fjl / gencodec

Command gencodec generates marshaling methods for Go struct types.
MIT License
55 stars 27 forks source link

Field override is failing #20

Open rjl493456442 opened 10 months ago

rjl493456442 commented 10 months ago

The issue occurs when I try to replace **big.Int with **hexutil.Big.

//go:generate go run github.com/fjl/gencodec -type OverrideAccount -field-override overrideMarshaling -out gen_override_json.go

// OverrideAccount indicates the overriding fields of account during the
// execution of a message call.
//
// Note, state and stateDiff can't be specified at the same time.
// - If state is set, state lookup will be performed in this state set first;
// - If statDiff is set, state lookup will be performed in the diff set first;
type OverrideAccount struct {
    Nonce     *uint64                      `json:"nonce"`
    Code      *[]byte                      `json:"code"`
    Balance   **big.Int                    `json:"balance"`
    State     *map[common.Hash]common.Hash `json:"state"`
    StateDiff *map[common.Hash]common.Hash `json:"stateDiff"`
}

// nolint
type overrideMarshaling struct {
    Nonce   *hexutil.Uint64
    Code    *hexutil.Bytes
    Balance **hexutil.Big
}

Error message is

/usr/local/go/bin/go generate -run go run github.com/fjl/gencodec -type OverrideAccount -field-override overrideMarshaling -out gen_override_json.go -: invalid field override: type **github.com/ethereum/go-ethereum/common/hexutil.Big is not convertible to **math/big.Int exit status 1 overrides.go:28: running "go": exit status 1

fjl commented 10 months ago

It doesn't support double-pointer conversion. We could add this, but I'm wondering why you need double-pointer in the first place?

fjl commented 10 months ago

Double pointer conversion is not hard to add if you really do need it though.

First you would have to allow it in this function here: https://github.com/fjl/gencodec/blob/f9840df7b83e876ee46553727f7523cebdc7e90b/types_util.go#L128

Then it needs to be handled in convert(): https://github.com/fjl/gencodec/blob/f9840df7b83e876ee46553727f7523cebdc7e90b/generate.go#L256

The output code for convert(**A, **B) where *A is trivially convertible to *B would be something like this:

if in.a == nil {
    out.b = nil
} else 
    v := (*B)(*in.a)
    out.b = &v
}
rjl493456442 commented 10 months ago

The reason I want double pointer conversion is the field is a double pointer.

e.g. **big.Int, an nil *big.Int means the balance is not overrided; while this override object needs to have JSONRPC marshaller and in which the *big.Int is encoded as a *hexutil.Big.

rjl493456442 commented 10 months ago

But it's no hurry at all! I can even make the change in gencodec tool by myself. It's not decided yet whether the whole conversion is needed or not.

Good to know how to support it.

fjl commented 10 months ago

e.g. *big.Int, an nil big.Int means the balance is not overrided;

What I find strange about this, is that usually it is enough to have a single pointer in this case, and *big.Int is already pointer. So in your OverrideAccount, if you set the Balance to a non-nil *big.Int, it can be considered overridden. You would only need the double pointer if you want to allow overriding with nil.

Same with slice/map values. Since they can be nil, it is usually OK to use them without pointer in this 'override' scenario.