Initial MPT implementation (2.x) #990

fyrchik · 2020-05-27T04:47:57Z

This is initial MPT implementation, containing most of the necessary methods.
Storage key transformation to NEO format will be done during integration.

codecov · 2020-05-27T04:49:51Z

Codecov Report

Merging #990 into neox-2.x will increase coverage by 0.97%.
The diff coverage is 94.80%.

@@             Coverage Diff              @@
##           neox-2.x     #990      +/-   ##
============================================
+ Coverage     67.97%   68.94%   +0.97%     
============================================
  Files           145      153       +8     
  Lines         14320    14859     +539     
============================================
+ Hits           9734    10245     +511     
- Misses         4126     4143      +17     
- Partials        460      471      +11

Impacted Files	Coverage Δ
pkg/core/storage/store.go	`34.61% <ø> (ø)`
pkg/core/mpt/hash.go	`83.78% <83.78%> (ø)`
pkg/core/mpt/branch.go	`89.47% <89.47%> (ø)`
pkg/core/mpt/trie.go	`93.24% <93.24%> (ø)`
pkg/core/mpt/leaf.go	`96.42% <96.42%> (ø)`
pkg/core/mpt/extension.go	`97.43% <97.43%> (ø)`
pkg/core/mpt/helpers.go	`100.00% <100.00%> (ø)`
pkg/core/mpt/node.go	`100.00% <100.00%> (ø)`
pkg/core/mpt/proof.go	`100.00% <100.00%> (ø)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a783b6...314430b. Read the comment docs.

roman-khimov

We absolutely need some tests that compare root hashes with ones from C# implementation (there are some UTs there that have samples of these)

pkg/core/mpt/branch.go

pkg/core/mpt/node_test.go

pkg/core/mpt/branch.go

pkg/core/mpt/node.go

pkg/core/mpt/trie.go

roman-khimov · 2020-05-30T16:14:56Z

pkg/core/mpt/node.go

+
+// toBytes is a helper for serializing node.
+func toBytes(n Node) []byte {
+	buf := io.NewBufBinWriter()


It should be OK for a start, but this probably will be allocating like crazy.

Yes and we can't use a single buffer, because this conversion is performed recursively, e.g. on Branch nodes. I decided not to use any kind of pools it for now, to keep this simple.

pkg/core/mpt/trie.go

roman-khimov

I think I'm done with the first four (for #964) and they're fine in general, just some minor tweaks are needed. I'll take a look at the rest of them a bit later (for #998, #1004, #1005). Maybe we'll also merge them separately (depends on their state of course).

pkg/core/mpt/trie.go

roman-khimov · 2020-05-30T17:27:01Z

pkg/core/mpt/trie.go

+	}
+
+	b := NewBranchNode()
+	r, err := t.putWithPath(b.Children[path[0]], path[1:], v)


You're a bit inconsistent here, putBranch uses splitPath in similar situation and given that you're using path[0] down below again, probably it's better to do so here too.

That's because we compare length of the path with 0 earlier.

pkg/core/mpt/trie.go

roman-khimov · 2020-05-30T19:08:13Z

pkg/core/mpt/proof.go

+}
+
+// VerifyProof verifies that path indeed belongs to a MPT with the specified root hash.
+func VerifyProof(rh util.Uint256, path []byte, proofs [][]byte) ([]byte, bool) {


It's an external interface, it should accept simple path and convert it to nibbles. BTW, that's the way VerifyProof() in C# works.

To be fair, I don't really like its interface with []byte returned, but maybe it's ok for now. I expect a verification of KV pair, at the moment it looks more like a GetVerifiedContentForTheKey. But probably it's not a problem as of yet.

pkg/core/mpt/proof.go

pkg/core/mpt/node.go

roman-khimov · 2020-05-30T19:24:08Z

pkg/core/mpt/node_test.go

@@ -74,6 +90,38 @@ func TestNode_Serializable(t *testing.T) {
 	})
 }

+func TestInvalidJSON(t *testing.T) {


Encoding/decoding and invalid JSON tests are all nice, but we also need some known-good JSON strings to decode. The ones generated by encoding are not appropriate as we need to follow a specific encoding standard set by C# implementation and internal enc/dec doesn't prove we're compliant with that.

Are there any that can be taken from C# tests or generated by the respective node?

Found only this https://github.com/neo-project/neo/blob/neox-2.x/neo.UnitTests/UT_MPTTrie.cs#L198
Added it.

pkg/core/mpt/trie.go

roman-khimov · 2020-05-31T08:45:03Z

В письме от воскресенье, 31 мая 2020 г. 11:28:07 MSK пользователь fyrchik написал:

This is a separate function, because we _must_ perform `Flush` every block, however `Collapse` can be called once per N blocks if needed. Not a big deal now probably, but can be useful e.g. during restore.

At the very minimum this should be explicitly documented.

roman-khimov · 2020-05-31T08:47:30Z

В письме от воскресенье, 31 мая 2020 г. 11:29:08 MSK пользователь fyrchik написал:

What about a `bool` flag to `Collapse` specifying if everything should be flushed?

I don't really like such flags, maybe a separate function will do better or maybe we can just document it for now. This function is obviously dangerous from an API point of view, probably that's what bugs me most, but at the same time there won't be a lot of users of this API and if we're to be careful with clearly written rules then maybe we're OK.

roman-khimov

Things obviously missing:

key grouping
p2p processing

I think we can concentrate here on base MPT structure and merge p2p/storage/consensus changes in other PRs.

pkg/core/mpt/trie.go

roman-khimov · 2020-05-31T09:12:28Z

pkg/core/dao/dao.go

@@ -86,7 +90,9 @@ func (dao *Simple) GetBatch() *storage.MemBatch {
 // GetWrapped returns new DAO instance with another layer of wrapped
 // MemCachedStore around the current DAO Store.
 func (dao *Simple) GetWrapped() DAO {
-	return NewSimple(dao.Store)
+	d := NewSimple(dao.Store)
+	d.MPT = dao.MPT


An interesting part is that this should work for Cached because of its storage item cache. Though it probably won't for the master branch.

roman-khimov · 2020-05-31T09:16:56Z

pkg/network/server.go

@@ -507,6 +508,8 @@ func (s *Server) handleGetDataCmd(p Peer, inv *payload.Inventory) error {
 			if err == nil {
 				msg = s.MkMsg(CMDBlock, b)
 			}
+		case payload.StateRootType:
+			return nil // TODO


Not there yet.

pkg/consensus/consensus.go

MPT is a trie with a branching factor = 16, i.e. it consists of sequences in 16-element alphabet.

Because there is no distinct type field in JSONized nodes, distinction is made via payload itself, thus all unmarshaling is done via NodeObject.

Because trie size is rather big, it can't be stored in memory. Thus some form of caching should also be implemented. To avoid marshaling/unmarshaling of items which are close to root and are used very frequenly we can save them across the persists. This commit implements pruning items at the specified depth, replacing them by hash nodes.

fyrchik mentioned this pull request May 27, 2020

Initial MPT implementation (2.x) #985

Closed

roman-khimov reviewed May 27, 2020

View reviewed changes

roman-khimov reviewed May 30, 2020

View reviewed changes

pkg/core/mpt/trie.go Show resolved Hide resolved

roman-khimov requested changes May 30, 2020

View reviewed changes

roman-khimov requested changes May 31, 2020

View reviewed changes

fyrchik added 5 commits June 1, 2020 18:14

mpt: implement MPT trie

861a163

MPT is a trie with a branching factor = 16, i.e. it consists of sequences in 16-element alphabet.

mpt: implement MPT proof Get and Verify

31d9aed

mpt: implement JSON marshaling/unmarshaling

9c47837

Because there is no distinct type field in JSONized nodes, distinction is made via payload itself, thus all unmarshaling is done via NodeObject.

mpt: implement NEO storage key conversion

314430b

roman-khimov approved these changes Jun 1, 2020

View reviewed changes

roman-khimov merged commit 2f90a06 into neox-2.x Jun 1, 2020

roman-khimov deleted the feature/mpt branch June 1, 2020 15:58

roman-khimov mentioned this pull request Jun 1, 2020

Implement MPT for neox-2.x #964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial MPT implementation (2.x) #990

Initial MPT implementation (2.x) #990

fyrchik commented May 27, 2020

codecov bot commented May 27, 2020 •

edited

Loading

roman-khimov left a comment

roman-khimov May 30, 2020

fyrchik May 31, 2020

roman-khimov left a comment

roman-khimov May 30, 2020

fyrchik May 31, 2020

roman-khimov May 30, 2020

roman-khimov May 30, 2020

roman-khimov May 30, 2020

roman-khimov Jun 1, 2020

fyrchik Jun 1, 2020

roman-khimov commented May 31, 2020 via email

roman-khimov commented May 31, 2020 via email

roman-khimov left a comment

roman-khimov May 31, 2020

roman-khimov May 31, 2020

Initial MPT implementation (2.x) #990

Initial MPT implementation (2.x) #990

Conversation

fyrchik commented May 27, 2020

codecov bot commented May 27, 2020 • edited Loading

Codecov Report

roman-khimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roman-khimov 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

Choose a reason for hiding this comment

roman-khimov commented May 31, 2020 via email

roman-khimov commented May 31, 2020 via email

roman-khimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 27, 2020 •

edited

Loading