Build deep tree structure by altering Verify method #3

Manav-Aggarwal · 2022-09-25T04:14:45Z

First step of implementation is to build deep tree structure. As pointed out by @musalbas in #1, this can be done by modifying RangeProofs Verify function.

Definition of done:

DeepTree structure is introduced
Verify functions are modified to collect required nodes
Tree structure is properly linked (by pointers in Node objects)
Root of underlying ImmutableTree is set properly
Key field of the inner nodes (ProofInnerNode) with the highest key in its left branch that we know of
Modified proof to include values on top of the already existing hashes of values in ProofLeafNode

Resolves #2.

musalbas · 2022-09-27T12:51:36Z

A reminder that you'll likely need to modify the proofs themselves too as mentioned here, eg adding a new optional field for deepnodes or similar to include nodes needed when appending or deleting from a deep tree: #1 (comment)

liamsi · 2022-09-27T09:45:48Z

proof_test.go

+	// nodes, err := dst.ndb.nodes()
+	// require.Nil(err)
+	// for _, node := range nodes {
+	// 	pnode, _ := dst.ndb.GetNode(node.hash)
+	// 	print(pnode.hash)
+	// 	print("\n")
+	// }


Why is this commented out? What purpose does this serve?

Apologies, this was being used while debugging. Removed it

* use `NoError` instead of `NotNil` for errors * un-hardcode loop counter

…root mismatch errors

… left branch that we know of

musalbas · 2022-10-02T15:10:54Z

proof_range.go

