Skip to content
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
fd0c346
enable lint errcheck
JonathanOppenheimer Sep 10, 2025
8e8947d
Update core/state_manager_test.go
JonathanOppenheimer Sep 10, 2025
73d6f52
Update plugin/evm/atomic/state/atomic_trie.go
JonathanOppenheimer Sep 10, 2025
ca5e761
in-line errors
JonathanOppenheimer Sep 10, 2025
ee18188
Fix TestConsecutiveAtomicTransactionsRevertSnapshot
JonathanOppenheimer Sep 10, 2025
e063e0d
lint
JonathanOppenheimer Sep 10, 2025
5b12fa2
Apply suggestions from code review
JonathanOppenheimer Sep 11, 2025
2b038aa
Stephen suggestions
JonathanOppenheimer Sep 11, 2025
97569ba
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 15, 2025
39a0e80
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 15, 2025
e7601ef
Apply suggestions from code review
JonathanOppenheimer Sep 15, 2025
ab4d3b8
Review suggestions
JonathanOppenheimer Sep 15, 2025
c769879
lint
JonathanOppenheimer Sep 15, 2025
be7513a
back to force add
JonathanOppenheimer Sep 15, 2025
07d4242
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 15, 2025
7321bed
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 17, 2025
9dbd1ff
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 22, 2025
726a4ce
align test
JonathanOppenheimer Sep 22, 2025
0496072
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 23, 2025
21b37d4
back to =
JonathanOppenheimer Sep 23, 2025
faae4e0
try this
JonathanOppenheimer Sep 23, 2025
f0012a4
don't panic!
JonathanOppenheimer Sep 23, 2025
cc4a475
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 23, 2025
1f6e70a
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 24, 2025
905e7ce
lint
JonathanOppenheimer Sep 24, 2025
956a0af
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 24, 2025
436538c
Austin suggestions
JonathanOppenheimer Sep 25, 2025
dbff479
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 25, 2025
71684ff
document why error isn't returned
JonathanOppenheimer Sep 25, 2025
9cb3fcc
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 25, 2025
d5917a1
lint
JonathanOppenheimer Sep 25, 2025
af2036c
Merge remote-tracking branch 'origin/master' into JonathanOppenheimer…
JonathanOppenheimer Sep 29, 2025
27a67b6
remove t
JonathanOppenheimer Sep 29, 2025
33ab908
Lint
JonathanOppenheimer Sep 29, 2025
42dd971
fix test
JonathanOppenheimer Sep 29, 2025
47183e1
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Sep 30, 2025
8262bea
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Oct 2, 2025
c25baa1
austin nits
JonathanOppenheimer Oct 8, 2025
24527b9
Merge branch 'master' into JonathanOppenheimer/lint-enable-errcheck
JonathanOppenheimer Oct 8, 2025
8402d4c
revert function
JonathanOppenheimer Oct 8, 2025
5eb4753
austin nits
JonathanOppenheimer Oct 8, 2025
916fc03
log error
JonathanOppenheimer Oct 8, 2025
4c6bccf
fix error format
JonathanOppenheimer Oct 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .avalanche-golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ linters:
- bodyclose
- copyloopvar
# - depguard
# - errcheck
- errcheck
- errorlint
# - forbidigo
- goconst
Expand Down
2 changes: 1 addition & 1 deletion core/blockchain_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/fifo_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions core/state_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

w.AcceptTrie(block)
if i <= tipBufferSize {
require.Zero(m.LastDereference, "should not have dereferenced block on accept")
} else {
Expand All @@ -71,7 +71,7 @@ func TestCappedMemoryTrieWriter(t *testing.T) {
m.LastCommit = common.Hash{}
}

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{}
Expand All @@ -96,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")

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{}

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{}
Expand Down
12 changes: 9 additions & 3 deletions plugin/evm/atomic/state/atomic_trie.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These trie error handling changes are lowkey terrifying... Need to dig into these to make sure they are safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: not scary at all. An error will only be returned if not called with HashDB (because geth is a nightmare)

Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ func (a *AtomicTrie) InsertTrie(nodes *trienode.NodeSet, root common.Hash) error
return err
}
}
a.trieDB.Reference(root, common.Hash{})
if err := a.trieDB.Reference(root, common.Hash{}); 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.
Expand Down Expand Up @@ -285,12 +287,16 @@ 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)
if err := a.trieDB.Dereference(a.lastAcceptedRoot); err != nil {
return false, err
}
a.lastAcceptedRoot = root
return hasCommitted, nil
}

