[wip] Adds a Merkle Tree library and "snapshotting" of tables' rows #455

brunocalza · 2023-02-06T14:47:58Z

Summary

This PR implements the architecture necessary to provide Inclusion Proofs in Tableland.

It relates to the GH project Brute force Merkle Tree of all tables (experimental).
An in-depth discussion about the architecture and design decisions can be found in an internal Notion doc.

Context

This can be potentially a very big PR. Not really sure how it will fully unfold. But as is, we have fully working parts. And others to come next, as described in the next section.

Implementation overview

Currently, we have the following parts implemented and ready for review:

Merkle Tree library
This is an independent package that implements a simple in-memory Merkle tree. It's currently not used by any code. But it will be used by Merkle Root publisher
"Snapshotting" of tables' rows
For every X block, we snapshot all rows for all tables into a stream of bytes row in a new table. This is described here.

Parts to come:

[GOT-43] Post the root to on-chain at some internal to an experimental validator registry #375
This part will fetch rows from '"Snapshotting" of tables' rows', build the Merkle Tree and publish the root to a smart contract
[GOT-42] Expose API endpoint to get a proof that a row is in a table #374

Todos

Remove calculate hash implementation

Notes

It is being run in staging
The Merkle Tree proof implementation was tested against OpenZeppelin implementation (this is probably an end-to-end test worth adding later)

Implementation details and review orientation

Checklist

Are changes backward compatible with existing SDKs, or is there a plan to manage it correctly?
Are changes covered by existing tests, or were new tests included?
Are code changes optimized for future code readers, commenting on problematic areas to understand (if any)?
Future-self question: Did you avoid unjustified/unnecessary complexity to achieve the goal?

.github/workflows/test.yml

brunocalza · 2023-02-08T16:35:07Z

pkg/eventprocessor/impl/eventprocessor.go

-	mEventExecutionCounter      syncint64.Counter
-	mTxnExecutionLatency        syncint64.Histogram
-	mHashCalculationElapsedTime atomic.Int64
+	mBaseLabels                       []attribute.KeyValue


Adds mTreeLeavesCalculationElapsedTime to measure how long it took to snapshot the leaves

brunocalza · 2023-02-08T16:36:23Z

pkg/eventprocessor/impl/eventprocessor.go

@@ -220,6 +221,11 @@ func (ep *EventProcessor) executeBlock(ctx context.Context, block eventfeed.Bloc
 		if err := ep.calculateHash(ctx, bs); err != nil {
 			return fmt.Errorf("calculate hash: %s", err)
 		}


At the same spot we calculate the hash we calculate the tree leaves. I'm keeping both implementations running so I can compare, but the calculateHash would be removed before merging

brunocalza · 2023-02-08T16:42:15Z

pkg/eventprocessor/impl/executor/impl/blockscope.go

@@ -215,6 +216,84 @@ func (bs *blockScope) StateHash(ctx context.Context, chainID tableland.ChainID)
 	return executor.NewStateHash(chainID, bs.scopeVars.BlockNumber, hash), nil
 }

+func (bs *blockScope) CalculateTreeLeaves(ctx context.Context, chainID tableland.ChainID) error {


This is where the snapshotting of the rows of each table happen. Here's how it works:

We fetch all tables for a given chain

For each table, we fetch all rows raw bytes. Each row is hashed.

The hashed rows for each table are concanated together into a single BLOB ([]byte)

An entry for that table is added in a new table called system_tree_leaves

The logic here has some implications. Every time we call CalculateTreeLeaves we scan all tables. Ideally we would scan only the tables that have been changed since last call of CalculateTreeLeaves. @jsign proposed an optimization back ago to add the block a table was last touched in the registry table.

Also, this implementation excludes system tables. That means we are losing the capability to compare a full database state. Maybe only for those tables, we can use the current state hash solution.

brunocalza · 2023-02-08T16:52:27Z

pkg/eventprocessor/impl/executor/impl/blockscope.go

+			return fmt.Errorf("table row scan: %s", err)
+		}
+
+		rowHash := sha3.New256()


