From 1478c18b9a32d458a8bb8501556fea9d63ba797f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Date: Mon, 30 Sep 2024 18:24:47 +0100 Subject: [PATCH] fix: `ExtraPayloads.SetOn{ChainConfig,Rules}()` overrides shallow copy (#42) --- params/config.libevm.go | 19 ++++++++++++------ params/config.libevm_test.go | 38 ++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/params/config.libevm.go b/params/config.libevm.go index f427e4003600..0e9981bcc32e 100644 --- a/params/config.libevm.go +++ b/params/config.libevm.go @@ -169,14 +169,18 @@ func (ExtraPayloads[C, R]) FromChainConfig(c *ChainConfig) C { // PointerFromChainConfig returns a pointer to the ChainConfig's extra payload. // This is guaranteed to be non-nil. +// +// Note that copying a ChainConfig by dereferencing a pointer will result in a +// shallow copy and that the *C returned here will therefore be shared by all +// copies. If this is not the desired behaviour, use +// [ExtraPayloads.SetOnChainConfig]. func (ExtraPayloads[C, R]) PointerFromChainConfig(c *ChainConfig) *C { return pseudo.MustPointerTo[C](c.extraPayload()).Value.Get() } -// SetOnChainConfig sets the ChainConfig's extra payload. It is equivalent to -// `*e.PointerFromChainConfig(cc) = val`. +// SetOnChainConfig sets the ChainConfig's extra payload. func (e ExtraPayloads[C, R]) SetOnChainConfig(cc *ChainConfig, val C) { - *e.PointerFromChainConfig(cc) = val + cc.extra = pseudo.From(val).Type } // hooksFromChainConfig is equivalent to FromChainConfig(), but returns an @@ -193,14 +197,17 @@ func (ExtraPayloads[C, R]) FromRules(r *Rules) R { // PointerFromRules returns a pointer to the Rules's extra payload. This is // guaranteed to be non-nil. +// +// Note that copying a Rules by dereferencing a pointer will result in a shallow +// copy and that the *R returned here will therefore be shared by all copies. If +// this is not the desired behaviour, use [ExtraPayloads.SetOnRules]. func (ExtraPayloads[C, R]) PointerFromRules(r *Rules) *R { return pseudo.MustPointerTo[R](r.extraPayload()).Value.Get() } -// SetOnRules sets the Rules' extra payload. It is equivalent to -// `*e.PointerFromRules(r) = val`. +// SetOnRules sets the Rules' extra payload. func (e ExtraPayloads[C, R]) SetOnRules(r *Rules, val R) { - *e.PointerFromRules(r) = val + r.extra = pseudo.From(val).Type } // hooksFromRules is the [RulesHooks] equivalent of hooksFromChainConfig(). diff --git a/params/config.libevm_test.go b/params/config.libevm_test.go index f9faae9b115d..1d9d64296913 100644 --- a/params/config.libevm_test.go +++ b/params/config.libevm_test.go @@ -189,20 +189,38 @@ func TestModificationOfZeroExtras(t *testing.T) { // The test of shallow copying is now guaranteed to fail. return } - t.Run("shallow copy", func(t *testing.T) { + t.Run("copy", func(t *testing.T) { + const ( + // Arbitrary test values. + seqUp = 123456789 + seqDown = 987654321 + ) + ccCopy := *config - rCopy := *rules + t.Run("ChainConfig", func(t *testing.T) { + assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "extras copied") - assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "ChainConfig extras copied") - assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "Rules extras copied") + extras.PointerFromChainConfig(&ccCopy).X = seqUp + assertChainConfigExtra(t, ccExtra{X: seqUp}, "original changed via copied.PointerFromChainConfig because copy only shallow") - const seqUp = 123456789 - extras.PointerFromChainConfig(&ccCopy).X = seqUp - assertChainConfigExtra(t, ccExtra{X: seqUp}, "original changed because copy only shallow") + ccReplace = ccExtra{X: seqDown} + extras.SetOnChainConfig(&ccCopy, ccReplace) + assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "SetOnChainConfig effect") + assertChainConfigExtra(t, ccExtra{X: seqUp}, "original unchanged after copied.SetOnChainConfig") + }) - const seqDown = 987654321 - extras.PointerFromRules(&rCopy).X = seqDown - assertRulesExtra(t, rulesExtra{X: seqDown}, "original changed because copy only shallow") + rCopy := *rules + t.Run("Rules", func(t *testing.T) { + assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "extras copied") + + extras.PointerFromRules(&rCopy).X = seqUp + assertRulesExtra(t, rulesExtra{X: seqUp}, "original changed via copied.PointerFromRuels because copy only shallow") + + rulesReplace = rulesExtra{X: seqDown} + extras.SetOnRules(&rCopy, rulesReplace) + assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "SetOnRules effect") + assertRulesExtra(t, rulesExtra{X: seqUp}, "original unchanged after copied.SetOnRules") + }) }) }