From 2d4c81a8ffd3824c6ce861c9f0d5e68ef55b2cc8 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Mon, 12 Aug 2024 14:57:09 -0700 Subject: [PATCH 01/17] test: Update test for loading patched initial version --- mutable_tree_test.go | 99 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 1e349328d..77a7d1104 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1447,16 +1447,91 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, initialVersion, version) + err = tree.ndb.batch.WriteSync() + require.NoError(t, err) + + // Save 2 empty versions after the initial version, should produce 2 refnodes + _, _, err = tree.SaveVersion() + require.NoError(t, err) + + _, _, err = tree.SaveVersion() + require.NoError(t, err) + + // ------------------------------ + // Verify + // 1. Node exists with Version 1, not InitialVersion + // 2. VersionExists(InitialVersion) - should return true + // 3. AvailableVersions() - should include InitialVersion and NOT version 1 + // 4. ndb.GetRoot(InitialVersion) - should return the root node + // 5. GetImmutable(InitialVersion) - should return the immutable tree + // 6. LoadVersion(InitialVersion) - loads tree even if node contains version 1. This is last since it runs some previous methods internally + // the nodes created at the first version are not assigned with the `InitialVersion` - rootKey := GetRootKey(1) - node, err := tree.ndb.GetNode(rootKey) + t.Run("1. node exists with root key version 1", func(t *testing.T) { + rootKey := GetRootKey(1) + node, err := tree.ndb.GetNode(rootKey) + require.NoError(t, err) + require.Equal(t, int64(1), node.nodeKey.version, "new nodes on new tree should be version 1") + + // Check fast node version + fastNode, err := tree.ndb.GetFastNode([]byte("hello")) + require.NoError(t, err) + require.Equal(t, int64(1), fastNode.GetVersionLastUpdatedAt(), "fast nodes be version 1") + }) + + t.Run("2. VersionExists(InitialVersion)", func(t *testing.T) { + hasVersion := tree.VersionExists(initialVersion) + require.True(t, hasVersion, "initial version should still be found") + }) + + t.Run("3. AvailableVersions()", func(t *testing.T) { + versions := tree.AvailableVersions() + require.Contains(t, versions, int(initialVersion), "initial version should be found") + require.NotContains(t, versions, int64(1), "version 1 should not be found") + }) + + t.Run("4. GetRoot(InitialVersion)", func(t *testing.T) { + root, err := tree.ndb.GetRoot(initialVersion) + + // Internally, this should use a refnode + require.NoError(t, err) + require.NotNil(t, root, "root node should be found at initial version") + }) + + t.Run("5. GetImmutable(InitialVersion)", func(t *testing.T) { + immutableTree, err := tree.GetImmutable(initialVersion) + require.NoError(t, err) + require.NotNil(t, immutableTree, "immutable tree should be found at initial version") + }) + + t.Run("6. LoadVersion(InitialVersion)", func(t *testing.T) { + // Final check - runs previous methods internally as well + _, err = tree.LoadVersion(initialVersion) + require.NoError(t, err) + }) + + // Internal ndb methods for good measure + firstVersion, err := tree.ndb.getFirstVersion() require.NoError(t, err) - require.Equal(t, int64(1), node.nodeKey.version, "new nodes on new tree should be version 1") + require.Equal(t, initialVersion, firstVersion, "first version should be the initialVersion") - // Check fast node version - fastNode, err := tree.ndb.GetFastNode([]byte("hello")) + latest, err := tree.ndb.getLatestVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, latest, "first version should be the initialVersion") + + // Check reference nodes point at the correct node key + err = tree.ndb.traversePrefix(nodeKeyFormat.Prefix(), func(key, value []byte) error { + if isRef, _ := isReferenceRoot(value); isRef { + nk := GetNodeKey(value[1:]) + val, err := tree.ndb.db.Get(nk.GetKey()) + + require.NoError(t, err) + require.NotNil(t, val, "reference node should point to a valid node") + } + + return nil + }) require.NoError(t, err) - require.Equal(t, int64(1), fastNode.GetVersionLastUpdatedAt(), "fast nodes be version 1") // ------------------------------ // Writes on existing tree @@ -1468,16 +1543,20 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, initialVersion+1, version, "new version should be initialVersion+1") - rootKey = GetRootKey(version) + rootKey := GetRootKey(version) // the following versions behaves normally - node, err = tree.ndb.GetNode(rootKey) + node, err := tree.ndb.GetNode(rootKey) require.NoError(t, err) require.Equal(t, initialVersion+1, node.nodeKey.version, "new nodes on existing tree should use initialVersion") // Check fast node version - fastNode, err = tree.ndb.GetFastNode([]byte("hello")) + fastNode, err := tree.ndb.GetFastNode([]byte("hello")) require.NoError(t, err) - require.Equal(t, initialVersion+1, fastNode.GetVersionLastUpdatedAt(), "fast nodes should have the same version as the tree") + require.Equal( + t, initialVersion+1, + fastNode.GetVersionLastUpdatedAt(), + "fast nodes should have the same version as the tree", + ) } func TestMutableTreeClose(t *testing.T) { From 628affc0bc5658f79161435c368f3a0960b51bea Mon Sep 17 00:00:00 2001 From: drklee3 Date: Mon, 12 Aug 2024 15:27:51 -0700 Subject: [PATCH 02/17] fix: Add refnode to initial version to point at version 1 --- mutable_tree.go | 14 ++++++++++++++ mutable_tree_test.go | 21 +++++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 9cfadc730..008c3ea73 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -746,6 +746,7 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { return nil, 0, err } } else { + // nodeKey already set, no changes to tree if tree.root.nodeKey != nil { // it means there are no updated nodes if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { @@ -761,12 +762,25 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { } } } else { + // No nodeKey, assign new nodeKeys and save nodes. This happens when + // any node in the tree has been updated as setting a key will + // reset all nodeKeys to nil if err := tree.saveNewNodes(); err != nil { return nil, 0, err } } } + // PATCH for backwards compatibility: + // On InitialVersion, this is a new tree. + // Add a reference node from InitialVersion -> 1 (root node) + // Note: Node version != nodekey.Version due to patch + if tree.ndb.opts.InitialVersion > 0 && version == int64(tree.ndb.opts.InitialVersion) { + if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { + return nil, 0, err + } + } + if err := tree.ndb.Commit(); err != nil { return nil, version, err } diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 77a7d1104..b7a247652 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1454,7 +1454,7 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { _, _, err = tree.SaveVersion() require.NoError(t, err) - _, _, err = tree.SaveVersion() + _, latestVersion, err := tree.SaveVersion() require.NoError(t, err) // ------------------------------ @@ -1517,14 +1517,19 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { latest, err := tree.ndb.getLatestVersion() require.NoError(t, err) - require.Equal(t, initialVersion, latest, "first version should be the initialVersion") + require.Equal(t, latestVersion, latest, "latest version should be updated") // Check reference nodes point at the correct node key err = tree.ndb.traversePrefix(nodeKeyFormat.Prefix(), func(key, value []byte) error { if isRef, _ := isReferenceRoot(value); isRef { + // key still contains prefix, so we need to remove first byte + refnodeKey := GetNodeKey(key[1:]) + nk := GetNodeKey(value[1:]) - val, err := tree.ndb.db.Get(nk.GetKey()) + t.Logf("ref node %s -> %s", refnodeKey.String(), nk.String()) + + val, err := tree.ndb.GetNode(nk.GetKey()) require.NoError(t, err) require.NotNil(t, val, "reference node should point to a valid node") } @@ -1536,24 +1541,28 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // ------------------------------ // Writes on existing tree + // Load the latest version again for writing + _, err = tree.LoadVersion(latestVersion) + require.NoError(t, err) + _, err = tree.Set([]byte("hello"), []byte("world1")) require.NoError(t, err) _, version, err = tree.SaveVersion() require.NoError(t, err) - require.Equal(t, initialVersion+1, version, "new version should be initialVersion+1") + require.Equal(t, latestVersion+1, version, "new version should be initialVersion+1") rootKey := GetRootKey(version) // the following versions behaves normally node, err := tree.ndb.GetNode(rootKey) require.NoError(t, err) - require.Equal(t, initialVersion+1, node.nodeKey.version, "new nodes on existing tree should use initialVersion") + require.Equal(t, latestVersion+1, node.nodeKey.version, "new nodes on existing tree should use initialVersion") // Check fast node version fastNode, err := tree.ndb.GetFastNode([]byte("hello")) require.NoError(t, err) require.Equal( - t, initialVersion+1, + t, latestVersion+1, fastNode.GetVersionLastUpdatedAt(), "fast nodes should have the same version as the tree", ) From 5cf60ddf6b55afd51eba72a5c626536c8bb3ca54 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Mon, 12 Aug 2024 15:33:30 -0700 Subject: [PATCH 03/17] docs: Update comments on patch --- mutable_tree.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 008c3ea73..b9a732063 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -771,11 +771,14 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { } } - // PATCH for backwards compatibility: - // On InitialVersion, this is a new tree. - // Add a reference node from InitialVersion -> 1 (root node) - // Note: Node version != nodekey.Version due to patch + // Addition to node version patch for backwards compatibility: + // This is to resolve queries at InitialVersion, loading the tree at + // InitialVersion when the root node has version 1. + // Note: version != nodekey.Version due to patch. if tree.ndb.opts.InitialVersion > 0 && version == int64(tree.ndb.opts.InitialVersion) { + // SaveRoot is meant for saving a tree when there are no updates, which + // simply creates a reference node to the root node. We reuse this to + // create a reference node from InitialVersion -> 1 (root node). if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { return nil, 0, err } From 33f1680b46fe4265d113cab5853b86fc06d0ba56 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Mon, 12 Aug 2024 16:22:10 -0700 Subject: [PATCH 04/17] fix: Update WorkingHash() to also use tree.version + 1 --- mutable_tree.go | 4 +++- tree_test.go | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index b9a732063..5afc2c0ce 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -137,7 +137,9 @@ func (tree *MutableTree) Hash() []byte { // WorkingHash returns the hash of the current working tree. func (tree *MutableTree) WorkingHash() []byte { - return tree.root.hashWithCount(tree.WorkingVersion()) + // Use tree.version instead of tree.WorkingVersion() because tree.version + // is always the latest version + return tree.root.hashWithCount(tree.version + 1) } func (tree *MutableTree) WorkingVersion() int64 { diff --git a/tree_test.go b/tree_test.go index 13f570035..00158664f 100644 --- a/tree_test.go +++ b/tree_test.go @@ -1936,6 +1936,9 @@ func TestWorkingHashWithInitialVersion(t *testing.T) { _, err = tree.Set([]byte("key1"), []byte("value1")) require.NoError(t, err) + // WorkingHash sets the hashes in the nodes, SaveVersion doesn't recompute it. + // This test ensures WorkingHash and SaveVersion are consistent, using the + // same version number. workingHash := tree.WorkingHash() commitHash, _, err := tree.SaveVersion() require.NoError(t, err) @@ -1952,5 +1955,5 @@ func TestWorkingHashWithInitialVersion(t *testing.T) { commitHash1, _, err := tree.SaveVersion() require.NoError(t, err) - require.Equal(t, commitHash1, commitHash) + assert.Equal(t, hex.EncodeToString(commitHash1), hex.EncodeToString(commitHash)) } From 882046b134c1cce1f75d5a98968ee6561b72f13c Mon Sep 17 00:00:00 2001 From: drklee3 Date: Mon, 12 Aug 2024 16:26:59 -0700 Subject: [PATCH 05/17] ci: Run test workflow on release branches --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0ecac740..0503e9848 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,7 @@ on: push: branches: - master + - release/* pull_request: jobs: From 2d28d39a250022dc1a8400bcb22fc13cd6dc160b Mon Sep 17 00:00:00 2001 From: drklee3 Date: Thu, 15 Aug 2024 11:51:38 -0700 Subject: [PATCH 06/17] fix: Prevent nil panic on reading tree.root.nodeKey on empty tree --- mutable_tree.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 5afc2c0ce..54e07044b 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -771,18 +771,18 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { return nil, 0, err } } - } - // Addition to node version patch for backwards compatibility: - // This is to resolve queries at InitialVersion, loading the tree at - // InitialVersion when the root node has version 1. - // Note: version != nodekey.Version due to patch. - if tree.ndb.opts.InitialVersion > 0 && version == int64(tree.ndb.opts.InitialVersion) { - // SaveRoot is meant for saving a tree when there are no updates, which - // simply creates a reference node to the root node. We reuse this to - // create a reference node from InitialVersion -> 1 (root node). - if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { - return nil, 0, err + // Addition to node version patch for backwards compatibility: + // This is to resolve queries at InitialVersion, loading the tree at + // InitialVersion when the root node has version 1. + // Note: version != nodekey.Version due to patch. + if tree.ndb.opts.InitialVersion > 0 && version == int64(tree.ndb.opts.InitialVersion) { + // SaveRoot is meant for saving a tree when there are no updates, which + // simply creates a reference node to the root node. We reuse this to + // create a reference node from InitialVersion -> 1 (root node). + if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { + return nil, 0, err + } } } From f73700beedd0ee98bb73c7fef2c0c61b2b69e045 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Thu, 15 Aug 2024 11:52:55 -0700 Subject: [PATCH 07/17] test: Add test for empty tree, pruning --- mutable_tree_test.go | 119 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 5 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index b7a247652..58a556836 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1432,6 +1432,19 @@ func TestNoFastStorageUpgrade_Integration_SaveVersion_Load_Iterate_Success(t *te }) } +func TestMutableTreeClose(t *testing.T) { + db := dbm.NewMemDB() + tree := NewMutableTree(db, 0, true, log.NewNopLogger()) + + _, err := tree.Set([]byte("hello"), []byte("world")) + require.NoError(t, err) + + _, _, err = tree.SaveVersion() + require.NoError(t, err) + + require.NoError(t, tree.Close()) +} + // TestMutableTree_InitialVersion_FirstVersion demonstrate the un-intuitive behavior, // when InitialVersion is set the nodes created in the first version are not assigned with expected version number. func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { @@ -1447,8 +1460,8 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, initialVersion, version) - err = tree.ndb.batch.WriteSync() - require.NoError(t, err) + // err = tree.ndb.batch.WriteSync() + // require.NoError(t, err) // Save 2 empty versions after the initial version, should produce 2 refnodes _, _, err = tree.SaveVersion() @@ -1510,7 +1523,20 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { require.NoError(t, err) }) + t.Run("6a. LoadVersion(1)", func(t *testing.T) { + _, err = tree.LoadVersion(1) + require.Error(t, err) + require.ErrorIs(t, err, ErrVersionDoesNotExist) + }) + // Internal ndb methods for good measure + + // TODO: AUDIT - use of ndb.hasVersion() + // Node has version 1, but it should only be found via InitialVersion + hasVersion1, err := tree.ndb.hasVersion(1) + require.NoError(t, err) + require.False(t, hasVersion1, "version 1 should not be found") + firstVersion, err := tree.ndb.getFirstVersion() require.NoError(t, err) require.Equal(t, initialVersion, firstVersion, "first version should be the initialVersion") @@ -1568,15 +1594,98 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { ) } -func TestMutableTreeClose(t *testing.T) { +func TestMutableTree_InitialVersion_Empty(t *testing.T) { + // No nodes created, empty tree, empty root + db := dbm.NewMemDB() - tree := NewMutableTree(db, 0, true, log.NewNopLogger()) + + initialVersion := int64(1000) + tree := NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) + + require.NotPanics(t, func() { + _, version, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, version) + }) + + err := tree.ndb.batch.WriteSync() + require.NoError(t, err) + + // Save 2 empty versions after the initial version, should produce 2 refnodes + _, _, err = tree.SaveVersion() + require.NoError(t, err) + _, latestVersion, err := tree.SaveVersion() + require.NoError(t, err) + + // ------------------------------ + // Verify + + _, err = tree.LoadVersion(initialVersion) + require.NoError(t, err) + + hasVersion1, err := tree.ndb.hasVersion(1) + require.NoError(t, err) + require.False(t, hasVersion1, "version 1 should not be found") + + // Internal ndb methods for good measure + firstVersion, err := tree.ndb.getFirstVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, firstVersion, "first version should be the initialVersion") + + latest, err := tree.ndb.getLatestVersion() + require.NoError(t, err) + require.Equal(t, latestVersion, latest, "latest version should be updated") +} + +func TestMutableTree_InitialVersion_Prune(t *testing.T) { + // Pruning should not affect loading versions + + db := dbm.NewMemDB() + + initialVersion := int64(1000) + tree := NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) _, err := tree.Set([]byte("hello"), []byte("world")) require.NoError(t, err) + require.NotPanics(t, func() { + _, version, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, version) + }) + + err = tree.ndb.batch.WriteSync() + require.NoError(t, err) + + // Save 2 empty versions after the initial version, should produce 2 refnodes _, _, err = tree.SaveVersion() require.NoError(t, err) + _, latestVersion, err := tree.SaveVersion() + require.NoError(t, err) - require.NoError(t, tree.Close()) + // Prune - inclusive of initialVersion + err = tree.DeleteVersionsTo(initialVersion) + require.NoError(t, err) + + // ------------------------------ + // Verify + + expFirstVersion := initialVersion + 1 + + _, err = tree.LoadVersion(expFirstVersion) + require.NoError(t, err) + + // TODO: Direct node version access + // hasVersion1, err := tree.ndb.hasVersion(1) + // require.NoError(t, err) + // require.False(t, hasVersion1, "version 1 should not be found") + + // Internal ndb methods for good measure + firstVersion, err := tree.ndb.getFirstVersion() + require.NoError(t, err) + require.Equal(t, expFirstVersion, firstVersion, "first version should be 1 after pruned") + + latest, err := tree.ndb.getLatestVersion() + require.NoError(t, err) + require.Equal(t, latestVersion, latest, "latest version should stay same") } From 6c6dbcfb2db4266407a9cf05cb7cd1c46a8a5e9b Mon Sep 17 00:00:00 2001 From: drklee3 Date: Thu, 15 Aug 2024 12:52:53 -0700 Subject: [PATCH 08/17] test: Temp skip ndb.hasVersion(1) --- mutable_tree_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 58a556836..435c3569c 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1533,9 +1533,9 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // TODO: AUDIT - use of ndb.hasVersion() // Node has version 1, but it should only be found via InitialVersion - hasVersion1, err := tree.ndb.hasVersion(1) - require.NoError(t, err) - require.False(t, hasVersion1, "version 1 should not be found") + // hasVersion1, err := tree.ndb.hasVersion(1) + // require.NoError(t, err) + // require.False(t, hasVersion1, "version 1 should not be found") firstVersion, err := tree.ndb.getFirstVersion() require.NoError(t, err) From 178156c249dd934751cd6ba3bac0420069e1f0c7 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Thu, 15 Aug 2024 17:37:58 -0700 Subject: [PATCH 09/17] test: Add cases for node db versions --- mutable_tree_test.go | 79 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 435c3569c..fff0eee2b 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1456,6 +1456,10 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { _, err := tree.Set([]byte("hello"), []byte("world")) require.NoError(t, err) + // More than 1 key/value pair to also check non-root nodes + _, err = tree.Set([]byte("goodbye"), []byte("world")) + require.NoError(t, err) + _, version, err := tree.SaveVersion() require.NoError(t, err) require.Equal(t, initialVersion, version) @@ -1480,11 +1484,16 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // 6. LoadVersion(InitialVersion) - loads tree even if node contains version 1. This is last since it runs some previous methods internally // the nodes created at the first version are not assigned with the `InitialVersion` - t.Run("1. node exists with root key version 1", func(t *testing.T) { - rootKey := GetRootKey(1) + t.Run("1. root node exists at RootKey(initialVersion) with nodeKey version 1", func(t *testing.T) { + t.Skip() + rootKey := GetRootKey(initialVersion) node, err := tree.ndb.GetNode(rootKey) require.NoError(t, err) - require.Equal(t, int64(1), node.nodeKey.version, "new nodes on new tree should be version 1") + + // Calling GetNode with rootKey actually sets the returned Node.nodeKey. + // nodeKey isn't persisted in db, and thus needs to be repopulated by + // the caller & the provided nodeKey + require.Equal(t, int64(1), node.nodeKey.version, "nodes on new tree should have nodeKey.version == 1") // Check fast node version fastNode, err := tree.ndb.GetFastNode([]byte("hello")) @@ -1533,9 +1542,9 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // TODO: AUDIT - use of ndb.hasVersion() // Node has version 1, but it should only be found via InitialVersion - // hasVersion1, err := tree.ndb.hasVersion(1) - // require.NoError(t, err) - // require.False(t, hasVersion1, "version 1 should not be found") + hasVersion1, err := tree.ndb.hasVersion(1) + require.NoError(t, err) + require.False(t, hasVersion1, "version 1 should not be found") firstVersion, err := tree.ndb.getFirstVersion() require.NoError(t, err) @@ -1675,10 +1684,10 @@ func TestMutableTree_InitialVersion_Prune(t *testing.T) { _, err = tree.LoadVersion(expFirstVersion) require.NoError(t, err) - // TODO: Direct node version access - // hasVersion1, err := tree.ndb.hasVersion(1) - // require.NoError(t, err) - // require.False(t, hasVersion1, "version 1 should not be found") + // Direct node version access + hasVersion1, err := tree.ndb.hasVersion(1) + require.NoError(t, err) + require.False(t, hasVersion1, "version 1 should not be found") // Internal ndb methods for good measure firstVersion, err := tree.ndb.getFirstVersion() @@ -1689,3 +1698,53 @@ func TestMutableTree_InitialVersion_Prune(t *testing.T) { require.NoError(t, err) require.Equal(t, latestVersion, latest, "latest version should stay same") } + +func TestMutableTree_InitialVersion_NodeDB_Version(t *testing.T) { + testInitialVersions := []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + + for _, initialVersion := range testInitialVersions { + t.Run(fmt.Sprintf("InitialVersion(%d)", initialVersion), func(t *testing.T) { + db := dbm.NewMemDB() + + tree := NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) + + _, err := tree.Set([]byte("hello"), []byte("world")) + require.NoError(t, err) + + _, version, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, version) + + // ------------------------------ + // Verify + + // Load a new tree with the same db and without initialVersion + tree = NewMutableTree(db, 0, false, log.NewNopLogger()) + + _, err = tree.LoadVersion(initialVersion) + require.NoError(t, err, "InitialVersion should be loadable in tree") + + if initialVersion == 1 { + _, err = tree.LoadVersion(1) + require.NoError(t, err, "Version 1 should exist when initialVersion is 1") + return + } + + // ndb.getFirstVersion() does a binary search and loads the node to find the + // first version. If using a reference node from InitialVersion -> version 1 + // Version 1 exists in the node db but shouldn't be used when actually + // trying to load version 1. + // + // When: + // InitialVersion == 2, node structure is identical to InitialVersion == 1 + // if version 2 has no changes. + // In this case we cannot determine if LoadVersion(1) should work or + // not since we no longer have ndb.opts.InitialVersion + // nodeKey v1 -> rootNode + // nodeKey v2 -> refNode(v1) + _, err = tree.LoadVersion(1) + require.Error(t, err, "Version 1 should not be loadable in tree") + require.ErrorIs(t, err, ErrVersionDoesNotExist) + }) + } +} From 2644f53d395a84173620328d8d8dda3ab0731512 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 11:23:34 -0700 Subject: [PATCH 10/17] fix: Hide root node at version 1 --- mutable_tree.go | 25 +++++++++++++++++++++++-- mutable_tree_test.go | 26 ++++++-------------------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 54e07044b..e18e62604 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -774,15 +774,36 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { // Addition to node version patch for backwards compatibility: // This is to resolve queries at InitialVersion, loading the tree at - // InitialVersion when the root node has version 1. + // InitialVersion when the root node has version 1. Must not be done + // when InitialVersion is 1 as it will write an unnecessary reference + // that overwrites the actual root node. // Note: version != nodekey.Version due to patch. - if tree.ndb.opts.InitialVersion > 0 && version == int64(tree.ndb.opts.InitialVersion) { + if tree.ndb.opts.InitialVersion > 1 && version == int64(tree.ndb.opts.InitialVersion) { // SaveRoot is meant for saving a tree when there are no updates, which // simply creates a reference node to the root node. We reuse this to // create a reference node from InitialVersion -> 1 (root node). + + // Save reference node from InitialVersion -> tree.root if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { return nil, 0, err } + + // Delete the root node to be replaced below with new key. Need to + // delete this otherwise there will be two keys (version, 1) and + // (version, 0) below pointing at the same node. + if err := tree.ndb.deleteFromPruning(tree.ndb.nodeKey(tree.root.nodeKey.GetKey())); err != nil { + return nil, 0, err + } + + // Use a nonce of 0 to match pruning behavior that hides it from + // the version list. + // Mark the root node as hidden, same as how pruning handles. + // Even if with the reference node via SaveRoot() above points at + // (version, 1), LoadVersion still checks (version, 0) if it exists. + tree.root.nodeKey.nonce = 0 + if err := tree.ndb.SaveNode(tree.root); err != nil { + return nil, 0, err + } } } diff --git a/mutable_tree_test.go b/mutable_tree_test.go index fff0eee2b..4db9b21de 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1554,25 +1554,6 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, latestVersion, latest, "latest version should be updated") - // Check reference nodes point at the correct node key - err = tree.ndb.traversePrefix(nodeKeyFormat.Prefix(), func(key, value []byte) error { - if isRef, _ := isReferenceRoot(value); isRef { - // key still contains prefix, so we need to remove first byte - refnodeKey := GetNodeKey(key[1:]) - - nk := GetNodeKey(value[1:]) - - t.Logf("ref node %s -> %s", refnodeKey.String(), nk.String()) - - val, err := tree.ndb.GetNode(nk.GetKey()) - require.NoError(t, err) - require.NotNil(t, val, "reference node should point to a valid node") - } - - return nil - }) - require.NoError(t, err) - // ------------------------------ // Writes on existing tree @@ -1654,7 +1635,8 @@ func TestMutableTree_InitialVersion_Prune(t *testing.T) { initialVersion := int64(1000) tree := NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) - _, err := tree.Set([]byte("hello"), []byte("world")) + key := []byte("hello") + _, err := tree.Set(key, []byte("world")) require.NoError(t, err) require.NotPanics(t, func() { @@ -1684,6 +1666,10 @@ func TestMutableTree_InitialVersion_Prune(t *testing.T) { _, err = tree.LoadVersion(expFirstVersion) require.NoError(t, err) + bz, err := tree.Get(key) + require.NoError(t, err) + require.Equal(t, []byte("world"), bz, "k/v should still be found") + // Direct node version access hasVersion1, err := tree.ndb.hasVersion(1) require.NoError(t, err) From 01ca777aa078021bb3e710f06d3c6c5846250f11 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 11:36:02 -0700 Subject: [PATCH 11/17] chore: remove todo comment --- mutable_tree_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 4db9b21de..9674569d0 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1540,8 +1540,7 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // Internal ndb methods for good measure - // TODO: AUDIT - use of ndb.hasVersion() - // Node has version 1, but it should only be found via InitialVersion + // Node has version 1, but it should only be found via reference nodes hasVersion1, err := tree.ndb.hasVersion(1) require.NoError(t, err) require.False(t, hasVersion1, "version 1 should not be found") From a6d56f910affca71430ab680fa7c7ca298c16230 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 11:56:39 -0700 Subject: [PATCH 12/17] refactor: Alternative way to set the initialversion root node --- mutable_tree.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index e18e62604..9e205cb93 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -787,23 +787,6 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { return nil, 0, err } - - // Delete the root node to be replaced below with new key. Need to - // delete this otherwise there will be two keys (version, 1) and - // (version, 0) below pointing at the same node. - if err := tree.ndb.deleteFromPruning(tree.ndb.nodeKey(tree.root.nodeKey.GetKey())); err != nil { - return nil, 0, err - } - - // Use a nonce of 0 to match pruning behavior that hides it from - // the version list. - // Mark the root node as hidden, same as how pruning handles. - // Even if with the reference node via SaveRoot() above points at - // (version, 1), LoadVersion still checks (version, 0) if it exists. - tree.root.nodeKey.nonce = 0 - if err := tree.ndb.SaveNode(tree.root); err != nil { - return nil, 0, err - } } } @@ -1068,6 +1051,9 @@ func (tree *MutableTree) balance(node *Node) (newSelf *Node, err error) { func (tree *MutableTree) saveNewNodes() error { nonce := uint32(0) newNodes := make([]*Node, 0) + + isInitialVersion := tree.ndb.opts.InitialVersion > 1 && tree.WorkingVersion() == int64(tree.ndb.opts.InitialVersion) + var recursiveAssignKey func(*Node) ([]byte, error) recursiveAssignKey = func(node *Node) ([]byte, error) { if node.nodeKey != nil { @@ -1081,6 +1067,12 @@ func (tree *MutableTree) saveNewNodes() error { nonce: nonce, } + // Save the root node at InitialVersion to nonce of 0 to hide it from + // the version list. + if isInitialVersion && nonce == 1 { + node.nodeKey.nonce = 0 + } + var err error // the inner nodes should have two children. if node.subtreeHeight > 0 { From 0e0bd3e093627fc6b3b5e1520e2e378ef9f78ab8 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 13:03:22 -0700 Subject: [PATCH 13/17] Revert "refactor: Alternative way to set the initialversion root node" This reverts commit 6365e4e96eee95a404afb2af5492349efa1ef6a8. Adding this just so we have reference of the other option --- mutable_tree.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 9e205cb93..e18e62604 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -787,6 +787,23 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { return nil, 0, err } + + // Delete the root node to be replaced below with new key. Need to + // delete this otherwise there will be two keys (version, 1) and + // (version, 0) below pointing at the same node. + if err := tree.ndb.deleteFromPruning(tree.ndb.nodeKey(tree.root.nodeKey.GetKey())); err != nil { + return nil, 0, err + } + + // Use a nonce of 0 to match pruning behavior that hides it from + // the version list. + // Mark the root node as hidden, same as how pruning handles. + // Even if with the reference node via SaveRoot() above points at + // (version, 1), LoadVersion still checks (version, 0) if it exists. + tree.root.nodeKey.nonce = 0 + if err := tree.ndb.SaveNode(tree.root); err != nil { + return nil, 0, err + } } } @@ -1051,9 +1068,6 @@ func (tree *MutableTree) balance(node *Node) (newSelf *Node, err error) { func (tree *MutableTree) saveNewNodes() error { nonce := uint32(0) newNodes := make([]*Node, 0) - - isInitialVersion := tree.ndb.opts.InitialVersion > 1 && tree.WorkingVersion() == int64(tree.ndb.opts.InitialVersion) - var recursiveAssignKey func(*Node) ([]byte, error) recursiveAssignKey = func(node *Node) ([]byte, error) { if node.nodeKey != nil { @@ -1067,12 +1081,6 @@ func (tree *MutableTree) saveNewNodes() error { nonce: nonce, } - // Save the root node at InitialVersion to nonce of 0 to hide it from - // the version list. - if isInitialVersion && nonce == 1 { - node.nodeKey.nonce = 0 - } - var err error // the inner nodes should have two children. if node.subtreeHeight > 0 { From 52376765d20f1dc654e97f3f0cafa0a3b32e4e98 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 15:57:36 -0700 Subject: [PATCH 14/17] doc: Clarify patch comments --- mutable_tree.go | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index e18e62604..08a526fae 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -773,33 +773,29 @@ func (tree *MutableTree) SaveVersion() ([]byte, int64, error) { } // Addition to node version patch for backwards compatibility: - // This is to resolve queries at InitialVersion, loading the tree at - // InitialVersion when the root node has version 1. Must not be done - // when InitialVersion is 1 as it will write an unnecessary reference - // that overwrites the actual root node. - // Note: version != nodekey.Version due to patch. + // This is to resolve queries at InitialVersion AND at version 1. + // Only applies when InitialVersion > 1 and on the first save. + // The root node on a new tree is always at version 1 to maintain app + // hash backwards compatibility: + // InitialVersion != nodekey.Version if tree.ndb.opts.InitialVersion > 1 && version == int64(tree.ndb.opts.InitialVersion) { // SaveRoot is meant for saving a tree when there are no updates, which // simply creates a reference node to the root node. We reuse this to - // create a reference node from InitialVersion -> 1 (root node). - - // Save reference node from InitialVersion -> tree.root + // create a reference node at InitialVersion with value of the root + // node at (1, 1). if err := tree.ndb.SaveRoot(version, tree.root.nodeKey); err != nil { return nil, 0, err } - // Delete the root node to be replaced below with new key. Need to - // delete this otherwise there will be two keys (version, 1) and - // (version, 0) below pointing at the same node. + // Delete the root node at (1, 1) to be re-saved at (1, 0) below. if err := tree.ndb.deleteFromPruning(tree.ndb.nodeKey(tree.root.nodeKey.GetKey())); err != nil { return nil, 0, err } // Use a nonce of 0 to match pruning behavior that hides it from // the version list. - // Mark the root node as hidden, same as how pruning handles. - // Even if with the reference node via SaveRoot() above points at - // (version, 1), LoadVersion still checks (version, 0) if it exists. + // ndb.GetRoot will check for (version, 0) if (version, 1) does not + // exist. tree.root.nodeKey.nonce = 0 if err := tree.ndb.SaveNode(tree.root); err != nil { return nil, 0, err From c1d6db02ef0229d720566a90dbff084b79731ad7 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Fri, 16 Aug 2024 15:57:59 -0700 Subject: [PATCH 15/17] test: Ensure apphash stays the same for different InitialVersions --- mutable_tree_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 9674569d0..31efa9f76 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1687,6 +1687,8 @@ func TestMutableTree_InitialVersion_Prune(t *testing.T) { func TestMutableTree_InitialVersion_NodeDB_Version(t *testing.T) { testInitialVersions := []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + var expTreeHash []byte + for _, initialVersion := range testInitialVersions { t.Run(fmt.Sprintf("InitialVersion(%d)", initialVersion), func(t *testing.T) { db := dbm.NewMemDB() @@ -1696,10 +1698,19 @@ func TestMutableTree_InitialVersion_NodeDB_Version(t *testing.T) { _, err := tree.Set([]byte("hello"), []byte("world")) require.NoError(t, err) - _, version, err := tree.SaveVersion() + treeHash, version, err := tree.SaveVersion() require.NoError(t, err) require.Equal(t, initialVersion, version) + if expTreeHash == nil { + expTreeHash = treeHash + } else { + require.NotEmpty(t, expTreeHash, "expected tree hash should not be empty") + require.Equal(t, expTreeHash, treeHash, "tree hash should be the same") + } + + t.Logf("treeHash: %X", treeHash) + // ------------------------------ // Verify @@ -1732,4 +1743,22 @@ func TestMutableTree_InitialVersion_NodeDB_Version(t *testing.T) { require.ErrorIs(t, err, ErrVersionDoesNotExist) }) } + + t.Run("No InitialVersion", func(t *testing.T) { + // Starts at 1, same as InitialVersion(1) + // This goes through the non-patched path + + db := dbm.NewMemDB() + tree := NewMutableTree(db, 0, false, log.NewNopLogger()) + + _, err := tree.Set([]byte("hello"), []byte("world")) + require.NoError(t, err) + + treeHash, version, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, int64(1), version) + + require.NotEmpty(t, expTreeHash, "expected tree hash should not be empty, should have been set in previous test") + require.Equal(t, expTreeHash, treeHash, "tree hash should be the same") + }) } From cdfc587cbee272313f159f4be224ff74c4c5a69d Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 20 Aug 2024 16:40:38 -0700 Subject: [PATCH 16/17] test: Update skipped (1, 0) root node test --- mutable_tree_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 31efa9f76..9fccc7def 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -1476,7 +1476,7 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // ------------------------------ // Verify - // 1. Node exists with Version 1, not InitialVersion + // 1. Node exists with Version 1 and nonce 0, not InitialVersion // 2. VersionExists(InitialVersion) - should return true // 3. AvailableVersions() - should include InitialVersion and NOT version 1 // 4. ndb.GetRoot(InitialVersion) - should return the root node @@ -1484,10 +1484,17 @@ func TestMutableTree_InitialVersion_FirstVersion(t *testing.T) { // 6. LoadVersion(InitialVersion) - loads tree even if node contains version 1. This is last since it runs some previous methods internally // the nodes created at the first version are not assigned with the `InitialVersion` - t.Run("1. root node exists at RootKey(initialVersion) with nodeKey version 1", func(t *testing.T) { - t.Skip() - rootKey := GetRootKey(initialVersion) - node, err := tree.ndb.GetNode(rootKey) + t.Run("1. root node exists at (1, 0) with nodeKey version 1", func(t *testing.T) { + _, err := tree.ndb.GetNode(GetRootKey(initialVersion)) + require.Error(t, err) + require.ErrorContains(t, err, "error reading Node.", "root node at initialVersion should be a reference node") + + _, err = tree.ndb.GetNode(GetRootKey(1)) + require.Error(t, err) + require.ErrorContains(t, err, "Value missing for key", "(1, 1) should not exist") + + rnk := &NodeKey{version: 1, nonce: 0} + node, err := tree.ndb.GetNode(rnk.GetKey()) require.NoError(t, err) // Calling GetNode with rootKey actually sets the returned Node.nodeKey. From d5920a190813647b6285575a8db7698e0d1b6c16 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 20 Aug 2024 16:55:49 -0700 Subject: [PATCH 17/17] test: Add test for rollbacks on new tree and existing ndb --- mutable_tree_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/mutable_tree_test.go b/mutable_tree_test.go index 9fccc7def..fcdc66c63 100644 --- a/mutable_tree_test.go +++ b/mutable_tree_test.go @@ -2,6 +2,7 @@ package iavl import ( "bytes" + "encoding/hex" "errors" "fmt" "runtime" @@ -1769,3 +1770,73 @@ func TestMutableTree_InitialVersion_NodeDB_Version(t *testing.T) { require.Equal(t, expTreeHash, treeHash, "tree hash should be the same") }) } + +func TestMutableTree_InitialVersion_LoadVersionForOverwriting(t *testing.T) { + // This test to ensure that WorkingHash() doesn't affect our InitialVersion + // patch. Since we only care about the InitialVersion for this: + // The code path to reach a WorkingHash() call is only via + // LoadVersionForOverwriting(). + // However - one possible case is when rolling back before InitialVersion, + // the tree isn't actually deleted, e.g. attempting to SaveVersion() on an + // existing version. + + db := dbm.NewMemDB() + + initialVersion := int64(1000) + tree := NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) + + key := []byte("hello") + _, err := tree.Set(key, []byte("world")) + require.NoError(t, err) + + hash1, version1, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, version1) + + // Save 2 empty versions after the initial version, should produce 2 refnodes + _, _, err = tree.SaveVersion() + require.NoError(t, err) + _, _, err = tree.SaveVersion() + require.NoError(t, err) + + err = tree.LoadVersionForOverwriting(initialVersion - 1) + require.Error(t, err, "cannot overwrite the initial version") + + // Overwrite initialversion + 1 + err = tree.LoadVersionForOverwriting(initialVersion) + require.NoError(t, err) + + workingVersion := tree.WorkingVersion() + require.Equal(t, initialVersion+1, workingVersion, "only can write InitialVersion + 1") + + hash2, version2, err := tree.SaveVersion() + require.NoError(t, err) + + // Hashes should match + require.Equal(t, initialVersion+1, version2, "version should be the same") + require.Equal( + t, + hex.EncodeToString(hash1), + hex.EncodeToString(hash2), + "hashes should match", + ) + + // ------------------------------ + // Repeat with the same database backend, but new tree. + // This replicates the case where a rollback before InitialVersion that does + // not delete the tree. + // Related: + // https://github.com/Kava-Labs/cosmos-sdk/pull/546 + + tree = NewMutableTree(db, 0, false, log.NewNopLogger(), InitialVersionOption(uint64(initialVersion))) + + _, err = tree.Set(key, []byte("world")) + require.NoError(t, err) + + hash3, version3, err := tree.SaveVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, version3) + + // Same as if the DB backend was empty + require.Equal(t, hex.EncodeToString(hash1), hex.EncodeToString(hash3), "hashes should match the first save") +}