Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
2ba967e
feat(params): `UnmarshalChainConfig` function
qdm12 Dec 17, 2024
4ba9321
`UnmarshalChainConfig` -> `UnmarshalChainConfigJSON`
qdm12 Dec 18, 2024
91f207c
Add extra field type to extra json decoding error
qdm12 Dec 18, 2024
9bcff55
Do not wrap error and use `%s` instead of `%w`
qdm12 Dec 18, 2024
512977d
rework UnmarshalChainConfigJSON
qdm12 Dec 19, 2024
8490a1b
update function comment
qdm12 Dec 19, 2024
0e3a68d
rework UnmarshalChainConfigJSON
qdm12 Dec 19, 2024
27688fe
reuseJSONRoot argument
qdm12 Dec 19, 2024
e8af129
inline body of unmarshalJSONWithRegisteredExtra and remove it
qdm12 Dec 19, 2024
ec9a9b3
use switch for shorter code
qdm12 Dec 19, 2024
5c4ac6c
chore: simplify code
qdm12 Dec 19, 2024
7497ab5
move UnmarshalChainConfigJSON below UnmarshalJSON chainConfig method
qdm12 Dec 19, 2024
a0d25d1
chore: split function into two functions `unmarshalChainConfigJSONExt…
qdm12 Dec 19, 2024
f0abde5
Update comment
qdm12 Dec 19, 2024
b20476d
Test fix: `.extra` -> `.Extra`
qdm12 Dec 20, 2024
81ee012
chore: merge encode/decode subtests in one
qdm12 Dec 20, 2024
46d8485
update function comment
qdm12 Dec 20, 2024
1c63271
Update TestUnmarshalChainConfigJSON
qdm12 Dec 20, 2024
f5dad1e
only use body of unmarshalChainConfigJSONExtraNotRegistered for every…
qdm12 Dec 20, 2024
03e825e
Add test cases to TestUnmarshalChainConfigJSON
qdm12 Dec 20, 2024
70b32e8
Make `UnmarshalChainConfigJSON` non generic to support `extra` interf…
qdm12 Dec 20, 2024
bf4557f
Revert function to be generic
qdm12 Dec 20, 2024
51cdb56
Return an error if extra argument is nil
qdm12 Dec 23, 2024
1ca9a4d
MarshalChainConfigJSON with test
qdm12 Dec 23, 2024
a9cd1d0
Update ChainConfig MarshalJSON comment
qdm12 Dec 23, 2024
86d9d03
Remove unneeded comments
qdm12 Dec 23, 2024
b8beaa9
style: inline variables
qdm12 Dec 23, 2024
f13f0f4
docs: fix typo in comment
qdm12 Dec 23, 2024
881dd0a
hotfix: fix TestUnmarshalChainConfigJSON
qdm12 Dec 23, 2024
ac3c989
UnmarshalChainConfigJSON decodes only once for re-use root case
qdm12 Dec 23, 2024
725250d
Use want* for test case fields
qdm12 Dec 23, 2024
afd7476
style: UnmarshalChainConfigJSON does not use switch anymore
qdm12 Dec 23, 2024
3120a53
`extraConstructors` -> `ec`
qdm12 Dec 23, 2024
5c3975f
Improved error wrapping for nil extra argument to `UnmarshalChainConf…
qdm12 Jan 6, 2025
496c608
Use short if forms for error checking
qdm12 Jan 6, 2025
98164b0
Use `C` instead of `T` for extra generic type
qdm12 Jan 6, 2025
740abb5
Clarify comment for `UnmarshalJSON`
qdm12 Jan 6, 2025
d1bea23
Clarify comment for `UnmarshalChainConfigJSON`
qdm12 Jan 6, 2025
ec81c21
Improve error wrappings
qdm12 Jan 6, 2025
5871a77
Simplify comment for `MarshalJSON`
qdm12 Jan 6, 2025
7d7aca1
Comment simplified for `MarshalChainConfigJSON`
qdm12 Jan 6, 2025
c9094dd
Don't use field names for inlined types in `UnmarshalChainConfigJSON`
qdm12 Jan 6, 2025
76f9928
Don't use field names for inlined types in `MarshalChainConfigJSON`
qdm12 Jan 6, 2025
3e068f5
Variable renamings in `TestChainConfigJSONRoundTrip`
qdm12 Jan 6, 2025
32fe286
Add error wrappings within `toJSONRawMessages`
qdm12 Jan 6, 2025
e3d25ea
Use regex for error messages expectations in tests
qdm12 Jan 6, 2025
b48fb8a
New unit tests focus on error cases only
qdm12 Jan 6, 2025
fa1a15b
Revert `TestChainConfigJSONRoundTrip` to its original state
qdm12 Jan 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 78 additions & 62 deletions params/json.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,108 +19,124 @@ package params
import (
"encoding/json"
"fmt"

"github.com/ava-labs/libevm/libevm/pseudo"
)

