Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ExtraPayloads.SetOn{ChainConfig,Rules}() overrides shallow copy #42

Merged
merged 1 commit into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions params/config.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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().
Expand Down
38 changes: 28 additions & 10 deletions params/config.libevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
})
}

Expand Down