From fd0c34676a39ad41baf20f49b0fb7b975f362114 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 14:20:22 -0400 Subject: [PATCH 01/28] enable lint errcheck --- .avalanche-golangci.yml | 2 +- core/blockchain_ext_test.go | 2 +- core/fifo_cache.go | 2 +- core/state_manager_test.go | 12 ++- plugin/evm/atomic/state/atomic_trie.go | 15 +++- plugin/evm/atomic/state/atomic_trie_test.go | 3 +- plugin/evm/atomic/vm/block_extension.go | 5 +- .../vm/gossiper_atomic_gossiping_test.go | 6 +- plugin/evm/atomic/vm/vm_test.go | 77 +++++++------------ plugin/evm/message/codec.go | 2 +- plugin/evm/vm.go | 10 ++- plugin/evm/vm_test.go | 3 +- plugin/evm/vmtest/genesis.go | 5 +- plugin/evm/vmtest/test_syncervm.go | 18 ++++- plugin/main.go | 6 +- sync/statesync/trie_sync_tasks.go | 5 +- triedb/firewood/database.go | 6 +- warp/verifier_backend_test.go | 3 +- 18 files changed, 107 insertions(+), 75 deletions(-) diff --git a/.avalanche-golangci.yml b/.avalanche-golangci.yml index dbe379846c..9bb36af396 100644 --- a/.avalanche-golangci.yml +++ b/.avalanche-golangci.yml @@ -55,7 +55,7 @@ linters: - bodyclose - copyloopvar # - depguard - # - errcheck + - errcheck - errorlint # - forbidigo - goconst diff --git a/core/blockchain_ext_test.go b/core/blockchain_ext_test.go index 7c0383e170..c3bbb25108 100644 --- a/core/blockchain_ext_test.go +++ b/core/blockchain_ext_test.go @@ -1886,7 +1886,7 @@ func ReexecCorruptedStateTest(t *testing.T, create ReexecTestFunc) { } // Simulate a crash by updating the acceptor tip - blockchain.writeBlockAcceptedIndices(chain[1]) + _ = blockchain.writeBlockAcceptedIndices(chain[1]) blockchain.Stop() // Restart blockchain with existing state diff --git a/core/fifo_cache.go b/core/fifo_cache.go index 9f1eb8cf1e..7802f44230 100644 --- a/core/fifo_cache.go +++ b/core/fifo_cache.go @@ -44,7 +44,7 @@ func (f *BufferFIFOCache[K, V]) Put(key K, val V) { f.l.Lock() defer f.l.Unlock() - f.buffer.Insert(key) // Insert will remove the oldest [K] if we are at the [limit] + _ = f.buffer.Insert(key) // Insert will remove the oldest [K] if we are at the [limit] f.m[key] = val } diff --git a/core/state_manager_test.go b/core/state_manager_test.go index 4622106bba..279a1698ff 100644 --- a/core/state_manager_test.go +++ b/core/state_manager_test.go @@ -57,7 +57,8 @@ func TestCappedMemoryTrieWriter(t *testing.T) { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on insert") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on insert") - w.AcceptTrie(block) + err := w.AcceptTrie(block) + assert.NoError(err) if i <= tipBufferSize { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on accept") } else { @@ -71,7 +72,8 @@ func TestCappedMemoryTrieWriter(t *testing.T) { m.LastCommit = common.Hash{} } - w.RejectTrie(block) + err = w.RejectTrie(block) + assert.NoError(err) assert.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on reject") m.LastDereference = common.Hash{} @@ -96,12 +98,14 @@ func TestNoPruningTrieWriter(t *testing.T) { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on insert") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on insert") - w.AcceptTrie(block) + err := w.AcceptTrie(block) + assert.NoError(err) assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on accept") assert.Equal(block.Root(), m.LastCommit, "should have committed block on accept") m.LastCommit = common.Hash{} - w.RejectTrie(block) + err = w.RejectTrie(block) + assert.NoError(err) assert.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on reject") m.LastDereference = common.Hash{} diff --git a/plugin/evm/atomic/state/atomic_trie.go b/plugin/evm/atomic/state/atomic_trie.go index 0fbdd59ff5..2764061729 100644 --- a/plugin/evm/atomic/state/atomic_trie.go +++ b/plugin/evm/atomic/state/atomic_trie.go @@ -244,7 +244,10 @@ func (a *AtomicTrie) InsertTrie(nodes *trienode.NodeSet, root common.Hash) error return err } } - a.trieDB.Reference(root, common.Hash{}) + err := a.trieDB.Reference(root, common.Hash{}) + if err != nil { + return err + } // The use of [Cap] in [insertTrie] prevents exceeding the configured memory // limit (and OOM) in case there is a large backlog of processing (unaccepted) blocks. @@ -285,12 +288,18 @@ func (a *AtomicTrie) AcceptTrie(height uint64, root common.Hash) (bool, error) { // - not committted, in which case the current root we are inserting contains // references to all the relevant data from the previous root, so the previous // root can be dereferenced. - a.trieDB.Dereference(a.lastAcceptedRoot) + err := a.trieDB.Dereference(a.lastAcceptedRoot) + if err != nil { + return false, err + } a.lastAcceptedRoot = root return hasCommitted, nil } func (a *AtomicTrie) RejectTrie(root common.Hash) error { - a.trieDB.Dereference(root) + err := a.trieDB.Dereference(root) + if err != nil { + return err + } return nil } diff --git a/plugin/evm/atomic/state/atomic_trie_test.go b/plugin/evm/atomic/state/atomic_trie_test.go index 4ce75aaae9..b0378dc0ba 100644 --- a/plugin/evm/atomic/state/atomic_trie_test.go +++ b/plugin/evm/atomic/state/atomic_trie_test.go @@ -644,7 +644,8 @@ func TestAtomicTrie_AcceptTrie(t *testing.T) { _, storageSize, _ := atomicTrie.trieDB.Size() require.NotZero(t, storageSize, "there should be a dirty node taking up storage space") } - atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight) + err = atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight) + require.NoError(t, err) hasCommitted, err := atomicTrie.AcceptTrie(testCase.height, testCase.root) require.NoError(t, err) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 349946b67b..ba43b7fce9 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -229,7 +229,10 @@ func (be *blockExtension) Reject() error { func (be *blockExtension) CleanupVerified() { vm := be.blockExtender.vm if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { - atomicState.Reject() + err = atomicState.Reject() + if err != nil { + log.Error("failed to reject atomic state", "err", err) + } } } diff --git a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go index 079eab5420..df70792cc8 100644 --- a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go +++ b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go @@ -143,7 +143,8 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { tx, conflictingTx := importTxs[0], importTxs[1] txID := tx.ID() - vm.AtomicMempool.AddRemoteTx(tx) + err := vm.AtomicMempool.AddRemoteTx(tx) + assert.NoError(err) vm.AtomicMempool.NextTx() vm.AtomicMempool.DiscardCurrentTx(txID) @@ -177,7 +178,8 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { // Conflicting tx must be submitted over the API to be included in push gossip. // (i.e., txs received via p2p are not included in push gossip) // This test adds it directly to the mempool + gossiper to simulate that. - vm.AtomicMempool.AddRemoteTx(conflictingTx) + err = vm.AtomicMempool.AddRemoteTx(conflictingTx) + assert.NoError(err) vm.AtomicTxPushGossiper.Add(conflictingTx) time.Sleep(500 * time.Millisecond) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index ac4480f270..11556ebfd2 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -190,7 +190,7 @@ func testIssueAtomicTxs(t *testing.T, scheme string) { utxos := map[ids.ShortID]uint64{ vmtest.TestShortIDAddrs[0]: importAmount, } - addUTXOs(tvm.AtomicMemory, vm.Ctx, utxos) + require.NoError(t, addUTXOs(tvm.AtomicMemory, vm.Ctx, utxos)) defer func() { if err := vm.Shutdown(context.Background()); err != nil { t.Fatal(err) @@ -198,34 +198,18 @@ func testIssueAtomicTxs(t *testing.T, scheme string) { }() importTx, err := vm.newImportTx(vm.Ctx.XChainID, vmtest.TestEthAddrs[0], vmtest.InitialBaseFee, vmtest.TestKeys[0:1]) - if err != nil { - t.Fatal(err) - } - - if err := vm.AtomicMempool.AddLocalTx(importTx); err != nil { - t.Fatal(err) - } - + require.NoError(t, err) + require.NoError(t, vm.AtomicMempool.AddLocalTx(importTx)) msg, err := vm.WaitForEvent(context.Background()) require.NoError(t, err) require.Equal(t, commonEng.PendingTxs, msg) blk, err := vm.BuildBlock(context.Background()) - if err != nil { - t.Fatal(err) - } - - if err := blk.Verify(context.Background()); err != nil { - t.Fatal(err) - } - - if err := vm.SetPreference(context.Background(), blk.ID()); err != nil { - t.Fatal(err) - } - - if err := blk.Accept(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, blk.Verify(context.Background())) + require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) + require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) + require.NoError(t, blk.Accept(context.Background())) if lastAcceptedID, err := vm.LastAccepted(context.Background()); err != nil { t.Fatal(err) @@ -235,36 +219,21 @@ func testIssueAtomicTxs(t *testing.T, scheme string) { vm.Ethereum().BlockChain().DrainAcceptorQueue() state, err := vm.Ethereum().BlockChain().State() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) wrappedState := extstate.New(state) exportTx, err := atomic.NewExportTx(vm.Ctx, vm.CurrentRules(), wrappedState, vm.Ctx.AVAXAssetID, importAmount-(2*ap0.AtomicTxFee), vm.Ctx.XChainID, vmtest.TestShortIDAddrs[0], vmtest.InitialBaseFee, vmtest.TestKeys[0:1]) - if err != nil { - t.Fatal(err) - } - - if err := vm.AtomicMempool.AddLocalTx(exportTx); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, vm.AtomicMempool.AddLocalTx(exportTx)) msg, err = vm.WaitForEvent(context.Background()) require.NoError(t, err) require.Equal(t, commonEng.PendingTxs, msg) blk2, err := vm.BuildBlock(context.Background()) - if err != nil { - t.Fatal(err) - } - - if err := blk2.Verify(context.Background()); err != nil { - t.Fatal(err) - } - - if err := blk2.Accept(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, blk2.Verify(context.Background())) + require.NoError(t, blk2.Accept(context.Background())) if lastAcceptedID, err := vm.LastAccepted(context.Background()); err != nil { t.Fatal(err) @@ -1102,8 +1071,14 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { // Add the two conflicting transactions directly to the mempool, so that two consecutive transactions // will fail verification when build block is called. - vm.AtomicMempool.AddRemoteTx(importTxs[1]) - vm.AtomicMempool.AddRemoteTx(importTxs[2]) + err = vm.AtomicMempool.AddRemoteTx(importTxs[1]) + if err != nil { + t.Fatal(err) + } + err = vm.AtomicMempool.AddRemoteTx(importTxs[2]) + if err != nil { + t.Fatal(err) + } if _, err := vm.BuildBlock(context.Background()); err == nil { t.Fatal("Expected build block to fail due to empty block") @@ -1149,7 +1124,10 @@ func TestAtomicTxBuildBlockDropsConflicts(t *testing.T) { t.Fatal("should conflict with the utxoSet in the mempool") } // force add the tx - vm.AtomicMempool.ForceAddTx(conflictTx) + err = vm.AtomicMempool.ForceAddTx(conflictTx) + if err != nil { + t.Fatal(err) + } conflictSets[index].Add(conflictTx.ID()) } msg, err := vm.WaitForEvent(context.Background()) @@ -2116,7 +2094,8 @@ func TestWaitForEvent(t *testing.T) { }, }}}})) testCase.testCase(t, vm, address, key) - vm.Shutdown(context.Background()) + err = vm.Shutdown(context.Background()) + require.NoError(t, err) }) } } diff --git a/plugin/evm/message/codec.go b/plugin/evm/message/codec.go index c3f1a6795f..aee9a70775 100644 --- a/plugin/evm/message/codec.go +++ b/plugin/evm/message/codec.go @@ -38,7 +38,7 @@ func init() { // See https://github.com/ava-labs/coreth/pull/999 c.SkipRegistrations(3) - Codec.RegisterCodec(Version, c) + errs.Add(Codec.RegisterCodec(Version, c)) if errs.Errored() { panic(errs.Err) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 6b07c32aa5..2156259534 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -468,7 +468,10 @@ func (vm *VM) Initialize( // Add p2p warp message warpHandler warpHandler := acp118.NewCachedHandler(meteredCache, vm.warpBackend, vm.ctx.WarpSigner) - vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler) + err = vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler) + if err != nil { + return err + } vm.stateSyncDone = make(chan struct{}) @@ -891,7 +894,10 @@ func (vm *VM) Shutdown(context.Context) error { for _, handler := range vm.rpcHandlers { handler.Stop() } - vm.eth.Stop() + err := vm.eth.Stop() + if err != nil { + return fmt.Errorf("error stopping eth: %w", err) + } vm.shutdownWg.Wait() return nil } diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index b8095e58b1..cee21a14f7 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -2027,7 +2027,8 @@ func TestWaitForEvent(t *testing.T) { Fork: &fork, }) testCase.testCase(t, vm) - vm.Shutdown(context.Background()) + err := vm.Shutdown(context.Background()) + require.NoError(t, err) }) } } diff --git a/plugin/evm/vmtest/genesis.go b/plugin/evm/vmtest/genesis.go index 4af162ce7d..a9f8b2c2dd 100644 --- a/plugin/evm/vmtest/genesis.go +++ b/plugin/evm/vmtest/genesis.go @@ -67,7 +67,10 @@ func NewTestGenesis(cfg *params.ChainConfig) *core.Genesis { g.Config = &cpy allocStr := `{"0100000000000000000000000000000000000000":{"code":"0x7300000000000000000000000000000000000000003014608060405260043610603d5760003560e01c80631e010439146042578063b6510bb314606e575b600080fd5b605c60048036036020811015605657600080fd5b503560b1565b60408051918252519081900360200190f35b818015607957600080fd5b5060af60048036036080811015608e57600080fd5b506001600160a01b03813516906020810135906040810135906060013560b6565b005b30cd90565b836001600160a01b031681836108fc8690811502906040516000604051808303818888878c8acf9550505050505015801560f4573d6000803e3d6000fd5b505050505056fea26469706673582212201eebce970fe3f5cb96bf8ac6ba5f5c133fc2908ae3dcd51082cfee8f583429d064736f6c634300060a0033","balance":"0x0"}}` - json.Unmarshal([]byte(allocStr), &g.Alloc) + err := json.Unmarshal([]byte(allocStr), &g.Alloc) + if err != nil { + panic(err) + } // After Durango, an additional account is funded in tests to use // with warp messages. if params.GetExtra(cfg).IsDurango(0) { diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index 887389ce48..d1be5ee5a1 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -161,7 +161,11 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup if !hasItem { t.Fatal("expected nodeSet to contain at least 1 nodeID") } - go testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request) + go func() { + if err := testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request); err != nil { + panic(err) + } + }() return nil } ResetMetrics(testSyncVMSetup.syncerVM.SnowCtx) @@ -246,7 +250,11 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] testSyncVMSetup.serverVM.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - go syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response) + go func() { + if err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response); err != nil { + panic(err) + } + }() } else { go test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) } @@ -387,7 +395,11 @@ func initSyncServerAndClientVMs(t *testing.T, test SyncTestParams, numBlocks int // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] serverTest.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - go syncerVM.AppResponse(ctx, nodeID, requestID, response) + go func() { + if err := syncerVM.AppResponse(ctx, nodeID, requestID, response); err != nil { + panic(err) + } + }() } else { go test.responseIntercept(syncerVM, nodeID, requestID, response) } diff --git a/plugin/main.go b/plugin/main.go index 28711dfbd7..49cc759f9e 100644 --- a/plugin/main.go +++ b/plugin/main.go @@ -30,5 +30,9 @@ func main() { fmt.Printf("failed to set fd limit correctly due to: %s\n", err) os.Exit(1) } - rpcchainvm.Serve(context.Background(), factory.NewPluginVM()) + err = rpcchainvm.Serve(context.Background(), factory.NewPluginVM()) + if err != nil { + fmt.Printf("failed to serve rpc chain vm: %s\n", err) + os.Exit(1) + } } diff --git a/sync/statesync/trie_sync_tasks.go b/sync/statesync/trie_sync_tasks.go index 070724fe42..d633789a21 100644 --- a/sync/statesync/trie_sync_tasks.go +++ b/sync/statesync/trie_sync_tasks.go @@ -75,7 +75,10 @@ func (m *mainTrieTask) OnLeafs(db ethdb.KeyValueWriter, keys, vals [][]byte) err // check if this account has storage root that we need to fetch if acc.Root != (common.Hash{}) && acc.Root != types.EmptyRootHash { - m.sync.trieQueue.RegisterStorageTrie(acc.Root, accountHash) + err := m.sync.trieQueue.RegisterStorageTrie(acc.Root, accountHash) + if err != nil { + return err + } } // check if this account has code and add it to codeHashes to fetch diff --git a/triedb/firewood/database.go b/triedb/firewood/database.go index dc6bf7f035..7765052bf9 100644 --- a/triedb/firewood/database.go +++ b/triedb/firewood/database.go @@ -547,7 +547,11 @@ func (db *Database) getProposalHash(parentRoot common.Hash, keys, values [][]byt ffiHashTimer.Inc(time.Since(start).Milliseconds()) // We succesffuly created a proposal, so we must drop it after use. - defer p.Drop() + defer func() { + if err := p.Drop(); err != nil { + log.Error("firewood: error dropping proposal after hash computation", "parentRoot", parentRoot.Hex(), "error", err) + } + }() rootBytes, err := p.Root() if err != nil { diff --git a/warp/verifier_backend_test.go b/warp/verifier_backend_test.go index 73cf9947a3..b299bc591a 100644 --- a/warp/verifier_backend_test.go +++ b/warp/verifier_backend_test.go @@ -53,7 +53,8 @@ func TestAddressedCallSignatures(t *testing.T) { signature, err := snowCtx.WarpSigner.Sign(msg) require.NoError(t, err) - backend.AddMessage(msg) + err = backend.AddMessage(msg) + require.NoError(t, err) return msg.Bytes(), signature }, verifyStats: func(t *testing.T, stats *verifierStats) { From 8e8947d05598dc3fa63020385b52265ae3cde533 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 14:52:52 -0400 Subject: [PATCH 02/28] Update core/state_manager_test.go Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- core/state_manager_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/state_manager_test.go b/core/state_manager_test.go index 279a1698ff..679eb075c6 100644 --- a/core/state_manager_test.go +++ b/core/state_manager_test.go @@ -57,8 +57,7 @@ func TestCappedMemoryTrieWriter(t *testing.T) { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on insert") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on insert") - err := w.AcceptTrie(block) - assert.NoError(err) + assert.NoError(w.AcceptTrie(block)) if i <= tipBufferSize { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on accept") } else { From 73d6f52b9b5e92455d84589e4795ec0876277da9 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 14:53:00 -0400 Subject: [PATCH 03/28] Update plugin/evm/atomic/state/atomic_trie.go Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- plugin/evm/atomic/state/atomic_trie.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/evm/atomic/state/atomic_trie.go b/plugin/evm/atomic/state/atomic_trie.go index 2764061729..68e3d68162 100644 --- a/plugin/evm/atomic/state/atomic_trie.go +++ b/plugin/evm/atomic/state/atomic_trie.go @@ -244,8 +244,7 @@ func (a *AtomicTrie) InsertTrie(nodes *trienode.NodeSet, root common.Hash) error return err } } - err := a.trieDB.Reference(root, common.Hash{}) - if err != nil { + if err := a.trieDB.Reference(root, common.Hash{}); err != nil { return err } From ca5e7612cb4f53211a345dab612feb6e53d67d0b Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 14:54:51 -0400 Subject: [PATCH 04/28] in-line errors --- core/state_manager_test.go | 10 +++------- plugin/evm/atomic/state/atomic_trie_test.go | 3 +-- plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go | 6 ++---- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/core/state_manager_test.go b/core/state_manager_test.go index 679eb075c6..722f3b45f1 100644 --- a/core/state_manager_test.go +++ b/core/state_manager_test.go @@ -56,7 +56,6 @@ func TestCappedMemoryTrieWriter(t *testing.T) { assert.NoError(w.InsertTrie(block)) assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on insert") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on insert") - assert.NoError(w.AcceptTrie(block)) if i <= tipBufferSize { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on accept") @@ -71,8 +70,7 @@ func TestCappedMemoryTrieWriter(t *testing.T) { m.LastCommit = common.Hash{} } - err = w.RejectTrie(block) - assert.NoError(err) + assert.NoError(w.RejectTrie(block)) assert.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on reject") m.LastDereference = common.Hash{} @@ -97,14 +95,12 @@ func TestNoPruningTrieWriter(t *testing.T) { assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on insert") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on insert") - err := w.AcceptTrie(block) - assert.NoError(err) + assert.NoError(w.AcceptTrie(block)) assert.Equal(common.Hash{}, m.LastDereference, "should not have dereferenced block on accept") assert.Equal(block.Root(), m.LastCommit, "should have committed block on accept") m.LastCommit = common.Hash{} - err = w.RejectTrie(block) - assert.NoError(err) + assert.NoError(w.RejectTrie(block)) assert.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") assert.Equal(common.Hash{}, m.LastCommit, "should not have committed block on reject") m.LastDereference = common.Hash{} diff --git a/plugin/evm/atomic/state/atomic_trie_test.go b/plugin/evm/atomic/state/atomic_trie_test.go index b0378dc0ba..c183d2c358 100644 --- a/plugin/evm/atomic/state/atomic_trie_test.go +++ b/plugin/evm/atomic/state/atomic_trie_test.go @@ -644,8 +644,7 @@ func TestAtomicTrie_AcceptTrie(t *testing.T) { _, storageSize, _ := atomicTrie.trieDB.Size() require.NotZero(t, storageSize, "there should be a dirty node taking up storage space") } - err = atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight) - require.NoError(t, err) + require.NoError(t, atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight)) hasCommitted, err := atomicTrie.AcceptTrie(testCase.height, testCase.root) require.NoError(t, err) diff --git a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go index df70792cc8..fae4144040 100644 --- a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go +++ b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go @@ -143,8 +143,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { tx, conflictingTx := importTxs[0], importTxs[1] txID := tx.ID() - err := vm.AtomicMempool.AddRemoteTx(tx) - assert.NoError(err) + assert.NoError(vm.AtomicMempool.AddRemoteTx(tx)) vm.AtomicMempool.NextTx() vm.AtomicMempool.DiscardCurrentTx(txID) @@ -178,8 +177,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { // Conflicting tx must be submitted over the API to be included in push gossip. // (i.e., txs received via p2p are not included in push gossip) // This test adds it directly to the mempool + gossiper to simulate that. - err = vm.AtomicMempool.AddRemoteTx(conflictingTx) - assert.NoError(err) + assert.NoError(vm.AtomicMempool.AddRemoteTx(conflictingTx)) vm.AtomicTxPushGossiper.Add(conflictingTx) time.Sleep(500 * time.Millisecond) From ee18188f465a8001b750990761eb5672e595e0f7 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 16:10:56 -0400 Subject: [PATCH 05/28] Fix TestConsecutiveAtomicTransactionsRevertSnapshot --- plugin/evm/atomic/vm/vm_test.go | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 11556ebfd2..04ddc627de 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1039,30 +1039,20 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { importTxs := createImportTxOptions(t, vm, tvm.AtomicMemory) // Issue the first import transaction, build, and accept the block. - if err := vm.AtomicMempool.AddLocalTx(importTxs[0]); err != nil { - t.Fatal(err) - } + require.NoError(t, vm.AtomicMempool.AddLocalTx(importTxs[0])) msg, err := vm.WaitForEvent(context.Background()) require.NoError(t, err) require.Equal(t, commonEng.PendingTxs, msg) blk, err := vm.BuildBlock(context.Background()) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if err := blk.Verify(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, blk.Verify(context.Background())) - if err := vm.SetPreference(context.Background(), blk.ID()); err != nil { - t.Fatal(err) - } + require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - if err := blk.Accept(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, blk.Accept(context.Background())) newHead := <-newTxPoolHeadChan if newHead.Head.Hash() != common.Hash(blk.ID()) { @@ -1071,14 +1061,8 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { // Add the two conflicting transactions directly to the mempool, so that two consecutive transactions // will fail verification when build block is called. - err = vm.AtomicMempool.AddRemoteTx(importTxs[1]) - if err != nil { - t.Fatal(err) - } - err = vm.AtomicMempool.AddRemoteTx(importTxs[2]) - if err != nil { - t.Fatal(err) - } + require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[1])) + require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[2])) if _, err := vm.BuildBlock(context.Background()); err == nil { t.Fatal("Expected build block to fail due to empty block") From e063e0d807ec895a32331c25c18b2d7de7a48fdb Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 10 Sep 2025 16:34:22 -0400 Subject: [PATCH 06/28] lint --- plugin/evm/atomic/vm/vm_test.go | 5 +---- plugin/evm/vm_test.go | 3 +-- warp/verifier_backend_test.go | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 04ddc627de..2769a986aa 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1049,9 +1049,7 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { require.NoError(t, err) require.NoError(t, blk.Verify(context.Background())) - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - require.NoError(t, blk.Accept(context.Background())) newHead := <-newTxPoolHeadChan @@ -2078,8 +2076,7 @@ func TestWaitForEvent(t *testing.T) { }, }}}})) testCase.testCase(t, vm, address, key) - err = vm.Shutdown(context.Background()) - require.NoError(t, err) + require.NoError(t, vm.Shutdown(context.Background())) }) } } diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index cee21a14f7..194aaf7c0c 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -2027,8 +2027,7 @@ func TestWaitForEvent(t *testing.T) { Fork: &fork, }) testCase.testCase(t, vm) - err := vm.Shutdown(context.Background()) - require.NoError(t, err) + require.NoError(t, vm.Shutdown(context.Background())) }) } } diff --git a/warp/verifier_backend_test.go b/warp/verifier_backend_test.go index b299bc591a..5c3699f8ea 100644 --- a/warp/verifier_backend_test.go +++ b/warp/verifier_backend_test.go @@ -52,9 +52,7 @@ func TestAddressedCallSignatures(t *testing.T) { require.NoError(t, err) signature, err := snowCtx.WarpSigner.Sign(msg) require.NoError(t, err) - - err = backend.AddMessage(msg) - require.NoError(t, err) + require.NoError(t, backend.AddMessage(msg)) return msg.Bytes(), signature }, verifyStats: func(t *testing.T, stats *verifierStats) { From 5b12fa22b3b48e51c4ac7eedf86fcdc937465d57 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Thu, 11 Sep 2025 14:15:00 -0400 Subject: [PATCH 07/28] Apply suggestions from code review Co-authored-by: Stephen Buttolph Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- plugin/evm/atomic/vm/vm_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 2769a986aa..c26dced33f 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -208,7 +208,6 @@ func testIssueAtomicTxs(t *testing.T, scheme string) { require.NoError(t, err) require.NoError(t, blk.Verify(context.Background())) require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) require.NoError(t, blk.Accept(context.Background())) if lastAcceptedID, err := vm.LastAccepted(context.Background()); err != nil { From 2b038aaefcf909a682290e7f582c783cee1759c2 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Thu, 11 Sep 2025 14:21:15 -0400 Subject: [PATCH 08/28] Stephen suggestions --- core/blockchain_ext_test.go | 2 +- plugin/evm/atomic/vm/block_extension.go | 5 +++-- plugin/evm/extension/config.go | 2 +- plugin/evm/vm.go | 5 ++--- plugin/evm/wrapped_block.go | 4 +++- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/core/blockchain_ext_test.go b/core/blockchain_ext_test.go index c3bbb25108..ad32f3931a 100644 --- a/core/blockchain_ext_test.go +++ b/core/blockchain_ext_test.go @@ -1886,7 +1886,7 @@ func ReexecCorruptedStateTest(t *testing.T, create ReexecTestFunc) { } // Simulate a crash by updating the acceptor tip - _ = blockchain.writeBlockAcceptedIndices(chain[1]) + require.NoError(t, blockchain.writeBlockAcceptedIndices(chain[1])) blockchain.Stop() // Restart blockchain with existing state diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index ba43b7fce9..c55b907d95 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -226,14 +226,15 @@ func (be *blockExtension) Reject() error { } // CleanupVerified is called when the block is cleaned up after a failed insertion. -func (be *blockExtension) CleanupVerified() { +func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { err = atomicState.Reject() if err != nil { - log.Error("failed to reject atomic state", "err", err) + return fmt.Errorf("failed to reject atomic state: %w", err) } } + return nil } // AtomicTxs returns the atomic transactions in this block. diff --git a/plugin/evm/extension/config.go b/plugin/evm/extension/config.go index 9386ba1977..0a92f458d4 100644 --- a/plugin/evm/extension/config.go +++ b/plugin/evm/extension/config.go @@ -103,7 +103,7 @@ type BlockExtension interface { // CleanupVerified is called when a block has passed SemanticVerify and SynctacticVerify, // and should be cleaned up due to error or verification runs under non-write mode. This // does not return an error because the block has already been verified. - CleanupVerified() + CleanupVerified() error // Accept is called when a block is accepted by the block manager. Accept takes a // database.Batch that contains the changes that were made to the database as a result // of accepting the block. The changes in the batch should be flushed to the database in this method. diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 2156259534..71b9a8ae7d 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -894,9 +894,8 @@ func (vm *VM) Shutdown(context.Context) error { for _, handler := range vm.rpcHandlers { handler.Stop() } - err := vm.eth.Stop() - if err != nil { - return fmt.Errorf("error stopping eth: %w", err) + if err := vm.eth.Stop(); err != nil { + log.Error("error stopping eth", "err", err) } vm.shutdownWg.Wait() return nil diff --git a/plugin/evm/wrapped_block.go b/plugin/evm/wrapped_block.go index a945da9e00..0d770328f3 100644 --- a/plugin/evm/wrapped_block.go +++ b/plugin/evm/wrapped_block.go @@ -268,7 +268,9 @@ func (b *wrappedBlock) verify(predicateContext *precompileconfig.PredicateContex // got an error while inserting to blockchain, we may need to cleanup the extension. // so that the extension can be garbage collected. if doCleanup := err != nil || !writes; b.extension != nil && doCleanup { - b.extension.CleanupVerified() + if err := b.extension.CleanupVerified(); err != nil { + return fmt.Errorf("failed to cleanup extension: %w", err) + } } return err } From e7601ef869c6bfd4f5780ee73e8a602541e70948 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 15 Sep 2025 13:32:00 -0400 Subject: [PATCH 09/28] Apply suggestions from code review Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- plugin/evm/atomic/state/atomic_trie.go | 6 ++---- plugin/evm/atomic/vm/vm_test.go | 3 +-- plugin/evm/vm.go | 3 +-- plugin/evm/vmtest/genesis.go | 3 +-- sync/statesync/trie_sync_tasks.go | 3 +-- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/plugin/evm/atomic/state/atomic_trie.go b/plugin/evm/atomic/state/atomic_trie.go index 68e3d68162..9976575a7f 100644 --- a/plugin/evm/atomic/state/atomic_trie.go +++ b/plugin/evm/atomic/state/atomic_trie.go @@ -287,8 +287,7 @@ func (a *AtomicTrie) AcceptTrie(height uint64, root common.Hash) (bool, error) { // - not committted, in which case the current root we are inserting contains // references to all the relevant data from the previous root, so the previous // root can be dereferenced. - err := a.trieDB.Dereference(a.lastAcceptedRoot) - if err != nil { + if err := a.trieDB.Dereference(a.lastAcceptedRoot); err != nil { return false, err } a.lastAcceptedRoot = root @@ -296,8 +295,7 @@ func (a *AtomicTrie) AcceptTrie(height uint64, root common.Hash) (bool, error) { } func (a *AtomicTrie) RejectTrie(root common.Hash) error { - err := a.trieDB.Dereference(root) - if err != nil { + if err := a.trieDB.Dereference(root); err != nil { return err } return nil diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index f4d6abd302..8ed5be0533 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1101,8 +1101,7 @@ func TestAtomicTxBuildBlockDropsConflicts(t *testing.T) { t.Fatal("should conflict with the utxoSet in the mempool") } // force add the tx - err = vm.AtomicMempool.ForceAddTx(conflictTx) - if err != nil { + if err := vm.AtomicMempool.ForceAddTx(conflictTx); err != nil { t.Fatal(err) } conflictSets[index].Add(conflictTx.ID()) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 17cd867d62..ab185b9102 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -468,8 +468,7 @@ func (vm *VM) Initialize( // Add p2p warp message warpHandler warpHandler := acp118.NewCachedHandler(meteredCache, vm.warpBackend, vm.ctx.WarpSigner) - err = vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler) - if err != nil { + if err = vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler); err != nil { return err } diff --git a/plugin/evm/vmtest/genesis.go b/plugin/evm/vmtest/genesis.go index a9f8b2c2dd..0cb6177fb6 100644 --- a/plugin/evm/vmtest/genesis.go +++ b/plugin/evm/vmtest/genesis.go @@ -67,8 +67,7 @@ func NewTestGenesis(cfg *params.ChainConfig) *core.Genesis { g.Config = &cpy allocStr := `{"0100000000000000000000000000000000000000":{"code":"0x7300000000000000000000000000000000000000003014608060405260043610603d5760003560e01c80631e010439146042578063b6510bb314606e575b600080fd5b605c60048036036020811015605657600080fd5b503560b1565b60408051918252519081900360200190f35b818015607957600080fd5b5060af60048036036080811015608e57600080fd5b506001600160a01b03813516906020810135906040810135906060013560b6565b005b30cd90565b836001600160a01b031681836108fc8690811502906040516000604051808303818888878c8acf9550505050505015801560f4573d6000803e3d6000fd5b505050505056fea26469706673582212201eebce970fe3f5cb96bf8ac6ba5f5c133fc2908ae3dcd51082cfee8f583429d064736f6c634300060a0033","balance":"0x0"}}` - err := json.Unmarshal([]byte(allocStr), &g.Alloc) - if err != nil { + if err := json.Unmarshal([]byte(allocStr), &g.Alloc); err != nil { panic(err) } // After Durango, an additional account is funded in tests to use diff --git a/sync/statesync/trie_sync_tasks.go b/sync/statesync/trie_sync_tasks.go index d633789a21..de3ea4c72f 100644 --- a/sync/statesync/trie_sync_tasks.go +++ b/sync/statesync/trie_sync_tasks.go @@ -75,8 +75,7 @@ func (m *mainTrieTask) OnLeafs(db ethdb.KeyValueWriter, keys, vals [][]byte) err // check if this account has storage root that we need to fetch if acc.Root != (common.Hash{}) && acc.Root != types.EmptyRootHash { - err := m.sync.trieQueue.RegisterStorageTrie(acc.Root, accountHash) - if err != nil { + if err := m.sync.trieQueue.RegisterStorageTrie(acc.Root, accountHash); err != nil { return err } } From ab4d3b87a8cfc701e25aa387a70181e71312ae89 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 15 Sep 2025 13:52:56 -0400 Subject: [PATCH 10/28] Review suggestions --- plugin/evm/atomic/vm/block_extension.go | 12 +++++++----- plugin/evm/vmtest/test_syncervm.go | 23 ++++++----------------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index b519d2f741..0e30a23e76 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -229,11 +229,13 @@ func (be *blockExtension) Reject() error { // CleanupVerified is called when the block is cleaned up after a failed insertion. func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm - if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { - err = atomicState.Reject() - if err != nil { - return fmt.Errorf("failed to reject atomic state: %w", err) - } + atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) + if err != nil { + // Atomic state not found + return nil + } + if err := atomicState.Reject(); err != nil { + return fmt.Errorf("failed to reject atomic state: %w", err) } return nil } diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index 03fa590bf6..f9e31d5421 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -161,11 +161,8 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup if !hasItem { t.Fatal("expected nodeSet to contain at least 1 nodeID") } - go func() { - if err := testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request); err != nil { - panic(err) - } - }() + require.NoError(t, testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request)) + return nil } ResetMetrics(testSyncVMSetup.syncerVM.SnowCtx) @@ -250,13 +247,9 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] testSyncVMSetup.serverVM.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - go func() { - if err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response); err != nil { - panic(err) - } - }() + require.NoError(t, syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response)) } else { - go test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) + test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) } return nil @@ -395,13 +388,9 @@ func initSyncServerAndClientVMs(t *testing.T, test SyncTestParams, numBlocks int // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] serverTest.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - go func() { - if err := syncerVM.AppResponse(ctx, nodeID, requestID, response); err != nil { - panic(err) - } - }() + require.NoError(syncerVM.AppResponse(ctx, nodeID, requestID, response)) } else { - go test.responseIntercept(syncerVM, nodeID, requestID, response) + test.responseIntercept(syncerVM, nodeID, requestID, response) } return nil From c769879236045d20d22fb46c26a74a8dcdc43c06 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 15 Sep 2025 14:10:36 -0400 Subject: [PATCH 11/28] lint --- plugin/evm/atomic/vm/block_extension.go | 3 +-- plugin/evm/atomic/vm/vm_test.go | 4 ++-- plugin/evm/vm_test.go | 18 +++++++----------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 0e30a23e76..d5395a4e2d 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -231,8 +231,7 @@ func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) if err != nil { - // Atomic state not found - return nil + return nil //nolint:nilerr // Atomic state not found } if err := atomicState.Reject(); err != nil { return fmt.Errorf("failed to reject atomic state: %w", err) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 8ed5be0533..a906bdc214 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1054,8 +1054,8 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { // Add the two conflicting transactions directly to the mempool, so that two consecutive transactions // will fail verification when build block is called. - require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[1])) - require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[2])) + require.NoError(t, vm.AtomicMempool.AddRemoteTx(importTxs[1])) + require.NoError(t, vm.AtomicMempool.AddRemoteTx(importTxs[2])) if _, err := vm.BuildBlock(context.Background()); err == nil { t.Fatal("Expected build block to fail due to empty block") diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 01c5e5a246..5979bee870 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1392,7 +1392,7 @@ func TestBuildTimeMilliseconds(t *testing.T) { Fork: &test.fork, }) - defer vm.Shutdown(context.Background()) + defer require.NoError(t, vm.Shutdown(context.Background())) vm.clock.Set(buildTime) signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(10), 21000, big.NewInt(ap0.MinGasPrice), nil) @@ -1422,9 +1422,7 @@ func testBuildApricotPhase1Block(t *testing.T, scheme string) { Scheme: scheme, }) defer func() { - if err := vm.Shutdown(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, vm.Shutdown(context.Background())) }() newTxPoolHeadChan := make(chan core.NewTxPoolReorgEvent, 1) @@ -1503,9 +1501,7 @@ func testLastAcceptedBlockNumberAllow(t *testing.T, scheme string) { }) defer func() { - if err := vm.Shutdown(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, vm.Shutdown(context.Background())) }() signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, big.NewInt(ap0.MinGasPrice), nil) @@ -1635,9 +1631,7 @@ func TestParentBeaconRootBlock(t *testing.T) { }) defer func() { - if err := vm.Shutdown(context.Background()); err != nil { - t.Fatal(err) - } + require.NoError(t, vm.Shutdown(context.Background())) }() signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) @@ -2209,7 +2203,9 @@ func TestDelegatePrecompile_BehaviorAcrossUpgrades(t *testing.T) { vmtest.SetupTestVM(t, vm, vmtest.TestVMConfig{ Fork: &tt.fork, }) - defer vm.Shutdown(ctx) + defer func() { + require.NoError(t, vm.Shutdown(ctx)) + }() if tt.preDeployTime != 0 { vm.clock.Set(time.Unix(tt.preDeployTime, 0)) From be7513a0f2b02d20703d38e8002b6b33dceeabc1 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 15 Sep 2025 15:36:04 -0400 Subject: [PATCH 12/28] back to force add --- plugin/evm/atomic/vm/vm_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index a906bdc214..8ed5be0533 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1054,8 +1054,8 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { // Add the two conflicting transactions directly to the mempool, so that two consecutive transactions // will fail verification when build block is called. - require.NoError(t, vm.AtomicMempool.AddRemoteTx(importTxs[1])) - require.NoError(t, vm.AtomicMempool.AddRemoteTx(importTxs[2])) + require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[1])) + require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[2])) if _, err := vm.BuildBlock(context.Background()); err == nil { t.Fatal("Expected build block to fail due to empty block") From 726a4ce61bb35dec0e7025905e908e5d808beaf3 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 22 Sep 2025 13:26:32 -0400 Subject: [PATCH 13/28] align test --- plugin/evm/vm_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index bcaa57ae36..d3a81a1a8f 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1400,7 +1400,9 @@ func TestBuildTimeMilliseconds(t *testing.T) { Fork: &test.fork, }) - defer require.NoError(t, vm.Shutdown(context.Background())) + defer func() { + require.NoError(t, vm.Shutdown(context.Background())) + }() vm.clock.Set(buildTime) signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(10), 21000, big.NewInt(ap0.MinGasPrice), nil) From 21b37d4a8102756d9b2905f2bed094cb7e997fb2 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 11:21:26 -0400 Subject: [PATCH 14/28] back to = --- plugin/evm/atomic/vm/block_extension.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index d5395a4e2d..8fdf28a7ad 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -230,7 +230,7 @@ func (be *blockExtension) Reject() error { func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) - if err != nil { + if err == nil { return nil //nolint:nilerr // Atomic state not found } if err := atomicState.Reject(); err != nil { From faae4e079e618ec8fb28d096227b41259ca8275d Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 12:12:33 -0400 Subject: [PATCH 15/28] try this --- plugin/evm/atomic/vm/block_extension.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 8fdf28a7ad..07340d71a2 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -231,10 +231,10 @@ func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) if err == nil { - return nil //nolint:nilerr // Atomic state not found - } - if err := atomicState.Reject(); err != nil { - return fmt.Errorf("failed to reject atomic state: %w", err) + err = atomicState.Reject() + if err != nil { + return fmt.Errorf("failed to reject atomic state: %w", err) + } } return nil } From f0012a4352b41031835eb44ae9ba71410263b56f Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 13:53:19 -0400 Subject: [PATCH 16/28] don't panic! --- plugin/evm/vmtest/test_syncervm.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index 2d36fc004e..2622109ef7 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -162,7 +162,11 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup if !hasItem { t.Fatal("expected nodeSet to contain at least 1 nodeID") } - require.NoError(t, testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request)) + go func() { + if err := testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request); err != nil { + t.Errorf("AppRequest failed: %v", err) + } + }() return nil } @@ -248,9 +252,13 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] testSyncVMSetup.serverVM.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - require.NoError(t, syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response)) + go func() { + if err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response); err != nil { + t.Errorf("AppResponse failed: %v", err) + } + }() } else { - test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) + go test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) } return nil @@ -389,9 +397,13 @@ func initSyncServerAndClientVMs(t *testing.T, test SyncTestParams, numBlocks int // override [serverVM]'s SendAppResponse function to trigger AppResponse on [syncerVM] serverTest.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { - require.NoError(syncerVM.AppResponse(ctx, nodeID, requestID, response)) + go func() { + if err := syncerVM.AppResponse(ctx, nodeID, requestID, response); err != nil { + t.Errorf("AppResponse failed: %v", err) + } + }() } else { - test.responseIntercept(syncerVM, nodeID, requestID, response) + go test.responseIntercept(syncerVM, nodeID, requestID, response) } return nil From 905e7ced7c04a08b2672b010bcbe7ef8e1f453bb Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 24 Sep 2025 12:23:52 -0400 Subject: [PATCH 17/28] lint --- core/state_manager_test.go | 11 +++++------ .../evm/atomic/vm/gossiper_atomic_gossiping_test.go | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/core/state_manager_test.go b/core/state_manager_test.go index 956070ca30..d9127ed9b0 100644 --- a/core/state_manager_test.go +++ b/core/state_manager_test.go @@ -56,8 +56,8 @@ func TestCappedMemoryTrieWriter(t *testing.T) { require.NoError(w.InsertTrie(block)) require.Zero(m.LastDereference, "should not have dereferenced block on insert") require.Zero(m.LastCommit, "should not have committed block on insert") - require.NoError(w.AcceptTrie(block)) - + require.NoError(w.AcceptTrie(block)) + if i <= tipBufferSize { require.Zero(m.LastDereference, "should not have dereferenced block on accept") } else { @@ -71,10 +71,9 @@ func TestCappedMemoryTrieWriter(t *testing.T) { m.LastCommit = common.Hash{} } - require.NoError(w.RejectTrie(block)) + require.NoError(w.RejectTrie(block)) require.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") require.Zero(m.LastCommit, "should not have committed block on reject") - m.LastDereference = common.Hash{} } } @@ -97,12 +96,12 @@ func TestNoPruningTrieWriter(t *testing.T) { require.Zero(m.LastDereference, "should not have dereferenced block on insert") require.Zero(m.LastCommit, "should not have committed block on insert") - require.NoError(w.AcceptTrie(block)) + require.NoError(w.AcceptTrie(block)) require.Zero(m.LastDereference, "should not have dereferenced block on accept") require.Equal(block.Root(), m.LastCommit, "should have committed block on accept") m.LastCommit = common.Hash{} - require.NoError(w.RejectTrie(block)) + require.NoError(w.RejectTrie(block)) require.Equal(block.Root(), m.LastDereference, "should have dereferenced block on reject") require.Zero(m.LastCommit, "should not have committed block on reject") m.LastDereference = common.Hash{} diff --git a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go index f73bd98863..a160c471fe 100644 --- a/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go +++ b/plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go @@ -143,7 +143,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { tx, conflictingTx := importTxs[0], importTxs[1] txID := tx.ID() - assert.NoError(vm.AtomicMempool.AddRemoteTx(tx)) + require.NoError(vm.AtomicMempool.AddRemoteTx(tx)) vm.AtomicMempool.NextTx() vm.AtomicMempool.DiscardCurrentTx(txID) @@ -177,7 +177,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) { // Conflicting tx must be submitted over the API to be included in push gossip. // (i.e., txs received via p2p are not included in push gossip) // This test adds it directly to the mempool + gossiper to simulate that. - assert.NoError(vm.AtomicMempool.AddRemoteTx(conflictingTx)) + require.NoError(vm.AtomicMempool.AddRemoteTx(conflictingTx)) vm.AtomicTxPushGossiper.Add(conflictingTx) time.Sleep(500 * time.Millisecond) From 436538c87f325d7238f66af22d05e1c1ba474613 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Thu, 25 Sep 2025 12:43:40 -0400 Subject: [PATCH 18/28] Austin suggestions --- plugin/evm/atomic/vm/block_extension.go | 9 +++------ plugin/evm/extension/config.go | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 07340d71a2..068300d951 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -230,13 +230,10 @@ func (be *blockExtension) Reject() error { func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) - if err == nil { - err = atomicState.Reject() - if err != nil { - return fmt.Errorf("failed to reject atomic state: %w", err) - } + if err != nil { + return err } - return nil + return atomicState.Reject() } // AtomicTxs returns the atomic transactions in this block. diff --git a/plugin/evm/extension/config.go b/plugin/evm/extension/config.go index 4f9e492e10..82cde803d5 100644 --- a/plugin/evm/extension/config.go +++ b/plugin/evm/extension/config.go @@ -101,8 +101,7 @@ type BlockExtension interface { // it can be implemented to extend inner block verification SemanticVerify() error // CleanupVerified is called when a block has passed SemanticVerify and SynctacticVerify, - // and should be cleaned up due to error or verification runs under non-write mode. This - // does not return an error because the block has already been verified. + // and should be cleaned up due to error or verification runs under non-write mode. CleanupVerified() error // Accept is called when a block is accepted by the block manager. Accept takes a // database.Batch that contains the changes that were made to the database as a result From 71684ffc08076e8743ab15716cb48bd83ae901a6 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Thu, 25 Sep 2025 16:09:13 -0400 Subject: [PATCH 19/28] document why error isn't returned --- plugin/evm/atomic/vm/block_extension.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 068300d951..7da9449f80 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -231,7 +231,9 @@ func (be *blockExtension) CleanupVerified() error { vm := be.blockExtender.vm atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) if err != nil { - return err + // If atomic state doesn't exist, it means verification failed before + // the state was created, so there's nothing to clean up + return nil } return atomicState.Reject() } From d5917a1718cfa9b99151b9ce130136037a0a0765 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Thu, 25 Sep 2025 16:23:55 -0400 Subject: [PATCH 20/28] lint --- plugin/evm/atomic/vm/block_extension.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 7da9449f80..dc4d8272aa 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -233,7 +233,7 @@ func (be *blockExtension) CleanupVerified() error { if err != nil { // If atomic state doesn't exist, it means verification failed before // the state was created, so there's nothing to clean up - return nil + return nil //nolint:nilerr } return atomicState.Reject() } From 27a67b6310259f99fcf6dc05d8fd12b04b7686ae Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 29 Sep 2025 16:46:59 -0400 Subject: [PATCH 21/28] remove t --- plugin/evm/atomic/vm/vm_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 52afa783c1..751cd685df 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -191,7 +191,7 @@ func testIssueAtomicTxs(t *testing.T, scheme string) { utxos := map[ids.ShortID]uint64{ vmtest.TestShortIDAddrs[0]: importAmount, } - require.NoError(t, addUTXOs(tvm.AtomicMemory, vm.Ctx, utxos)) + require.NoError(addUTXOs(tvm.AtomicMemory, vm.Ctx, utxos)) defer func() { require.NoError(vm.Shutdown(context.Background())) }() @@ -837,8 +837,8 @@ func TestConsecutiveAtomicTransactionsRevertSnapshot(t *testing.T) { // Add the two conflicting transactions directly to the mempool, so that two consecutive transactions // will fail verification when build block is called. - require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[1])) - require.NoError(t, vm.AtomicMempool.ForceAddTx(importTxs[2])) + require.NoError(vm.AtomicMempool.ForceAddTx(importTxs[1])) + require.NoError(vm.AtomicMempool.ForceAddTx(importTxs[2])) _, err = vm.BuildBlock(context.Background()) require.ErrorIs(err, ErrEmptyBlock) From 33ab908da0f92cb9712b45e834984b2319ff8630 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 29 Sep 2025 16:51:22 -0400 Subject: [PATCH 22/28] Lint --- plugin/evm/atomic/vm/vm_test.go | 4 +--- plugin/evm/vmtest/test_syncervm.go | 10 +++++----- sync/statesync/code_queue_test.go | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 751cd685df..835eabdee9 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -875,9 +875,7 @@ func TestAtomicTxBuildBlockDropsConflicts(t *testing.T) { err = vm.AtomicMempool.AddLocalTx(conflictTx) require.ErrorIs(err, txpool.ErrConflict) // force add the tx - if err := vm.AtomicMempool.ForceAddTx(conflictTx); err != nil { - t.Fatal(err) - } + require.NoError(vm.AtomicMempool.ForceAddTx(conflictTx)) conflictSets[index].Add(conflictTx.ID()) } msg, err := vm.WaitForEvent(context.Background()) diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index a593c9e07f..b5c7ffaad5 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -156,7 +156,9 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup appSender.SendAppRequestF = func(ctx context.Context, nodeSet set.Set[ids.NodeID], requestID uint32, request []byte) error { nodeID, hasItem := nodeSet.Pop() require.True(hasItem, "expected nodeSet to contain at least 1 nodeID") - go testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request) + go func() { + require.NoError(testSyncVMSetup.serverVM.VM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request)) + }() return nil } ResetMetrics(testSyncVMSetup.syncerVM.SnowCtx) @@ -223,7 +225,7 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup if test.responseIntercept == nil { go func() { if err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response); err != nil { - t.Errorf("AppResponse failed: %v", err) + require.NoError(err, "AppResponse failed") } }() } else { @@ -368,9 +370,7 @@ func initSyncServerAndClientVMs(t *testing.T, test SyncTestParams, numBlocks int serverTest.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { go func() { - if err := syncerVM.AppResponse(ctx, nodeID, requestID, response); err != nil { - t.Errorf("AppResponse failed: %v", err) - } + require.NoError(syncerVM.AppResponse(ctx, nodeID, requestID, response)) }() } else { go test.responseIntercept(syncerVM, nodeID, requestID, response) diff --git a/sync/statesync/code_queue_test.go b/sync/statesync/code_queue_test.go index b46ad1d07f..e57a2e729b 100644 --- a/sync/statesync/code_queue_test.go +++ b/sync/statesync/code_queue_test.go @@ -235,7 +235,7 @@ func TestQuitAndAddCodeRace(t *testing.T) { in := []common.Hash{{}} ready.Done() <-start - q.AddCode(in) + require.NoError(t, q.AddCode(in)) }() ready.Wait() From 42dd97199ad265d0ab8fac0b56ca1041e5f2c9a8 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Mon, 29 Sep 2025 17:05:36 -0400 Subject: [PATCH 23/28] fix test --- sync/statesync/code_queue_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sync/statesync/code_queue_test.go b/sync/statesync/code_queue_test.go index e57a2e729b..bb6c14f40f 100644 --- a/sync/statesync/code_queue_test.go +++ b/sync/statesync/code_queue_test.go @@ -235,7 +235,9 @@ func TestQuitAndAddCodeRace(t *testing.T) { in := []common.Hash{{}} ready.Done() <-start - require.NoError(t, q.AddCode(in)) + // Due to the race condition, AddCode may either succeed or fail + // depending on whether the quit channel is closed first + _ = q.AddCode(in) }() ready.Wait() From c25baa1134d4ad32181b0080dd545e9e7120a355 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 8 Oct 2025 14:44:33 -0400 Subject: [PATCH 24/28] austin nits --- plugin/evm/atomic/vm/block_extension.go | 11 +++-------- plugin/evm/extension/config.go | 2 +- plugin/evm/vmtest/test_syncervm.go | 5 ++--- plugin/evm/wrapped_block.go | 4 +--- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index dc4d8272aa..23ff7aeba5 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -227,15 +227,10 @@ func (be *blockExtension) Reject() error { } // CleanupVerified is called when the block is cleaned up after a failed insertion. -func (be *blockExtension) CleanupVerified() error { +func (be *blockExtension) CleanupVerified() { vm := be.blockExtender.vm - atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) - if err != nil { - // If atomic state doesn't exist, it means verification failed before - // the state was created, so there's nothing to clean up - return nil //nolint:nilerr - } - return atomicState.Reject() + atomicState, _ := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) + atomicState.Reject() } // AtomicTxs returns the atomic transactions in this block. diff --git a/plugin/evm/extension/config.go b/plugin/evm/extension/config.go index 82cde803d5..3fe676866f 100644 --- a/plugin/evm/extension/config.go +++ b/plugin/evm/extension/config.go @@ -102,7 +102,7 @@ type BlockExtension interface { SemanticVerify() error // CleanupVerified is called when a block has passed SemanticVerify and SynctacticVerify, // and should be cleaned up due to error or verification runs under non-write mode. - CleanupVerified() error + CleanupVerified() // Accept is called when a block is accepted by the block manager. Accept takes a // database.Batch that contains the changes that were made to the database as a result // of accepting the block. The changes in the batch should be flushed to the database in this method. diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index b5c7ffaad5..c151836d33 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -224,9 +224,8 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup testSyncVMSetup.serverVM.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { go func() { - if err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response); err != nil { - require.NoError(err, "AppResponse failed") - } + err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response) + require.NoError(err, "AppResponse failed") }() } else { go test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) diff --git a/plugin/evm/wrapped_block.go b/plugin/evm/wrapped_block.go index ea5b2aeaab..8ff47e5cba 100644 --- a/plugin/evm/wrapped_block.go +++ b/plugin/evm/wrapped_block.go @@ -275,9 +275,7 @@ func (b *wrappedBlock) verify(predicateContext *precompileconfig.PredicateContex // got an error while inserting to blockchain, we may need to cleanup the extension. // so that the extension can be garbage collected. if doCleanup := err != nil || !writes; b.extension != nil && doCleanup { - if err := b.extension.CleanupVerified(); err != nil { - return fmt.Errorf("failed to cleanup extension: %w", err) - } + b.extension.CleanupVerified() } return err } From 8402d4c82a97c00860d31e8dc76ecb042a82d187 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 8 Oct 2025 14:52:12 -0400 Subject: [PATCH 25/28] revert function --- plugin/evm/atomic/vm/block_extension.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 23ff7aeba5..6171ef5b86 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -229,8 +229,9 @@ func (be *blockExtension) Reject() error { // CleanupVerified is called when the block is cleaned up after a failed insertion. func (be *blockExtension) CleanupVerified() { vm := be.blockExtender.vm - atomicState, _ := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()) - atomicState.Reject() + if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { + atomicState.Reject() + } } // AtomicTxs returns the atomic transactions in this block. From 5eb475370992bd4a2110b2cc7717dc3eaf33cc49 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 8 Oct 2025 15:10:36 -0400 Subject: [PATCH 26/28] austin nits --- plugin/evm/vmtest/test_syncervm.go | 3 +-- plugin/main.go | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/plugin/evm/vmtest/test_syncervm.go b/plugin/evm/vmtest/test_syncervm.go index c151836d33..d070f9c234 100644 --- a/plugin/evm/vmtest/test_syncervm.go +++ b/plugin/evm/vmtest/test_syncervm.go @@ -224,8 +224,7 @@ func StateSyncToggleEnabledToDisabledTest(t *testing.T, testSetup *SyncTestSetup testSyncVMSetup.serverVM.AppSender.SendAppResponseF = func(ctx context.Context, nodeID ids.NodeID, requestID uint32, response []byte) error { if test.responseIntercept == nil { go func() { - err := syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response) - require.NoError(err, "AppResponse failed") + require.NoError(syncReEnabledVM.AppResponse(ctx, nodeID, requestID, response), "AppResponse failed") }() } else { go test.responseIntercept(syncReEnabledVM, nodeID, requestID, response) diff --git a/plugin/main.go b/plugin/main.go index e91bd6f826..6804079836 100644 --- a/plugin/main.go +++ b/plugin/main.go @@ -32,8 +32,8 @@ func main() { fmt.Printf("failed to set fd limit correctly due to: %s\n", err) os.Exit(1) } - err = rpcchainvm.Serve(context.Background(), factory.NewPluginVM()) - if err != nil { + + if err := rpcchainvm.Serve(context.Background(), factory.NewPluginVM()); err != nil { fmt.Printf("failed to serve rpc chain vm: %s\n", err) os.Exit(1) } From 916fc03548b9a29ee5fc49e945fd7af2692db73e Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 8 Oct 2025 15:13:14 -0400 Subject: [PATCH 27/28] log error --- plugin/evm/atomic/vm/block_extension.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 6171ef5b86..43f60c75a0 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -230,7 +230,10 @@ func (be *blockExtension) Reject() error { func (be *blockExtension) CleanupVerified() { vm := be.blockExtender.vm if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { - atomicState.Reject() + err := atomicState.Reject() + if err != nil { + log.Error("failed to reject atomic state", "err", err) + } } } From 4c6bccfc77371b26272119f12ba2ae1078e5e0f9 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 8 Oct 2025 15:21:16 -0400 Subject: [PATCH 28/28] fix error format --- plugin/evm/atomic/vm/block_extension.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/evm/atomic/vm/block_extension.go b/plugin/evm/atomic/vm/block_extension.go index 43f60c75a0..1899e88b5f 100644 --- a/plugin/evm/atomic/vm/block_extension.go +++ b/plugin/evm/atomic/vm/block_extension.go @@ -230,8 +230,7 @@ func (be *blockExtension) Reject() error { func (be *blockExtension) CleanupVerified() { vm := be.blockExtender.vm if atomicState, err := vm.AtomicBackend.GetVerifiedAtomicState(be.block.GetEthBlock().Hash()); err == nil { - err := atomicState.Reject() - if err != nil { + if err := atomicState.Reject(); err != nil { log.Error("failed to reject atomic state", "err", err) } }