func (a *AtomicTrie) RejectTrie(root common.Hash) error {
a.trieDB.Dereference(root)
if err := a.trieDB.Dereference(root); err != nil {
return err
}
return nil
}
2 changes: 1 addition & 1 deletion plugin/evm/atomic/state/atomic_trie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,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")
}
atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight)
require.NoError(t, atomicTrie.updateLastCommitted(testCase.lastCommittedRoot, testCase.lastCommittedHeight))

hasCommitted, err := atomicTrie.AcceptTrie(testCase.height, testCase.root)
require.NoError(t, err)
Expand Down
11 changes: 8 additions & 3 deletions plugin/evm/atomic/vm/block_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,16 @@ 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 {
atomicState.Reject()
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)
}
}
return nil
}

// AtomicTxs returns the atomic transactions in this block.
Expand Down
4 changes: 2 additions & 2 deletions plugin/evm/atomic/vm/gossiper_atomic_gossiping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) {
tx, conflictingTx := importTxs[0], importTxs[1]
txID := tx.ID()

vm.AtomicMempool.AddRemoteTx(tx)
require.NoError(vm.AtomicMempool.AddRemoteTx(tx))
vm.AtomicMempool.NextTx()
vm.AtomicMempool.DiscardCurrentTx(txID)

Expand Down Expand Up @@ -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.
vm.AtomicMempool.AddRemoteTx(conflictingTx)
require.NoError(vm.AtomicMempool.AddRemoteTx(conflictingTx))
vm.AtomicTxPushGossiper.Add(conflictingTx)
time.Sleep(500 * time.Millisecond)

Expand Down
90 changes: 24 additions & 66 deletions plugin/evm/atomic/vm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,42 +190,25 @@ 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)
}
}()

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, blk.Accept(context.Background()))

if lastAcceptedID, err := vm.LastAccepted(context.Background()); err != nil {
t.Fatal(err)
Expand All @@ -235,36 +218,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)
Expand Down Expand Up @@ -1067,30 +1035,18 @@ 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)
}

if err := blk.Verify(context.Background()); err != nil {
t.Fatal(err)
}

if err := vm.SetPreference(context.Background(), blk.ID()); err != nil {
t.Fatal(err)
}
require.NoError(t, err)

if err := blk.Accept(context.Background()); err != nil {
t.Fatal(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
if newHead.Head.Hash() != common.Hash(blk.ID()) {
Expand All @@ -1099,8 +1055,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.
vm.AtomicMempool.AddRemoteTx(importTxs[1])
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")
Expand Down Expand Up @@ -1146,7 +1102,9 @@ func TestAtomicTxBuildBlockDropsConflicts(t *testing.T) {
t.Fatal("should conflict with the utxoSet in the mempool")
}
// force add the tx
vm.AtomicMempool.ForceAddTx(conflictTx)
if err := vm.AtomicMempool.ForceAddTx(conflictTx); err != nil {
t.Fatal(err)
}
conflictSets[index].Add(conflictTx.ID())
}
msg, err := vm.WaitForEvent(context.Background())
Expand Down Expand Up @@ -2112,7 +2070,7 @@ func TestWaitForEvent(t *testing.T) {
},
}}}}))
testCase.testCase(t, vm, address, key)
vm.Shutdown(context.Background())
require.NoError(t, vm.Shutdown(context.Background()))
})
}
}
2 changes: 1 addition & 1 deletion plugin/evm/extension/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion plugin/evm/message/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions plugin/evm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,9 @@ func (vm *VM) Initialize(

// Add p2p warp message warpHandler
warpHandler := acp118.NewCachedHandler(meteredCache, vm.warpBackend, vm.ctx.WarpSigner)
vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler)
if err = vm.Network.AddHandler(p2p.SignatureRequestHandlerID, warpHandler); err != nil {
return err
}

vm.stateSyncDone = make(chan struct{})

Expand Down Expand Up @@ -888,7 +890,9 @@ func (vm *VM) Shutdown(context.Context) error {
for _, handler := range vm.rpcHandlers {
handler.Stop()
}
vm.eth.Stop()
if err := vm.eth.Stop(); err != nil {
log.Error("error stopping eth", "err", err)
}
vm.shutdownWg.Wait()
return nil
}
Expand Down
Loading
Loading