Skip to content

Commit

Permalink
fix: ExtraPayloads.SetOn{ChainConfig,Rules}() overrides shallow copy (
Browse files Browse the repository at this point in the history
  • Loading branch information
ARR4N authored Sep 30, 2024
1 parent 210f8ab commit 1478c18
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 16 deletions.
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

0 comments on commit 1478c18

Please sign in to comment.