We're hashing the entire row here using. There's not a real need for that at this point. I did that so all rows have the same size, and the column leaves of the table system_tree_leaves would have a predictable length. If we don't hash it, we would have to add a byte to indicate where one row ends and another starts, which is fine. Also, the hashing of rows has some consequences on the storage size. For small rows (less than the size of the hash) we would be increasing storage more than necessary, but for big rows that would be a gain.

Note: All leaves are hashed again using a different hash when building the Merkle Tree. This means that the final encoding of a row really is keccak256(sha3(row)), in other words, keccak256(sha3(row)) is the real leaf of the tree, and sha3(row) is what's stored.

The sha3 was quite arbitrary. Can change to another hash if it makes more sense.

Makes sense. As a comment, since the leaf is later keccak-ed, we don't strictly need a cryptographic hash function like sha3, so if that is changed in the future it can be switched to something faster.

oh right, I always go for sha and forget about if the cryptographic properties are needed. good callout. any suggestions on standard go implementations? maybe crc32?

fnv is pretty fast and supported in stdlib. It can be an option.

brunocalza · 2023-02-08T17:13:30Z

pkg/merkletree/tree_test.go

+	})
+}
+
+func TestProperties(t *testing.T) {


Experimenting with the testing/quick library to test properties of the tree and catch problems. It was quite good actually, found 2 bugs in my first implementation.

brunocalza · 2023-02-08T17:14:43Z

pkg/merkletree/tree_test.go

+		},
+	}
+
+	t.Run("tree holds merkle tree property", func(t *testing.T) {


this tests if the tree holds the merkle tree property: the hashing of the leaves leads to the merkle root

brunocalza · 2023-02-08T17:15:39Z

pkg/merkletree/tree_test.go

+		}
+	})
+
+	t.Run("leaves are always sorted", func(t *testing.T) {


This test that we did not messed up with the order along the way

brunocalza · 2023-02-08T17:16:00Z

pkg/merkletree/tree_test.go

+		}
+	})
+
+	t.Run("height of the tree is correct", func(t *testing.T) {


This tests that the height property of the tree

brunocalza · 2023-02-08T17:16:46Z

pkg/merkletree/tree_test.go

+		}
+	})
+
+	t.Run("every leaf proof is correctly verifiable", func(t *testing.T) {


This tests that every leaf can be correctly verified by getting the proof of each one and verifying

jsign

LGTM!

I left some comments that I think you might want to take a look at.

.github/workflows/test.yml

pkg/eventprocessor/impl/eventprocessor.go

pkg/eventprocessor/impl/executor/impl/blockscope.go

pkg/merkletree/tree.go

jsign · 2023-02-09T13:54:08Z

pkg/merkletree/tree.go

+		}
+		leaf, parent = parent, parent.parent
+	}
+	return proof


Nit: may worth adding to the method comment that the provided leaf in the parameter, isn't part of the proof. That's fine and normal, since technically speaking that is known to the client.

Just to say that the proof isn't "self-contained" (i.e: you need something extra apart from the proof to verify)

pkg/merkletree/tree.go

jsign · 2023-02-09T14:00:49Z

pkg/merkletree/tree_test.go

+	})
+}
+
+func TestProperties(t *testing.T) {


jsign · 2023-02-09T14:03:42Z

pkg/eventprocessor/impl/executor/impl/blockscope.go

+			rowHash.Write(col)
+		}
+
+		leaves = append(leaves, rowHash.Sum(nil)...)


It maybe worth deduping here? The hash will be the same if two rows have the same column values. That would make the underlying tree have duplicate leaves next to each other and make the tree bigger unnecessarily.

joewagner · 2023-02-09T19:28:34Z

pkg/eventprocessor/impl/executor/impl/blockscope.go

