conversion: replace MarshalSSZTo with MarshalSSZAppend and MarshalSSZInto

[holiman]

This fixes the append vs overwrite part of #170 .

However, this is technically a breaking change. On the other hand, the way it now works is incompatible with the fastssz encoding library. And since nobody has raised this earlier, I assume nobody has really used it in production.

So, should be fine to merge, on that reasoning.

(?)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (70cbe2b) to head (e685369).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #171   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines         1632      1638    +6     
=========================================
+ Hits          1632      1638    +6     

[jshufro]

(?)

I'd recommend going through the paces with deprecation instead, or at the very least keeping around a function that preserves the old behavior (under a new symbol).

[holiman]

Deprecating MarshalSSZTo doesn't really work -- it's an interface method, so I think calls to to it are via not direct, and the deprecation won't be noticed by tooling.

Possibly, I could remove it, and add it back a few releases later. At least then the breakage would be obvious. And that's perhaps better than what this PR does, which breaks things in a way which will only show in runtime, not compile-time.

[jshufro]

calls to to it are via not direct

I can't imagine anyone is calling it any way except direct.

sszgen/fastssz do not really support external interfaces. The -include flag for sszgen expects a local package only. I had to do all this: https://github.com/jshufro/smartnode/blob/e55d271c981833425987fb84d20d03e6177a7276/shared/services/rewards/ssz_types/big/uint256.go

[holiman]

type Uint256 struct {
	*big.Int
}

Hm, why can't you use uint256.Int directly, or, alternatively, alias it?

type Uint256 uint256.Int`

But I agree, perhaps best to just deprecate MarshalSSZTo in this PR, and create MarshalSSZAppend as a new method which does the append.

[holiman]

Possibly, I could remove it, and add it back a few releases later. At least then the breakage would be obvious.

I have now changed this PR to remove MarshalSSZTo, to force users to choose either MarshalSSZAppend or MarshalSSZInto. The former is the future MarshalSSZTo, the latter is already deprecated.

@jshufro SGTY?