@@ -472,6 +518,7 @@ func (t *ImmutableTree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof
 				h := sha256.Sum256(node.value)
 				leaves = append(leaves, ProofLeafNode{
 					Key:       node.key,
+					Value:     node.value,


Why do we need the values for the leaf nodes? I believe you need the values for the sibling inner nodes for rebalancing, rather than leaf nodes.

Sorry I misinterpreted it to mean values of the node, not just the valueHash. I'll add the values of the leftNode and rightNode in ProofInnerNode in addition to the already existing leftHash and rightHash

To clarify I believe we need the preimage of sibling inner nodes, not their hashes, to get their "size" field to enable set() to do rebalancing. This is why doing the full test I suggested below will be important - as I'm not sure sets will work right now as they don't have the data needed to do rebalancing.

musalbas · 2022-10-02T15:12:42Z

Is there a test for updating and deleting from the deepsubtree and checking that it returns the correct roots? Something like this: https://ethresear.ch/t/data-availability-proof-friendly-state-tree-transitions/1453/23

musalbas · 2022-10-02T15:15:31Z

proof_range.go

@@ -509,8 +556,9 @@ func (t *ImmutableTree) getRangeProof(keyStart, keyEnd []byte, limit int) (proof
 					Height:  node.subtreeHeight,
 					Size:    node.size,
 					Version: node.version,
-					Left:    nil,
-					Right:   node.rightHash,
+					// Need this information to know where we came from while constructing a DeepSubTree


I think this should not be necessary to send over the wire. You should be able to compute this locally, no?

I tried computing it locally earlier, and reproduced it again here: https://github.com/celestiaorg/iavl/pull/4/files

However, in computeRootHash inside PathToLeaf, the hash computed through pin.Hash(hash) is not always the same as what pin.Left contains and that leads to inconsistent hashing. Found removing this optimization to work, so incorporated it for now.

Please correct me if there's a different way you can think of/something obvious I'm missing here.

If pin.Hash(hash) returns different things than pin.Left then shouldn't the Merkle proof be impossible to verify in the first place? I think something doesn't add up.

Manav-Aggarwal · 2022-10-03T00:27:57Z

Is there a test for updating and deleting from the deepsubtree and checking that it returns the correct roots? Something like this: https://ethresear.ch/t/data-availability-proof-friendly-state-tree-transitions/1453/23

Thanks for sharing an example, will add one.

Manav-Aggarwal · 2022-10-03T07:27:44Z

proto/iavl/proof.proto

@@ -32,11 +32,13 @@ message ProofInnerNode {
  int64  version = 3;
  bytes  left    = 4;
  bytes  right   = 5;
+  Node leftNode = 6;


How to add generic pointers in ProtoBuf?

This reverts commit 718defe.

tzdybal · 2022-10-31T09:01:01Z

This was a great primer for further work, enabling deep understanding of IAVL internals, but now it's obsolete.
See: #5, #9.

Manav-Aggarwal added 2 commits September 23, 2022 10:48

Add deep tree file and extend mutable tree

d6d0f40

Modify verify functions to collect required nodes

3afbf64

Manav-Aggarwal linked an issue Sep 25, 2022 that may be closed by this pull request

Create deep tree structure using an ICS23-style approach: Updates #2

Closed

6 tasks

Manav-Aggarwal added 2 commits September 25, 2022 00:43

Link tree structure properly

4519777

Set root of underlying Immutable tree

d7a0db9

Manav-Aggarwal changed the title ~~Add Deep Subtrees~~ Build deep tree structure by altering Verify method Sep 25, 2022

Manav-Aggarwal requested a review from tzdybal September 25, 2022 04:57

Manav-Aggarwal self-assigned this Sep 25, 2022

Manav-Aggarwal marked this pull request as ready for review September 25, 2022 04:57

Manav-Aggarwal added the enhancement New feature or request label Sep 25, 2022

Manav-Aggarwal added 5 commits September 27, 2022 02:17

Add nil checks for deepsubtree

65cacb1

WIP: Write test for deep subtree

ca8067d

Add root hash check in deep subtree verify

2eb81a2

Save version of trees so we can access nodes

a97f20e

Add left and right pointers when available

0cbb93a

liamsi reviewed Sep 27, 2022

View reviewed changes

tzdybal marked this pull request as draft September 27, 2022 13:49

Manav-Aggarwal and others added 11 commits September 27, 2022 15:05

Remove commented code

6e05e47

testing: small improvements

fe5459f

* use `NoError` instead of `NotNil` for errors * un-hardcode loop counter

refactor: improve error handling

b0ccb3b

fix: save proper hashes and leaf node

688ac59

Change tree components in test

dc1458b

Add left and right node hashes, tree links properly

49483b0

Calculate rootHash properly

5dd3b66

Able to create deepsubtrees now, but get still get not blocking some …

7c3f21c

…root mismatch errors

Remove verification code bc assumption broken now

718defe

Add values to leaf nodes in addition to already existing hashes

1581db4

populate the key field of the inner nodes with the highest key in its…

1cc629b

… left branch that we know of

Manav-Aggarwal marked this pull request as ready for review October 1, 2022 10:30

Manav-Aggarwal requested review from musalbas and liamsi October 1, 2022 10:31

Describe unit test added

6980866

Manav-Aggarwal linked an issue Oct 1, 2022 that may be closed by this pull request

Add deep trees for IAVL, for fraud proofs and stateless execution #1

Closed

9 tasks

Refactor printDeepsubtreeNode

aa798eb

musalbas reviewed Oct 2, 2022

View reviewed changes

Manav-Aggarwal force-pushed the manav/add_deepsubtree branch from 7b89c40 to 5f062cb Compare October 3, 2022 07:27

Manav-Aggarwal commented Oct 3, 2022

View reviewed changes

Manav-Aggarwal added 2 commits October 7, 2022 14:03

Revert "Remove verification code bc assumption broken now"

e710bfd

This reverts commit 718defe.

Fix hash calculation

c31bff8

Manav-Aggarwal force-pushed the manav/add_deepsubtree branch from 5f062cb to c31bff8 Compare October 7, 2022 18:38

Manav-Aggarwal added 2 commits October 10, 2022 02:05

Fix inner key set

4090442

Add save version calls

af17d46

This was unlinked from issues Oct 16, 2022

Add deep trees for IAVL, for fraud proofs and stateless execution #1

Closed

Create deep tree structure using an ICS23-style approach: Updates #2

Closed

tzdybal closed this Oct 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build deep tree structure by altering Verify method #3

Build deep tree structure by altering Verify method #3

Manav-Aggarwal commented Sep 25, 2022 •

edited

Loading

musalbas commented Sep 27, 2022

liamsi Sep 27, 2022

Manav-Aggarwal Sep 27, 2022

musalbas Oct 2, 2022 •

edited

Loading

Manav-Aggarwal Oct 3, 2022

musalbas Oct 3, 2022

musalbas commented Oct 2, 2022

musalbas Oct 2, 2022

Manav-Aggarwal Oct 2, 2022

musalbas Oct 3, 2022

Manav-Aggarwal commented Oct 3, 2022

Manav-Aggarwal Oct 3, 2022

tzdybal commented Oct 31, 2022

Build deep tree structure by altering Verify method #3

Build deep tree structure by altering Verify method #3

Conversation

Manav-Aggarwal commented Sep 25, 2022 • edited Loading

musalbas commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

musalbas Oct 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

musalbas commented Oct 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Manav-Aggarwal commented Oct 3, 2022

Choose a reason for hiding this comment

tzdybal commented Oct 31, 2022

Manav-Aggarwal commented Sep 25, 2022 •

edited

Loading

musalbas Oct 2, 2022 •

edited

Loading