Skip to content

Commit 1bccf4f

Browse files
authored
refactor!: temporary extras require proof of global lock (#238)
## Why this should be merged The `temporary.WithTempRegisteredExtras()` global lock introduced in #234 wasn't fit for purpose when used in `coreth` as it required central coordination of registration, types, and usage of the payload accessor. ## How this works Instead of a central registration point, the new `libevm.WithTemporaryExtrasLock()` function takes out a global lock and provides the caller with a handle that proves the lock is held. All of the override functions, e.g. `params.WithTempRegisteredExtras()` now require a current lock, which will be propagated by the respective `coreth` functions. See ava-labs/coreth#1328 for intended usage in `coreth` and `subnet-evm`. A consumer of both of these can then safely do the following: ```go import ( "github.com/ava-labs/libevm/libevm" coreth "github.com/ava-labs/coreth/plugin/evm" subnet "github.com/ava-labs/subnet-evm/plugin/evm" ) // asCChain calls `fn` while emulating `coreth`. It is safe for concurrent usage with [asSubnetEVM]. func asCChain(fn func() error) error { return libevm.WithTemporaryExtrasLock(func(l libevm.ExtrasLock) error { return coreth.WithTempRegisteredLibEVMExtras(l, fn) }) } // asSubnetEVM calls `fn` while emulating `subnet-evm`. It is safe for concurrent usage with [asCChain]. func asSubnetEVM(fn func() error) error { return libevm.WithTemporaryExtrasLock(func(l libevm.ExtrasLock) error { return subnet.WithTempRegisteredLibEVMExtras(l, fn) }) } ``` ## How this was tested Unit test of the new function plus existing integration tests of all modified code.
1 parent 51afede commit 1bccf4f

File tree

13 files changed

+213
-114
lines changed

13 files changed

+213
-114
lines changed

core/state/statedb.libevm.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"github.com/ava-labs/libevm/common"
2323
"github.com/ava-labs/libevm/core/state/snapshot"
24+
"github.com/ava-labs/libevm/libevm"
2425
"github.com/ava-labs/libevm/libevm/register"
2526
"github.com/ava-labs/libevm/libevm/stateconf"
2627
)
@@ -91,8 +92,11 @@ func RegisterExtras(s StateDBHooks) {
9192
// consumers that require access to extras. Said consumers SHOULD NOT, however
9293
// call this function directly. Use the libevm/temporary.WithRegisteredExtras()
9394
// function instead as it atomically overrides all possible packages.
94-
func WithTempRegisteredExtras(s StateDBHooks, fn func()) {
95-
registeredExtras.TempOverride(s, fn)
95+
func WithTempRegisteredExtras(lock libevm.ExtrasLock, s StateDBHooks, fn func() error) error {
96+
if err := lock.Verify(); err != nil {
97+
return err
98+
}
99+
return registeredExtras.TempOverride(s, fn)
96100
}
97101

98102
// TestOnlyClearRegisteredExtras clears the arguments previously passed to

core/state/statedb.libevm_test.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/ava-labs/libevm/core/state/snapshot"
2828
"github.com/ava-labs/libevm/core/types"
2929
"github.com/ava-labs/libevm/ethdb"
30+
"github.com/ava-labs/libevm/libevm"
3031
"github.com/ava-labs/libevm/libevm/stateconf"
3132
"github.com/ava-labs/libevm/trie"
3233
"github.com/ava-labs/libevm/trie/trienode"
@@ -216,13 +217,17 @@ func TestTransformStateKey(t *testing.T) {
216217
assertCommittedEq(t, flippedKey, flippedVal, noTransform)
217218

218219
t.Run("WithTempRegisteredExtras", func(t *testing.T) {
219-
WithTempRegisteredExtras(noopHooks{}, func() {
220-
// No-op hooks are equivalent to using the `noTransform` option.
221-
// NOTE this is NOT the intended usage of [WithTempRegisteredExtras]
222-
// and is simply an easy way to test the temporary registration.
223-
assertEq(t, regularKey, regularVal)
224-
assertEq(t, flippedKey, flippedVal)
220+
err := libevm.WithTemporaryExtrasLock(func(lock libevm.ExtrasLock) error {
221+
return WithTempRegisteredExtras(lock, noopHooks{}, func() error {
222+
// No-op hooks are equivalent to using the `noTransform` option.
223+
// NOTE this is NOT the intended usage of [WithTempRegisteredExtras]
224+
// and is simply an easy way to test the temporary registration.
225+
assertEq(t, regularKey, regularVal)
226+
assertEq(t, flippedKey, flippedVal)
227+
return nil
228+
})
225229
})
230+
require.NoError(t, err)
226231
})
227232

228233
updatedVal := common.Hash{'u', 'p', 'd', 'a', 't', 'e', 'd'}

core/types/rlp_payload.libevm.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"io"
2222

2323
"github.com/ava-labs/libevm/common"
24+
"github.com/ava-labs/libevm/libevm"
2425
"github.com/ava-labs/libevm/libevm/pseudo"
2526
"github.com/ava-labs/libevm/libevm/register"
2627
"github.com/ava-labs/libevm/libevm/testonly"
@@ -114,9 +115,12 @@ func WithTempRegisteredExtras[
114115
H, B, SA any,
115116
HPtr HeaderHooksPointer[H],
116117
BPtr BlockBodyHooksPointer[B, BPtr],
117-
](fn func(ExtraPayloads[HPtr, BPtr, SA])) {
118+
](lock libevm.ExtrasLock, fn func(ExtraPayloads[HPtr, BPtr, SA]) error) error {
119+
if err := lock.Verify(); err != nil {
120+
return err
121+
}
118122
payloads, ctors := payloadsAndConstructors[H, HPtr, B, BPtr, SA]()
119-
registeredExtras.TempOverride(ctors, func() { fn(payloads) })
123+
return registeredExtras.TempOverride(ctors, func() error { return fn(payloads) })
120124
}
121125

122126
// A HeaderHooksPointer is a type constraint for an implementation of

core/types/tempextras.libevm_test.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
"github.com/stretchr/testify/require"
2424

25+
"github.com/ava-labs/libevm/libevm"
2526
"github.com/ava-labs/libevm/rlp"
2627
)
2728

@@ -58,19 +59,23 @@ func TestTempRegisteredExtras(t *testing.T) {
5859

5960
t.Run("before_temp", testPrimaryExtras)
6061
t.Run("WithTempRegisteredExtras", func(t *testing.T) {
61-
WithTempRegisteredExtras(func(extras ExtraPayloads[*NOOPHeaderHooks, *tempBlockBodyHooks, bool]) {
62-
const val = "Hello, world"
63-
b := new(Block)
64-
payload := &tempBlockBodyHooks{X: val}
65-
extras.Block.Set(b, payload)
62+
err := libevm.WithTemporaryExtrasLock(func(lock libevm.ExtrasLock) error {
63+
return WithTempRegisteredExtras(lock, func(extras ExtraPayloads[*NOOPHeaderHooks, *tempBlockBodyHooks, bool]) error {
64+
const val = "Hello, world"
65+
b := new(Block)
66+
payload := &tempBlockBodyHooks{X: val}
67+
extras.Block.Set(b, payload)
6668

67-
got, err := rlp.EncodeToBytes(b)
68-
require.NoErrorf(t, err, "rlp.EncodeToBytes(%T) with %T hooks", b, extras.Block.Get(b))
69-
want, err := rlp.EncodeToBytes([]string{val})
70-
require.NoErrorf(t, err, "rlp.EncodeToBytes(%T{%[1]v})", []string{val})
69+
got, err := rlp.EncodeToBytes(b)
70+
require.NoErrorf(t, err, "rlp.EncodeToBytes(%T) with %T hooks", b, extras.Block.Get(b))
71+
want, err := rlp.EncodeToBytes([]string{val})
72+
require.NoErrorf(t, err, "rlp.EncodeToBytes(%T{%[1]v})", []string{val})
7173

72-
assert.Equalf(t, want, got, "rlp.EncodeToBytes(%T) with %T hooks", b, payload)
74+
assert.Equalf(t, want, got, "rlp.EncodeToBytes(%T) with %T hooks", b, payload)
75+
return nil
76+
})
7377
})
78+
require.NoError(t, err)
7479
})
7580
t.Run("after_temp", testPrimaryExtras)
7681
}

core/vm/evm.libevm_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
"github.com/stretchr/testify/require"
2424

25+
"github.com/ava-labs/libevm/libevm"
2526
"github.com/ava-labs/libevm/params"
2627
)
2728

@@ -72,10 +73,14 @@ func TestOverrideNewEVMArgs(t *testing.T) {
7273
assertChainID(t, chainID)
7374

7475
t.Run("WithTempRegisteredHooks", func(t *testing.T) {
75-
override := evmArgOverrider{newEVMchainID: 24680}
76-
WithTempRegisteredHooks(&override, func() {
77-
assertChainID(t, override.newEVMchainID)
76+
err := libevm.WithTemporaryExtrasLock(func(lock libevm.ExtrasLock) error {
77+
override := evmArgOverrider{newEVMchainID: 24680}
78+
return WithTempRegisteredHooks(lock, &override, func() error {
79+
assertChainID(t, override.newEVMchainID)
80+
return nil
81+
})
7882
})
83+
require.NoError(t, err)
7984
t.Run("after", func(t *testing.T) {
8085
assertChainID(t, chainID)
8186
})

core/vm/hooks.libevm.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package vm
1818

1919
import (
20+
"github.com/ava-labs/libevm/libevm"
2021
"github.com/ava-labs/libevm/libevm/register"
2122
"github.com/ava-labs/libevm/params"
2223
)
@@ -36,8 +37,11 @@ func RegisterHooks(h Hooks) {
3637
// consumers that require access to extras. Said consumers SHOULD NOT, however
3738
// call this function directly. Use the libevm/temporary.WithRegisteredExtras()
3839
// function instead as it atomically overrides all possible packages.
39-
func WithTempRegisteredHooks(h Hooks, fn func()) {
40-
libevmHooks.TempOverride(h, fn)
40+
func WithTempRegisteredHooks(lock libevm.ExtrasLock, h Hooks, fn func() error) error {
41+
if err := lock.Verify(); err != nil {
42+
return err
43+
}
44+
return libevmHooks.TempOverride(h, fn)
4145
}
4246

4347
// TestOnlyClearRegisteredHooks clears the [Hooks] previously passed to

libevm/extraslock.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright 2025 the libevm authors.
2+
//
3+
// The libevm additions to go-ethereum are free software: you can redistribute
4+
// them and/or modify them under the terms of the GNU Lesser General Public License
5+
// as published by the Free Software Foundation, either version 3 of the License,
6+
// or (at your option) any later version.
7+
//
8+
// The libevm additions are distributed in the hope that they will be useful,
9+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
11+
// General Public License for more details.
12+
//
13+
// You should have received a copy of the GNU Lesser General Public License
14+
// along with the go-ethereum library. If not, see
15+
// <http://www.gnu.org/licenses/>.
16+
17+
package libevm
18+
19+
import (
20+
"errors"
21+
"sync"
22+
"sync/atomic"
23+
)
24+
25+
var (
26+
extrasMu sync.Mutex
27+
extrasHandle atomic.Uint64
28+
)
29+
30+
// An ExtrasLock is a handle that proves a current call to
31+
// [WithTemporaryExtrasLock].
32+
type ExtrasLock struct {
33+
handle *uint64
34+
}
35+
36+
// WithTemporaryExtrasLock takes a global lock and calls `fn` with a handle that
37+
// can be used to prove that the lock is held. All package-specific temporary
38+
// overrides require this proof.
39+
//
40+
// WithTemporaryExtrasLock MUST NOT be used on a live chain. It is solely
41+
// intended for off-chain consumers that require access to extras.
42+
func WithTemporaryExtrasLock(fn func(lock ExtrasLock) error) error {
43+
extrasMu.Lock()
44+
defer func() {
45+
extrasHandle.Add(1)
46+
extrasMu.Unlock()
47+
}()
48+
49+
v := extrasHandle.Load()
50+
return fn(ExtrasLock{&v})
51+
}
52+
53+
// ErrExpiredExtrasLock is returned by [ExtrasLock.Verify] if the lock has been
54+
// persisted beyond the call to [WithTemporaryExtrasLock] that created it.
55+
var ErrExpiredExtrasLock = errors.New("libevm.ExtrasLock expired")
56+
57+
// Verify verifies that the lock is valid.
58+
func (l ExtrasLock) Verify() error {
59+
if *l.handle != extrasHandle.Load() {
60+
return ErrExpiredExtrasLock
61+
}
62+
return nil
63+
}

libevm/extraslock_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2025 the libevm authors.
2+
//
3+
// The libevm additions to go-ethereum are free software: you can redistribute
4+
// them and/or modify them under the terms of the GNU Lesser General Public License
5+
// as published by the Free Software Foundation, either version 3 of the License,
6+
// or (at your option) any later version.
7+
//
8+
// The libevm additions are distributed in the hope that they will be useful,
9+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
11+
// General Public License for more details.
12+
//
13+
// You should have received a copy of the GNU Lesser General Public License
14+
// along with the go-ethereum library. If not, see
15+
// <http://www.gnu.org/licenses/>.
16+
17+
package libevm_test
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
25+
// Testing from outside the package to guarantee usage of the public API only.
26+
. "github.com/ava-labs/libevm/libevm"
27+
)
28+
29+
func TestExtrasLock(t *testing.T) {
30+
var zero ExtrasLock
31+
assert.Panics(t, func() { _ = zero.Verify() }, "Verify() method of zero-value ExtrasLock{}")
32+
33+
testIntegration := func(t *testing.T) {
34+
t.Helper()
35+
require.NoError(t,
36+
WithTemporaryExtrasLock((ExtrasLock).Verify),
37+
"WithTemporaryExtrasLock((ExtrasLock).Verify)",
38+
)
39+
}
40+
t.Run("initial_usage", testIntegration)
41+
42+
t.Run("lock_expiration", func(t *testing.T) {
43+
var persisted ExtrasLock
44+
require.NoError(t, WithTemporaryExtrasLock(func(l ExtrasLock) error {
45+
persisted = l
46+
return l.Verify()
47+
}))
48+
assert.ErrorIs(t, persisted.Verify(), ErrExpiredExtrasLock, "Verify() of persisted ExtrasLock")
49+
})
50+
51+
t.Run("repeat_usage", testIntegration)
52+
}

libevm/register/register.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,22 +72,23 @@ func (o *AtMostOnce[T]) TestOnlyClear() {
7272
//
7373
// It is valid to call this method with or without a prior call to
7474
// [AtMostOnce.Register].
75-
func (o *AtMostOnce[T]) TempOverride(with T, fn func()) {
76-
o.temp(&with, fn)
75+
func (o *AtMostOnce[T]) TempOverride(with T, fn func() error) error {
76+
return o.temp(&with, fn)
7777
}
7878

7979
// TempClear calls `fn`, clearing any registered `T`, but only for the life of
8080
// the call. It is not threadsafe.
8181
//
8282
// It is valid to call this method with or without a prior call to
8383
// [AtMostOnce.Register].
84-
func (o *AtMostOnce[T]) TempClear(fn func()) {
85-
o.temp(nil, fn)
84+
func (o *AtMostOnce[T]) TempClear(fn func() error) error {
85+
return o.temp(nil, fn)
8686
}
8787

88-
func (o *AtMostOnce[T]) temp(with *T, fn func()) {
88+
func (o *AtMostOnce[T]) temp(with *T, fn func() error) error {
8989
old := o.v
9090
o.v = with
91-
fn()
91+
err := fn()
9292
o.v = old
93+
return err
9394
}

libevm/register/register_test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package register
1818

1919
import (
20+
"errors"
2021
"testing"
2122

2223
"github.com/stretchr/testify/assert"
@@ -56,9 +57,10 @@ func TestAtMostOnce(t *testing.T) {
5657

5758
t.Run("TempOverride", func(t *testing.T) {
5859
t.Run("during", func(t *testing.T) {
59-
sut.TempOverride(val+1, func() {
60+
require.NoError(t, sut.TempOverride(val+1, func() error {
6061
assertRegistered(t, val+1)
61-
})
62+
return nil
63+
}))
6264
})
6365
t.Run("after", func(t *testing.T) {
6466
assertRegistered(t, val)
@@ -67,12 +69,20 @@ func TestAtMostOnce(t *testing.T) {
6769

6870
t.Run("TempClear", func(t *testing.T) {
6971
t.Run("during", func(t *testing.T) {
70-
sut.TempClear(func() {
72+
require.NoError(t, sut.TempClear(func() error {
7173
assert.False(t, sut.Registered(), "Registered()")
72-
})
74+
return nil
75+
}))
7376
})
7477
t.Run("after", func(t *testing.T) {
7578
assertRegistered(t, val)
7679
})
7780
})
81+
82+
t.Run("error_propagation", func(t *testing.T) {
83+
errFoo := errors.New("foo")
84+
fn := func() error { return errFoo }
85+
assert.ErrorIs(t, sut.TempOverride(0, fn), errFoo, "TempOverride()") //nolint:testifylint // Blindly using require is an anti-pattern!!!
86+
assert.ErrorIs(t, sut.TempClear(fn), errFoo, "TempClear()")
87+
})
7888
}

0 commit comments

Comments
 (0)