From 2ba967eac0cf2f58090a7f7e108e2a83e2869f33 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Tue, 17 Dec 2024 19:07:40 +0100 Subject: [PATCH 01/48] feat(params): `UnmarshalChainConfig` function --- params/json.libevm.go | 24 ++++++++++++++++ params/json.libevm_test.go | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/params/json.libevm.go b/params/json.libevm.go index c9acefe9497..5a517fba980 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -124,3 +124,27 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { } return msgs, nil } + +// UnmarshalChainConfig JSON decodes the given data into the config and the extra +// pointer arguments. The root JSON keys except the "extra" key are decoded into +// the config argument, and the object at the "extra" key, if present, is decoded +// into the extra argument. Note the extra argument must be a non-nil pointer or +// an error would be returned. +func UnmarshalChainConfig(data []byte, config *ChainConfig, extra any) (err error) { + err = json.Unmarshal(data, config) + if err != nil { + return fmt.Errorf("json decoding root chain config: %w", err) + } + + jsonExtra := struct { + Extra any `json:"extra"` + }{ + Extra: extra, + } + err = json.Unmarshal(data, &jsonExtra) + if err != nil { + return fmt.Errorf("json decoding extra chain config: %w", err) + } + + return nil +} diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 70014e0d8d1..0c09028125f 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -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" @@ -144,3 +145,60 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { }) } } + +func Test_UnmarshalChainConfig(t *testing.T) { + t.Parallel() + + type testExtra struct { + Field string `json:"field"` + } + + testCases := map[string]struct { + jsonData string // string for convenience + expectedConfig ChainConfig + expectedExtra any + errMessage string + }{ + "invalid_json": { + expectedExtra: testExtra{}, + errMessage: "json decoding root chain config: unexpected end of JSON input", + }, + "no_extra": { + jsonData: `{"chainId": 1}`, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{}, + }, + "wrong_extra_type": { + jsonData: `{"chainId": 1, "extra": 1}`, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{}, + errMessage: "json decoding extra chain config: " + + "json: cannot unmarshal number into Go struct field " + + ".extra of type params.testExtra", + }, + "extra_success": { + jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{Field: "value"}, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + data := []byte(testCase.jsonData) + config := ChainConfig{} + var extra testExtra + err := UnmarshalChainConfig(data, &config, &extra) + if testCase.errMessage == "" { + require.NoError(t, err) + } else { + require.EqualError(t, err, testCase.errMessage) + } + assert.Equal(t, testCase.expectedConfig, config) + assert.Equal(t, testCase.expectedExtra, extra) + }) + } +} From 4ba9321dd015c037d22bd8a3cbb2efa4e099e1a8 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 18 Dec 2024 17:46:20 +0100 Subject: [PATCH 02/48] `UnmarshalChainConfig` -> `UnmarshalChainConfigJSON` --- params/json.libevm.go | 4 ++-- params/json.libevm_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 5a517fba980..a29bb6b44b6 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -125,12 +125,12 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { return msgs, nil } -// UnmarshalChainConfig JSON decodes the given data into the config and the extra +// UnmarshalChainConfigJSON JSON decodes the given data into the config and the extra // pointer arguments. The root JSON keys except the "extra" key are decoded into // the config argument, and the object at the "extra" key, if present, is decoded // into the extra argument. Note the extra argument must be a non-nil pointer or // an error would be returned. -func UnmarshalChainConfig(data []byte, config *ChainConfig, extra any) (err error) { +func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any) (err error) { err = json.Unmarshal(data, config) if err != nil { return fmt.Errorf("json decoding root chain config: %w", err) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 0c09028125f..d63d387f555 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -146,7 +146,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { } } -func Test_UnmarshalChainConfig(t *testing.T) { +func Test_UnmarshalChainConfigJSON(t *testing.T) { t.Parallel() type testExtra struct { @@ -191,7 +191,7 @@ func Test_UnmarshalChainConfig(t *testing.T) { data := []byte(testCase.jsonData) config := ChainConfig{} var extra testExtra - err := UnmarshalChainConfig(data, &config, &extra) + err := UnmarshalChainConfigJSON(data, &config, &extra) if testCase.errMessage == "" { require.NoError(t, err) } else { From 91f207ca6d6c508e53a9aacefab810c3333bd79e Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 18 Dec 2024 17:56:53 +0100 Subject: [PATCH 03/48] Add extra field type to extra json decoding error --- params/json.libevm.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index a29bb6b44b6..f48292d9e5a 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -143,7 +143,8 @@ func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any) (err } err = json.Unmarshal(data, &jsonExtra) if err != nil { - return fmt.Errorf("json decoding extra chain config: %w", err) + return fmt.Errorf("json decoding extra chain config to %T: %w", + jsonExtra.Extra, err) } return nil From 9bcff55543d92fb868eb5ff079477f2ae8dbb171 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Wed, 18 Dec 2024 17:57:45 +0100 Subject: [PATCH 04/48] Do not wrap error and use `%s` instead of `%w` --- params/json.libevm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index f48292d9e5a..ff099e27b79 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -133,7 +133,7 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any) (err error) { err = json.Unmarshal(data, config) if err != nil { - return fmt.Errorf("json decoding root chain config: %w", err) + return fmt.Errorf("json decoding root chain config: %s", err) } jsonExtra := struct { @@ -143,7 +143,7 @@ func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any) (err } err = json.Unmarshal(data, &jsonExtra) if err != nil { - return fmt.Errorf("json decoding extra chain config to %T: %w", + return fmt.Errorf("json decoding extra chain config to %T: %s", jsonExtra.Extra, err) } From 512977daaceecf1258c9e8360bdfe64daf3ffc24 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 12:13:36 +0100 Subject: [PATCH 05/48] rework UnmarshalChainConfigJSON --- params/json.libevm.go | 60 ++++++++++++++++++++++++++------------ params/json.libevm_test.go | 1 + 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index ff099e27b79..d45a3ae267c 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -47,13 +47,13 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { 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 { + if err := UnmarshalChainConfigJSON(data, nil, c.extra); err != nil { c.extra = nil return err } fallthrough // Important! We've only unmarshalled the extra field. default: // reg == nil - return json.Unmarshal(data, (*chainConfigWithoutMethods)(c)) + return UnmarshalChainConfigJSON[struct{}](data, c, nil) } } @@ -130,22 +130,46 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { // the config argument, and the object at the "extra" key, if present, is decoded // into the extra argument. Note the extra argument must be a non-nil pointer or // an error would be returned. -func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any) (err error) { - err = json.Unmarshal(data, config) - if err != nil { - return fmt.Errorf("json decoding root chain config: %s", err) - } +func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) (err error) { + switch { + case config == nil && extra == nil: + return fmt.Errorf("chain config and extra config are both nil") + case extra == nil: + // non-registered extra call from ChainConfig.UnmarshalJSON + // we only want to decode to the chain config, ignoring the + // "extra" JSON key. + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + if err != nil { + return fmt.Errorf("decoding chain config without extra: %s", err) + } + return nil + case config == nil: + // decode the data to the extra argument only. + // this originates from the registered extra + re-use JSON + // root call from ChainConfig.UnmarshalJSON. + err = json.Unmarshal(data, extra) + if err != nil { + extra = nil + return fmt.Errorf("decoding chain config to %T: %s", extra, err) + } + return nil + default: + // Decode the data separately to the chain config and the extra. + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + if err != nil { + return fmt.Errorf("decoding root chain config: %s", err) + } - jsonExtra := struct { - Extra any `json:"extra"` - }{ - Extra: extra, - } - err = json.Unmarshal(data, &jsonExtra) - if err != nil { - return fmt.Errorf("json decoding extra chain config to %T: %s", - jsonExtra.Extra, err) + jsonExtra := struct { + Extra *T `json:"extra"` + }{ + Extra: extra, + } + err = json.Unmarshal(data, &jsonExtra) + if err != nil { + return fmt.Errorf("decoding extra config to %T: %s", + extra, err) + } + return nil } - - return nil } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index d63d387f555..edb30ece264 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -148,6 +148,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { func Test_UnmarshalChainConfigJSON(t *testing.T) { t.Parallel() + t.Skip() type testExtra struct { Field string `json:"field"` From 8490a1b0594148f042b6f69bf1f49b5383c49676 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 17:00:15 +0100 Subject: [PATCH 06/48] update function comment --- params/json.libevm.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index d45a3ae267c..340c833b891 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -125,11 +125,13 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { return msgs, nil } -// UnmarshalChainConfigJSON JSON decodes the given data into the config and the extra -// pointer arguments. The root JSON keys except the "extra" key are decoded into -// the config argument, and the object at the "extra" key, if present, is decoded -// into the extra argument. Note the extra argument must be a non-nil pointer or -// an error would be returned. +// UnmarshalChainConfigJSON JSON decodes the given data according to its arguments: +// - if only `extra` is nil, the data is decoded into `config` and the "extra" JSON +// key field is ignored. +// - if only `config` is nil, the data is decoded into `extra` only. +// - if both `config` and `extra` are non-nil, the data is first decoded into `config` +// and then the "extra" JSON field is decoded into `extra.` +// - if both `config` and `extra` are nil pointers, an error is returned. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) (err error) { switch { case config == nil && extra == nil: From 0e3a68d9dcb895b5df30ef50e871d76ee59c8d7e Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 17:27:49 +0100 Subject: [PATCH 07/48] rework UnmarshalChainConfigJSON --- params/json.libevm.go | 82 ++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 340c833b891..e45e532473e 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -41,26 +41,14 @@ type chainConfigWithExportedExtra struct { // 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) - - case reg.Registered() && reg.Get().reuseJSONRoot: // although the latter is redundant, it's clearer - c.extra = reg.Get().newChainConfig() - if err := UnmarshalChainConfigJSON(data, nil, c.extra); err != nil { - c.extra = nil - return err - } - fallthrough // Important! We've only unmarshalled the extra field. - default: // reg == nil - return UnmarshalChainConfigJSON[struct{}](data, c, nil) - } + extra := (*struct{})(nil) + return UnmarshalChainConfigJSON(data, c, extra) } -// unmarshalJSONWithExtra unmarshals JSON under the assumption that the +// unmarshalJSONWithRegisteredExtra 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 { +func (c *ChainConfig) unmarshalJSONWithRegisteredExtra(data []byte) error { cc := &chainConfigWithExportedExtra{ chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c), Extra: registeredExtras.Get().newChainConfig(), @@ -125,43 +113,23 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { return msgs, nil } -// UnmarshalChainConfigJSON JSON decodes the given data according to its arguments: -// - if only `extra` is nil, the data is decoded into `config` and the "extra" JSON -// key field is ignored. -// - if only `config` is nil, the data is decoded into `extra` only. -// - if both `config` and `extra` are non-nil, the data is first decoded into `config` -// and then the "extra" JSON field is decoded into `extra.` -// - if both `config` and `extra` are nil pointers, an error is returned. +// UnmarshalChainConfigJSON JSON decodes the given data: +// - if the extra is NOT registered AND the `extra` argument is nil, the chain config +// is decoded in `config` and the "extra" JSON field is ignored. +// - if the extra is NOT registered AND the `extra` argument is NOT nil, the chain config +// is decoded in `config` and the "extra" JSON field is decoded in `extra`. +// - if the extra is registered, whether the root JSON is re-used for the extra or not, +// the chain config is decoded only in the `config` argument. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) (err error) { - switch { - case config == nil && extra == nil: - return fmt.Errorf("chain config and extra config are both nil") - case extra == nil: - // non-registered extra call from ChainConfig.UnmarshalJSON - // we only want to decode to the chain config, ignoring the - // "extra" JSON key. - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) - if err != nil { - return fmt.Errorf("decoding chain config without extra: %s", err) - } - return nil - case config == nil: - // decode the data to the extra argument only. - // this originates from the registered extra + re-use JSON - // root call from ChainConfig.UnmarshalJSON. - err = json.Unmarshal(data, extra) - if err != nil { - extra = nil - return fmt.Errorf("decoding chain config to %T: %s", extra, err) - } - return nil - default: - // Decode the data separately to the chain config and the extra. + if !registeredExtras.Registered() { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) if err != nil { return fmt.Errorf("decoding root chain config: %s", err) } + if extra == nil { // ignore the "extra" JSON key + return nil + } jsonExtra := struct { Extra *T `json:"extra"` }{ @@ -174,4 +142,24 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) } return nil } + + // registered extra and extra config is in the "extra" JSON field. + if !registeredExtras.Get().reuseJSONRoot { + return config.unmarshalJSONWithRegisteredExtra(data) + } + + // registered extra and re-use JSON root, the extra config is contained + // in the root config object. + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + if err != nil { + return fmt.Errorf("decoding chain config: %s", err) + } + + config.extra = registeredExtras.Get().newChainConfig() + err = json.Unmarshal(data, config.extra) + if err != nil { + config.extra = nil + return fmt.Errorf("decoding chain config to %T: %s", config.extra, err) + } + return nil } From 27688fe8e83772c02707506ef2601880017766d4 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:26:08 +0100 Subject: [PATCH 08/48] reuseJSONRoot argument --- params/json.libevm.go | 41 ++++++++++++++++++++++++++------------ params/json.libevm_test.go | 3 ++- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index e45e532473e..16da77c6071 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -42,7 +42,8 @@ type chainConfigWithExportedExtra struct { // UnmarshalJSON implements the [json.Unmarshaler] interface. func (c *ChainConfig) UnmarshalJSON(data []byte) error { extra := (*struct{})(nil) - return UnmarshalChainConfigJSON(data, c, extra) + const reuseJSONRoot = false + return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) } // unmarshalJSONWithRegisteredExtra unmarshals JSON under the assumption that the @@ -120,7 +121,7 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { // is decoded in `config` and the "extra" JSON field is decoded in `extra`. // - if the extra is registered, whether the root JSON is re-used for the extra or not, // the chain config is decoded only in the `config` argument. -func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) (err error) { +func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { if !registeredExtras.Registered() { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) if err != nil { @@ -130,17 +131,31 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) if extra == nil { // ignore the "extra" JSON key return nil } - jsonExtra := struct { - Extra *T `json:"extra"` - }{ - Extra: extra, - } - err = json.Unmarshal(data, &jsonExtra) - if err != nil { - return fmt.Errorf("decoding extra config to %T: %s", - extra, err) + + if reuseJSONRoot { + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + if err != nil { + return fmt.Errorf("decoding chain config: %s", err) + } + + err = json.Unmarshal(data, extra) + if err != nil { + return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) + } + return nil + } else { + jsonExtra := struct { + Extra *T `json:"extra"` + }{ + Extra: extra, + } + err = json.Unmarshal(data, &jsonExtra) + if err != nil { + return fmt.Errorf("decoding extra config to %T: %s", + extra, err) + } + return nil } - return nil } // registered extra and extra config is in the "extra" JSON field. @@ -159,7 +174,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T) err = json.Unmarshal(data, config.extra) if err != nil { config.extra = nil - return fmt.Errorf("decoding chain config to %T: %s", config.extra, err) + return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) } return nil } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index edb30ece264..90323d66664 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -156,6 +156,7 @@ func Test_UnmarshalChainConfigJSON(t *testing.T) { testCases := map[string]struct { jsonData string // string for convenience + reuseJSONRoot bool expectedConfig ChainConfig expectedExtra any errMessage string @@ -192,7 +193,7 @@ func Test_UnmarshalChainConfigJSON(t *testing.T) { data := []byte(testCase.jsonData) config := ChainConfig{} var extra testExtra - err := UnmarshalChainConfigJSON(data, &config, &extra) + err := UnmarshalChainConfigJSON(data, &config, &extra, testCase.reuseJSONRoot) if testCase.errMessage == "" { require.NoError(t, err) } else { From e8af129da1911f81e4de462c3ad48ad8a29ca1b3 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:28:56 +0100 Subject: [PATCH 09/48] inline body of unmarshalJSONWithRegisteredExtra and remove it --- params/json.libevm.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 16da77c6071..1ca6e88418d 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -46,21 +46,6 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) } -// unmarshalJSONWithRegisteredExtra 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) unmarshalJSONWithRegisteredExtra(data []byte) error { - cc := &chainConfigWithExportedExtra{ - chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c), - Extra: registeredExtras.Get().newChainConfig(), - } - if err := json.Unmarshal(data, cc); err != nil { - return err - } - c.extra = cc.Extra - return nil -} - // MarshalJSON implements the [json.Marshaler] interface. func (c *ChainConfig) MarshalJSON() ([]byte, error) { switch reg := registeredExtras; { @@ -158,14 +143,25 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, } } + chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) + // registered extra and extra config is in the "extra" JSON field. if !registeredExtras.Get().reuseJSONRoot { - return config.unmarshalJSONWithRegisteredExtra(data) + configWrapper := &chainConfigWithExportedExtra{ + chainConfigWithoutMethods: chainConfigWithoutMethods, + Extra: registeredExtras.Get().newChainConfig(), + } + err = json.Unmarshal(data, configWrapper) + if err != nil { + return fmt.Errorf("decoding chain config and extra: %s", err) + } + config.extra = configWrapper.extra + return nil } // registered extra and re-use JSON root, the extra config is contained // in the root config object. - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + err = json.Unmarshal(data, chainConfigWithoutMethods) if err != nil { return fmt.Errorf("decoding chain config: %s", err) } From ec9a9b3cc2d8f6ae9d35ae30e9d126629437df3a Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:30:44 +0100 Subject: [PATCH 10/48] use switch for shorter code --- params/json.libevm.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 1ca6e88418d..c59299536b2 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -109,15 +109,11 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { if !registeredExtras.Registered() { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) - if err != nil { + switch { + case err != nil: return fmt.Errorf("decoding root chain config: %s", err) - } - - if extra == nil { // ignore the "extra" JSON key - return nil - } - - if reuseJSONRoot { + case extra == nil: // ignore the "extra" JSON key + case reuseJSONRoot: err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) if err != nil { return fmt.Errorf("decoding chain config: %s", err) @@ -127,8 +123,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, if err != nil { return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) } - return nil - } else { + default: jsonExtra := struct { Extra *T `json:"extra"` }{ @@ -139,8 +134,8 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, return fmt.Errorf("decoding extra config to %T: %s", extra, err) } - return nil } + return nil } chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) From 5c4ac6c0f2a8ad0c01b64029c2573fdf4dcaccbe Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:31:37 +0100 Subject: [PATCH 11/48] chore: simplify code --- params/json.libevm.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index c59299536b2..97a69c4c029 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -107,18 +107,15 @@ func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { // - if the extra is registered, whether the root JSON is re-used for the extra or not, // the chain config is decoded only in the `config` argument. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { + chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) + if !registeredExtras.Registered() { - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + err = json.Unmarshal(data, chainConfigWithoutMethods) switch { case err != nil: return fmt.Errorf("decoding root chain config: %s", err) case extra == nil: // ignore the "extra" JSON key case reuseJSONRoot: - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) - if err != nil { - return fmt.Errorf("decoding chain config: %s", err) - } - err = json.Unmarshal(data, extra) if err != nil { return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) @@ -138,13 +135,12 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, return nil } - chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) - // registered extra and extra config is in the "extra" JSON field. - if !registeredExtras.Get().reuseJSONRoot { + registeredExtraConstructors := registeredExtras.Get() + if !registeredExtraConstructors.reuseJSONRoot { configWrapper := &chainConfigWithExportedExtra{ chainConfigWithoutMethods: chainConfigWithoutMethods, - Extra: registeredExtras.Get().newChainConfig(), + Extra: registeredExtraConstructors.newChainConfig(), } err = json.Unmarshal(data, configWrapper) if err != nil { @@ -161,7 +157,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, return fmt.Errorf("decoding chain config: %s", err) } - config.extra = registeredExtras.Get().newChainConfig() + config.extra = registeredExtraConstructors.newChainConfig() err = json.Unmarshal(data, config.extra) if err != nil { config.extra = nil From 7497ab5b3b486838339fce0a68b8207407459fb7 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:35:43 +0100 Subject: [PATCH 12/48] move UnmarshalChainConfigJSON below UnmarshalJSON chainConfig method --- params/json.libevm.go | 106 +++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 97a69c4c029..4fe8d565a3c 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -46,59 +46,6 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) } -// MarshalJSON implements the [json.Marshaler] interface. -func (c *ChainConfig) MarshalJSON() ([]byte, error) { - switch reg := registeredExtras; { - case !reg.Registered(): - return json.Marshal((*chainConfigWithoutMethods)(c)) - - 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 - } - extra, err := toJSONRawMessages(c.extra) - if err != nil { - return nil, err - } - - 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) - } -} - -// marshalJSONWithExtra is the inverse of unmarshalJSONWithExtra(). -func (c *ChainConfig) marshalJSONWithExtra() ([]byte, error) { - cc := &chainConfigWithExportedExtra{ - chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c), - Extra: c.extra, - } - return json.Marshal(cc) -} - -func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { - buf, err := json.Marshal(v) - if err != nil { - return nil, err - } - msgs := make(map[string]json.RawMessage) - if err := json.Unmarshal(buf, &msgs); err != nil { - return nil, err - } - return msgs, nil -} - // UnmarshalChainConfigJSON JSON decodes the given data: // - if the extra is NOT registered AND the `extra` argument is nil, the chain config // is decoded in `config` and the "extra" JSON field is ignored. @@ -165,3 +112,56 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, } return nil } + +// MarshalJSON implements the [json.Marshaler] interface. +func (c *ChainConfig) MarshalJSON() ([]byte, error) { + switch reg := registeredExtras; { + case !reg.Registered(): + return json.Marshal((*chainConfigWithoutMethods)(c)) + + 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 + } + extra, err := toJSONRawMessages(c.extra) + if err != nil { + return nil, err + } + + 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) + } +} + +// marshalJSONWithExtra is the inverse of unmarshalJSONWithExtra(). +func (c *ChainConfig) marshalJSONWithExtra() ([]byte, error) { + cc := &chainConfigWithExportedExtra{ + chainConfigWithoutMethods: (*chainConfigWithoutMethods)(c), + Extra: c.extra, + } + return json.Marshal(cc) +} + +func toJSONRawMessages(v any) (map[string]json.RawMessage, error) { + buf, err := json.Marshal(v) + if err != nil { + return nil, err + } + msgs := make(map[string]json.RawMessage) + if err := json.Unmarshal(buf, &msgs); err != nil { + return nil, err + } + return msgs, nil +} From a0d25d170fa54d27cc64cbd7619cd9959cfb3555 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:48:02 +0100 Subject: [PATCH 13/48] chore: split function into two functions `unmarshalChainConfigJSONExtraNotRegistered` and `unmarshalChainConfigJSONExtraRegistered` --- params/json.libevm.go | 55 ++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 4fe8d565a3c..32adeb011b3 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -54,33 +54,40 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { // - if the extra is registered, whether the root JSON is re-used for the extra or not, // the chain config is decoded only in the `config` argument. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { - chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) - if !registeredExtras.Registered() { - err = json.Unmarshal(data, chainConfigWithoutMethods) - switch { - case err != nil: - return fmt.Errorf("decoding root chain config: %s", err) - case extra == nil: // ignore the "extra" JSON key - case reuseJSONRoot: - err = json.Unmarshal(data, extra) - if err != nil { - return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) - } - default: - jsonExtra := struct { - Extra *T `json:"extra"` - }{ - Extra: extra, - } - err = json.Unmarshal(data, &jsonExtra) - if err != nil { - return fmt.Errorf("decoding extra config to %T: %s", - extra, err) - } + return unmarshalChainConfigJSONExtraNotRegistered(data, config, extra, reuseJSONRoot) + } + return unmarshalChainConfigJSONExtraRegistered(data, config) +} + +func unmarshalChainConfigJSONExtraNotRegistered[T any](data []byte, config *ChainConfig, + extra *T, reuseJSONRoot bool) (err error) { + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + switch { + case err != nil: + return fmt.Errorf("decoding root chain config: %s", err) + case extra == nil: // ignore the "extra" JSON key + case reuseJSONRoot: + err = json.Unmarshal(data, extra) + if err != nil { + return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) + } + default: + jsonExtra := struct { + Extra *T `json:"extra"` + }{ + Extra: extra, + } + err = json.Unmarshal(data, &jsonExtra) + if err != nil { + return fmt.Errorf("decoding extra config to %T: %s", extra, err) } - return nil } + return nil +} + +func unmarshalChainConfigJSONExtraRegistered(data []byte, config *ChainConfig) (err error) { + chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) // registered extra and extra config is in the "extra" JSON field. registeredExtraConstructors := registeredExtras.Get() From f0abde543d19436afce5bd6714d7926d2dc0c419 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Thu, 19 Dec 2024 18:53:48 +0100 Subject: [PATCH 14/48] Update comment --- params/json.libevm.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 32adeb011b3..7dcead21e5e 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -46,13 +46,8 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) } -// UnmarshalChainConfigJSON JSON decodes the given data: -// - if the extra is NOT registered AND the `extra` argument is nil, the chain config -// is decoded in `config` and the "extra" JSON field is ignored. -// - if the extra is NOT registered AND the `extra` argument is NOT nil, the chain config -// is decoded in `config` and the "extra" JSON field is decoded in `extra`. -// - if the extra is registered, whether the root JSON is re-used for the extra or not, -// the chain config is decoded only in the `config` argument. +// UnmarshalChainConfigJSON JSON decodes the given `data` into `config`, and into `extra` if +// and only if the extra is not registered. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { if !registeredExtras.Registered() { return unmarshalChainConfigJSONExtraNotRegistered(data, config, extra, reuseJSONRoot) From b20476da0e7f5749432c3df496d63022be024ab8 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 10:58:12 +0100 Subject: [PATCH 15/48] Test fix: `.extra` -> `.Extra` --- params/json.libevm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 7dcead21e5e..e16efe32bce 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -95,7 +95,7 @@ func unmarshalChainConfigJSONExtraRegistered(data []byte, config *ChainConfig) ( if err != nil { return fmt.Errorf("decoding chain config and extra: %s", err) } - config.extra = configWrapper.extra + config.extra = configWrapper.Extra // Note: "Extra" field, not "extra" return nil } From 81ee0126470a7b6ddc9f0b0057f9bd4ce4ab46bf Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 11:54:09 +0100 Subject: [PATCH 16/48] chore: merge encode/decode subtests in one --- params/json.libevm_test.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 90323d66664..53485616436 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -128,20 +128,18 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { t.Cleanup(TestOnlyClearRegisteredExtras) tt.register() - t.Run("json.Unmarshal()", func(t *testing.T) { - got := new(ChainConfig) - require.NoError(t, json.Unmarshal([]byte(tt.jsonInput), got)) - require.Equal(t, tt.want, got) - }) - - t.Run("json.Marshal()", func(t *testing.T) { - var want bytes.Buffer - require.NoError(t, json.Compact(&want, []byte(tt.jsonInput)), "json.Compact()") - - got, err := json.Marshal(tt.want) - require.NoError(t, err, "json.Marshal()") - require.Equal(t, want.String(), string(got)) - }) + expectedEncoded := bytes.NewBuffer(nil) + err := json.Compact(expectedEncoded, []byte(tt.jsonInput)) + require.NoError(t, err) + + encoded, err := json.Marshal(tt.want) + require.NoError(t, err, "encoding error") + require.Equal(t, expectedEncoded.String(), string(encoded)) + + decoded := new(ChainConfig) + err = json.Unmarshal(encoded, decoded) + require.NoError(t, err, "decoding error") + require.Equal(t, tt.want, decoded) }) } } From 46d84850da44d1d55e8577035b2a4508534d2192 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 12:14:09 +0100 Subject: [PATCH 17/48] update function comment --- params/json.libevm.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index e16efe32bce..f5ecc474e75 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -46,8 +46,18 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) error { return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) } -// UnmarshalChainConfigJSON JSON decodes the given `data` into `config`, and into `extra` if -// and only if the extra is not registered. +// UnmarshalChainConfigJSON JSON decodes `data` according to the following. +// - extra is not registered, `extra` is not nil and `reuseJSONRoot` is false: +// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. +// - extra is not registered, `extra` is not nil and `reuseJSONRoot` is true: +// `data` is decoded into `config` and `data` is decoded into `extra`. +// - extra is not registered, `extra` is nil: +// `data` is decoded into `config` and the extra, whether at the root JSON depth +// of `data` or at the "extra" JSON field, is ignored. +// - extra is registered and the registered reuseJSONRoot field is false: +// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `config.extra`. +// - extra is registered and the registered reuseJSONRoot field is true: +// `data` is decoded into `config` and `data` is decoded into `config.extra`. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { if !registeredExtras.Registered() { return unmarshalChainConfigJSONExtraNotRegistered(data, config, extra, reuseJSONRoot) @@ -61,7 +71,7 @@ func unmarshalChainConfigJSONExtraNotRegistered[T any](data []byte, config *Chai switch { case err != nil: return fmt.Errorf("decoding root chain config: %s", err) - case extra == nil: // ignore the "extra" JSON key + case extra == nil: // ignore the extra, whether at the root JSON depth, or at the "extra" JSON field. case reuseJSONRoot: err = json.Unmarshal(data, extra) if err != nil { From 1c6327171617f253f0bcb645fdca2547b5993ace Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 13:37:31 +0100 Subject: [PATCH 18/48] Update TestUnmarshalChainConfigJSON --- params/json.libevm_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 53485616436..92d9dbf0397 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -144,9 +144,8 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { } } -func Test_UnmarshalChainConfigJSON(t *testing.T) { +func TestUnmarshalChainConfigJSON(t *testing.T) { t.Parallel() - t.Skip() type testExtra struct { Field string `json:"field"` @@ -161,7 +160,7 @@ func Test_UnmarshalChainConfigJSON(t *testing.T) { }{ "invalid_json": { expectedExtra: testExtra{}, - errMessage: "json decoding root chain config: unexpected end of JSON input", + errMessage: "decoding root chain config: unexpected end of JSON input", }, "no_extra": { jsonData: `{"chainId": 1}`, @@ -172,7 +171,7 @@ func Test_UnmarshalChainConfigJSON(t *testing.T) { jsonData: `{"chainId": 1, "extra": 1}`, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: testExtra{}, - errMessage: "json decoding extra chain config: " + + errMessage: "decoding extra config to *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, From f5dad1e779f9914a1494292741ffa542ad0274f6 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 15:28:57 +0100 Subject: [PATCH 19/48] only use body of unmarshalChainConfigJSONExtraNotRegistered for everything and remove functions --- params/json.libevm.go | 79 ++++++++++++------------------------------- 1 file changed, 22 insertions(+), 57 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index f5ecc474e75..84995ae6705 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -39,34 +39,33 @@ type chainConfigWithExportedExtra struct { Extra *pseudo.Type `json:"extra"` // `c.extra` is otherwise unexported } -// UnmarshalJSON implements the [json.Unmarshaler] interface. -func (c *ChainConfig) UnmarshalJSON(data []byte) error { - extra := (*struct{})(nil) - const reuseJSONRoot = false - return UnmarshalChainConfigJSON(data, c, extra, reuseJSONRoot) -} - -// UnmarshalChainConfigJSON JSON decodes `data` according to the following. -// - extra is not registered, `extra` is not nil and `reuseJSONRoot` is false: -// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. -// - extra is not registered, `extra` is not nil and `reuseJSONRoot` is true: -// `data` is decoded into `config` and `data` is decoded into `extra`. -// - extra is not registered, `extra` is nil: -// `data` is decoded into `config` and the extra, whether at the root JSON depth -// of `data` or at the "extra" JSON field, is ignored. +// UnmarshalJSON implements the [json.Unmarshaler] interface and JSON decodes +// `data` according to the following: +// - extra is not registered: +// `data` is decoded into `c` and the extra is ignored. // - extra is registered and the registered reuseJSONRoot field is false: -// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `config.extra`. +// `data` is decoded into `c` and the "extra" JSON field in `data` is decoded into `c.extra`. // - extra is registered and the registered reuseJSONRoot field is true: -// `data` is decoded into `config` and `data` is decoded into `config.extra`. -func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { +// `data` is decoded into `c` and `data` is decoded into `c.extra`. +func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { if !registeredExtras.Registered() { - return unmarshalChainConfigJSONExtraNotRegistered(data, config, extra, reuseJSONRoot) + // assume there is no extra + return json.Unmarshal(data, (*chainConfigWithoutMethods)(c)) } - return unmarshalChainConfigJSONExtraRegistered(data, config) + extraConstructors := registeredExtras.Get() + c.extra = extraConstructors.newChainConfig() + reuseJSONRoot := extraConstructors.reuseJSONRoot + return UnmarshalChainConfigJSON(data, c, c.extra, reuseJSONRoot) } -func unmarshalChainConfigJSONExtraNotRegistered[T any](data []byte, config *ChainConfig, - extra *T, reuseJSONRoot bool) (err error) { +// UnmarshalChainConfigJSON JSON decodes `data` according to the following. +// - `extra` is not nil and `reuseJSONRoot` is false: +// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. +// - `extra` is not nil and `reuseJSONRoot` is true: +// `data` is decoded into `config` and `data` is decoded into `extra`. +// - `extra` is nil: +// `data` is decoded into `config` and the extra is ignored. +func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) switch { case err != nil: @@ -75,7 +74,7 @@ func unmarshalChainConfigJSONExtraNotRegistered[T any](data []byte, config *Chai case reuseJSONRoot: err = json.Unmarshal(data, extra) if err != nil { - return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) + return fmt.Errorf("decoding extra config to %T: %s", extra, err) } default: jsonExtra := struct { @@ -91,40 +90,6 @@ func unmarshalChainConfigJSONExtraNotRegistered[T any](data []byte, config *Chai return nil } -func unmarshalChainConfigJSONExtraRegistered(data []byte, config *ChainConfig) (err error) { - chainConfigWithoutMethods := (*chainConfigWithoutMethods)(config) - - // registered extra and extra config is in the "extra" JSON field. - registeredExtraConstructors := registeredExtras.Get() - if !registeredExtraConstructors.reuseJSONRoot { - configWrapper := &chainConfigWithExportedExtra{ - chainConfigWithoutMethods: chainConfigWithoutMethods, - Extra: registeredExtraConstructors.newChainConfig(), - } - err = json.Unmarshal(data, configWrapper) - if err != nil { - return fmt.Errorf("decoding chain config and extra: %s", err) - } - config.extra = configWrapper.Extra // Note: "Extra" field, not "extra" - return nil - } - - // registered extra and re-use JSON root, the extra config is contained - // in the root config object. - err = json.Unmarshal(data, chainConfigWithoutMethods) - if err != nil { - return fmt.Errorf("decoding chain config: %s", err) - } - - config.extra = registeredExtraConstructors.newChainConfig() - err = json.Unmarshal(data, config.extra) - if err != nil { - config.extra = nil - return fmt.Errorf("decoding extra config to %T: %s", config.extra, err) - } - return nil -} - // MarshalJSON implements the [json.Marshaler] interface. func (c *ChainConfig) MarshalJSON() ([]byte, error) { switch reg := registeredExtras; { From 03e825e37ce3c38118bd4161349c5d05bfa19b4b Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 15:43:48 +0100 Subject: [PATCH 20/48] Add test cases to TestUnmarshalChainConfigJSON --- params/json.libevm_test.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 92d9dbf0397..4c60cedbec8 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -162,12 +162,18 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { expectedExtra: testExtra{}, errMessage: "decoding root chain config: unexpected end of JSON input", }, - "no_extra": { + "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: testExtra{}, }, - "wrong_extra_type": { + "no_extra_at_root_depth": { + jsonData: `{"chainId": 1}`, + reuseJSONRoot: true, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{}, + }, + "wrong_extra_type_at_extra_key": { jsonData: `{"chainId": 1, "extra": 1}`, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: testExtra{}, @@ -175,11 +181,26 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, - "extra_success": { + "wrong_extra_type_at_root_depth": { + jsonData: `{"chainId": 1, "field": 1}`, + reuseJSONRoot: true, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{}, + errMessage: "decoding extra config to *params.testExtra: " + + "json: cannot unmarshal number into Go struct field " + + "testExtra.field of type string", + }, + "extra_success_at_extra_key": { jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: testExtra{Field: "value"}, }, + "extra_success_at_root_depth": { + jsonData: `{"chainId": 1, "field":"value"}`, + reuseJSONRoot: true, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: testExtra{Field: "value"}, + }, } for name, testCase := range testCases { From 70b32e821adcad29ed1a3201bb15d1c0b29894f1 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 15:46:29 +0100 Subject: [PATCH 21/48] Make `UnmarshalChainConfigJSON` non generic to support `extra` interface arguments --- params/json.libevm.go | 4 ++-- params/json.libevm_test.go | 33 +++++++++++++++++++++++---------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 84995ae6705..79a4bc41c8d 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -65,7 +65,7 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // `data` is decoded into `config` and `data` is decoded into `extra`. // - `extra` is nil: // `data` is decoded into `config` and the extra is ignored. -func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { +func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any, reuseJSONRoot bool) (err error) { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) switch { case err != nil: @@ -78,7 +78,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, } default: jsonExtra := struct { - Extra *T `json:"extra"` + Extra any `json:"extra"` }{ Extra: extra, } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 4c60cedbec8..aa87883524b 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -153,53 +153,67 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { testCases := map[string]struct { jsonData string // string for convenience + extra any reuseJSONRoot bool expectedConfig ChainConfig expectedExtra any errMessage string }{ "invalid_json": { - expectedExtra: testExtra{}, + extra: &testExtra{}, + expectedExtra: &testExtra{}, errMessage: "decoding root chain config: unexpected end of JSON input", }, + "nil_extra": { + jsonData: `{"chainId": 1}`, + extra: &testExtra{}, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: &testExtra{}, + }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, + extra: &testExtra{}, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{}, + expectedExtra: &testExtra{}, }, "no_extra_at_root_depth": { jsonData: `{"chainId": 1}`, + extra: &testExtra{}, reuseJSONRoot: true, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{}, + expectedExtra: &testExtra{}, }, "wrong_extra_type_at_extra_key": { jsonData: `{"chainId": 1, "extra": 1}`, + extra: &testExtra{}, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{}, + expectedExtra: &testExtra{}, errMessage: "decoding extra config to *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, "wrong_extra_type_at_root_depth": { jsonData: `{"chainId": 1, "field": 1}`, + extra: &testExtra{}, reuseJSONRoot: true, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{}, + expectedExtra: &testExtra{}, errMessage: "decoding extra config to *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + "testExtra.field of type string", }, "extra_success_at_extra_key": { jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, + extra: &testExtra{}, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{Field: "value"}, + expectedExtra: &testExtra{Field: "value"}, }, "extra_success_at_root_depth": { jsonData: `{"chainId": 1, "field":"value"}`, + extra: &testExtra{}, reuseJSONRoot: true, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: testExtra{Field: "value"}, + expectedExtra: &testExtra{Field: "value"}, }, } @@ -210,15 +224,14 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { data := []byte(testCase.jsonData) config := ChainConfig{} - var extra testExtra - err := UnmarshalChainConfigJSON(data, &config, &extra, testCase.reuseJSONRoot) + err := UnmarshalChainConfigJSON(data, &config, testCase.extra, testCase.reuseJSONRoot) if testCase.errMessage == "" { require.NoError(t, err) } else { require.EqualError(t, err, testCase.errMessage) } assert.Equal(t, testCase.expectedConfig, config) - assert.Equal(t, testCase.expectedExtra, extra) + assert.Equal(t, testCase.expectedExtra, testCase.extra) }) } } From bf4557f94f0ca11f52ad79d620b2102e7af0a266 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Fri, 20 Dec 2024 17:29:44 +0100 Subject: [PATCH 22/48] Revert function to be generic --- params/json.libevm.go | 4 ++-- params/json.libevm_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 79a4bc41c8d..84995ae6705 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -65,7 +65,7 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // `data` is decoded into `config` and `data` is decoded into `extra`. // - `extra` is nil: // `data` is decoded into `config` and the extra is ignored. -func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any, reuseJSONRoot bool) (err error) { +func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) switch { case err != nil: @@ -78,7 +78,7 @@ func UnmarshalChainConfigJSON(data []byte, config *ChainConfig, extra any, reuse } default: jsonExtra := struct { - Extra any `json:"extra"` + Extra *T `json:"extra"` }{ Extra: extra, } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index aa87883524b..de18c7ad4a9 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -153,7 +153,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { testCases := map[string]struct { jsonData string // string for convenience - extra any + extra *testExtra reuseJSONRoot bool expectedConfig ChainConfig expectedExtra any @@ -166,9 +166,9 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { }, "nil_extra": { jsonData: `{"chainId": 1}`, - extra: &testExtra{}, + extra: nil, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{}, + expectedExtra: (*testExtra)(nil), }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, From 51cdb56d655b8071b14bbc9e99b181da084f4017 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 09:44:05 +0100 Subject: [PATCH 23/48] Return an error if extra argument is nil --- params/json.libevm.go | 10 +++++----- params/json.libevm_test.go | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 84995ae6705..7b44aa012ac 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -59,18 +59,18 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { } // UnmarshalChainConfigJSON JSON decodes `data` according to the following. -// - `extra` is not nil and `reuseJSONRoot` is false: +// - `reuseJSONRoot` is false: // `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. -// - `extra` is not nil and `reuseJSONRoot` is true: +// - `reuseJSONRoot` is true: // `data` is decoded into `config` and `data` is decoded into `extra`. -// - `extra` is nil: -// `data` is decoded into `config` and the extra is ignored. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { + if extra == nil { + return fmt.Errorf("extra pointer argument is nil") + } err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) switch { case err != nil: return fmt.Errorf("decoding root chain config: %s", err) - case extra == nil: // ignore the extra, whether at the root JSON depth, or at the "extra" JSON field. case reuseJSONRoot: err = json.Unmarshal(data, extra) if err != nil { diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index de18c7ad4a9..593a3d1dd29 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -164,11 +164,20 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { expectedExtra: &testExtra{}, errMessage: "decoding root chain config: unexpected end of JSON input", }, - "nil_extra": { + "nil_extra_at_root_depth": { + jsonData: `{"chainId": 1}`, + extra: nil, + reuseJSONRoot: true, + expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, + expectedExtra: (*testExtra)(nil), + errMessage: "extra pointer argument is nil", + }, + "nil_extra_at_extra_key": { jsonData: `{"chainId": 1}`, extra: nil, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: (*testExtra)(nil), + errMessage: "extra pointer argument is nil", }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, From 1ca9a4da2a8937874f6dabe357fdbbcbdafafb7c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 12:32:02 +0100 Subject: [PATCH 24/48] MarshalChainConfigJSON with test --- params/json.libevm.go | 80 ++++++++++++++++++++------------------ params/json.libevm_test.go | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 37 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 7b44aa012ac..2314436787c 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -19,8 +19,6 @@ package params import ( "encoding/json" "fmt" - - "github.com/ava-labs/libevm/libevm/pseudo" ) var _ interface { @@ -32,13 +30,6 @@ var _ interface { // [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 and JSON decodes // `data` according to the following: // - extra is not registered: @@ -92,43 +83,58 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, // MarshalJSON implements the [json.Marshaler] interface. func (c *ChainConfig) MarshalJSON() ([]byte, error) { - switch reg := registeredExtras; { - case !reg.Registered(): + if !registeredExtras.Registered() { + // assume there is no extra return json.Marshal((*chainConfigWithoutMethods)(c)) + } + extraConstructors := registeredExtras.Get() + reuseJSONRoot := extraConstructors.reuseJSONRoot + return MarshalChainConfigJSON(*c, c.extra, 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 JSON encodes `config` and `extra` according to the following. +// - `reuseJSONRoot` is false: +// `config` is encoded with `extra` encoded at the "extra" JSON field. +// - `reuseJSONRoot` is true: +// `config` is encoded with `extra` encoded at the root depth of the JSON object. +func MarshalChainConfigJSON[T any](config ChainConfig, extra T, reuseJSONRoot bool) (data []byte, err error) { + if !reuseJSONRoot { + jsonExtra := struct { + ChainConfig + Extra T `json:"extra,omitempty"` + }{ + ChainConfig: config, + Extra: extra, } - extra, err := toJSONRawMessages(c.extra) + data, err = json.Marshal(jsonExtra) if err != nil { - return nil, err + return nil, fmt.Errorf("encoding config with extra: %s", 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) + } + 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) { diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 593a3d1dd29..54b16d0653f 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -244,3 +244,76 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { }) } } + +func TestMarshalChainConfigJSON(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + config ChainConfig + extra any + reuseJSONRoot bool + jsonData string // string for convenience + errMessage string + }{ + "invalid_extra_at_extra_key": { + extra: struct { + Field chan struct{} `json:"field"` + }{}, + errMessage: "encoding config with extra: " + + "json: unsupported type: chan struct {}", + }, + "nil_extra_at_extra_key": { + jsonData: `{"chainId":null}`, + }, + "extra_at_extra_key": { + extra: struct { + Field string `json:"field"` + }{Field: "value"}, + jsonData: `{"chainId":null,"extra":{"field":"value"}}`, + }, + "invalid_extra_at_root_depth": { + extra: struct { + Field chan struct{} `json:"field"` + }{}, + reuseJSONRoot: true, + errMessage: "converting extra config to JSON raw messages: " + + "json: unsupported type: chan struct {}", + }, + "duplicate_key": { + extra: struct { + Field string `json:"chainId"` + }{}, + reuseJSONRoot: true, + errMessage: `duplicate JSON key "chainId" in ChainConfig` + + ` and extra struct { Field string "json:\"chainId\"" }`, + }, + "nil_extra_at_root_depth": { + extra: nil, + reuseJSONRoot: true, + jsonData: `{"chainId":null}`, + }, + "extra_at_root_depth": { + extra: struct { + Field string `json:"field"` + }{}, + reuseJSONRoot: true, + jsonData: `{"chainId":null,"field":""}`, + }, + } + + 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.errMessage == "" { + require.NoError(t, err) + } else { + require.EqualError(t, err, testCase.errMessage) + } + assert.Equal(t, testCase.jsonData, string(data)) + }) + } +} From a9cd1d011e9a5aa905ddeb42f0596a1ddeaebe39 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 12:46:48 +0100 Subject: [PATCH 25/48] Update ChainConfig MarshalJSON comment --- params/json.libevm.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 2314436787c..b3bb7945bb5 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -81,7 +81,14 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, return nil } -// MarshalJSON implements the [json.Marshaler] interface. +// MarshalJSON implements the [json.Marshaler] interface and JSON encodes +// the chain config `c` according to the following: +// - extra is not registered: +// `c` is encoded into `data` and the extra is ignored. +// - extra is registered and the registered reuseJSONRoot field is false: +// `c` is encoded with `c.extra` encoded at the "extra" JSON field. +// - extra is registered and the registered reuseJSONRoot field is true: +// `c` is encoded with `c.extra` encoded at the root depth of the JSON object. func (c *ChainConfig) MarshalJSON() ([]byte, error) { if !registeredExtras.Registered() { // assume there is no extra From 86d9d03e43de4acc3522b4a590236451ceec715d Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 12:47:06 +0100 Subject: [PATCH 26/48] Remove unneeded comments --- params/json.libevm.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index b3bb7945bb5..1627b3f1ac0 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -40,7 +40,6 @@ type chainConfigWithoutMethods ChainConfig // `data` is decoded into `c` and `data` is decoded into `c.extra`. func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { if !registeredExtras.Registered() { - // assume there is no extra return json.Unmarshal(data, (*chainConfigWithoutMethods)(c)) } extraConstructors := registeredExtras.Get() @@ -91,7 +90,6 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, // `c` is encoded with `c.extra` encoded at the root depth of the JSON object. func (c *ChainConfig) MarshalJSON() ([]byte, error) { if !registeredExtras.Registered() { - // assume there is no extra return json.Marshal((*chainConfigWithoutMethods)(c)) } extraConstructors := registeredExtras.Get() From b8beaa937d7f0ab08a20a66122f76cf0550bb425 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:11:44 +0100 Subject: [PATCH 27/48] style: inline variables --- params/json.libevm.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 1627b3f1ac0..49e144aa070 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -44,8 +44,7 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { } extraConstructors := registeredExtras.Get() c.extra = extraConstructors.newChainConfig() - reuseJSONRoot := extraConstructors.reuseJSONRoot - return UnmarshalChainConfigJSON(data, c, c.extra, reuseJSONRoot) + return UnmarshalChainConfigJSON(data, c, c.extra, extraConstructors.reuseJSONRoot) } // UnmarshalChainConfigJSON JSON decodes `data` according to the following. @@ -93,8 +92,7 @@ func (c *ChainConfig) MarshalJSON() ([]byte, error) { return json.Marshal((*chainConfigWithoutMethods)(c)) } extraConstructors := registeredExtras.Get() - reuseJSONRoot := extraConstructors.reuseJSONRoot - return MarshalChainConfigJSON(*c, c.extra, reuseJSONRoot) + return MarshalChainConfigJSON(*c, c.extra, extraConstructors.reuseJSONRoot) } // MarshalChainConfigJSON JSON encodes `config` and `extra` according to the following. From f13f0f4a6fa66f535022ba707b9fd1346df05bd9 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:12:09 +0100 Subject: [PATCH 28/48] docs: fix typo in comment --- params/json.libevm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 49e144aa070..a642b7050f3 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -26,7 +26,7 @@ var _ interface { json.Unmarshaler } = (*ChainConfig)(nil) -// chainConfigWithoutMethods avoids infinite recurion into +// chainConfigWithoutMethods avoids infinite recursion into // [ChainConfig.UnmarshalJSON]. type chainConfigWithoutMethods ChainConfig From 881dd0a11270820ca75e275e2b20ebd901e8ae51 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:19:53 +0100 Subject: [PATCH 29/48] hotfix: fix TestUnmarshalChainConfigJSON --- params/json.libevm_test.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 54b16d0653f..c3e0692c9e5 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -165,19 +165,17 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { errMessage: "decoding root chain config: unexpected end of JSON input", }, "nil_extra_at_root_depth": { - jsonData: `{"chainId": 1}`, - extra: nil, - reuseJSONRoot: true, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: (*testExtra)(nil), - errMessage: "extra pointer argument is nil", + jsonData: `{"chainId": 1}`, + extra: nil, + reuseJSONRoot: true, + expectedExtra: (*testExtra)(nil), + errMessage: "extra pointer argument is nil", }, "nil_extra_at_extra_key": { - jsonData: `{"chainId": 1}`, - extra: nil, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: (*testExtra)(nil), - errMessage: "extra pointer argument is nil", + jsonData: `{"chainId": 1}`, + extra: nil, + expectedExtra: (*testExtra)(nil), + errMessage: "extra pointer argument is nil", }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, From ac3c989bdf90b9dbac4071c312bc303b864ec4cf Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:20:59 +0100 Subject: [PATCH 30/48] UnmarshalChainConfigJSON decodes only once for re-use root case --- params/json.libevm.go | 24 +++++++++++++----------- params/json.libevm_test.go | 5 +++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index a642b7050f3..5c1bab01bd9 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -53,27 +53,29 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // - `reuseJSONRoot` is true: // `data` is decoded into `config` and `data` is decoded into `extra`. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { - if extra == nil { - return fmt.Errorf("extra pointer argument is nil") - } - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) switch { - case err != nil: - return fmt.Errorf("decoding root chain config: %s", err) + case extra == nil: + return fmt.Errorf("extra pointer argument is nil") case reuseJSONRoot: + err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) + if err != nil { + return fmt.Errorf("decoding root chain config: %s", err) + } err = json.Unmarshal(data, extra) if err != nil { return fmt.Errorf("decoding extra config to %T: %s", extra, err) } - default: - jsonExtra := struct { + case !reuseJSONRoot: + combined := struct { + *chainConfigWithoutMethods Extra *T `json:"extra"` }{ - Extra: extra, + chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), + Extra: extra, } - err = json.Unmarshal(data, &jsonExtra) + err = json.Unmarshal(data, &combined) if err != nil { - return fmt.Errorf("decoding extra config to %T: %s", extra, err) + return fmt.Errorf("decoding config to combined of chain config and %T: %s", extra, err) } } return nil diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index c3e0692c9e5..b63298c3f25 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -162,7 +162,8 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { "invalid_json": { extra: &testExtra{}, expectedExtra: &testExtra{}, - errMessage: "decoding root chain config: unexpected end of JSON input", + errMessage: "decoding config to combined of chain config and *params.testExtra: " + + "unexpected end of JSON input", }, "nil_extra_at_root_depth": { jsonData: `{"chainId": 1}`, @@ -195,7 +196,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { extra: &testExtra{}, expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, expectedExtra: &testExtra{}, - errMessage: "decoding extra config to *params.testExtra: " + + errMessage: "decoding config to combined of chain config and *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, From 725250db1a93dc851673fd25b1e3b6621c3fc71b Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:22:57 +0100 Subject: [PATCH 31/48] Use want* for test case fields --- params/json.libevm_test.go | 126 ++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index b63298c3f25..3b18daf8101 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -155,73 +155,73 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { jsonData string // string for convenience extra *testExtra reuseJSONRoot bool - expectedConfig ChainConfig - expectedExtra any - errMessage string + wantConfig ChainConfig + wantExtra any + wantErrMessage string }{ "invalid_json": { - extra: &testExtra{}, - expectedExtra: &testExtra{}, - errMessage: "decoding config to combined of chain config and *params.testExtra: " + + extra: &testExtra{}, + wantExtra: &testExtra{}, + wantErrMessage: "decoding config to combined of chain config and *params.testExtra: " + "unexpected end of JSON input", }, "nil_extra_at_root_depth": { - jsonData: `{"chainId": 1}`, - extra: nil, - reuseJSONRoot: true, - expectedExtra: (*testExtra)(nil), - errMessage: "extra pointer argument is nil", + jsonData: `{"chainId": 1}`, + extra: nil, + reuseJSONRoot: true, + wantExtra: (*testExtra)(nil), + wantErrMessage: "extra pointer argument is nil", }, "nil_extra_at_extra_key": { - jsonData: `{"chainId": 1}`, - extra: nil, - expectedExtra: (*testExtra)(nil), - errMessage: "extra pointer argument is nil", + jsonData: `{"chainId": 1}`, + extra: nil, + wantExtra: (*testExtra)(nil), + wantErrMessage: "extra pointer argument is nil", }, "no_extra_at_extra_key": { - jsonData: `{"chainId": 1}`, - extra: &testExtra{}, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{}, + jsonData: `{"chainId": 1}`, + extra: &testExtra{}, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{}, }, "no_extra_at_root_depth": { - jsonData: `{"chainId": 1}`, - extra: &testExtra{}, - reuseJSONRoot: true, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{}, + jsonData: `{"chainId": 1}`, + extra: &testExtra{}, + reuseJSONRoot: true, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{}, }, "wrong_extra_type_at_extra_key": { - jsonData: `{"chainId": 1, "extra": 1}`, - extra: &testExtra{}, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{}, - errMessage: "decoding config to combined of chain config and *params.testExtra: " + + jsonData: `{"chainId": 1, "extra": 1}`, + extra: &testExtra{}, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{}, + wantErrMessage: "decoding config to combined of chain config and *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, "wrong_extra_type_at_root_depth": { - jsonData: `{"chainId": 1, "field": 1}`, - extra: &testExtra{}, - reuseJSONRoot: true, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{}, - errMessage: "decoding extra config to *params.testExtra: " + + jsonData: `{"chainId": 1, "field": 1}`, + extra: &testExtra{}, + reuseJSONRoot: true, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{}, + wantErrMessage: "decoding extra config to *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + "testExtra.field of type string", }, "extra_success_at_extra_key": { - jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, - extra: &testExtra{}, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{Field: "value"}, + jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, + extra: &testExtra{}, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{Field: "value"}, }, "extra_success_at_root_depth": { - jsonData: `{"chainId": 1, "field":"value"}`, - extra: &testExtra{}, - reuseJSONRoot: true, - expectedConfig: ChainConfig{ChainID: big.NewInt(1)}, - expectedExtra: &testExtra{Field: "value"}, + jsonData: `{"chainId": 1, "field":"value"}`, + extra: &testExtra{}, + reuseJSONRoot: true, + wantConfig: ChainConfig{ChainID: big.NewInt(1)}, + wantExtra: &testExtra{Field: "value"}, }, } @@ -233,13 +233,13 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { data := []byte(testCase.jsonData) config := ChainConfig{} err := UnmarshalChainConfigJSON(data, &config, testCase.extra, testCase.reuseJSONRoot) - if testCase.errMessage == "" { + if testCase.wantErrMessage == "" { require.NoError(t, err) } else { - require.EqualError(t, err, testCase.errMessage) + require.EqualError(t, err, testCase.wantErrMessage) } - assert.Equal(t, testCase.expectedConfig, config) - assert.Equal(t, testCase.expectedExtra, testCase.extra) + assert.Equal(t, testCase.wantConfig, config) + assert.Equal(t, testCase.wantExtra, testCase.extra) }) } } @@ -248,34 +248,34 @@ func TestMarshalChainConfigJSON(t *testing.T) { t.Parallel() testCases := map[string]struct { - config ChainConfig - extra any - reuseJSONRoot bool - jsonData string // string for convenience - errMessage string + config ChainConfig + extra any + reuseJSONRoot bool + wantJSONData string // string for convenience + wantErrMessage string }{ "invalid_extra_at_extra_key": { extra: struct { Field chan struct{} `json:"field"` }{}, - errMessage: "encoding config with extra: " + + wantErrMessage: "encoding config with extra: " + "json: unsupported type: chan struct {}", }, "nil_extra_at_extra_key": { - jsonData: `{"chainId":null}`, + wantJSONData: `{"chainId":null}`, }, "extra_at_extra_key": { extra: struct { Field string `json:"field"` }{Field: "value"}, - jsonData: `{"chainId":null,"extra":{"field":"value"}}`, + wantJSONData: `{"chainId":null,"extra":{"field":"value"}}`, }, "invalid_extra_at_root_depth": { extra: struct { Field chan struct{} `json:"field"` }{}, reuseJSONRoot: true, - errMessage: "converting extra config to JSON raw messages: " + + wantErrMessage: "converting extra config to JSON raw messages: " + "json: unsupported type: chan struct {}", }, "duplicate_key": { @@ -283,20 +283,20 @@ func TestMarshalChainConfigJSON(t *testing.T) { Field string `json:"chainId"` }{}, reuseJSONRoot: true, - errMessage: `duplicate JSON key "chainId" in ChainConfig` + + wantErrMessage: `duplicate JSON key "chainId" in ChainConfig` + ` and extra struct { Field string "json:\"chainId\"" }`, }, "nil_extra_at_root_depth": { extra: nil, reuseJSONRoot: true, - jsonData: `{"chainId":null}`, + wantJSONData: `{"chainId":null}`, }, "extra_at_root_depth": { extra: struct { Field string `json:"field"` }{}, reuseJSONRoot: true, - jsonData: `{"chainId":null,"field":""}`, + wantJSONData: `{"chainId":null,"field":""}`, }, } @@ -307,12 +307,12 @@ func TestMarshalChainConfigJSON(t *testing.T) { config := ChainConfig{} data, err := MarshalChainConfigJSON(config, testCase.extra, testCase.reuseJSONRoot) - if testCase.errMessage == "" { + if testCase.wantErrMessage == "" { require.NoError(t, err) } else { - require.EqualError(t, err, testCase.errMessage) + require.EqualError(t, err, testCase.wantErrMessage) } - assert.Equal(t, testCase.jsonData, string(data)) + assert.Equal(t, testCase.wantJSONData, string(data)) }) } } From afd74769783002bc8273c81b94038810b6feee6d Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:31:53 +0100 Subject: [PATCH 32/48] style: UnmarshalChainConfigJSON does not use switch anymore --- params/json.libevm.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 5c1bab01bd9..4bcc9d8829c 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -53,10 +53,11 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // - `reuseJSONRoot` is true: // `data` is decoded into `config` and `data` is decoded into `extra`. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { - switch { - case extra == nil: + if extra == nil { return fmt.Errorf("extra pointer argument is nil") - case reuseJSONRoot: + } + + if reuseJSONRoot { err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) if err != nil { return fmt.Errorf("decoding root chain config: %s", err) @@ -65,18 +66,19 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, if err != nil { return fmt.Errorf("decoding extra config to %T: %s", extra, err) } - case !reuseJSONRoot: - combined := struct { - *chainConfigWithoutMethods - Extra *T `json:"extra"` - }{ - chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), - Extra: extra, - } - err = json.Unmarshal(data, &combined) - if err != nil { - return fmt.Errorf("decoding config to combined of chain config and %T: %s", extra, err) - } + return nil + } + + combined := struct { + *chainConfigWithoutMethods + Extra *T `json:"extra"` + }{ + chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), + Extra: extra, + } + err = json.Unmarshal(data, &combined) + if err != nil { + return fmt.Errorf("decoding config to combined of chain config and %T: %s", extra, err) } return nil } From 3120a53bfc84b03fdb32c9187ce59a046d2507ce Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 23 Dec 2024 13:54:18 +0100 Subject: [PATCH 33/48] `extraConstructors` -> `ec` --- params/json.libevm.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 4bcc9d8829c..b3505b3bca3 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -42,9 +42,9 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { if !registeredExtras.Registered() { return json.Unmarshal(data, (*chainConfigWithoutMethods)(c)) } - extraConstructors := registeredExtras.Get() - c.extra = extraConstructors.newChainConfig() - return UnmarshalChainConfigJSON(data, c, c.extra, extraConstructors.reuseJSONRoot) + ec := registeredExtras.Get() + c.extra = ec.newChainConfig() + return UnmarshalChainConfigJSON(data, c, c.extra, ec.reuseJSONRoot) } // UnmarshalChainConfigJSON JSON decodes `data` according to the following. @@ -95,8 +95,8 @@ func (c *ChainConfig) MarshalJSON() ([]byte, error) { if !registeredExtras.Registered() { return json.Marshal((*chainConfigWithoutMethods)(c)) } - extraConstructors := registeredExtras.Get() - return MarshalChainConfigJSON(*c, c.extra, extraConstructors.reuseJSONRoot) + ec := registeredExtras.Get() + return MarshalChainConfigJSON(*c, c.extra, ec.reuseJSONRoot) } // MarshalChainConfigJSON JSON encodes `config` and `extra` according to the following. From 5c3975fe189621f219b18e41a8c719c441b3a2a1 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 6 Jan 2025 13:56:42 +0100 Subject: [PATCH 34/48] Improved error wrapping for nil extra argument to `UnmarshalChainConfigJSON` Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- params/json.libevm.go | 2 +- params/json.libevm_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index b3505b3bca3..d9c6140c68a 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -54,7 +54,7 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // `data` is decoded into `config` and `data` is decoded into `extra`. func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { if extra == nil { - return fmt.Errorf("extra pointer argument is nil") + return fmt.Errorf("%T argument is nil; use %T.UnmarshalJSON() directly", extra, config) } if reuseJSONRoot { diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 3b18daf8101..e07c77e1336 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -170,13 +170,13 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { extra: nil, reuseJSONRoot: true, wantExtra: (*testExtra)(nil), - wantErrMessage: "extra pointer argument is nil", + wantErrMessage: "*params.testExtra argument is nil; use *params.ChainConfig.UnmarshalJSON() directly", }, "nil_extra_at_extra_key": { jsonData: `{"chainId": 1}`, extra: nil, wantExtra: (*testExtra)(nil), - wantErrMessage: "extra pointer argument is nil", + wantErrMessage: "*params.testExtra argument is nil; use *params.ChainConfig.UnmarshalJSON() directly", }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, From 496c608bfee43999ba11efda77cb0e2205a246b1 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 13:49:47 +0100 Subject: [PATCH 35/48] Use short if forms for error checking --- params/json.libevm.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index d9c6140c68a..b10d48835bb 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -58,12 +58,10 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, } if reuseJSONRoot { - err = json.Unmarshal(data, (*chainConfigWithoutMethods)(config)) - if err != nil { + if err := json.Unmarshal(data, (*chainConfigWithoutMethods)(config)); err != nil { return fmt.Errorf("decoding root chain config: %s", err) } - err = json.Unmarshal(data, extra) - if err != nil { + if err := json.Unmarshal(data, extra); err != nil { return fmt.Errorf("decoding extra config to %T: %s", extra, err) } return nil @@ -76,8 +74,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), Extra: extra, } - err = json.Unmarshal(data, &combined) - if err != nil { + if err := json.Unmarshal(data, &combined); err != nil { return fmt.Errorf("decoding config to combined of chain config and %T: %s", extra, err) } return nil From 98164b0f56412a9f1dc17df8126d65b239607ec7 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 13:55:45 +0100 Subject: [PATCH 36/48] Use `C` instead of `T` for extra generic type --- params/json.libevm.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index b10d48835bb..eba42ff2736 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -52,7 +52,7 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { // `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. // - `reuseJSONRoot` is true: // `data` is decoded into `config` and `data` is decoded into `extra`. -func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, reuseJSONRoot bool) (err error) { +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) } @@ -69,7 +69,7 @@ func UnmarshalChainConfigJSON[T any](data []byte, config *ChainConfig, extra *T, combined := struct { *chainConfigWithoutMethods - Extra *T `json:"extra"` + Extra *C `json:"extra"` }{ chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), Extra: extra, @@ -101,11 +101,11 @@ func (c *ChainConfig) MarshalJSON() ([]byte, error) { // `config` is encoded with `extra` encoded at the "extra" JSON field. // - `reuseJSONRoot` is true: // `config` is encoded with `extra` encoded at the root depth of the JSON object. -func MarshalChainConfigJSON[T any](config ChainConfig, extra T, reuseJSONRoot bool) (data []byte, err error) { +func MarshalChainConfigJSON[C any](config ChainConfig, extra C, reuseJSONRoot bool) (data []byte, err error) { if !reuseJSONRoot { jsonExtra := struct { ChainConfig - Extra T `json:"extra,omitempty"` + Extra C `json:"extra,omitempty"` }{ ChainConfig: config, Extra: extra, From 740abb5aadb8413f87048b214b2db0ff6c3c7eac Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 14:06:01 +0100 Subject: [PATCH 37/48] Clarify comment for `UnmarshalJSON` --- params/json.libevm.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index eba42ff2736..56338ee8a8f 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -30,14 +30,10 @@ var _ interface { // [ChainConfig.UnmarshalJSON]. type chainConfigWithoutMethods ChainConfig -// UnmarshalJSON implements the [json.Unmarshaler] interface and JSON decodes -// `data` according to the following: -// - extra is not registered: -// `data` is decoded into `c` and the extra is ignored. -// - extra is registered and the registered reuseJSONRoot field is false: -// `data` is decoded into `c` and the "extra" JSON field in `data` is decoded into `c.extra`. -// - extra is registered and the registered reuseJSONRoot field is true: -// `data` is decoded into `c` and `data` is decoded into `c.extra`. +// 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)) From d1bea234da1ee441c8070cc4dee527392f76a375 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 14:11:51 +0100 Subject: [PATCH 38/48] Clarify comment for `UnmarshalChainConfigJSON` --- params/json.libevm.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 56338ee8a8f..16c941793ac 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -43,11 +43,9 @@ func (c *ChainConfig) UnmarshalJSON(data []byte) (err error) { return UnmarshalChainConfigJSON(data, c, c.extra, ec.reuseJSONRoot) } -// UnmarshalChainConfigJSON JSON decodes `data` according to the following. -// - `reuseJSONRoot` is false: -// `data` is decoded into `config` and the "extra" JSON field in `data` is decoded into `extra`. -// - `reuseJSONRoot` is true: -// `data` is decoded into `config` and `data` is decoded into `extra`. +// 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) From ec81c2181693bfcf42aee0200c1dc0014ae9fac6 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 14:33:16 +0100 Subject: [PATCH 39/48] Improve error wrappings --- params/json.libevm.go | 8 ++++---- params/json.libevm_test.go | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 16c941793ac..030d3e1100f 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -53,10 +53,10 @@ func UnmarshalChainConfigJSON[C any](data []byte, config *ChainConfig, extra *C, if reuseJSONRoot { if err := json.Unmarshal(data, (*chainConfigWithoutMethods)(config)); err != nil { - return fmt.Errorf("decoding root chain config: %s", err) + return fmt.Errorf("decoding JSON into %T: %s", config, err) } if err := json.Unmarshal(data, extra); err != nil { - return fmt.Errorf("decoding extra config to %T: %s", extra, err) + return fmt.Errorf("decoding JSON into %T: %s", extra, err) } return nil } @@ -69,7 +69,7 @@ func UnmarshalChainConfigJSON[C any](data []byte, config *ChainConfig, extra *C, Extra: extra, } if err := json.Unmarshal(data, &combined); err != nil { - return fmt.Errorf("decoding config to combined of chain config and %T: %s", extra, err) + return fmt.Errorf(`decoding JSON into combination of %T and %T (as "extra" key): %s`, config, extra, err) } return nil } @@ -106,7 +106,7 @@ func MarshalChainConfigJSON[C any](config ChainConfig, extra C, reuseJSONRoot bo } data, err = json.Marshal(jsonExtra) if err != nil { - return nil, fmt.Errorf("encoding config with extra: %s", err) + return nil, fmt.Errorf(`encoding combination of %T and %T (as "extra" key) to JSON: %s`, config, extra, err) } return data, nil } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index e07c77e1336..ac93af3c80c 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -162,7 +162,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { "invalid_json": { extra: &testExtra{}, wantExtra: &testExtra{}, - wantErrMessage: "decoding config to combined of chain config and *params.testExtra: " + + wantErrMessage: `decoding JSON into combination of *params.ChainConfig and *params.testExtra (as "extra" key): ` + "unexpected end of JSON input", }, "nil_extra_at_root_depth": { @@ -196,7 +196,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { extra: &testExtra{}, wantConfig: ChainConfig{ChainID: big.NewInt(1)}, wantExtra: &testExtra{}, - wantErrMessage: "decoding config to combined of chain config and *params.testExtra: " + + wantErrMessage: `decoding JSON into combination of *params.ChainConfig and *params.testExtra (as "extra" key): ` + "json: cannot unmarshal number into Go struct field " + ".extra of type params.testExtra", }, @@ -206,7 +206,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { reuseJSONRoot: true, wantConfig: ChainConfig{ChainID: big.NewInt(1)}, wantExtra: &testExtra{}, - wantErrMessage: "decoding extra config to *params.testExtra: " + + wantErrMessage: "decoding JSON into *params.testExtra: " + "json: cannot unmarshal number into Go struct field " + "testExtra.field of type string", }, @@ -258,7 +258,8 @@ func TestMarshalChainConfigJSON(t *testing.T) { extra: struct { Field chan struct{} `json:"field"` }{}, - wantErrMessage: "encoding config with extra: " + + wantErrMessage: "encoding combination of params.ChainConfig and " + + `struct { Field chan struct {} "json:\"field\"" } (as "extra" key) to JSON: ` + "json: unsupported type: chan struct {}", }, "nil_extra_at_extra_key": { From 5871a771cc16dc70eaadf2b200e8e9fd71b566c4 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 15:19:07 +0100 Subject: [PATCH 40/48] Simplify comment for `MarshalJSON` --- params/json.libevm.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 030d3e1100f..f5cd22cfb5b 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -74,14 +74,10 @@ func UnmarshalChainConfigJSON[C any](data []byte, config *ChainConfig, extra *C, return nil } -// MarshalJSON implements the [json.Marshaler] interface and JSON encodes -// the chain config `c` according to the following: -// - extra is not registered: -// `c` is encoded into `data` and the extra is ignored. -// - extra is registered and the registered reuseJSONRoot field is false: -// `c` is encoded with `c.extra` encoded at the "extra" JSON field. -// - extra is registered and the registered reuseJSONRoot field is true: -// `c` is encoded with `c.extra` encoded at the root depth of the JSON object. +// 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) { if !registeredExtras.Registered() { return json.Marshal((*chainConfigWithoutMethods)(c)) From 7d7aca13509b81d907370a464b938785b5488ef6 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 15:21:03 +0100 Subject: [PATCH 41/48] Comment simplified for `MarshalChainConfigJSON` --- params/json.libevm.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index f5cd22cfb5b..f7ac23dda3e 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -86,11 +86,9 @@ func (c *ChainConfig) MarshalJSON() ([]byte, error) { return MarshalChainConfigJSON(*c, c.extra, ec.reuseJSONRoot) } -// MarshalChainConfigJSON JSON encodes `config` and `extra` according to the following. -// - `reuseJSONRoot` is false: -// `config` is encoded with `extra` encoded at the "extra" JSON field. -// - `reuseJSONRoot` is true: -// `config` is encoded with `extra` encoded at the root depth of the JSON object. +// 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 { From c9094dd6fa0e73de35d6193815c8feeaad4fd182 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 6 Jan 2025 15:23:37 +0100 Subject: [PATCH 42/48] Don't use field names for inlined types in `UnmarshalChainConfigJSON` Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- params/json.libevm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index f7ac23dda3e..9391b822032 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -65,8 +65,8 @@ func UnmarshalChainConfigJSON[C any](data []byte, config *ChainConfig, extra *C, *chainConfigWithoutMethods Extra *C `json:"extra"` }{ - chainConfigWithoutMethods: (*chainConfigWithoutMethods)(config), - Extra: extra, + (*chainConfigWithoutMethods)(config), + extra, } 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) From 76f9928f4ab44ff05b2a32ec83dc925f22d9e7eb Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 6 Jan 2025 15:23:59 +0100 Subject: [PATCH 43/48] Don't use field names for inlined types in `MarshalChainConfigJSON` Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Quentin McGaw --- params/json.libevm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 9391b822032..7cb844a65bb 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -95,8 +95,8 @@ func MarshalChainConfigJSON[C any](config ChainConfig, extra C, reuseJSONRoot bo ChainConfig Extra C `json:"extra,omitempty"` }{ - ChainConfig: config, - Extra: extra, + config, + extra, } data, err = json.Marshal(jsonExtra) if err != nil { From 3e068f5cd7e20bd9381281ec4ac3110677fda5ee Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 15:43:36 +0100 Subject: [PATCH 44/48] Variable renamings in `TestChainConfigJSONRoundTrip` --- params/json.libevm_test.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index ac93af3c80c..16c1b8208b0 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -41,10 +41,10 @@ type rootJSONChainConfigExtra struct { func TestChainConfigJSONRoundTrip(t *testing.T) { tests := []struct { - name string - register func() - jsonInput string - want *ChainConfig + name string + register func() + jsonInput string + wantDecoded *ChainConfig }{ { name: "no registered extras", @@ -52,7 +52,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { jsonInput: `{ "chainId": 1234 }`, - want: &ChainConfig{ + wantDecoded: &ChainConfig{ ChainID: big.NewInt(1234), }, }, @@ -67,7 +67,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 5678, "foo": "hello" }`, - want: &ChainConfig{ + wantDecoded: &ChainConfig{ ChainID: big.NewInt(5678), extra: pseudo.From(rootJSONChainConfigExtra{TopLevelFoo: "hello"}).Type, }, @@ -83,7 +83,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 5678, "foo": "hello" }`, - want: &ChainConfig{ + wantDecoded: &ChainConfig{ ChainID: big.NewInt(5678), extra: pseudo.From(&rootJSONChainConfigExtra{TopLevelFoo: "hello"}).Type, }, @@ -99,7 +99,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 42, "extra": {"foo": "world"} }`, - want: &ChainConfig{ + wantDecoded: &ChainConfig{ ChainID: big.NewInt(42), extra: pseudo.From(nestedChainConfigExtra{NestedFoo: "world"}).Type, }, @@ -115,7 +115,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 42, "extra": {"foo": "world"} }`, - want: &ChainConfig{ + wantDecoded: &ChainConfig{ ChainID: big.NewInt(42), extra: pseudo.From(&nestedChainConfigExtra{NestedFoo: "world"}).Type, }, @@ -128,18 +128,19 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { t.Cleanup(TestOnlyClearRegisteredExtras) tt.register() - expectedEncoded := bytes.NewBuffer(nil) - err := json.Compact(expectedEncoded, []byte(tt.jsonInput)) + buffer := bytes.NewBuffer(nil) + err := json.Compact(buffer, []byte(tt.jsonInput)) require.NoError(t, err) + wantEncoded := buffer.String() - encoded, err := json.Marshal(tt.want) - require.NoError(t, err, "encoding error") - require.Equal(t, expectedEncoded.String(), string(encoded)) + gotEncoded, err := json.Marshal(tt.wantDecoded) + require.NoError(t, err, "json.Marshal()") + require.Equal(t, wantEncoded, string(gotEncoded)) - decoded := new(ChainConfig) - err = json.Unmarshal(encoded, decoded) - require.NoError(t, err, "decoding error") - require.Equal(t, tt.want, decoded) + gotDecoded := new(ChainConfig) + err = json.Unmarshal(gotEncoded, gotDecoded) + require.NoError(t, err, "json.Unmarshal()") + require.Equal(t, tt.wantDecoded, gotDecoded) }) } } From 32fe286f4686b860e57c3c07858e3c498bf7af7c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 16:04:18 +0100 Subject: [PATCH 45/48] Add error wrappings within `toJSONRawMessages` --- params/json.libevm.go | 4 ++-- params/json.libevm_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/params/json.libevm.go b/params/json.libevm.go index 7cb844a65bb..639bfe18f67 100644 --- a/params/json.libevm.go +++ b/params/json.libevm.go @@ -132,11 +132,11 @@ func MarshalChainConfigJSON[C any](config ChainConfig, extra C, reuseJSONRoot bo 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 } diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 16c1b8208b0..2d87c9189b0 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -278,6 +278,7 @@ func TestMarshalChainConfigJSON(t *testing.T) { }{}, reuseJSONRoot: true, wantErrMessage: "converting extra config to JSON raw messages: " + + `encoding struct { Field chan struct {} "json:\"field\"" }: ` + "json: unsupported type: chan struct {}", }, "duplicate_key": { From e3d25ead3ebe035e390f6fedede4f2c1f0fa3233 Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 16:32:00 +0100 Subject: [PATCH 46/48] Use regex for error messages expectations in tests --- params/json.libevm_test.go | 84 +++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 2d87c9189b0..26630213245 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -153,31 +153,30 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { } testCases := map[string]struct { - jsonData string // string for convenience - extra *testExtra - reuseJSONRoot bool - wantConfig ChainConfig - wantExtra any - wantErrMessage string + jsonData string // string for convenience + extra *testExtra + reuseJSONRoot bool + wantConfig ChainConfig + wantExtra any + wantErrRegex string }{ "invalid_json": { - extra: &testExtra{}, - wantExtra: &testExtra{}, - wantErrMessage: `decoding JSON into combination of *params.ChainConfig and *params.testExtra (as "extra" key): ` + - "unexpected end of JSON input", + 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), - wantErrMessage: "*params.testExtra argument is nil; use *params.ChainConfig.UnmarshalJSON() directly", + 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), - wantErrMessage: "*params.testExtra argument is nil; use *params.ChainConfig.UnmarshalJSON() directly", + jsonData: `{"chainId": 1}`, + extra: nil, + wantExtra: (*testExtra)(nil), + wantErrRegex: `^\*.+\.testExtra argument is nil; use \*.+\.ChainConfig.UnmarshalJSON\(\) directly$`, }, "no_extra_at_extra_key": { jsonData: `{"chainId": 1}`, @@ -193,13 +192,11 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { wantExtra: &testExtra{}, }, "wrong_extra_type_at_extra_key": { - jsonData: `{"chainId": 1, "extra": 1}`, - extra: &testExtra{}, - wantConfig: ChainConfig{ChainID: big.NewInt(1)}, - wantExtra: &testExtra{}, - wantErrMessage: `decoding JSON into combination of *params.ChainConfig and *params.testExtra (as "extra" key): ` + - "json: cannot unmarshal number into Go struct field " + - ".extra of type params.testExtra", + 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}`, @@ -207,9 +204,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { reuseJSONRoot: true, wantConfig: ChainConfig{ChainID: big.NewInt(1)}, wantExtra: &testExtra{}, - wantErrMessage: "decoding JSON into *params.testExtra: " + - "json: cannot unmarshal number into Go struct field " + - "testExtra.field of type string", + wantErrRegex: `^decoding JSON into \*.+\.testExtra: .+`, }, "extra_success_at_extra_key": { jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, @@ -234,10 +229,11 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { data := []byte(testCase.jsonData) config := ChainConfig{} err := UnmarshalChainConfigJSON(data, &config, testCase.extra, testCase.reuseJSONRoot) - if testCase.wantErrMessage == "" { + if testCase.wantErrRegex == "" { require.NoError(t, err) } else { - require.EqualError(t, err, testCase.wantErrMessage) + require.Error(t, err) + require.Regexp(t, testCase.wantErrRegex, err.Error()) } assert.Equal(t, testCase.wantConfig, config) assert.Equal(t, testCase.wantExtra, testCase.extra) @@ -249,19 +245,17 @@ func TestMarshalChainConfigJSON(t *testing.T) { t.Parallel() testCases := map[string]struct { - config ChainConfig - extra any - reuseJSONRoot bool - wantJSONData string // string for convenience - wantErrMessage string + 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"` }{}, - wantErrMessage: "encoding combination of params.ChainConfig and " + - `struct { Field chan struct {} "json:\"field\"" } (as "extra" key) to JSON: ` + - "json: unsupported type: chan struct {}", + wantErrRegex: `^encoding combination of .+\.ChainConfig and .+ to JSON: .+$`, }, "nil_extra_at_extra_key": { wantJSONData: `{"chainId":null}`, @@ -277,17 +271,14 @@ func TestMarshalChainConfigJSON(t *testing.T) { Field chan struct{} `json:"field"` }{}, reuseJSONRoot: true, - wantErrMessage: "converting extra config to JSON raw messages: " + - `encoding struct { Field chan struct {} "json:\"field\"" }: ` + - "json: unsupported type: chan struct {}", + wantErrRegex: "^converting extra config to JSON raw messages: .+$", }, "duplicate_key": { extra: struct { Field string `json:"chainId"` }{}, reuseJSONRoot: true, - wantErrMessage: `duplicate JSON key "chainId" in ChainConfig` + - ` and extra struct { Field string "json:\"chainId\"" }`, + wantErrRegex: `^duplicate JSON key "chainId" in ChainConfig and extra struct .+$`, }, "nil_extra_at_root_depth": { extra: nil, @@ -310,10 +301,11 @@ func TestMarshalChainConfigJSON(t *testing.T) { config := ChainConfig{} data, err := MarshalChainConfigJSON(config, testCase.extra, testCase.reuseJSONRoot) - if testCase.wantErrMessage == "" { + if testCase.wantErrRegex == "" { require.NoError(t, err) } else { - require.EqualError(t, err, testCase.wantErrMessage) + require.Error(t, err) + assert.Regexp(t, testCase.wantErrRegex, err.Error()) } assert.Equal(t, testCase.wantJSONData, string(data)) }) From b48fb8af5ac8f054c08580544815bbdbcc84e42c Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Mon, 6 Jan 2025 17:16:34 +0100 Subject: [PATCH 47/48] New unit tests focus on error cases only --- params/json.libevm_test.go | 48 ++------------------------------------ 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index 26630213245..b76d0fcd6c6 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -145,7 +145,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { } } -func TestUnmarshalChainConfigJSON(t *testing.T) { +func TestUnmarshalChainConfigJSON_Errors(t *testing.T) { t.Parallel() type testExtra struct { @@ -178,19 +178,6 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { wantExtra: (*testExtra)(nil), wantErrRegex: `^\*.+\.testExtra argument is nil; use \*.+\.ChainConfig.UnmarshalJSON\(\) directly$`, }, - "no_extra_at_extra_key": { - jsonData: `{"chainId": 1}`, - extra: &testExtra{}, - wantConfig: ChainConfig{ChainID: big.NewInt(1)}, - wantExtra: &testExtra{}, - }, - "no_extra_at_root_depth": { - jsonData: `{"chainId": 1}`, - extra: &testExtra{}, - reuseJSONRoot: true, - wantConfig: ChainConfig{ChainID: big.NewInt(1)}, - wantExtra: &testExtra{}, - }, "wrong_extra_type_at_extra_key": { jsonData: `{"chainId": 1, "extra": 1}`, extra: &testExtra{}, @@ -206,19 +193,6 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { wantExtra: &testExtra{}, wantErrRegex: `^decoding JSON into \*.+\.testExtra: .+`, }, - "extra_success_at_extra_key": { - jsonData: `{"chainId": 1, "extra": {"field":"value"}}`, - extra: &testExtra{}, - wantConfig: ChainConfig{ChainID: big.NewInt(1)}, - wantExtra: &testExtra{Field: "value"}, - }, - "extra_success_at_root_depth": { - jsonData: `{"chainId": 1, "field":"value"}`, - extra: &testExtra{}, - reuseJSONRoot: true, - wantConfig: ChainConfig{ChainID: big.NewInt(1)}, - wantExtra: &testExtra{Field: "value"}, - }, } for name, testCase := range testCases { @@ -241,7 +215,7 @@ func TestUnmarshalChainConfigJSON(t *testing.T) { } } -func TestMarshalChainConfigJSON(t *testing.T) { +func TestMarshalChainConfigJSON_Errors(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -260,12 +234,6 @@ func TestMarshalChainConfigJSON(t *testing.T) { "nil_extra_at_extra_key": { wantJSONData: `{"chainId":null}`, }, - "extra_at_extra_key": { - extra: struct { - Field string `json:"field"` - }{Field: "value"}, - wantJSONData: `{"chainId":null,"extra":{"field":"value"}}`, - }, "invalid_extra_at_root_depth": { extra: struct { Field chan struct{} `json:"field"` @@ -280,18 +248,6 @@ func TestMarshalChainConfigJSON(t *testing.T) { reuseJSONRoot: true, wantErrRegex: `^duplicate JSON key "chainId" in ChainConfig and extra struct .+$`, }, - "nil_extra_at_root_depth": { - extra: nil, - reuseJSONRoot: true, - wantJSONData: `{"chainId":null}`, - }, - "extra_at_root_depth": { - extra: struct { - Field string `json:"field"` - }{}, - reuseJSONRoot: true, - wantJSONData: `{"chainId":null,"field":""}`, - }, } for name, testCase := range testCases { From fa1a15bc29f4d2bcf5e4bdbfbc2698a91eb9479f Mon Sep 17 00:00:00 2001 From: Quentin Mc Gaw Date: Sun, 12 Jan 2025 15:00:13 +0100 Subject: [PATCH 48/48] Revert `TestChainConfigJSONRoundTrip` to its original state --- params/json.libevm_test.go | 41 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/params/json.libevm_test.go b/params/json.libevm_test.go index b76d0fcd6c6..84b04b35427 100644 --- a/params/json.libevm_test.go +++ b/params/json.libevm_test.go @@ -41,10 +41,10 @@ type rootJSONChainConfigExtra struct { func TestChainConfigJSONRoundTrip(t *testing.T) { tests := []struct { - name string - register func() - jsonInput string - wantDecoded *ChainConfig + name string + register func() + jsonInput string + want *ChainConfig }{ { name: "no registered extras", @@ -52,7 +52,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { jsonInput: `{ "chainId": 1234 }`, - wantDecoded: &ChainConfig{ + want: &ChainConfig{ ChainID: big.NewInt(1234), }, }, @@ -67,7 +67,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 5678, "foo": "hello" }`, - wantDecoded: &ChainConfig{ + want: &ChainConfig{ ChainID: big.NewInt(5678), extra: pseudo.From(rootJSONChainConfigExtra{TopLevelFoo: "hello"}).Type, }, @@ -83,7 +83,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 5678, "foo": "hello" }`, - wantDecoded: &ChainConfig{ + want: &ChainConfig{ ChainID: big.NewInt(5678), extra: pseudo.From(&rootJSONChainConfigExtra{TopLevelFoo: "hello"}).Type, }, @@ -99,7 +99,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 42, "extra": {"foo": "world"} }`, - wantDecoded: &ChainConfig{ + want: &ChainConfig{ ChainID: big.NewInt(42), extra: pseudo.From(nestedChainConfigExtra{NestedFoo: "world"}).Type, }, @@ -115,7 +115,7 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { "chainId": 42, "extra": {"foo": "world"} }`, - wantDecoded: &ChainConfig{ + want: &ChainConfig{ ChainID: big.NewInt(42), extra: pseudo.From(&nestedChainConfigExtra{NestedFoo: "world"}).Type, }, @@ -128,19 +128,20 @@ func TestChainConfigJSONRoundTrip(t *testing.T) { t.Cleanup(TestOnlyClearRegisteredExtras) tt.register() - buffer := bytes.NewBuffer(nil) - err := json.Compact(buffer, []byte(tt.jsonInput)) - require.NoError(t, err) - wantEncoded := buffer.String() + t.Run("json.Unmarshal()", func(t *testing.T) { + got := new(ChainConfig) + require.NoError(t, json.Unmarshal([]byte(tt.jsonInput), got)) + require.Equal(t, tt.want, got) + }) - gotEncoded, err := json.Marshal(tt.wantDecoded) - require.NoError(t, err, "json.Marshal()") - require.Equal(t, wantEncoded, string(gotEncoded)) + t.Run("json.Marshal()", func(t *testing.T) { + var want bytes.Buffer + require.NoError(t, json.Compact(&want, []byte(tt.jsonInput)), "json.Compact()") - gotDecoded := new(ChainConfig) - err = json.Unmarshal(gotEncoded, gotDecoded) - require.NoError(t, err, "json.Unmarshal()") - require.Equal(t, tt.wantDecoded, gotDecoded) + got, err := json.Marshal(tt.want) + require.NoError(t, err, "json.Marshal()") + require.Equal(t, want.String(), string(got)) + }) }) } }