var _ interface {
json.Marshaler
json.Unmarshaler
} = (*ChainConfig)(nil)

// chainConfigWithoutMethods avoids infinite recurion into
// chainConfigWithoutMethods avoids infinite recursion into
// [ChainConfig.UnmarshalJSON].
type chainConfigWithoutMethods ChainConfig

// chainConfigWithExportedExtra supports JSON (un)marshalling of a [ChainConfig]
// while exposing the `extra` field as the "extra" JSON key.
type chainConfigWithExportedExtra struct {
*chainConfigWithoutMethods // embedded to achieve regular JSON unmarshalling
Extra *pseudo.Type `json:"extra"` // `c.extra` is otherwise unexported
// UnmarshalJSON implements the [json.Unmarshaler] interface. If extra payloads
// were registered, UnmarshalJSON decodes data as described by [Extras] and
// [RegisterExtras] otherwise it unmarshals directly into c as if ChainConfig
// didn't implement json.Unmarshaler.
func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) {
if !registeredExtras.Registered() {
return json.Unmarshal(data, (*chainConfigWithoutMethods)(c))
}
ec := registeredExtras.Get()
c.extra = ec.newChainConfig()
return UnmarshalChainConfigJSON(data, c, c.extra, ec.reuseJSONRoot)
}

// UnmarshalJSON implements the [json.Unmarshaler] interface.
func (c *ChainConfig) UnmarshalJSON(data []byte) error {
switch reg := registeredExtras; {
case reg.Registered() && !reg.Get().reuseJSONRoot:
return c.unmarshalJSONWithExtra(data)
// UnmarshalChainConfigJSON is equivalent to [ChainConfig.UnmarshalJSON]
// had [Extras] with `C` been registered, but without the need to call
// [RegisterExtras]. The `extra` argument MUST NOT be nil.
func UnmarshalChainConfigJSON[C any](data []byte, config *ChainConfig, extra *C, reuseJSONRoot bool) (err error) {
if extra == nil {
return fmt.Errorf("%T argument is nil; use %T.UnmarshalJSON() directly", extra, config)
}

case reg.Registered() && reg.Get().reuseJSONRoot: // although the latter is redundant, it's clearer
c.extra = reg.Get().newChainConfig()
if err := json.Unmarshal(data, c.extra); err != nil {
c.extra = nil
return err
if reuseJSONRoot {
if err := json.Unmarshal(data, (*chainConfigWithoutMethods)(config)); err != nil {
return fmt.Errorf("decoding JSON into %T: %s", config, err)
}
fallthrough // Important! We've only unmarshalled the extra field.
default: // reg == nil
return json.Unmarshal(data, (*chainConfigWithoutMethods)(c))
if err := json.Unmarshal(data, extra); err != nil {
return fmt.Errorf("decoding JSON into %T: %s", extra, err)
}
return nil
}
}

// unmarshalJSONWithExtra unmarshals JSON under the assumption that the
// registered [Extras] payload is in the JSON "extra" key. All other
// unmarshalling is performed as if no [Extras] were registered.
func (c *ChainConfig) unmarshalJSONWithExtra(data []byte) error {
cc := &chainConfigWithExportedExtra{
chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c),
Extra: registeredExtras.Get().newChainConfig(),
combined := struct {
*chainConfigWithoutMethods
Extra *C `json:"extra"`
}{
(*chainConfigWithoutMethods)(config),
extra,
}
if err := json.Unmarshal(data, cc); err != nil {
return err
if err := json.Unmarshal(data, &combined); err != nil {
return fmt.Errorf(`decoding JSON into combination of %T and %T (as "extra" key): %s`, config, extra, err)
}
c.extra = cc.Extra
return nil
}

// MarshalJSON implements the [json.Marshaler] interface.
// If extra payloads were registered, MarshalJSON encodes JSON as
// described by [Extras] and [RegisterExtras] otherwise it marshals
// `c` as if ChainConfig didn't implement json.Marshaler.
func (c *ChainConfig) MarshalJSON() ([]byte, error) {
switch reg := registeredExtras; {
case !reg.Registered():
if !registeredExtras.Registered() {
return json.Marshal((*chainConfigWithoutMethods)(c))
}
ec := registeredExtras.Get()
return MarshalChainConfigJSON(*c, c.extra, ec.reuseJSONRoot)
}

case !reg.Get().reuseJSONRoot:
return c.marshalJSONWithExtra()

default: // reg.reuseJSONRoot == true
// The inverse of reusing the JSON root is merging two JSON buffers,
// which isn't supported by the native package. So we use
// map[string]json.RawMessage intermediates.
geth, err := toJSONRawMessages((*chainConfigWithoutMethods)(c))
if err != nil {
return nil, err
// MarshalChainConfigJSON is equivalent to [ChainConfig.MarshalJSON]
// had [Extras] with `C` been registered, but without the need to
// call [RegisterExtras].
func MarshalChainConfigJSON[C any](config ChainConfig, extra C, reuseJSONRoot bool) (data []byte, err error) {
if !reuseJSONRoot {
jsonExtra := struct {
ChainConfig
Extra C `json:"extra,omitempty"`
}{
config,
extra,
}
extra, err := toJSONRawMessages(c.extra)
data, err = json.Marshal(jsonExtra)
if err != nil {
return nil, err
return nil, fmt.Errorf(`encoding combination of %T and %T (as "extra" key) to JSON: %s`, config, extra, err)
}
return data, nil
}

for k, v := range extra {
if _, ok := geth[k]; ok {
return nil, fmt.Errorf("duplicate JSON key %q in both %T and registered extra", k, c)
}
geth[k] = v
}
return json.Marshal(geth)
// The inverse of reusing the JSON root is merging two JSON buffers,
// which isn't supported by the native package. So we use
// map[string]json.RawMessage intermediates.
// Note we cannot encode a combined struct directly because of the extra
// type generic nature which cannot be embedded in such a combined struct.
configJSONRaw, err := toJSONRawMessages((chainConfigWithoutMethods)(config))
if err != nil {
return nil, fmt.Errorf("converting config to JSON raw messages: %s", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for this and the extras too we should just return a raw error and decorate it in toJSONRawMessages() instead, for consistency. I'd add a suggestion but GitHub review UI sucks and I can't comment on unmodified code in the same file.

fmt.Errorf("encoding %T into %T: %s", v, msgs, err) on line 154.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think fmt.Errorf("encoding %T into %T: %s", v, msgs, err) is a bit confusing, since technically we are encoding then decoding into a different Go element (map). I still think converting config to JSON raw messages explains it alright. I've added error wrappings in 359448d66f0a15e7f62b8aaef7bd734872690975 but it might be a bit too-muchey, feel free to let me know and i'll drop that commit.

}
extraJSONRaw, err := toJSONRawMessages(extra)
if err != nil {
return nil, fmt.Errorf("converting extra config to JSON raw messages: %s", err)
}
}

// marshalJSONWithExtra is the inverse of unmarshalJSONWithExtra().
func (c *ChainConfig) marshalJSONWithExtra() ([]byte, error) {
cc := &chainConfigWithExportedExtra{
chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c),
Extra: c.extra,
for k, v := range extraJSONRaw {
_, ok := configJSONRaw[k]
if ok {
return nil, fmt.Errorf("duplicate JSON key %q in ChainConfig and extra %T", k, extra)
}
configJSONRaw[k] = v
}
return json.Marshal(cc)
return json.Marshal(configJSONRaw)
}

func toJSONRawMessages(v any) (map[string]json.RawMessage, error) {
buf, err := json.Marshal(v)
if err != nil {
return nil, err
return nil, fmt.Errorf("encoding %T: %s", v, err)
}
msgs := make(map[string]json.RawMessage)
if err := json.Unmarshal(buf, &msgs); err != nil {
return nil, err
return nil, fmt.Errorf("decoding JSON encoding of %T into %T: %s", v, msgs, err)
}
return msgs, nil
}
124 changes: 124 additions & 0 deletions params/json.libevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"math/big"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ava-labs/libevm/libevm/pseudo"
Expand Down Expand Up @@ -144,3 +145,126 @@ func TestChainConfigJSONRoundTrip(t *testing.T) {
})
}
}

func TestUnmarshalChainConfigJSON_Errors(t *testing.T) {
t.Parallel()

type testExtra struct {
Field string `json:"field"`
}

testCases := map[string]struct {
jsonData string // string for convenience
extra *testExtra
reuseJSONRoot bool
wantConfig ChainConfig
wantExtra any
wantErrRegex string
}{
"invalid_json": {
extra: &testExtra{},
wantExtra: &testExtra{},
wantErrRegex: `^decoding JSON into combination of \*.+\.ChainConfig and \*.+\.testExtra \(as "extra" key\): .+$`,
},
"nil_extra_at_root_depth": {
jsonData: `{"chainId": 1}`,
extra: nil,
reuseJSONRoot: true,
wantExtra: (*testExtra)(nil),
wantErrRegex: `^\*.+.testExtra argument is nil; use \*.+\.ChainConfig\.UnmarshalJSON\(\) directly$`,
},
"nil_extra_at_extra_key": {
jsonData: `{"chainId": 1}`,
extra: nil,
wantExtra: (*testExtra)(nil),
wantErrRegex: `^\*.+\.testExtra argument is nil; use \*.+\.ChainConfig.UnmarshalJSON\(\) directly$`,
},
"wrong_extra_type_at_extra_key": {
jsonData: `{"chainId": 1, "extra": 1}`,
extra: &testExtra{},
wantConfig: ChainConfig{ChainID: big.NewInt(1)},
wantExtra: &testExtra{},
wantErrRegex: `^decoding JSON into combination of \*.+\.ChainConfig and \*.+\.testExtra \(as "extra" key\): .+$`,
},
"wrong_extra_type_at_root_depth": {
jsonData: `{"chainId": 1, "field": 1}`,
extra: &testExtra{},
reuseJSONRoot: true,
wantConfig: ChainConfig{ChainID: big.NewInt(1)},
wantExtra: &testExtra{},
wantErrRegex: `^decoding JSON into \*.+\.testExtra: .+`,
},
}

for name, testCase := range testCases {
testCase := testCase
t.Run(name, func(t *testing.T) {
t.Parallel()

data := []byte(testCase.jsonData)
config := ChainConfig{}
err := UnmarshalChainConfigJSON(data, &config, testCase.extra, testCase.reuseJSONRoot)
if testCase.wantErrRegex == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Regexp(t, testCase.wantErrRegex, err.Error())
}
assert.Equal(t, testCase.wantConfig, config)
assert.Equal(t, testCase.wantExtra, testCase.extra)
})
}
}

func TestMarshalChainConfigJSON_Errors(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
config ChainConfig
extra any
reuseJSONRoot bool
wantJSONData string // string for convenience
wantErrRegex string
}{
"invalid_extra_at_extra_key": {
extra: struct {
Field chan struct{} `json:"field"`
}{},
wantErrRegex: `^encoding combination of .+\.ChainConfig and .+ to JSON: .+$`,
},
"nil_extra_at_extra_key": {
wantJSONData: `{"chainId":null}`,
},
"invalid_extra_at_root_depth": {
extra: struct {
Field chan struct{} `json:"field"`
}{},
reuseJSONRoot: true,
wantErrRegex: "^converting extra config to JSON raw messages: .+$",
},
"duplicate_key": {
extra: struct {
Field string `json:"chainId"`
}{},
reuseJSONRoot: true,
wantErrRegex: `^duplicate JSON key "chainId" in ChainConfig and extra struct .+$`,
},
}

for name, testCase := range testCases {
testCase := testCase
t.Run(name, func(t *testing.T) {
t.Parallel()

config := ChainConfig{}
data, err := MarshalChainConfigJSON(config, testCase.extra, testCase.reuseJSONRoot)
if testCase.wantErrRegex == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
assert.Regexp(t, testCase.wantErrRegex, err.Error())
}
assert.Equal(t, testCase.wantJSONData, string(data))
})
}
}
Loading