Skip to content

Commit cc35af9

Browse files
committed
fix: ExtraPayloads.SetOn{ChainConfig,Rules}() overrides shallow copy
1 parent 210f8ab commit cc35af9

File tree

2 files changed

+41
-16
lines changed

2 files changed

+41
-16
lines changed

params/config.libevm.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,18 @@ func (ExtraPayloads[C, R]) FromChainConfig(c *ChainConfig) C {
169169

170170
// PointerFromChainConfig returns a pointer to the ChainConfig's extra payload.
171171
// This is guaranteed to be non-nil.
172+
//
173+
// Note that copying a ChainConfig by dereferencing a pointer will result in a
174+
// shallow copy and that the *C returned here will therefore be shared by all
175+
// copies. If this is not the desired behaviour, use
176+
// [ExtraPayloads.SetOnChainConfig].
172177
func (ExtraPayloads[C, R]) PointerFromChainConfig(c *ChainConfig) *C {
173178
return pseudo.MustPointerTo[C](c.extraPayload()).Value.Get()
174179
}
175180

176-
// SetOnChainConfig sets the ChainConfig's extra payload. It is equivalent to
177-
// `*e.PointerFromChainConfig(cc) = val`.
181+
// SetOnChainConfig sets the ChainConfig's extra payload.
178182
func (e ExtraPayloads[C, R]) SetOnChainConfig(cc *ChainConfig, val C) {
179-
*e.PointerFromChainConfig(cc) = val
183+
cc.extra = pseudo.From(val).Type
180184
}
181185

182186
// hooksFromChainConfig is equivalent to FromChainConfig(), but returns an
@@ -193,14 +197,17 @@ func (ExtraPayloads[C, R]) FromRules(r *Rules) R {
193197

194198
// PointerFromRules returns a pointer to the Rules's extra payload. This is
195199
// guaranteed to be non-nil.
200+
//
201+
// Note that copying a Rules by dereferencing a pointer will result in a shallow
202+
// copy and that the *R returned here will therefore be shared by all copies. If
203+
// this is not the desired behaviour, use [ExtraPayloads.SetOnRules].
196204
func (ExtraPayloads[C, R]) PointerFromRules(r *Rules) *R {
197205
return pseudo.MustPointerTo[R](r.extraPayload()).Value.Get()
198206
}
199207

200-
// SetOnRules sets the Rules' extra payload. It is equivalent to
201-
// `*e.PointerFromRules(r) = val`.
208+
// SetOnRules sets the Rules' extra payload.
202209
func (e ExtraPayloads[C, R]) SetOnRules(r *Rules, val R) {
203-
*e.PointerFromRules(r) = val
210+
r.extra = pseudo.From(val).Type
204211
}
205212

206213
// hooksFromRules is the [RulesHooks] equivalent of hooksFromChainConfig().

params/config.libevm_test.go

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -189,20 +189,38 @@ func TestModificationOfZeroExtras(t *testing.T) {
189189
// The test of shallow copying is now guaranteed to fail.
190190
return
191191
}
192-
t.Run("shallow copy", func(t *testing.T) {
192+
t.Run("copy", func(t *testing.T) {
193+
const (
194+
// Arbitrary test values.
195+
seqUp = 123456789
196+
seqDown = 987654321
197+
)
198+
193199
ccCopy := *config
194-
rCopy := *rules
200+
t.Run("ChainConfig", func(t *testing.T) {
201+
assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "extras copied")
195202

196-
assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "ChainConfig extras copied")
197-
assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "Rules extras copied")
203+
extras.PointerFromChainConfig(&ccCopy).X = seqUp
204+
assertChainConfigExtra(t, ccExtra{X: seqUp}, "original changed via copied.PointerFromChainConfig because copy only shallow")
198205

199-
const seqUp = 123456789
200-
extras.PointerFromChainConfig(&ccCopy).X = seqUp
201-
assertChainConfigExtra(t, ccExtra{X: seqUp}, "original changed because copy only shallow")
206+
ccReplace = ccExtra{X: seqDown}
207+
extras.SetOnChainConfig(&ccCopy, ccReplace)
208+
assert.Equal(t, extras.FromChainConfig(&ccCopy), ccReplace, "SetOnChainConfig effect")
209+
assertChainConfigExtra(t, ccExtra{X: seqUp}, "original unchanged after copied.SetOnChainConfig")
210+
})
202211

203-
const seqDown = 987654321
204-
extras.PointerFromRules(&rCopy).X = seqDown
205-
assertRulesExtra(t, rulesExtra{X: seqDown}, "original changed because copy only shallow")
212+
rCopy := *rules
213+
t.Run("Rules", func(t *testing.T) {
214+
assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "extras copied")
215+
216+
extras.PointerFromRules(&rCopy).X = seqUp
217+
assertRulesExtra(t, rulesExtra{X: seqUp}, "original changed via copied.PointerFromRuels because copy only shallow")
218+
219+
rulesReplace = rulesExtra{X: seqDown}
220+
extras.SetOnRules(&rCopy, rulesReplace)
221+
assert.Equal(t, extras.FromRules(&rCopy), rulesReplace, "SetOnRules effect")
222+
assertRulesExtra(t, rulesExtra{X: seqUp}, "original unchanged after copied.SetOnRules")
223+
})
206224
})
207225
}
208226

0 commit comments

Comments
 (0)