aergo icon indicating copy to clipboard operation
aergo copied to clipboard

add new function Walk() to trie

Open p4u opened this issue 3 years ago • 12 comments

Walk allows iteration over all values stored into the SMT.

This is a useful method for exporting the tree (and importing afterwards) and also for search specific values.

Signed-off-by: p4u [email protected]

p4u avatar Oct 29 '20 10:10 p4u

This is not 100% correct... Sometimes the test will fail and I don't know why (yet).

Maybe someone with more knowledge on this SMT implementation can find the problem.

p4u avatar Oct 30 '20 15:10 p4u

So with this last implementation, go test -timeout 30s . -run ^TestTrieWalk$ -count=1 works always but -count=10 crashes... I'm not sure if this is something from my changes or just anything wrong with the trie_test.go

p4u avatar Oct 30 '20 15:10 p4u

Thank you @p4u for your input. Have you checked this repository which does the export of an Aergo state snapshot ? https://github.com/aergoio/state-tools

paouvrard avatar Nov 02 '20 01:11 paouvrard

I did not know about the existence of that repository, I'll take a look.

However, from a SMT API consumer perspective with the experience of using several goLang Merkletree implementations for blockchain development, I think the following methods are missing:

  1. A Walk() as proposed here
  2. A Snapshot(root) in order to make an immutable tree (will not change over time)
  3. Get and Walk should allow specifying a root, something like smt.Get(key, root []byte)
  4. A Count(root) method in order to get the number of leafs
  5. A way to export and import the tree in order to generate the same exact Root hash

Does it makes sense to you? To this end, some functions from the state-tools repository might be added to the SMT trie package. I think it would help on the adoption of this trie implementation by other open source blockchain projects.

This is our interface and our current implementations (where I want to add Aergo SMT): https://gitlab.com/vocdoni/go-dvote/-/tree/master/statedb

p4u avatar Nov 02 '20 08:11 p4u

Definitely makes sense.

I know some projects used the same trie but usually adapt it to their needs. So if there is consensus on an interface/usage that is useful to many, I'm happy to add features. We might have to refactor outside of the Aergo client code. The interface you linked mentions Iterate, is that the same as Walk but for iterating from a subtree?

There is also the new approach https://github.com/ledgerwatch/turbo-geth which stores the leaves directly instead of querying from the root in logN, and re-constructs the root when a leaf is updated. It brings performance benefits but changes how we think about the trie (need to keep track of what is the current version of the trie).

paouvrard avatar Nov 03 '20 02:11 paouvrard

Hey @paouvrard I have been out for some days but now I'm back. Thank you for your review, I am going to incorporate the changes.

p4u avatar Dec 10 '20 18:12 p4u

I applied your suggestions and now go test -timeout 30s . -run=^TestTrieWalk$ -count=100 works fine, thank you!

I added another function named GetWithRoot in order to Get a value for a specific Root. And in addition I added the root as a parameter to Walk() to the caller can choose on which Root he wants to walk.

I keep everything on the same PR because in my local repository I did not split it, I hope that's not a problem.

Looking forward for your review :)

p4u avatar Dec 10 '20 19:12 p4u

Damn it, go test -timeout 30s . -count=100 fails... I'll look into it.

p4u avatar Dec 10 '20 20:12 p4u

Fixed! Now go test -timeout 120s . -count=100 works for me.

p4u avatar Dec 11 '20 08:12 p4u

The last version solves concurrency problems and it is data race safe. Tested with go test -timeout 120s . -run ^TestTrieWalk$ -count=100 -race

p4u avatar Dec 12 '20 18:12 p4u

If this PR gets finally merged, next step could be to add a String() function, Export() and Import(), which IMO would make the API more powerful. Something like:

func (t *trie) String() string {
	buf := bytes.Buffer{}
	t.Walk(nil, func(k, v []byte) int32 {
		buf.WriteString(fmt.Sprintf("%x => %x\n", k, v))
		return 0
	})
	return buf.String()
}

p4u avatar Dec 12 '20 19:12 p4u

So I see there is probably no interest on having this new methods on the SMT. That's fine. I forked the code and applied some changes (including a new abstraction layer). If anyone is interested, the code is here: https://github.com/p4u/asmt

p4u avatar Mar 27 '21 17:03 p4u

The aergoio/SMT repo appears to be better for this

The aergoio/state-tools is also related

kroggen avatar May 04 '23 17:05 kroggen