From 36808b89049cbbcbcd5afb3c6f395ca7fe2c2adc Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 27 Aug 2021 16:58:27 +0300 Subject: [PATCH] core: clone MPT node while restoring it multiple times We need this to avoid collapse collisions. Example of such collapse described in https://github.com/nspcc-dev/neo-go/pull/2019#discussion_r689629704. --- pkg/core/mpt/branch.go | 6 ++ pkg/core/mpt/empty.go | 3 + pkg/core/mpt/extension.go | 6 ++ pkg/core/mpt/hash.go | 7 ++ pkg/core/mpt/leaf.go | 6 ++ pkg/core/mpt/node.go | 1 + pkg/core/statesync/module.go | 4 +- pkg/core/statesync/module_test.go | 106 ++++++++++++++++++++++++++++++ 8 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 pkg/core/statesync/module_test.go diff --git a/pkg/core/mpt/branch.go b/pkg/core/mpt/branch.go index 0338ff4a7..d2fc84dfc 100644 --- a/pkg/core/mpt/branch.go +++ b/pkg/core/mpt/branch.go @@ -89,6 +89,12 @@ func (b *BranchNode) UnmarshalJSON(data []byte) error { return errors.New("expected branch node") } +// Clone implements Node interface. +func (b *BranchNode) Clone() Node { + res := *b + return &res +} + // splitPath splits path for a branch node. func splitPath(path []byte) (byte, []byte) { if len(path) != 0 { diff --git a/pkg/core/mpt/empty.go b/pkg/core/mpt/empty.go index 6669ef8c1..bc4f4914a 100644 --- a/pkg/core/mpt/empty.go +++ b/pkg/core/mpt/empty.go @@ -54,3 +54,6 @@ func (e EmptyNode) Type() NodeType { func (e EmptyNode) Bytes() []byte { return nil } + +// Clone implements Node interface. +func (EmptyNode) Clone() Node { return EmptyNode{} } diff --git a/pkg/core/mpt/extension.go b/pkg/core/mpt/extension.go index 2dcbcb66b..1266c6acb 100644 --- a/pkg/core/mpt/extension.go +++ b/pkg/core/mpt/extension.go @@ -98,3 +98,9 @@ func (e *ExtensionNode) UnmarshalJSON(data []byte) error { } return errors.New("expected extension node") } + +// Clone implements Node interface. +func (e *ExtensionNode) Clone() Node { + res := *e + return &res +} diff --git a/pkg/core/mpt/hash.go b/pkg/core/mpt/hash.go index 6ad66924d..03dc47a36 100644 --- a/pkg/core/mpt/hash.go +++ b/pkg/core/mpt/hash.go @@ -77,3 +77,10 @@ func (h *HashNode) UnmarshalJSON(data []byte) error { } return errors.New("expected hash node") } + +// Clone implements Node interface. +func (h *HashNode) Clone() Node { + res := *h + res.Collapsed = false + return &res +} diff --git a/pkg/core/mpt/leaf.go b/pkg/core/mpt/leaf.go index 0f3072b85..0efa45e70 100644 --- a/pkg/core/mpt/leaf.go +++ b/pkg/core/mpt/leaf.go @@ -77,3 +77,9 @@ func (n *LeafNode) UnmarshalJSON(data []byte) error { } return errors.New("expected leaf node") } + +// Clone implements Node interface. +func (n *LeafNode) Clone() Node { + res := *n + return &res +} diff --git a/pkg/core/mpt/node.go b/pkg/core/mpt/node.go index af35286c1..a5f6cc814 100644 --- a/pkg/core/mpt/node.go +++ b/pkg/core/mpt/node.go @@ -34,6 +34,7 @@ type Node interface { json.Marshaler json.Unmarshaler Size() int + Clone() Node BaseNodeIface } diff --git a/pkg/core/statesync/module.go b/pkg/core/statesync/module.go index fb775c388..801b51a97 100644 --- a/pkg/core/statesync/module.go +++ b/pkg/core/statesync/module.go @@ -371,7 +371,9 @@ func (s *Module) restoreNode(n mpt.Node) error { } var childrenPaths = make(map[util.Uint256][][]byte) for _, path := range nPaths { - err := s.billet.RestoreHashNode(path, n) + // Must clone here in order to avoid future collapse collisions. If the node's refcount>1 then MPT pool + // will manage all paths for this node and call RestoreHashNode separately for each of the paths. + err := s.billet.RestoreHashNode(path, n.Clone()) if err != nil { return fmt.Errorf("failed to restore MPT node with hash %s and path %s: %w", n.Hash().StringBE(), hex.EncodeToString(path), err) } diff --git a/pkg/core/statesync/module_test.go b/pkg/core/statesync/module_test.go new file mode 100644 index 000000000..c1bf6b117 --- /dev/null +++ b/pkg/core/statesync/module_test.go @@ -0,0 +1,106 @@ +package statesync + +import ( + "fmt" + "testing" + + "github.com/nspcc-dev/neo-go/pkg/core/dao" + "github.com/nspcc-dev/neo-go/pkg/core/mpt" + "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/util/slice" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +func TestModule_PR2019_discussion_r689629704(t *testing.T) { + expectedStorage := storage.NewMemCachedStore(storage.NewMemoryStore()) + tr := mpt.NewTrie(nil, true, expectedStorage) + require.NoError(t, tr.Put([]byte{0x03}, []byte("leaf1"))) + require.NoError(t, tr.Put([]byte{0x01, 0xab, 0x02}, []byte("leaf2"))) + require.NoError(t, tr.Put([]byte{0x01, 0xab, 0x04}, []byte("leaf3"))) + require.NoError(t, tr.Put([]byte{0x06, 0x01, 0xde, 0x02}, []byte("leaf2"))) // <-- the same `leaf2` and `leaf3` values are put in the storage, + require.NoError(t, tr.Put([]byte{0x06, 0x01, 0xde, 0x04}, []byte("leaf3"))) // <-- but the path should differ. + require.NoError(t, tr.Put([]byte{0x06, 0x03}, []byte("leaf4"))) + + sr := tr.StateRoot() + tr.Flush() + + // Keep MPT nodes in a map in order not to repeat them. We'll use `nodes` map to ask + // state sync module to restore the nodes. + var ( + nodes = make(map[util.Uint256][]byte) + expectedItems []storage.KeyValue + ) + expectedStorage.Seek(storage.DataMPT.Bytes(), func(k, v []byte) { + key := slice.Copy(k) + value := slice.Copy(v) + expectedItems = append(expectedItems, storage.KeyValue{ + Key: key, + Value: value, + }) + hash, err := util.Uint256DecodeBytesBE(key[1:]) + require.NoError(t, err) + nodeBytes := value[:len(value)-4] + nodes[hash] = nodeBytes + }) + + actualStorage := storage.NewMemCachedStore(storage.NewMemoryStore()) + // These actions are done in module.Init(), but it's not the point of the test. + // Here we want to test only MPT restoring process. + stateSync := &Module{ + log: zaptest.NewLogger(t), + syncPoint: 1000500, + syncStage: headersSynced, + syncInterval: 100500, + dao: dao.NewSimple(actualStorage, true, false), + mptpool: NewPool(), + } + stateSync.billet = mpt.NewBillet(sr, true, actualStorage) + stateSync.mptpool.Add(sr, []byte{}) + + // The test itself: we'll ask state sync module to restore each node exactly once. + // After that storage content (including storage items and refcounts) must + // match exactly the one got from real MPT trie. MPT pool must be empty. + // State sync module must have mptSynced state in the end. + // MPT Billet root must become a collapsed hashnode (it was checked manually). + requested := make(map[util.Uint256]struct{}) + for { + unknownHashes := stateSync.GetUnknownMPTNodesBatch(1) // restore nodes one-by-one + if len(unknownHashes) == 0 { + break + } + h := unknownHashes[0] + node, ok := nodes[h] + if !ok { + if _, ok = requested[h]; ok { + t.Fatal("node was requested twice") + } + t.Fatal("unknown node was requested") + } + require.NotPanics(t, func() { + err := stateSync.AddMPTNodes([][]byte{node}) + require.NoError(t, err) + }, fmt.Errorf("hash=%s, value=%s", h.StringBE(), string(node))) + requested[h] = struct{}{} + delete(nodes, h) + if len(nodes) == 0 { + break + } + } + require.Equal(t, headersSynced|mptSynced, stateSync.syncStage, "all nodes were sent exactly ones, but MPT wasn't restored") + require.Equal(t, 0, len(nodes), "not all nodes were requested by state sync module") + require.Equal(t, 0, stateSync.mptpool.Count(), "MPT was restored, but MPT pool still contains items") + + // Compare resulting storage items and refcounts. + var actualItems []storage.KeyValue + expectedStorage.Seek(storage.DataMPT.Bytes(), func(k, v []byte) { + key := slice.Copy(k) + value := slice.Copy(v) + actualItems = append(actualItems, storage.KeyValue{ + Key: key, + Value: value, + }) + }) + require.ElementsMatch(t, expectedItems, actualItems) +}