+
+		rowHash := sha3.New256()
+		for _, col := range rawBuffer {
+			rowHash.Write(col)


After the chat in the code walk through in regard to column ordering, I found this forum post. I'm not sure of when or how it would be possible, but one of the answering posts suggests that select * from does not guarantee column order. If that is the case, would we need to amend the above query, or sort the columns in the query results?

From what I understood there it's that it does not follow any standard behavior, but it has a deterministic behavior:

it is the current habit of choice of SQLite specifically to return the columns in the order they are created in the internal structure, I assume which is simply because it happens to be the order in which they are parsed, which happens to be the order in which they appear in the CREATE statement

The problem is that they can change that behavior in a future version (because it's not standard) and also ALTER tables can mess up things (we don't support ALTER TABLE, at least yet).

We could sort it but that implies that the client when producing the leaf for verification, would have to sort it too. Maybe that's ok? I think so.

We could sort it but that implies that the client when producing the leaf for verification, would have to sort it too. Maybe that's ok? I think so.

The client sorting seems ok to me too. Either way as you mention this isn't a problem at the moment, but maybe something to consider in the future.

Signed-off-by: Bruno Calza <[email protected]>

Signed-off-by: Bruno Calza <[email protected]> Signed-off-by: Bruno Calza <[email protected]>

Signed-off-by: Bruno Calza <[email protected]>

brunocalza force-pushed the bcalza/merkle branch from e416356 to 18d5269 Compare February 7, 2023 17:08

brunocalza commented Feb 8, 2023

View reviewed changes

brunocalza changed the title ~~[wip] Adds a Merkle Tree implementation~~ [wip] Adds a Merkle Tree library and "snapshotting" of tables' rows Feb 8, 2023

jsign previously approved these changes Feb 9, 2023

View reviewed changes

joewagner reviewed Feb 9, 2023

View reviewed changes

brunocalza dismissed jsign’s stale review via 48b51c5 February 14, 2023 13:21

brunocalza force-pushed the bcalza/merkle branch 2 times, most recently from ccb0412 to 1950c27 Compare February 14, 2023 13:28

brunocalza force-pushed the bcalza/merkle branch from aaf4df1 to 347f5bb Compare February 24, 2023 17:57

brunocalza added 6 commits March 7, 2023 15:08

Adds a Merkle Tree implementation

cf9323b

Signed-off-by: Bruno Calza <[email protected]>

Implements the snapshot of table rows into another table [stagingdeploy]

1892a7e

Signed-off-by: Bruno Calza <[email protected]>

removes metric collection

79a4cbf

Signed-off-by: Bruno Calza <[email protected]>

Fixes after review [stagingdeploy]

648d1a3

Signed-off-by: Bruno Calza <[email protected]>

Implements a merkle tree root publisher [stagingdeploy]

5415202

Signed-off-by: Bruno Calza <[email protected]> Signed-off-by: Bruno Calza <[email protected]>

Adds root registry

d2c4818

Signed-off-by: Bruno Calza <[email protected]>

brunocalza force-pushed the bcalza/merkle branch from 6f4990d to d2c4818 Compare March 9, 2023 14:01

Make use of buffer

ff40346

Signed-off-by: Bruno Calza <[email protected]>

brunocalza closed this Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wip] Adds a Merkle Tree library and "snapshotting" of tables' rows #455

[wip] Adds a Merkle Tree library and "snapshotting" of tables' rows #455

brunocalza commented Feb 6, 2023 •

edited

Loading

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

jsign Feb 9, 2023

brunocalza Feb 9, 2023

jsign Feb 9, 2023

brunocalza Feb 8, 2023

jsign Feb 9, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

brunocalza Feb 8, 2023

jsign left a comment

jsign Feb 9, 2023

jsign Feb 9, 2023

jsign Feb 9, 2023

joewagner Feb 9, 2023

brunocalza Feb 9, 2023

joewagner Feb 9, 2023

[wip] Adds a Merkle Tree library and "snapshotting" of tables' rows #455

[wip] Adds a Merkle Tree library and "snapshotting" of tables' rows #455

Conversation

brunocalza commented Feb 6, 2023 • edited Loading

Summary

Context

Implementation overview

Todos

Notes

Implementation details and review orientation

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunocalza commented Feb 6, 2023 •

edited

Loading