From ee310ed4dee545c3e46babb9aff73ec832a49a40 Mon Sep 17 00:00:00 2001 From: rfxfxfx Date: Mon, 31 Aug 2026 17:26:33 +0800 Subject: [PATCH] fix(store): make node.copy() actually copy node.copy() was 'return &(*x)', which is just x: dereferencing and immediately re-addressing a pointer yields the same pointer. Every caller that asked for a copy got an alias to the original node, so mutating the 'copy' mutated the original. staticcheck reports the expression as SA4001. This is already known to be load-bearing. store.go carries a comment saying the SMT node cache 'MUST NOT be persisted across blocks' precisely because 'node.copy() is a no-op alias, so the parallel commit mutates cached *node objects in place', and works around it with a fresh per-block cache. Fixing the copy removes the reason that workaround exists, though this change leaves the workaround in place. The node is rebuilt field by field rather than with '*x', because lib.Node embeds a protobuf MessageState and a plain struct copy would be a lock copy that 'go vet' rejects. Byte slices and the *key are still shared, which is what the existing 'shallow copy' doc comment describes. Verification: full suite passes ('go test ./... -p=1'), and the state root over a deterministic 500-write / 167-delete workload is byte-identical before and after the change: main ROOT=3944f19bc128472bd163bd081b5bc648fd97f535ca823b88f6b5da8548e391a6 fix/smt-node-copy-no-op ROOT=3944f19bc128472bd163bd081b5bc648fd97f535ca823b88f6b5da8548e391a6 Given this is consensus-critical storage code, reviewers may still want to run it against a real chain replay before merging. --- store/smt.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/store/smt.go b/store/smt.go index 3449c4964b..38e64d36e1 100644 --- a/store/smt.go +++ b/store/smt.go @@ -1177,7 +1177,28 @@ func (x *node) replaceChild(oldKey, newKey []byte) { } // copy() returns a shallow copy of the node -func (x *node) copy() *node { return &(*x) } +// +// The node struct is rebuilt field by field rather than dereferenced and +// re-addressed: `&(*x)` is simply `x` and returns the original pointer, and a +// plain `*x` struct copy would copy the protobuf MessageState embedded in +// lib.Node (a lock copy, reported by `go vet`). The byte slices and the *key +// are shared with the original, which is what "shallow" means here. +func (x *node) copy() *node { + if x == nil { + return nil + } + return &node{ + Key: x.Key, + Node: lib.Node{ + Value: x.Node.Value, + LeftChildKey: x.Node.LeftChildKey, + RightChildKey: x.Node.RightChildKey, + Key: x.Node.Key, + Bitmask: x.Node.Bitmask, + }, + delete: x.delete, + } +} // NODE LIST CODE BELOW