Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-increment version on save #16

Merged
merged 11 commits into from
Dec 7, 2017
Merged

Conversation

cloudhead
Copy link
Contributor

@cloudhead cloudhead commented Nov 29, 2017

This changes the API for saving versions from:

SaveVersion(version int64) (hash []byte, err error)

to

SaveVersion() (hash []byte, version int64, err error)

Where the new SaveVersion auto-increments the version, starting from 1.

@cloudhead cloudhead changed the base branch from master to sdk2 November 29, 2017 14:49
Copy link
Contributor

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

In general, quite nice.
Still not sure why the version is all over the proof.
Also, really don't like updating version during clone.

If you clean that up, I'm happy to approve it.

if node.isLeaf() {
switch bytes.Compare(key, node.key) {
case -1:
return &Node{
key: node.key,
height: 1,
size: 2,
leftNode: NewNode(key, value),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, so set version once on insert rather than that traversing callback craziness in Save.
That was super disconcerting. Glad you changed that,

👍

tree.ndb.SaveBranch(tree.root, func(node *Node) {
// The node version is set here since it isn't known until we save.
Copy link
Contributor

Choose a reason for hiding this comment

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

😌

leafNode := proofLeafNode{
KeyBytes: keys[i],
ValueBytes: values[i],
Version: proof.Version,
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, version in leaf node proof is fair enough, as we do use in the hash, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's part of the hash for leaves.

@@ -231,7 +235,7 @@ func (t *Tree) getRangeWithProof(keyStart, keyEnd []byte, limit int) (
}
t.root.hashWithCount() // Ensure that all hashes are calculated.

rangeProof = &KeyRangeProof{RootHash: t.root.hash}
rangeProof = &KeyRangeProof{RootHash: t.root.hash, Version: t.root.version}
Copy link
Contributor

Choose a reason for hiding this comment

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

but why do we need the version on the root?

Isn't enough just to include version in the leaf to differentiate them, then any inner/root node automatically changes if a key updates, even to the same value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that I look at this, I think there's a flaw in how we do range proofs with versioned keys. There's missing information: we don't have the version of the leaf nodes we're trying to check, so if they are all different versions, the proof won't verify. It only verifies here because the keys happen to have the same version as the root..

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we need to look into proofs and versions deeper. But let's make this a different pr. Fo now, it is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. So does that mean range proofs are temporarily broken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, or rather, they never worked properly with historical queries.

tree.go Outdated
@@ -12,8 +12,9 @@ import (

// Tree is an immutable AVL+ Tree. Note that this tree is not thread-safe.
Copy link
Contributor

Choose a reason for hiding this comment

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

Tree is actually a reference to a the root of an immutibel AVL+ Tree
Plus mutable metadata.

Make it clear that root is immutiable, but tree is mutable

tree.go Outdated
ndb *nodeDB
root *Node
ndb *nodeDB
version int64
}

// NewTree creates both im-memory and persistent instances
Copy link
Contributor

Choose a reason for hiding this comment

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

s/im-memory/in-memory/

tree.go Outdated
@@ -73,7 +74,7 @@ func (t *Tree) set(key []byte, value []byte) (orphaned []*Node, updated bool) {
cmn.PanicSanity(cmn.Fmt("Attempt to store nil value at key '%s'", key))
}
if t.root == nil {
t.root = NewNode(key, value)
t.root = NewNode(key, value, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

first node is always version 1?
not t.version?

Remember the case where the empty tree is saved 5 times before the first right, we want version to update each time so it remains in sync with the block height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if the root is nil, it means it's a brand new tree, and as it starts with 0, it will always be 1, even with t.version+1.

Note that we can't currently save an empty tree, so the first save will always be 1, and then it should increment from there since the root isn't nil anymore. Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense, but I know we found a bug in some test code with this case a while ago.

If you save but t.root == nil, then we still do t.version++ ?
Then make my change here and if we save empty tree 5 times, on save 6 it has a value, it starts at version 6 (to match block 6)?

Maybe odd, but trying to keep in sync with the block height.

tree.go Outdated
ndb: tree.ndb,
root: tree.root,
ndb: tree.ndb,
version: tree.version + 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

update on clone???
not after SaveVersion?

I would suggest starting at 1 on init, and increment after SaveVersion, seems cleaner...
Expecially, as we want to clone Tree for caching, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That clone method is private and only used by VersionedTree, but I hear you - will change.

Copy link
Contributor Author

@cloudhead cloudhead Dec 1, 2017

Choose a reason for hiding this comment

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

func (tree *VersionedTree) SaveVersion(version int64) ([]byte, error) {
// the tree. Returns the hash and new version number.
func (tree *VersionedTree) SaveVersion() ([]byte, int64, error) {
version := tree.latestVersion + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, you do set the version here. very nicely actually (disregard that part of my previous comment).
however, I don't like the assumption that it is increaded by clone.
rather you set the version in clone()

then it is clear what happens, eg:

  • init: VersionedTree.latestVersion=0, orphanedTree.version=1
  • SaveVersion:
    • v := VersionedTree.latestVersion+1
    • VersionedTreeversions[v] = ...
    • orphanTree = ...clone()
    • orphanTree.version = v+1

That is super clear and only happens in init and save, not hidden in clone, which is not expected to have side-effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap. Let me know what you think of the changes. We have to set the version before Save() for orphanTree because it uses it to manage orphans, but the rest is as you describe.

@cloudhead cloudhead force-pushed the feature/version-auto-increment branch from 039828e to d22102c Compare December 1, 2017 15:55
@cloudhead cloudhead force-pushed the feature/version-auto-increment branch from c65db1b to 19b2143 Compare December 4, 2017 12:18
Copy link
Contributor

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

I think it is acceptable.
If you make a few pieces of code cleanup, I think it would be very nice.

Merge it in as you feel fit.

// Used for serialization of proofs.
keyExistsMagicNumber = 0x50
keyAbsentMagicNumber = 0x51
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You really, really don't like the type bytes, register interface in go-wire, do you? Just reproducing pieces of this functionality by hand.

Won't block it, but I think we need to standardize... so we don't have 5 different encoding mechanisms. Merge this in. Then discuss with jae and rudi about the future of type switching in serialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yeah this is me giving up after 1 minute of trying to figure out how to do this with go-wire. Obviously not the way to go, so let's do this properly once we figure out what we want to do.

proof_key.go Outdated
proof := new(KeyAbsentProof)
err := wire.ReadBinaryBytes(data, &proof)
return proof, err
}

func ReadKeyProof(data []byte) (KeyProof, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the Bytes() method, you serialize a byte slice with go wire, then prepend a type.

Why not just do the same here?

b, val := data[0], data[1:]
switch b {
case keyExistsMagicNumber:
 return readKeyExistsProof(val)
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh yeah, that's much simpler.

require.NotNil(err)
require.NoError(proof2.Verify(key, val, root))

if _, ok := proof2.(*KeyExistsProof); !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the if, not
require.True(ok, "Proof should be...")?
They both do the same, one seems more standard.

if version <= tree.latestVersion {
return nil, errors.Errorf("version must be greater than latest (%d <= %d)",
version, tree.latestVersion)
return nil, 0, ErrNilRoot
Copy link
Contributor

Choose a reason for hiding this comment

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

Just ask for tree.latestVersion = version in this case with my rambling about multiple saves and matching block height.

@cloudhead cloudhead merged commit 7358fc6 into sdk2 Dec 7, 2017
@cloudhead cloudhead deleted the feature/version-auto-increment branch December 7, 2017 14:47

// SaveEmptyRoot creates an entry on disk for an empty root.
func (ndb *nodeDB) SaveEmptyRoot(version int64) error {
return ndb.saveRoot([]byte{}, version)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just call ndb.SaveRoot() here and keep the mtx in the exposed method, unless I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exposed method takes a root node though, which we don't have here.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok.

rightNode: node,
version: version,
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, so we just save these on disk (it would help with debugging) but they're not otherwise used? (e.g. not hashed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was always the case actually, the version of inner nodes was saved but not hashed. The difference is where we set the version. Now we set it here, in set(), previously we would set it on SaveVersion().

// SaveAs saves the underlying Tree and assigns it a new version.
// Saves orphans too.
func (tree *orphaningTree) SaveAs(version int64) {
tree.version = version
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets assert here that version = tree.version + 1 before doing this? (e.g. panic otherwise). I can imagine that in the future we might want to be able to load an outdated version and resume from there, but for now I don't think the assertion should fail.

(BTW, If *Tree is never assigned, then it can just be Tree. Since the containing struct is always used as a pointer, golang lets orphaningTree.Tree be a pointer receiver.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I was about to do this, I realised we can make a simplification now by getting rid of latestVersion, and then we set the version in *VersionedTree. Check it out and let me know if you still think we need to check (v' == v+1): 4e42312

Copy link
Contributor

Choose a reason for hiding this comment

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

I think now the semantics of latestVersion changes. See my comments...
What does it meant to load a previous version of the tree, and then call Hash or LatestVersion? Maybe there should be LatestHash() and Hash() as well as LatestVersion() and Version()?

}

tree.latestVersion = version
tree.versions[version] = tree.orphaningTree.Tree

tree.orphaningTree.SaveVersion(version)
tree.orphaningTree.SaveAs(version)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have just made it tree.orphaningTree.Save() and I would assert the return value here. I like "SaveAs()" too, same difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change I mentioned above means orphaningTree doesn't set the version anymore. So this shouldn't be needed anymore.

ridenaio pushed a commit to idena-network/iavl that referenced this pull request Jul 1, 2019
tac0turtle pushed a commit that referenced this pull request Apr 9, 2022
* Add unbounded key string to KeyFormat

* Add test vectors for unbounded length keys

* Add some notes

* update .gitignore

* Add FastNode struct

* WIP: make Get work with new FastNode

* when retrieving fastnode fails, return errors vs. panic

* add comments clarifying what index represents

* make the linter happy

* Add small tweaks to fast_node.go

* add TODO & small linter tweaks

* Update comment

* update fast node cache in set

* add debugging output when falling back to original logic

* return error instead of panic

* WIP: refactor set ops to work with fast store

* update Set of mutable tree, begin unit testing

* update GetVersioned to check fast nodes before trying the immutable

* check fast node version before nil value check in get of immutable tree

* fix small bugs and typos, continue writing unit tests for Set

* unit test saveFastNodeVersion

* simplify storing unsaved fast nodes

* resolve a bug with not writing prefix for fast node to disk

* remove fast nodes from disk on save and clear fast cache when version is deleted, fix all tests but random and with index

* resolve an issue with randomized tests caused by the fast node cache not being cleared when latest version is saved

* split unsaved fast node changes into additions and removals

* save fast node removals

* move fast node cache clearing to nodedb

* use regular logic only when fast node version is greater than immutable tree'

* clean up tree_random_test.go

* clear unsaved fast node removals on rollback

* fix randomized test failures caused by a typo in ndb DeleteVersion for loop

* implement GetFast method to preserve Get with correct index return, convert unit tests from Get to GetFast where applicable

* ensure Get and GetFast return the same values in tree_random_test.go

* test fast node cache is live in random tree tests

* improve mutable tree unit tests related to new functionality

* clean up tree_test.go

* implement GetVersionedFast to preserve the index in GetVersioned

* restore accidentally deleted test in mutable tree test

* spurious whitespace

* refactor mutable tree

* fix comment in mutable tree

* add benchmark results

* avoid redundant full tree search in GetFast of immutable tree when fast node is nil and tree is latest

* fix naming for bench test get on random keys

* use method for get latestversio in get fast

* optimize GetFast, perform a refactor to ensure that fast nodes on disk are matched and better test

* add latest bench

* Fast Node Iteration (#7)

* propagate errors from nodedb and avoid panicking

* begin implementing fast node iteration

* resolve rebase issue in random tests

* fix iteration to deserialize fast node for extracting the correct value

* finalzie iteration and let all unit tests pass

* add benchmarks

* merge GetVersioned and GetVersionedFast

* remove fast node deletion from DeleteVersion and DeleteVersionRange and benchmark

* fix and unit test iteration on mutable and immutable trees

* implement tests for iterator and iterate, begin testing fast iterator

* fix and unit test fast iterator

* refactor iterate methods of mutable and immutable trees

* resolve some warnings

* remove old bench results

* refactor bench tests for iteration

* Fast Cache Migration (#9)

* implement nodedb changes to set and get chain version from the database

* implement and unit test upgrade to fast storage in mutable tree

* refactor for auto upgrade to fast version in mutable tree contructor, load version and lazy load version

* use proper functionality for getting latest version

* remove unused error

* fix comment

* use fast version value in tests

* spurious tab

* fix style problems and remove redundant code in tests

* Rename Get to GetWithIndex and GetFast to Get

* refactor and clean up bench tests

* remove extra byte from fast node length

* clean up immutable tree

* refactor nil tree or ndb error for the iterators and their tests

* avoid exporting methods for getting unsaved additions and removals

* refactor fast upgrade to read from tree instead of traversing disk nodes and orphans, update unit tests

* remove unneeded comment

* refer to storage version consistently across the project

* fix more warnings

* optimize removal of fast nodes from cache on deletion

* small changes in teh mutable tree

* correct storage version key

* auto set fast version in SaveVersion

* avoid exporting new methods of the immutable tree

* go fmt

* Fix comment in fast iterator

Co-authored-by: Dev Ojha <[email protected]>

* add extra comment for domain in fast iterator

Co-authored-by: Dev Ojha <[email protected]>

* add comments for moving the iterator before the first element

* add comment for describing what mirror is in assertIterator of testutils

* fix checking the mirror for descending iterator in tests

* Update testutils_test.go with a comment

Co-authored-by: Dev Ojha <[email protected]>

* Update benchmarks/bench_test.go with a comment for runKnownQueriesFast

Co-authored-by: Dev Ojha <[email protected]>

* Update benchmarks/bench_test.go with a comment for runQueriesFast

Co-authored-by: Dev Ojha <[email protected]>

* export IsFastCacheEnabled and add an assert in bench tests

* Update comment immutable_tree.go

Co-authored-by: Dev Ojha <[email protected]>

* Update comment for migration in mutable_tree.go

Co-authored-by: Dev Ojha <[email protected]>

* simlify Iterate in mutable tree, add debug log for

* Fast Cache - Downgrade - reupgrade protection and other improvements (#12)

* add leaf hash to fast node and unit test

* refactor get with index and get by index, fix migration in load version and lazy load version

* use Get in GetVersioned of mutable tree

* refactor non membership proof to use fast storage if available

* bench non-membership proof

* fix bench tests to work with the new changes

* add downgrade-reupgrade protection and unit test

* remove leaf hash from fast node

* resolve multithreading bug related to iterators not being closed

* clean up

* use correct tree in bench tests

* add cache to tree used to bench non membership proofs

* add benc tests for GetWithIndex and GetByIndex

* revert GetWithIndex and GetByIndex

* remove unused import

* unit test re-upgrade protection and fix small issues

* remove redundant setStorageVersion method

* fix bug with appending to live stage version to storage version and nit test

* add comment for setFastStorageVersionToBatch

* refactor and improve unit tests for reupgrade protection

* rename ndb's isFastStorageEnabled to hasUpgradedToFastStorage and add comments

* comment out new implementation for GetNonMembershipProof

* update comments in nodedb to reflect the difference between hasUpgradedToFastStorage and shouldForceFastStorageUpdate

* refactor nodedb tests

* downgrade tendermint to 0.34.14 - osmosis's latest cosmos sdk does not support 0.35.0

* fix bug where fast storage was not enabled when version 0 was attempted to be loaded

* implement unsaved fast iterator to be used in mutable tree (#16)

* address comments from unsaved fast iterator PR

* expose isUpgradeable method on mutable tree and unit test (#17)

* expose isUpgradeable method on mutable tree and unit test

* go fmt

* resolve problems with rebasing

* update CHANGELOG.md

* tendermint v0.35.0

* fix String() bench

* fix duplication lint in iterator_test.go

* fix lint for tree.ndb.DeleteFastNode error return not checked

* fix Error return value of `ndb.resetBatch` is not checked

* fix Error return value of `ndb.traversePrefix` is not checked

* fix Error: struct of size 64 bytes could be of size 56 bytes

* fix Error: `comitted` is a misspelling of `committed`

* fix issues in basic_test.go

* address comments in fast_iterator.go

* address comments in immutable tree

* address comments in iterator.go

* address comments in key_format.go

* address remaining comments

* fix Error: Error return value of `ndb.batch.Write` is not checked

* fix Error: receiver name t should be consistent with previous receiver name tree for MutableTree

* fix Error: struct of size 48 bytes could be of size 40 bytes

* go fmt

* more linter fixes

* fix remaining linter problems

* upgrade tm-db and comment out broken bencher databases

* skip iterations for BenchmarkLevelDBLargeData bench

* attempt to fix linter

* force GC, no cache during migration, auto heap profile (#19)

* force GC, no cache during migration, auto heap profile

* resolve a potential deadlock from racing between reset and stop

* fix small lint issue

* remove logs and pprof logic

* remove unused libraries

* add comment explaining the reason for RAM optimizations

* sync access to fast node cache to avoid concurrent write fatal error (#23)

* update go.mod and go.sum to match master versions

* revert #23 (sync access to fast node cache), fix bug related to old height export (#33)

* Revert "sync access to fast node cache to avoid concurrent write fatal error (#23)"

This reverts commit 2a1daf4.

* return correct iterator in mutable tree

* fix concurrent map panic when querying and comittting concurrently

* avoid clearing fast node cache during pruning (#35)

* fix data race related to VersionExists (#36)

* fix data race related to VersionExists

* use regular lock instead of RW in mutable_tree.go

* hardcode fast node cache size to  100k

* go fmt

* restore proof_ics23.go

* fix linter

Co-authored-by: ValarDragon <[email protected]>
Co-authored-by: jtieri <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Roman Akhtariev <[email protected]>
Co-authored-by: Roman <[email protected]>
roysc pushed a commit to roysc/iavl that referenced this pull request Jul 16, 2022
…osmos#12)

* add leaf hash to fast node and unit test

* refactor get with index and get by index, fix migration in load version and lazy load version

* use Get in GetVersioned of mutable tree

* refactor non membership proof to use fast storage if available

* bench non-membership proof

* fix bench tests to work with the new changes

* add downgrade-reupgrade protection and unit test

* remove leaf hash from fast node

* resolve multithreading bug related to iterators not being closed

* clean up

* use correct tree in bench tests

* add cache to tree used to bench non membership proofs

* add benc tests for GetWithIndex and GetByIndex

* revert GetWithIndex and GetByIndex

* remove unused import

* unit test re-upgrade protection and fix small issues

* remove redundant setStorageVersion method

* fix bug with appending to live stage version to storage version and nit test

* add comment for setFastStorageVersionToBatch

* refactor and improve unit tests for reupgrade protection

* rename ndb's isFastStorageEnabled to hasUpgradedToFastStorage and add comments

* comment out new implementation for GetNonMembershipProof

* update comments in nodedb to reflect the difference between hasUpgradedToFastStorage and shouldForceFastStorageUpdate

* refactor nodedb tests

* downgrade tendermint to 0.34.14 - osmosis's latest cosmos sdk does not support 0.35.0

* fix bug where fast storage was not enabled when version 0 was attempted to be loaded

* implement unsaved fast iterator to be used in mutable tree (cosmos#16)
ulbqb pushed a commit to ulbqb/iavl that referenced this pull request Jun 6, 2023
* Add go fuzz tests

* Add membership proof for existing keys

* Build tree after adding membership proof

* Make batch add fuzz tests work

* Do not commit version twice for dst

* Save version out of dst.Set condition

* Set rootHash to workingHash

* Fix edge cases

* Refactor DST Non-Existence Proof

* Change cacheSize

* Add test data and sibling nodes for each node in path

* Fix GetSiblingNodes

* Add more test data

* testing: fuzz: failing test case

* Use set for keys

* Add more test data

* Refactor existence proofs code

* Add required info for remove operation

* Add children of siblings as well

* Remove debug code

* Add testdata that breaks current code

* Fix bug

* Add failing testcase

* Add breaking testcase

* IAVL with tracing

* Fuzz tests pass

* Refactor tracing code

* Remove redundant code

* Remove working hash in calls to AddExistenceProof

* Clean up flag

* Make build tree a private method

* Add back whitespace in node.go

* Add new ci for fuzz

* Refactor more

* Refactor out getKey method

* Change name to reapInclusionProofs

* Refactor set and remove in DST

* Factor out add existence proofs from Remove DST for consistency with Set

* Refactor into testContext

* Clean up setInDST

Co-authored-by: Tomasz Zdybał <[email protected]>
ulbqb pushed a commit to ulbqb/iavl that referenced this pull request Jun 12, 2023
* Add go fuzz tests

* Add membership proof for existing keys

* Build tree after adding membership proof

* Make batch add fuzz tests work

* Do not commit version twice for dst

* Save version out of dst.Set condition

* Set rootHash to workingHash

* Fix edge cases

* Refactor DST Non-Existence Proof

* Change cacheSize

* Add test data and sibling nodes for each node in path

* Fix GetSiblingNodes

* Add more test data

* testing: fuzz: failing test case

* Use set for keys

* Add more test data

* Refactor existence proofs code

* Add required info for remove operation

* Add children of siblings as well

* Remove debug code

* Add testdata that breaks current code

* Fix bug

* Add failing testcase

* Add breaking testcase

* IAVL with tracing

* Fuzz tests pass

* Refactor tracing code

* Remove redundant code

* Remove working hash in calls to AddExistenceProof

* Clean up flag

* Make build tree a private method

* Add back whitespace in node.go

* Add new ci for fuzz

* Refactor more

* Refactor out getKey method

* Change name to reapInclusionProofs

* Refactor set and remove in DST

* Factor out add existence proofs from Remove DST for consistency with Set

* Refactor into testContext

* Clean up setInDST

Co-authored-by: Tomasz Zdybał <[email protected]>
ulbqb added a commit to ulbqb/iavl that referenced this pull request Jun 12, 2023
danwt pushed a commit to dymensionxyz/iavl that referenced this pull request Apr 2, 2024
* feat: Deep tree structure with Updates (cosmos#13)

* ICS23-style approach for DeepSubTree creation

* Fix deepsubtree, all checks pass

* Update documentation for deep subtrees

* Fix deepsubtree to accomodate left/right paths both

* Refactor test code

* Refactor TestDeepSubtreeStepByStep

* Address comments

* Refactor code

* Disable gocritic and unused

* Add description

* Refactor code to check err in Set

* Modify traversal conditions to be clearer

* feat: build deep subtree from ICS23 inclusion proofs (cosmos#9)

* feat: build deep subtree from ICS23 inclusion proofs

* feat: handle non-existence proofs

* linter: goimports deepsubtree.go

* refactor: addExistenceProofProof -> addExistenceProof

* fix: un-hardcode size of byte array

* Add more comments

* Refactor ndb.Commit call outside for loop

* verify that in the case that dst.root != nil that the root node hash matches the provided hash and check dst.root != nil first

* Add strings and use strings.Repeat

* Delete addPath and AddPath

* Remove print statements

* Refactor recomputeHash usage

Co-authored-by: Matthew Sevey <[email protected]>

* return err directly

Co-authored-by: Matthew Sevey <[email protected]>

* Address linting checks

* Use tm-db instead of cosmos-db

* Fix linter

* Update comment

Co-authored-by: Matthew Sevey <[email protected]>

* Add error checking for node hash

* turn lengthByte into a const

* Refactor linkNode

* Update comment

Co-authored-by: Tomasz Zdybał <[email protected]>
Co-authored-by: Matthew Sevey <[email protected]>

* feat: Support Adds in a Deep Subtree (cosmos#8)

* ICS23-style approach for DeepSubTree creation

* Fix deepsubtree, all checks pass

* Update documentation for deep subtrees

* Fix deepsubtree to accomodate left/right paths both

* Refactor test code

* Refactor TestDeepSubtreeStepByStep

* Address comments

* Refactor code

* Disable gocritic and unused

* Add description

* Refactor code to check err in Set

* Modify traversal conditions to be clearer

* feat: build deep subtree from ICS23 inclusion proofs (cosmos#9)

* feat: build deep subtree from ICS23 inclusion proofs

* feat: handle non-existence proofs

* linter: goimports deepsubtree.go

* refactor: addExistenceProofProof -> addExistenceProof

* fix: un-hardcode size of byte array

* Add more comments

* Refactor ndb.Commit call outside for loop

* verify that in the case that dst.root != nil that the root node hash matches the provided hash and check dst.root != nil first

* Add strings and use strings.Repeat

* Delete addPath and AddPath

* Remove print statements

* Refactor recomputeHash usage

Co-authored-by: Matthew Sevey <[email protected]>

* return err directly

Co-authored-by: Matthew Sevey <[email protected]>

* Address linting checks

* Use tm-db instead of cosmos-db

* Fix linter

* Add test to check add operation in deepsubtree: WIP

* Add insert key functionality WIP

* Try replicating non-inclusion proofs

* Add sibling nodes to nonExistenceProofs

* Refactor test code

* Finish adding add functionality to dst

* Add Remove functionality to dst

* Fix linting errors

* Fix GetSiblingNode

* Remove more print statements

* Add comment for each case in recursive set

* Change which methods are exported and document exported methods

* feat: Support Empty Hashes and Add constructor (cosmos#11)

* Export siblings

* Add deepsubtree constructor

* Support empty root hashes

* Use working hash instead of root.hash

* Use tm-db instead of cosmos-db

* Return nil in BuildTree

* Address comments

* Address more comments

Co-authored-by: Tomasz Zdybał <[email protected]>
Co-authored-by: Matthew Sevey <[email protected]>

* Add randomized tests for adding/removing keys (cosmos#16)

* Add go fuzz tests

* Add membership proof for existing keys

* Build tree after adding membership proof

* Make batch add fuzz tests work

* Do not commit version twice for dst

* Save version out of dst.Set condition

* Set rootHash to workingHash

* Fix edge cases

* Refactor DST Non-Existence Proof

* Change cacheSize

* Add test data and sibling nodes for each node in path

* Fix GetSiblingNodes

* Add more test data

* testing: fuzz: failing test case

* Use set for keys

* Add more test data

* Refactor existence proofs code

* Add required info for remove operation

* Add children of siblings as well

* Remove debug code

* Add testdata that breaks current code

* Fix bug

* Add failing testcase

* Add breaking testcase

* IAVL with tracing

* Fuzz tests pass

* Refactor tracing code

* Remove redundant code

* Remove working hash in calls to AddExistenceProof

* Clean up flag

* Make build tree a private method

* Add back whitespace in node.go

* Add new ci for fuzz

* Refactor more

* Refactor out getKey method

* Change name to reapInclusionProofs

* Refactor set and remove in DST

* Factor out add existence proofs from Remove DST for consistency with Set

* Refactor into testContext

* Clean up setInDST

Co-authored-by: Tomasz Zdybał <[email protected]>

* Add tracing to Trees and Deep Subtrees (cosmos#18)

* Add go fuzz tests

* Add membership proof for existing keys

* Build tree after adding membership proof

* Make batch add fuzz tests work

* Do not commit version twice for dst

* Save version out of dst.Set condition

* Set rootHash to workingHash

* Fix edge cases

* Refactor DST Non-Existence Proof

* Change cacheSize

* Add test data and sibling nodes for each node in path

* Fix GetSiblingNodes

* Add more test data

* testing: fuzz: failing test case

* Use set for keys

* Add more test data

* Refactor existence proofs code

* Add required info for remove operation

* Add children of siblings as well

* Remove debug code

* Add testdata that breaks current code

* Fix bug

* Add failing testcase

* Add breaking testcase

* IAVL with tracing

* Fuzz tests pass

* Refactor tracing code

* Remove redundant code

* Remove working hash in calls to AddExistenceProof

* Clean up flag

* Make build tree a private method

* Add back whitespace in node.go

* Add new ci for fuzz

* Refactor more

* Refactor out getKey method

* Change name to reapInclusionProofs

* Refactor set and remove in DST

* Factor out add existence proofs from Remove DST for consistency with Set

* Refactor into testContext

* Clean up setInDST

* Add method for get

* Export methods

* Add witness data to deep subtree

* Verify operation in witness data

* Refactor and verify operation for get and remove

* Add set witness data

* Add tracing to tree

* add getter for witness data

* Verify existence proofs in dst

* Cleanup

* Reset witness data on tracing enabled

* Add node to keysAccessed even not in cache

* Add initial root hash

* Refactor GetInitialRootHash

* Modify GetInitialRootHash

* Add test data

* Add get to dst tests: fails right now

* Refactor and add tracing for root key as well

* Add docs

* Add more docs

* Update comments

* Update log message

Co-authored-by: Tomasz Zdybał <[email protected]>

* allocate length

Co-authored-by: Tomasz Zdybał <[email protected]>

* fix: remove `RangeProofs` (cosmos#586)

Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Marko <[email protected]>

* Update go.mod file

* Add new line at end of .golangci.yml

Co-authored-by: Tomasz Zdybał <[email protected]>
Co-authored-by: Matthew Sevey <[email protected]>
Co-authored-by: cool-developer <[email protected]>
Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Marko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants