From 999f2c0147489aae8516aeab50d3a42dd1d81dee Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 1 Oct 2025 17:16:54 +0100 Subject: [PATCH 1/6] feat: `trienode` payloads for `Node`, `NodeSet`, and `MergedNodeSet` --- trie/trienode/node.go | 10 +- trie/trienode/node.libevm.go | 153 ++++++++++++++++++++++++++++++ trie/trienode/node.libevm_test.go | 85 +++++++++++++++++ 3 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 trie/trienode/node.libevm.go create mode 100644 trie/trienode/node.libevm_test.go diff --git a/trie/trienode/node.go b/trie/trienode/node.go index 8bd0a18ba38..f1618635c5a 100644 --- a/trie/trienode/node.go +++ b/trie/trienode/node.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/libevm/pseudo" ) // Node is a wrapper which contains the encoded blob of the trie node and its @@ -30,6 +31,8 @@ import ( type Node struct { Hash common.Hash // Node hash, empty for deleted node Blob []byte // Encoded node blob, nil for the deleted node + + extra *pseudo.Type // libevm } // Size returns the total memory size used by this node. @@ -64,6 +67,8 @@ type NodeSet struct { Nodes map[string]*Node updates int // the count of updated and inserted nodes deletes int // the count of deleted nodes + + extra *pseudo.Type // libevm } // NewNodeSet initializes a node set. The owner is zero for the account trie and @@ -97,6 +102,7 @@ func (set *NodeSet) AddNode(path []byte, n *Node) { set.updates += 1 } set.Nodes[string(path)] = n + set.mergePayload(path, n) } // Merge adds a set of nodes into the set. @@ -164,6 +170,8 @@ func (set *NodeSet) Summary() string { // MergedNodeSet represents a merged node set for a group of tries. type MergedNodeSet struct { Sets map[common.Hash]*NodeSet + + extra *pseudo.Type // libevm } // NewMergedNodeSet initializes an empty merged set. @@ -180,7 +188,7 @@ func NewWithNodeSet(set *NodeSet) *MergedNodeSet { // Merge merges the provided dirty nodes of a trie into the set. The assumption // is held that no duplicated set belonging to the same trie will be merged twice. -func (set *MergedNodeSet) Merge(other *NodeSet) error { +func (set *MergedNodeSet) merge(other *NodeSet) error { subset, present := set.Sets[other.Owner] if present { return subset.Merge(other.Owner, other.Nodes) diff --git a/trie/trienode/node.libevm.go b/trie/trienode/node.libevm.go new file mode 100644 index 00000000000..d9b6eecda7d --- /dev/null +++ b/trie/trienode/node.libevm.go @@ -0,0 +1,153 @@ +// Copyright 2025 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +package trienode + +import ( + "github.com/ava-labs/libevm/libevm/pseudo" + "github.com/ava-labs/libevm/libevm/register" +) + +// MergedNodeSetHooks +type MergedNodeSetHooks interface { + Merge(into *MergedNodeSet, _ *NodeSet) error +} + +// NodeSetHooks +type NodeSetHooks interface { + Add(into *NodeSet, path []byte, _ *Node) +} + +// RegisterExtras +func RegisterExtras[ + MNS, NS, N any, + MNSPtr interface { + MergedNodeSetHooks + *MNS + }, + NSPtr interface { + NodeSetHooks + *NS + }, +]() ExtraPayloads[MNSPtr, NSPtr, N] { + payloads := ExtraPayloads[MNSPtr, NSPtr, N]{ + MergedNodeSet: pseudo.NewAccessor[*MergedNodeSet, MNSPtr]( + (*MergedNodeSet).extraPayload, + func(s *MergedNodeSet, t *pseudo.Type) { s.extra = t }, + ), + NodeSet: pseudo.NewAccessor[*NodeSet, NSPtr]( + (*NodeSet).extraPayload, + func(s *NodeSet, t *pseudo.Type) { s.extra = t }, + ), + Node: pseudo.NewAccessor[*Node, N]( + (*Node).extraPayload, + func(n *Node, t *pseudo.Type) { n.extra = t }, + ), + } + + registeredExtras.MustRegister(&extraConstructors{ + newMergedNodeSet: pseudo.NewConstructor[MNS]().NewPointer, + newNodeSet: pseudo.NewConstructor[NS]().NewPointer, + newNode: pseudo.NewConstructor[N]().Zero, + hooks: payloads, + }) + + return payloads +} + +// TestOnlyClearRegisteredExtras +func TestOnlyClearRegisteredExtras() { + registeredExtras.TestOnlyClear() +} + +var registeredExtras register.AtMostOnce[*extraConstructors] + +type extraConstructors struct { + newMergedNodeSet func() *pseudo.Type + newNodeSet func() *pseudo.Type + newNode func() *pseudo.Type + hooks interface { + hooksFromMNS(*MergedNodeSet) MergedNodeSetHooks + hooksFromNS(*NodeSet) NodeSetHooks + } +} + +// Merge merges the provided dirty nodes of a trie into the set. The assumption +// is held that no duplicated set belonging to the same trie will be merged twice. +func (set *MergedNodeSet) Merge(other *NodeSet) error { + if err := set.merge(other); err != nil { + return err + } + if r := registeredExtras; r.Registered() { + return r.Get().hooks.hooksFromMNS(set).Merge(set, other) + } + return nil +} + +func (set *NodeSet) mergePayload(path []byte, n *Node) { + if r := registeredExtras; r.Registered() { + r.Get().hooks.hooksFromNS(set).Add(set, path, n) + } +} + +// ExtraPayloads +type ExtraPayloads[ + MNS MergedNodeSetHooks, + NS NodeSetHooks, + N any, +] struct { + MergedNodeSet pseudo.Accessor[*MergedNodeSet, MNS] + NodeSet pseudo.Accessor[*NodeSet, NS] + Node pseudo.Accessor[*Node, N] +} + +func (e ExtraPayloads[MNS, NS, N]) hooksFromMNS(s *MergedNodeSet) MergedNodeSetHooks { + return e.MergedNodeSet.Get(s) +} + +func (e ExtraPayloads[MNS, NS, N]) hooksFromNS(s *NodeSet) NodeSetHooks { + return e.NodeSet.Get(s) +} + +func extraPayloadOrSetDefault(field **pseudo.Type, construct func(*extraConstructors) *pseudo.Type) *pseudo.Type { + r := registeredExtras + if !r.Registered() { + // See params.ChainConfig.extraPayload() for panic rationale. + panic(".extraPayload() called before RegisterExtras()") + } + if *field == nil { + *field = construct(r.Get()) + } + return *field +} + +func (set *MergedNodeSet) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&set.extra, func(c *extraConstructors) *pseudo.Type { + return c.newMergedNodeSet() + }) +} + +func (set *NodeSet) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&set.extra, func(c *extraConstructors) *pseudo.Type { + return c.newNodeSet() + }) +} + +func (n *Node) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&n.extra, func(c *extraConstructors) *pseudo.Type { + return c.newNode() + }) +} diff --git a/trie/trienode/node.libevm_test.go b/trie/trienode/node.libevm_test.go new file mode 100644 index 00000000000..e2ad6701be2 --- /dev/null +++ b/trie/trienode/node.libevm_test.go @@ -0,0 +1,85 @@ +// Copyright 2025 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +package trienode + +import ( + "maps" + "testing" + + "github.com/ava-labs/libevm/common" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) + +type nodePayload struct { + x uint64 +} + +type setPayload struct { + added map[string]uint64 +} + +func (p *setPayload) Add(_ *NodeSet, path []byte, n *Node) { + if p.added == nil { + p.added = make(map[string]uint64) + } + p.added[string(path)] = extras.Node.Get(n).x +} + +type mergedSetPayload struct { + merged []map[string]uint64 +} + +func (p *mergedSetPayload) Merge(_ *MergedNodeSet, ns *NodeSet) error { + p.merged = append(p.merged, maps.Clone(extras.NodeSet.Get(ns).added)) + return nil +} + +var extras ExtraPayloads[*mergedSetPayload, *setPayload, nodePayload] + +func TestExtras(t *testing.T) { + extras = RegisterExtras[mergedSetPayload, setPayload, nodePayload]() + t.Cleanup(TestOnlyClearRegisteredExtras) + + n1 := New(common.Hash{0}, nil) + extras.Node.Set(n1, nodePayload{x: 1}) + n42 := New(common.Hash{1}, nil) + extras.Node.Set(n42, nodePayload{x: 42}) + + set := NewNodeSet(common.Hash{}) + merge := NewMergedNodeSet() + set.AddNode([]byte("n1"), n1) + require.NoError(t, merge.Merge(set)) + + set.AddNode([]byte("n42"), n42) + require.NoError(t, merge.Merge(set)) + + got := extras.MergedNodeSet.Get(merge).merged + want := []map[string]uint64{ + { + "n1": 1, + }, + { + "n1": 1, + "n42": 42, + }, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("%T payload diff (-want +got):\n%s", merge, diff) + } +} From 42a256ef19639cf1e3208a951e8d6d4ec00ee68c Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 1 Oct 2025 21:08:35 +0100 Subject: [PATCH 2/6] feat: `libevm/triedb/firewood` scaffolding with integration test for proposal propagation --- libevm/triedb/firewood/firewood.go | 25 ++++ libevm/triedb/firewood/proposals.go | 56 +++++++++ libevm/triedb/firewood/proposals_test.go | 141 +++++++++++++++++++++++ trie/trienode/node.libevm.go | 8 +- trie/trienode/node.libevm_test.go | 4 +- 5 files changed, 228 insertions(+), 6 deletions(-) create mode 100644 libevm/triedb/firewood/firewood.go create mode 100644 libevm/triedb/firewood/proposals.go create mode 100644 libevm/triedb/firewood/proposals_test.go diff --git a/libevm/triedb/firewood/firewood.go b/libevm/triedb/firewood/firewood.go new file mode 100644 index 00000000000..6c18dfa4ec6 --- /dev/null +++ b/libevm/triedb/firewood/firewood.go @@ -0,0 +1,25 @@ +// Copyright 2025 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +// The firewood package provides a [triedb.BackendDB] based on [Firewood]. +// +// [Firewood]: https://github.com/ava-labs/firewood +package firewood + +import "github.com/ava-labs/libevm/triedb" + +// Protects the import in this file so [triedb.BackendDB] linked comments work. +var _ triedb.BackendDB diff --git a/libevm/triedb/firewood/proposals.go b/libevm/triedb/firewood/proposals.go new file mode 100644 index 00000000000..6a38f608eac --- /dev/null +++ b/libevm/triedb/firewood/proposals.go @@ -0,0 +1,56 @@ +// Copyright 2025 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +package firewood + +import ( + "fmt" + + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/trie/trienode" +) + +// RegisterExtras +func RegisterExtras() { + extras = trienode.RegisterExtras[proposal, proposal, struct{}]() +} + +var extras trienode.ExtraPayloads[*proposal, *proposal, struct{}] + +type proposal struct { + handle *handle +} + +// TODO(alarso16) this type is entirely arbitrary and exists only to allow +// initial integration testing. +type handle struct { + root common.Hash + memoryFreed bool +} + +// MergeNodeSet implements [trienode.MergedNodeSetHooks], copying at most one +// proposal handle into the merged set. +func (h *proposal) MergeNodeSet(into *trienode.MergedNodeSet, set *trienode.NodeSet) error { + merged := extras.MergedNodeSet.Get(into) + if merged.handle != nil { + return fmt.Errorf(">1 %T carrying non-nil %T", set, merged.handle) + } + merged.handle = extras.NodeSet.Get(set).handle + return nil +} + +// AddNode implements [trienode.NodeSetHooks] as a noop. +func (h *proposal) AddNode(*trienode.NodeSet, []byte, *trienode.Node) {} diff --git a/libevm/triedb/firewood/proposals_test.go b/libevm/triedb/firewood/proposals_test.go new file mode 100644 index 00000000000..102d6b0f4dc --- /dev/null +++ b/libevm/triedb/firewood/proposals_test.go @@ -0,0 +1,141 @@ +// Copyright 2025 the libevm authors. +// +// The libevm additions to go-ethereum are free software: you can redistribute +// them and/or modify them under the terms of the GNU Lesser General Public License +// as published by the Free Software Foundation, either version 3 of the License, +// or (at your option) any later version. +// +// The libevm additions are distributed in the hope that they will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser +// General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see +// . + +package firewood + +import ( + "os" + "runtime" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/rawdb" + "github.com/ava-labs/libevm/core/state" + "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/ethdb" + "github.com/ava-labs/libevm/libevm/stateconf" + "github.com/ava-labs/libevm/trie" + "github.com/ava-labs/libevm/trie/trienode" + "github.com/ava-labs/libevm/trie/triestate" + "github.com/ava-labs/libevm/triedb" + "github.com/ava-labs/libevm/triedb/database" + "github.com/ava-labs/libevm/triedb/hashdb" +) + +func TestMain(m *testing.M) { + RegisterExtras() + os.Exit(m.Run()) +} + +type hashDBWithDummyProposals struct { + *hashdb.Database + gotProposalHandle *handle +} + +func (db *hashDBWithDummyProposals) Reader(root common.Hash) (database.Reader, error) { + return db.Database.Reader(root) +} + +func (db *hashDBWithDummyProposals) Update(root, parent common.Hash, block uint64, nodes *trienode.MergedNodeSet, states *triestate.Set, opts ...stateconf.TrieDBUpdateOption) error { + db.gotProposalHandle = extras.MergedNodeSet.Get(nodes).handle + return db.Database.Update(root, parent, block, nodes, states, opts...) +} + +type cacheWithDummyProposals struct { + state.Database +} + +func (db *cacheWithDummyProposals) OpenTrie(root common.Hash) (state.Trie, error) { + t, err := db.Database.OpenTrie(root) + if err != nil { + return nil, err + } + return &trieWithDummyProposals{Trie: t}, nil +} + +func (db *cacheWithDummyProposals) CopyTrie(t state.Trie) state.Trie { + return &trieWithDummyProposals{ + Trie: db.Database.CopyTrie(t.(*trieWithDummyProposals).Trie), // let it panic, see if I care! + } +} + +type trieWithDummyProposals struct { + state.Trie +} + +func (t *trieWithDummyProposals) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet, error) { + root, set, err := t.Trie.Commit(collectLeaf) + if err != nil { + return common.Hash{}, nil, err + } + + // This, combined with [proposalPayload.MergeNodeSet], is where the magic + // happens. We use the existing geth plumbing to carry the proposal back to + // [hashDBWithDummyProposals.Update], knowing that the Go GC will trigger + // the FFI call to free the Rust memory. + payload := &proposal{ + handle: &handle{root: root}, + } + runtime.SetFinalizer(payload, func(p *proposal) { + p.handle.memoryFreed = true + }) + extras.NodeSet.Set(set, payload) + + return root, set, nil +} + +func TestProposalPropagation(t *testing.T) { + db := rawdb.NewMemoryDatabase() + backend := &hashDBWithDummyProposals{ + Database: hashdb.New(db, nil, trie.MerkleResolver{}), + } + tdb := triedb.NewDatabase(db, &triedb.Config{ + DBOverride: func(db ethdb.Database) triedb.DBOverride { + return backend + }, + }) + + cache := &cacheWithDummyProposals{ + Database: state.NewDatabaseWithNodeDB(db, tdb), + } + sdb, err := state.New(types.EmptyRootHash, cache, nil) + require.NoError(t, err, "state.New([empty root], ...)") + + sdb.SetState(common.Address{}, common.Hash{}, common.Hash{42}) + root, err := sdb.Commit(1, false) + require.NoErrorf(t, err, "%T.Commit()", sdb) + + got := backend.gotProposalHandle + want := &handle{ + root: root, + memoryFreed: false, + } + if diff := cmp.Diff(want, got, cmp.AllowUnexported(handle{})); diff != "" { + t.Errorf("diff (-want +got):\n%s", diff) + } + + // Ensure that the proposal payload is no longer reachable. + sdb = nil + cache = nil + tdb = nil + backend = nil + runtime.GC() + assert.True(t, got.memoryFreed) +} diff --git a/trie/trienode/node.libevm.go b/trie/trienode/node.libevm.go index d9b6eecda7d..771ef0034b3 100644 --- a/trie/trienode/node.libevm.go +++ b/trie/trienode/node.libevm.go @@ -23,12 +23,12 @@ import ( // MergedNodeSetHooks type MergedNodeSetHooks interface { - Merge(into *MergedNodeSet, _ *NodeSet) error + MergeNodeSet(into *MergedNodeSet, _ *NodeSet) error } // NodeSetHooks type NodeSetHooks interface { - Add(into *NodeSet, path []byte, _ *Node) + AddNode(into *NodeSet, path []byte, _ *Node) } // RegisterExtras @@ -92,14 +92,14 @@ func (set *MergedNodeSet) Merge(other *NodeSet) error { return err } if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromMNS(set).Merge(set, other) + return r.Get().hooks.hooksFromMNS(set).MergeNodeSet(set, other) } return nil } func (set *NodeSet) mergePayload(path []byte, n *Node) { if r := registeredExtras; r.Registered() { - r.Get().hooks.hooksFromNS(set).Add(set, path, n) + r.Get().hooks.hooksFromNS(set).AddNode(set, path, n) } } diff --git a/trie/trienode/node.libevm_test.go b/trie/trienode/node.libevm_test.go index e2ad6701be2..244a69dcfc4 100644 --- a/trie/trienode/node.libevm_test.go +++ b/trie/trienode/node.libevm_test.go @@ -33,7 +33,7 @@ type setPayload struct { added map[string]uint64 } -func (p *setPayload) Add(_ *NodeSet, path []byte, n *Node) { +func (p *setPayload) AddNode(_ *NodeSet, path []byte, n *Node) { if p.added == nil { p.added = make(map[string]uint64) } @@ -44,7 +44,7 @@ type mergedSetPayload struct { merged []map[string]uint64 } -func (p *mergedSetPayload) Merge(_ *MergedNodeSet, ns *NodeSet) error { +func (p *mergedSetPayload) MergeNodeSet(_ *MergedNodeSet, ns *NodeSet) error { p.merged = append(p.merged, maps.Clone(extras.NodeSet.Get(ns).added)) return nil } From 7cb934736a5c28355eeb674f757f6706e560f59c Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 2 Oct 2025 16:11:07 +0100 Subject: [PATCH 3/6] refactor: collapse `handle` into `proposal` --- libevm/triedb/firewood/firewood.go | 35 ++++++++-- libevm/triedb/firewood/proposals.go | 67 +++++++++++++----- libevm/triedb/firewood/proposals_test.go | 89 ++++++++++++------------ trie/trienode/node.libevm.go | 53 ++++++++------ trie/trienode/node.libevm_test.go | 10 +-- 5 files changed, 161 insertions(+), 93 deletions(-) diff --git a/libevm/triedb/firewood/firewood.go b/libevm/triedb/firewood/firewood.go index 6c18dfa4ec6..c8fe6345a87 100644 --- a/libevm/triedb/firewood/firewood.go +++ b/libevm/triedb/firewood/firewood.go @@ -14,12 +14,39 @@ // along with the go-ethereum library. If not, see // . -// The firewood package provides a [triedb.BackendDB] based on [Firewood]. +// The firewood package provides a [triedb.DBOverride] backed by [Firewood]. // // [Firewood]: https://github.com/ava-labs/firewood package firewood -import "github.com/ava-labs/libevm/triedb" +import ( + "errors" + "runtime" -// Protects the import in this file so [triedb.BackendDB] linked comments work. -var _ triedb.BackendDB + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/libevm/stateconf" + "github.com/ava-labs/libevm/trie/trienode" + "github.com/ava-labs/libevm/trie/triestate" + "github.com/ava-labs/libevm/triedb" +) + +var _ triedb.DBOverride = (*database)(nil) + +type database struct { + triedb.DBOverride // TODO(alarso16) remove once this type implements the interface +} + +func (db *database) Update(root, parent common.Hash, block uint64, nodes *trienode.MergedNodeSet, states *triestate.Set, opts ...stateconf.TrieDBUpdateOption) error { + // TODO(alarso16) + var _ *proposal = extras.MergedNodeSet.Get(nodes) + + db.afterUpdate(nodes) // MUST be the last statement before the final return + return errors.New("unimplemented") +} + +// afterUpdate MUST be called at the end of [database.Update] to ensure that the +// Rust handle isn't freed any earlier. This is an overly cautious, defensive +// approach that will make Rustaceans scream "I told you so". +func (db *database) afterUpdate(nodes *trienode.MergedNodeSet) { + runtime.KeepAlive(extras.MergedNodeSet.Get(nodes)) +} diff --git a/libevm/triedb/firewood/proposals.go b/libevm/triedb/firewood/proposals.go index 6a38f608eac..7412fbf87ff 100644 --- a/libevm/triedb/firewood/proposals.go +++ b/libevm/triedb/firewood/proposals.go @@ -18,39 +18,72 @@ package firewood import ( "fmt" + "runtime" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/trie/trienode" ) -// RegisterExtras +// RegisterExtras registers Firewood proposals with [trienode.RegisterExtras]. +// This MUST be called in and only in tests / package main to avoid polluting +// other packages. A call to RegisterExtras is required for the rest of this +// package to function correctly. func RegisterExtras() { extras = trienode.RegisterExtras[proposal, proposal, struct{}]() } -var extras trienode.ExtraPayloads[*proposal, *proposal, struct{}] +var extras trienode.ExtraPayloads[*proposal, *proposal, *struct{}] +// A proposal is embedded as a payload in the [trienode.NodeSet] object returned +// by trie `Commit()`. A preceding call to [RegisterExtras] ensures that the +// proposal will be propagated to [Database.Update]. +// +// After construction, [proposal.setFinalizer] SHOULD be called to ensure +// release of resources via [proposal.free] once the proposal is garbage +// collected. type proposal struct { - handle *handle + // root MUST match the argument returned by the trie's `Commit()` method. + root common.Hash + + // TODO(alarso16) add handles etc. here and clean them up in [proposal.free] + + finalized chan struct{} // https://go.dev/doc/gc-guide#Testing_object_death +} + +func (p *proposal) injectInto(ns *trienode.NodeSet) { + extras.NodeSet.Set(ns, p) +} + +// setFinalizer calls [runtime.SetFinalizer] with `p`. +func (p *proposal) setFinalizer() { + p.finalized = make(chan struct{}) + runtime.SetFinalizer(p, (*proposal).finalizer) +} + +// finalizer is expected to be passed to [runtime.SetFinalizer], abstracted as a +// method to guarantee that it doesn't accidentally capture the value being +// collected, thus resurrecting it. +func (p *proposal) finalizer() { + p.free() + close(p.finalized) } -// TODO(alarso16) this type is entirely arbitrary and exists only to allow -// initial integration testing. -type handle struct { - root common.Hash - memoryFreed bool +// free is called when the [proposal] is no longer reachable. +func (p *proposal) free() { + // TODO(alarso16) free the Rust object(s). } -// MergeNodeSet implements [trienode.MergedNodeSetHooks], copying at most one -// proposal handle into the merged set. -func (h *proposal) MergeNodeSet(into *trienode.MergedNodeSet, set *trienode.NodeSet) error { - merged := extras.MergedNodeSet.Get(into) - if merged.handle != nil { - return fmt.Errorf(">1 %T carrying non-nil %T", set, merged.handle) +// AfterMergeNodeSet implements [trienode.MergedNodeSetHooks], copying at most +// one proposal handle into the merged set. +func (h *proposal) AfterMergeNodeSet(into *trienode.MergedNodeSet, ns *trienode.NodeSet) error { + if p := extras.MergedNodeSet.Get(into); p.root != (common.Hash{}) { + return fmt.Errorf(">1 %T carrying non-zero %T", ns, p) } - merged.handle = extras.NodeSet.Get(set).handle + // The GC finalizer is attached to the [payload], not to the [handle], so + // we have to copy the entire object to ensure that it remains reachable. + extras.MergedNodeSet.Set(into, extras.NodeSet.Get(ns)) return nil } -// AddNode implements [trienode.NodeSetHooks] as a noop. -func (h *proposal) AddNode(*trienode.NodeSet, []byte, *trienode.Node) {} +// AfterAddNode implements [trienode.NodeSetHooks] as a noop. +func (h *proposal) AfterAddNode(*trienode.NodeSet, []byte, *trienode.Node) {} diff --git a/libevm/triedb/firewood/proposals_test.go b/libevm/triedb/firewood/proposals_test.go index 102d6b0f4dc..0a1fea0215d 100644 --- a/libevm/triedb/firewood/proposals_test.go +++ b/libevm/triedb/firewood/proposals_test.go @@ -20,9 +20,8 @@ import ( "os" "runtime" "testing" + "time" - "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ava-labs/libevm/common" @@ -35,7 +34,7 @@ import ( "github.com/ava-labs/libevm/trie/trienode" "github.com/ava-labs/libevm/trie/triestate" "github.com/ava-labs/libevm/triedb" - "github.com/ava-labs/libevm/triedb/database" + databasepkg "github.com/ava-labs/libevm/triedb/database" "github.com/ava-labs/libevm/triedb/hashdb" ) @@ -44,20 +43,8 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -type hashDBWithDummyProposals struct { - *hashdb.Database - gotProposalHandle *handle -} - -func (db *hashDBWithDummyProposals) Reader(root common.Hash) (database.Reader, error) { - return db.Database.Reader(root) -} - -func (db *hashDBWithDummyProposals) Update(root, parent common.Hash, block uint64, nodes *trienode.MergedNodeSet, states *triestate.Set, opts ...stateconf.TrieDBUpdateOption) error { - db.gotProposalHandle = extras.MergedNodeSet.Get(nodes).handle - return db.Database.Update(root, parent, block, nodes, states, opts...) -} - +// A cacheWithDummyProposals overrides `OpenTrie()` to returns a +// [trieWithDummyProposals]. type cacheWithDummyProposals struct { state.Database } @@ -70,12 +57,8 @@ func (db *cacheWithDummyProposals) OpenTrie(root common.Hash) (state.Trie, error return &trieWithDummyProposals{Trie: t}, nil } -func (db *cacheWithDummyProposals) CopyTrie(t state.Trie) state.Trie { - return &trieWithDummyProposals{ - Trie: db.Database.CopyTrie(t.(*trieWithDummyProposals).Trie), // let it panic, see if I care! - } -} - +// A trieWithDummyProposals overrides `Commit()` to inject a [proposal] into the +// returned [trienode.NodeSet]. type trieWithDummyProposals struct { state.Trie } @@ -85,22 +68,33 @@ func (t *trieWithDummyProposals) Commit(collectLeaf bool) (common.Hash, *trienod if err != nil { return common.Hash{}, nil, err } - // This, combined with [proposalPayload.MergeNodeSet], is where the magic // happens. We use the existing geth plumbing to carry the proposal back to // [hashDBWithDummyProposals.Update], knowing that the Go GC will trigger // the FFI call to free the Rust memory. - payload := &proposal{ - handle: &handle{root: root}, - } - runtime.SetFinalizer(payload, func(p *proposal) { - p.handle.memoryFreed = true - }) - extras.NodeSet.Set(set, payload) + p := &proposal{root: root} + p.setFinalizer() + p.injectInto(set) return root, set, nil } +// A hashDBWithDummyProposals overrides `Update()` to capture the [proposal] +// propagated from [trieWithDummyProposals.Commit]. +type hashDBWithDummyProposals struct { + *hashdb.Database + got *proposal +} + +func (db *hashDBWithDummyProposals) Reader(root common.Hash) (databasepkg.Reader, error) { + return db.Database.Reader(root) +} + +func (db *hashDBWithDummyProposals) Update(root, parent common.Hash, block uint64, nodes *trienode.MergedNodeSet, states *triestate.Set, opts ...stateconf.TrieDBUpdateOption) error { + db.got = extras.MergedNodeSet.Get(nodes) + return db.Database.Update(root, parent, block, nodes, states, opts...) +} + func TestProposalPropagation(t *testing.T) { db := rawdb.NewMemoryDatabase() backend := &hashDBWithDummyProposals{ @@ -122,20 +116,25 @@ func TestProposalPropagation(t *testing.T) { root, err := sdb.Commit(1, false) require.NoErrorf(t, err, "%T.Commit()", sdb) - got := backend.gotProposalHandle - want := &handle{ - root: root, - memoryFreed: false, - } - if diff := cmp.Diff(want, got, cmp.AllowUnexported(handle{})); diff != "" { - t.Errorf("diff (-want +got):\n%s", diff) + if got, want := backend.got.root, root; got != want { + t.Errorf("got %v; want %v", got, want) } - // Ensure that the proposal payload is no longer reachable. - sdb = nil - cache = nil - tdb = nil - backend = nil - runtime.GC() - assert.True(t, got.memoryFreed) + t.Run("GC_finalizer_invoked", func(t *testing.T) { + finalized := backend.got.finalized + + sdb = nil + cache = nil + tdb = nil + backend = nil + + // Note that [runtime.GC] doesn't block on finalizers; see + // https://go.dev/doc/gc-guide#Testing_object_death + runtime.GC() + select { + case <-finalized: + case <-time.After(5 * time.Second): + t.Errorf("%T finalizer did not run", &proposal{}) + } + }) } diff --git a/trie/trienode/node.libevm.go b/trie/trienode/node.libevm.go index 771ef0034b3..31061907aa0 100644 --- a/trie/trienode/node.libevm.go +++ b/trie/trienode/node.libevm.go @@ -21,17 +21,19 @@ import ( "github.com/ava-labs/libevm/libevm/register" ) -// MergedNodeSetHooks +// MergedNodeSetHooks are called as part of standard [MergedNodeSet] behaviour. type MergedNodeSetHooks interface { - MergeNodeSet(into *MergedNodeSet, _ *NodeSet) error + AfterMergeNodeSet(into *MergedNodeSet, _ *NodeSet) error } -// NodeSetHooks +// NodeSetHooks are called as part of standard [NodeSet] behaviour. type NodeSetHooks interface { - AddNode(into *NodeSet, path []byte, _ *Node) + AfterAddNode(into *NodeSet, path []byte, _ *Node) } -// RegisterExtras +// RegisterExtras registers types `MNSPtr`, `NSPtr`, and `N` to be carried as +// extra payloads in [MergedNodeSet], [NodeSet], and [Node] objects +// respectively. It MUST NOT be called more than once. func RegisterExtras[ MNS, NS, N any, MNSPtr interface { @@ -42,8 +44,9 @@ func RegisterExtras[ NodeSetHooks *NS }, -]() ExtraPayloads[MNSPtr, NSPtr, N] { - payloads := ExtraPayloads[MNSPtr, NSPtr, N]{ + NPtr interface{ *N }, +]() ExtraPayloads[MNSPtr, NSPtr, NPtr] { + payloads := ExtraPayloads[MNSPtr, NSPtr, NPtr]{ MergedNodeSet: pseudo.NewAccessor[*MergedNodeSet, MNSPtr]( (*MergedNodeSet).extraPayload, func(s *MergedNodeSet, t *pseudo.Type) { s.extra = t }, @@ -52,23 +55,24 @@ func RegisterExtras[ (*NodeSet).extraPayload, func(s *NodeSet, t *pseudo.Type) { s.extra = t }, ), - Node: pseudo.NewAccessor[*Node, N]( + Node: pseudo.NewAccessor[*Node, NPtr]( (*Node).extraPayload, func(n *Node, t *pseudo.Type) { n.extra = t }, ), } registeredExtras.MustRegister(&extraConstructors{ - newMergedNodeSet: pseudo.NewConstructor[MNS]().NewPointer, - newNodeSet: pseudo.NewConstructor[NS]().NewPointer, - newNode: pseudo.NewConstructor[N]().Zero, + newMergedNodeSet: pseudo.NewConstructor[MNS]().NewPointer, // i.e. non-nil MNSPtr + newNodeSet: pseudo.NewConstructor[NS]().NewPointer, // i.e. non-nil NSPtr + newNode: pseudo.NewConstructor[N]().NewPointer, // i.e. non-nil N hooks: payloads, }) return payloads } -// TestOnlyClearRegisteredExtras +// TestOnlyClearRegisteredExtras clears any previous call to [RegisterExtras]. +// It panics if called from a non-testing call stack. func TestOnlyClearRegisteredExtras() { registeredExtras.TestOnlyClear() } @@ -86,32 +90,37 @@ type extraConstructors struct { } // Merge merges the provided dirty nodes of a trie into the set. The assumption -// is held that no duplicated set belonging to the same trie will be merged twice. +// is held that no duplicated set belonging to the same trie will be merged +// twice. func (set *MergedNodeSet) Merge(other *NodeSet) error { if err := set.merge(other); err != nil { return err } if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromMNS(set).MergeNodeSet(set, other) + return r.Get().hooks.hooksFromMNS(set).AfterMergeNodeSet(set, other) } return nil } func (set *NodeSet) mergePayload(path []byte, n *Node) { if r := registeredExtras; r.Registered() { - r.Get().hooks.hooksFromNS(set).AddNode(set, path, n) + r.Get().hooks.hooksFromNS(set).AfterAddNode(set, path, n) } } -// ExtraPayloads +// ExtraPayloads provides strongly typed access to the extra payloads carried by +// [MergedNodeSet], [NodeSet], and [Node] ojects. The only valid way to +// construct an instance is by a call to [RegisterExtras]. The default `MNSPtr` +// and `NSPtr` default values, returned by [pseudo.Accessor.Get] are guaranteed +// to be non-nil pointers to zero values, equivalent to, e.g. `new(MNS)`. type ExtraPayloads[ - MNS MergedNodeSetHooks, - NS NodeSetHooks, - N any, + MNSPtr MergedNodeSetHooks, + NSPtr NodeSetHooks, + NPtr any, ] struct { - MergedNodeSet pseudo.Accessor[*MergedNodeSet, MNS] - NodeSet pseudo.Accessor[*NodeSet, NS] - Node pseudo.Accessor[*Node, N] + MergedNodeSet pseudo.Accessor[*MergedNodeSet, MNSPtr] + NodeSet pseudo.Accessor[*NodeSet, NSPtr] + Node pseudo.Accessor[*Node, NPtr] } func (e ExtraPayloads[MNS, NS, N]) hooksFromMNS(s *MergedNodeSet) MergedNodeSetHooks { diff --git a/trie/trienode/node.libevm_test.go b/trie/trienode/node.libevm_test.go index 244a69dcfc4..b897ae8a883 100644 --- a/trie/trienode/node.libevm_test.go +++ b/trie/trienode/node.libevm_test.go @@ -33,7 +33,7 @@ type setPayload struct { added map[string]uint64 } -func (p *setPayload) AddNode(_ *NodeSet, path []byte, n *Node) { +func (p *setPayload) AfterAddNode(_ *NodeSet, path []byte, n *Node) { if p.added == nil { p.added = make(map[string]uint64) } @@ -44,21 +44,21 @@ type mergedSetPayload struct { merged []map[string]uint64 } -func (p *mergedSetPayload) MergeNodeSet(_ *MergedNodeSet, ns *NodeSet) error { +func (p *mergedSetPayload) AfterMergeNodeSet(_ *MergedNodeSet, ns *NodeSet) error { p.merged = append(p.merged, maps.Clone(extras.NodeSet.Get(ns).added)) return nil } -var extras ExtraPayloads[*mergedSetPayload, *setPayload, nodePayload] +var extras ExtraPayloads[*mergedSetPayload, *setPayload, *nodePayload] func TestExtras(t *testing.T) { extras = RegisterExtras[mergedSetPayload, setPayload, nodePayload]() t.Cleanup(TestOnlyClearRegisteredExtras) n1 := New(common.Hash{0}, nil) - extras.Node.Set(n1, nodePayload{x: 1}) + extras.Node.Set(n1, &nodePayload{x: 1}) n42 := New(common.Hash{1}, nil) - extras.Node.Set(n42, nodePayload{x: 42}) + extras.Node.Set(n42, &nodePayload{x: 42}) set := NewNodeSet(common.Hash{}) merge := NewMergedNodeSet() From 7bb95a504ed46faed23e4180ee2e992f500c2acb Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 2 Oct 2025 16:26:05 +0100 Subject: [PATCH 4/6] chore: placate the linter --- trie/trienode/node.libevm_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trie/trienode/node.libevm_test.go b/trie/trienode/node.libevm_test.go index b897ae8a883..e6bc6e334eb 100644 --- a/trie/trienode/node.libevm_test.go +++ b/trie/trienode/node.libevm_test.go @@ -20,9 +20,10 @@ import ( "maps" "testing" - "github.com/ava-labs/libevm/common" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" + + "github.com/ava-labs/libevm/common" ) type nodePayload struct { From 52530091b1ce1cb26f909a9d6d853ef01dabe7e8 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 3 Oct 2025 10:45:28 +0100 Subject: [PATCH 5/6] doc: `trienode` hook methods + `proposal` finalizer test --- libevm/triedb/firewood/proposals_test.go | 2 ++ trie/trienode/node.go | 6 +++--- trie/trienode/node.libevm.go | 8 ++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/libevm/triedb/firewood/proposals_test.go b/libevm/triedb/firewood/proposals_test.go index 0a1fea0215d..3dfd62800f5 100644 --- a/libevm/triedb/firewood/proposals_test.go +++ b/libevm/triedb/firewood/proposals_test.go @@ -123,6 +123,8 @@ func TestProposalPropagation(t *testing.T) { t.Run("GC_finalizer_invoked", func(t *testing.T) { finalized := backend.got.finalized + // Everything that might still hold a reference to the `proposal`, + // stopping it from being garbage collected. sdb = nil cache = nil tdb = nil diff --git a/trie/trienode/node.go b/trie/trienode/node.go index f1618635c5a..939d042efe7 100644 --- a/trie/trienode/node.go +++ b/trie/trienode/node.go @@ -32,7 +32,7 @@ type Node struct { Hash common.Hash // Node hash, empty for deleted node Blob []byte // Encoded node blob, nil for the deleted node - extra *pseudo.Type // libevm + extra *pseudo.Type } // Size returns the total memory size used by this node. @@ -68,7 +68,7 @@ type NodeSet struct { updates int // the count of updated and inserted nodes deletes int // the count of deleted nodes - extra *pseudo.Type // libevm + extra *pseudo.Type } // NewNodeSet initializes a node set. The owner is zero for the account trie and @@ -171,7 +171,7 @@ func (set *NodeSet) Summary() string { type MergedNodeSet struct { Sets map[common.Hash]*NodeSet - extra *pseudo.Type // libevm + extra *pseudo.Type } // NewMergedNodeSet initializes an empty merged set. diff --git a/trie/trienode/node.libevm.go b/trie/trienode/node.libevm.go index 31061907aa0..b45c0436640 100644 --- a/trie/trienode/node.libevm.go +++ b/trie/trienode/node.libevm.go @@ -23,12 +23,16 @@ import ( // MergedNodeSetHooks are called as part of standard [MergedNodeSet] behaviour. type MergedNodeSetHooks interface { - AfterMergeNodeSet(into *MergedNodeSet, _ *NodeSet) error + // AfterMergeNodeSet is called at the end of [MergedNodeSet.Merge], with the + // method receiver and argument propagated. + AfterMergeNodeSet(receiver *MergedNodeSet, _ *NodeSet) error } // NodeSetHooks are called as part of standard [NodeSet] behaviour. type NodeSetHooks interface { - AfterAddNode(into *NodeSet, path []byte, _ *Node) + // AfterAddNode is called at the end of [NodeSet.AddNode], with the method + // receiver and arguments propagated. + AfterAddNode(receiver *NodeSet, path []byte, _ *Node) } // RegisterExtras registers types `MNSPtr`, `NSPtr`, and `N` to be carried as From 058bc81d894c1775d6b9607a88391031f0fa211f Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 3 Oct 2025 16:12:28 +0100 Subject: [PATCH 6/6] feat: allow multiple handles per proposal payload --- libevm/triedb/firewood/firewood.go | 2 +- libevm/triedb/firewood/proposals.go | 54 +++++++++++++----------- libevm/triedb/firewood/proposals_test.go | 41 ++++++++++++++---- 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/libevm/triedb/firewood/firewood.go b/libevm/triedb/firewood/firewood.go index c8fe6345a87..5448dbde738 100644 --- a/libevm/triedb/firewood/firewood.go +++ b/libevm/triedb/firewood/firewood.go @@ -38,7 +38,7 @@ type database struct { func (db *database) Update(root, parent common.Hash, block uint64, nodes *trienode.MergedNodeSet, states *triestate.Set, opts ...stateconf.TrieDBUpdateOption) error { // TODO(alarso16) - var _ *proposal = extras.MergedNodeSet.Get(nodes) + var _ *proposals = extras.MergedNodeSet.Get(nodes) db.afterUpdate(nodes) // MUST be the last statement before the final return return errors.New("unimplemented") diff --git a/libevm/triedb/firewood/proposals.go b/libevm/triedb/firewood/proposals.go index 7412fbf87ff..5918381391a 100644 --- a/libevm/triedb/firewood/proposals.go +++ b/libevm/triedb/firewood/proposals.go @@ -29,53 +29,59 @@ import ( // other packages. A call to RegisterExtras is required for the rest of this // package to function correctly. func RegisterExtras() { - extras = trienode.RegisterExtras[proposal, proposal, struct{}]() + extras = trienode.RegisterExtras[proposals, proposals, struct{}]() } -var extras trienode.ExtraPayloads[*proposal, *proposal, *struct{}] +var extras trienode.ExtraPayloads[*proposals, *proposals, *struct{}] -// A proposal is embedded as a payload in the [trienode.NodeSet] object returned -// by trie `Commit()`. A preceding call to [RegisterExtras] ensures that the -// proposal will be propagated to [Database.Update]. -// -// After construction, [proposal.setFinalizer] SHOULD be called to ensure -// release of resources via [proposal.free] once the proposal is garbage -// collected. -type proposal struct { +// A proposals carrier is embedded as a payload in the [trienode.NodeSet] object +// returned by trie `Commit()`. A preceding call to [RegisterExtras] ensures +// that the proposals will be propagated to [Database.Update]. +type proposals struct { // root MUST match the argument returned by the trie's `Commit()` method. root common.Hash - - // TODO(alarso16) add handles etc. here and clean them up in [proposal.free] - - finalized chan struct{} // https://go.dev/doc/gc-guide#Testing_object_death + // handles MAY carry >=1 handle, based off different parents, but all MUST + // result in the same root (i.e. the one specified in the other field). + handles []*handle } -func (p *proposal) injectInto(ns *trienode.NodeSet) { +func (p *proposals) injectInto(ns *trienode.NodeSet) { extras.NodeSet.Set(ns, p) } +// A handle carries a Firewood FFI proposal handle (i.e. Rust-owned memory). +// After construction, [handle.setFinalizer] SHOULD be called to ensure release +// of resources via [handle.free] once the handle is garbage collected. +type handle struct { + // TODO(alarso16) place the FFI handle here + + // finalized is set by [handle.setFinalizer] to signal when said finalizer + // has run; see https://go.dev/doc/gc-guide#Testing_object_death + finalized chan struct{} +} + // setFinalizer calls [runtime.SetFinalizer] with `p`. -func (p *proposal) setFinalizer() { - p.finalized = make(chan struct{}) - runtime.SetFinalizer(p, (*proposal).finalizer) +func (h *handle) setFinalizer() { + h.finalized = make(chan struct{}) + runtime.SetFinalizer(h, (*handle).finalizer) } // finalizer is expected to be passed to [runtime.SetFinalizer], abstracted as a // method to guarantee that it doesn't accidentally capture the value being // collected, thus resurrecting it. -func (p *proposal) finalizer() { - p.free() - close(p.finalized) +func (h *handle) finalizer() { + h.free() + close(h.finalized) } // free is called when the [proposal] is no longer reachable. -func (p *proposal) free() { +func (h *handle) free() { // TODO(alarso16) free the Rust object(s). } // AfterMergeNodeSet implements [trienode.MergedNodeSetHooks], copying at most // one proposal handle into the merged set. -func (h *proposal) AfterMergeNodeSet(into *trienode.MergedNodeSet, ns *trienode.NodeSet) error { +func (p *proposals) AfterMergeNodeSet(into *trienode.MergedNodeSet, ns *trienode.NodeSet) error { if p := extras.MergedNodeSet.Get(into); p.root != (common.Hash{}) { return fmt.Errorf(">1 %T carrying non-zero %T", ns, p) } @@ -86,4 +92,4 @@ func (h *proposal) AfterMergeNodeSet(into *trienode.MergedNodeSet, ns *trienode. } // AfterAddNode implements [trienode.NodeSetHooks] as a noop. -func (h *proposal) AfterAddNode(*trienode.NodeSet, []byte, *trienode.Node) {} +func (p *proposals) AfterAddNode(*trienode.NodeSet, []byte, *trienode.Node) {} diff --git a/libevm/triedb/firewood/proposals_test.go b/libevm/triedb/firewood/proposals_test.go index 3dfd62800f5..1f96cec5987 100644 --- a/libevm/triedb/firewood/proposals_test.go +++ b/libevm/triedb/firewood/proposals_test.go @@ -17,12 +17,15 @@ package firewood import ( + "context" + "fmt" "os" "runtime" "testing" "time" "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/rawdb" @@ -72,8 +75,13 @@ func (t *trieWithDummyProposals) Commit(collectLeaf bool) (common.Hash, *trienod // happens. We use the existing geth plumbing to carry the proposal back to // [hashDBWithDummyProposals.Update], knowing that the Go GC will trigger // the FFI call to free the Rust memory. - p := &proposal{root: root} - p.setFinalizer() + p := &proposals{ + root: root, + handles: []*handle{{}, {}}, + } + for _, h := range p.handles { + h.setFinalizer() + } p.injectInto(set) return root, set, nil @@ -83,7 +91,7 @@ func (t *trieWithDummyProposals) Commit(collectLeaf bool) (common.Hash, *trienod // propagated from [trieWithDummyProposals.Commit]. type hashDBWithDummyProposals struct { *hashdb.Database - got *proposal + got *proposals } func (db *hashDBWithDummyProposals) Reader(root common.Hash) (databasepkg.Reader, error) { @@ -120,8 +128,11 @@ func TestProposalPropagation(t *testing.T) { t.Errorf("got %v; want %v", got, want) } - t.Run("GC_finalizer_invoked", func(t *testing.T) { - finalized := backend.got.finalized + t.Run("GC_finalizers_invoked", func(t *testing.T) { + var finalized []chan struct{} + for _, h := range backend.got.handles { + finalized = append(finalized, h.finalized) + } // Everything that might still hold a reference to the `proposal`, // stopping it from being garbage collected. @@ -133,10 +144,22 @@ func TestProposalPropagation(t *testing.T) { // Note that [runtime.GC] doesn't block on finalizers; see // https://go.dev/doc/gc-guide#Testing_object_death runtime.GC() - select { - case <-finalized: - case <-time.After(5 * time.Second): - t.Errorf("%T finalizer did not run", &proposal{}) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + g, ctx := errgroup.WithContext(ctx) + + for i, ch := range finalized { + g.Go(func() error { + select { + case <-ch: + return nil + case <-ctx.Done(): + return fmt.Errorf("%T[%d] finalizer didn't run", &handle{}, i) + } + }) } + + require.NoError(t, g.Wait()) }) }