Initial SMT store type #12

tzdybal · 2021-01-13T12:23:13Z

Description

This PR introduces SMT store layer. We need to wait for conclusion of ADR-040 before moving forward.

Current state implements most interfaces (except Queryable), and provides compatibility layer for Tendermint ProofOp's.

SMT (Sparse Merkle Tree) is intended to replace IAVL. New type implements same interfaces as iavl.Store.

Sparse Merkle Tree does not support iteration over keys in order. To provide drop-in replacement for IAVL, Iterator and ReverseIterator has to be implemented. SMT Store implementation use the underlying KV store to: - maintain a list of keys (under a prefix) - iterate over a keys Values are stored only in SMT.

codecov-io · 2021-01-14T23:52:33Z

Codecov Report

Merging #12 (19e07ee) into master (85b5fa9) will increase coverage by 0.01%.
The diff coverage is 65.82%.

@@            Coverage Diff             @@
##           master      #12      +/-   ##
==========================================
+ Coverage   61.91%   61.92%   +0.01%     
==========================================
  Files         609      611       +2     
  Lines       35102    35178      +76     
==========================================
+ Hits        21732    21784      +52     
- Misses      11086    11105      +19     
- Partials     2284     2289       +5

Impacted Files	Coverage Δ
store/types/store.go	`57.14% <0.00%> (-2.86%)`	⬇️
store/smt/store.go	`45.23% <45.23%> (ø)`
store/smt/iterator.go	`94.28% <94.28%> (ø)`
x/ibc/core/02-client/module.go	`25.00% <0.00%> (-8.34%)`	⬇️
.../light-clients/07-tendermint/types/misbehaviour.go	`73.01% <0.00%> (ø)`
...light-clients/06-solomachine/types/misbehaviour.go	`96.15% <0.00%> (ø)`
simapp/app.go	`84.79% <0.00%> (+0.77%)`	⬆️
...ght-clients/07-tendermint/types/consensus_state.go	`100.00% <0.00%> (+11.76%)`	⬆️

tzdybal · 2021-01-18T09:11:40Z

store/smt/iterator_test.go

+)
+
+func TestIteration(t *testing.T) {
+	pairs := []struct{ key, val []byte }{


Table driven tests

tzdybal · 2021-01-18T09:14:13Z

store/smt/store.go

+	val, err := s.tree.Get(key)
+	// TODO(tzdybal): how to handle this err?
+	if err != nil {
+		return nil


SMT propagates errors from underlying storage, so we should panic (and probably log before)

liamsi

Great start to get the SMT to adhere to the required interfaces 👍🏼

liamsi · 2021-01-28T10:31:30Z

store/smt/iterator.go

+var (
+	indexPrefix = []byte("smt-ordering-idx-")
+	afterIndex  = []byte("smt-ordering-idx.") // '.' is next after '-' in ASCII
+)


This isn't really self-explanatory. Add a brief explanation (even if it is unexported).

Nit: I'd prefer using string constants instead and wrap them with []byte() where they are used.

store/smt/proof.go

liamsi · 2021-01-28T10:53:49Z

We need to wait for conclusion of ADR-040 before moving forward.

Does that mean we should hold off from merging as well?

tzdybal · 2021-01-28T11:06:11Z

Does that mean we should hold off from merging as well?

Current state of implementation is a complete, usable store, with support for proofs. We can merge it, and then continue work, or we can just leave it as is, to be a starting point for further work.

robert-zaremba · 2021-01-28T19:25:01Z

store/smt/proof_test.go

+	r, err := storeProofOp.Run([][]byte{value})
+	assert.NoError(t, err)
+	assert.NotEmpty(t, r)
+	assert.Equal(t, root, r[0])


instead of adding t to every assert or require I usually shadow the package names:

assert := assert.New(t) require := require.New(t) // and then: assert.Equal(root, r[0]) // no need to pass `t`

liamsi

I'm OK with merging this in its current form into our fork.

I would certainly hold-off from replacing iavl with the smt untill we have a MVP without fraud-proofs. Maybe, we should even implement fraud proofs using iavl first.

robert-zaremba · 2021-02-03T18:38:21Z

I think the plan was to make a new PR and target it to cosmos-sdk/store/ll-smt and then review.

liamsi · 2021-02-03T21:11:52Z

That's right! I actually forgot. But we could do both. I'll leave it up to @tzdybal to decide.

robert-zaremba · 2021-02-03T21:31:31Z

That's right! I actually forgot. But we could do both. I'll leave it up to @tzdybal to decide.

What's the advantage of doing both? Wouldn't it complicate?

liamsi · 2021-02-03T21:43:51Z

It would only complicate things if we'd have to versions that we try to keep in sync, one in that branch and one in our fork.

tzdybal · 2021-02-03T23:37:44Z

Drafted cosmos#8507.

adlerjohn · 2021-08-31T17:06:40Z

What's the status on implementing the SMT here @tzdybal?

adlerjohn · 2022-04-12T14:24:14Z

Converting to draft but keeping around since we may need this branch soon

adlerjohn · 2022-04-13T09:44:58Z

cc @Manav-Aggarwal @jbowen93

This code is copied from #12 with following changes: * updated smt to v0.2.0 * added CacheWrapWithListeners to fully implement KVStore interface

evan-forbes · 2022-12-19T13:25:02Z

can we close this?

evan-forbes · 2022-12-19T18:25:14Z

closing as I think this is outdated

pls reopen or move if not slightly_smiling_face

tzdybal added 2 commits January 13, 2021 13:11

Initial SMT store type

1945a92

SMT (Sparse Merkle Tree) is intended to replace IAVL. New type implements same interfaces as iavl.Store.

tzdybal added 2 commits January 15, 2021 14:08

Migrate to smt v0.1.1

f8fd015

Extra test for SMT iterator

19e07ee

tzdybal commented Jan 18, 2021

View reviewed changes

store/smt/iterator_test.go

)

func TestIteration(t *testing.T) {

pairs := []struct{ key, val []byte }{

Copy link

Member Author

tzdybal Jan 18, 2021

Choose a reason for hiding this comment

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

Table driven tests

tzdybal commented Jan 18, 2021

View reviewed changes

tac0turtle and others added 8 commits January 21, 2021 09:45

ci: fix liveness test / rbuilder script (#15)

dde3c29

CommitStore implementation for SMT store

e89cbb5

Merge remote-tracking branch 'origin/master' into tzdybal/add_smt

8d79be8

Use interface instead of concrete type

40ce86d

Add telemetry to SMT store

04dc9ee

SMT: version->root mapping, cleanup

54e3909

SMT proofs - initial code

f1ac2cc

Tests for SMT store ProofOp implementation

122a1bc

tzdybal marked this pull request as ready for review January 28, 2021 09:37

tzdybal requested review from adlerjohn, evan-forbes, liamsi, tac0turtle and musalbas January 28, 2021 09:37

tzdybal force-pushed the tzdybal/add_smt branch 2 times, most recently from 3291c1f to bab3f84 Compare January 28, 2021 09:48

Merge remote-tracking branch 'origin/master' into tzdybal/add_smt

9eb31c8

tzdybal force-pushed the tzdybal/add_smt branch from bab3f84 to 9eb31c8 Compare January 28, 2021 10:07

liamsi reviewed Jan 28, 2021

View reviewed changes

Fix linter errors

6171e35

tzdybal force-pushed the tzdybal/add_smt branch from e79dc4a to 6171e35 Compare January 28, 2021 11:11

robert-zaremba reviewed Jan 28, 2021

View reviewed changes

liamsi approved these changes Feb 3, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

github-actions bot added the Stale label Mar 21, 2021

liamsi removed the Stale label Mar 23, 2021

adlerjohn marked this pull request as draft April 12, 2022 14:23

adlerjohn assigned tzdybal Apr 12, 2022

musalbas mentioned this pull request Sep 16, 2022

Don't recompute the root on every update; only on commit celestiaorg/smt#72

Open

tzdybal added a commit that referenced this pull request Sep 16, 2022

feat: initial import of SMT code from #12

c3e22e8

This code is copied from #12 with following changes: * updated smt to v0.2.0 * added CacheWrapWithListeners to fully implement KVStore interface

tzdybal mentioned this pull request Sep 16, 2022

Initial import of SMT store (v1 interface) on top of v0.45.8 #278

Closed

evan-forbes closed this Dec 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial SMT store type #12

Initial SMT store type #12

tzdybal commented Jan 13, 2021 •

edited

Loading

codecov-io commented Jan 14, 2021 •

edited

Loading

tzdybal Jan 18, 2021

tzdybal Jan 18, 2021

liamsi left a comment

liamsi Jan 28, 2021

liamsi commented Jan 28, 2021

tzdybal commented Jan 28, 2021

robert-zaremba Jan 28, 2021

tzdybal Jan 29, 2021

liamsi left a comment

robert-zaremba commented Feb 3, 2021

liamsi commented Feb 3, 2021

robert-zaremba commented Feb 3, 2021

liamsi commented Feb 3, 2021

tzdybal commented Feb 3, 2021

This comment has been minimized.

adlerjohn commented Aug 31, 2021

adlerjohn commented Apr 12, 2022

adlerjohn commented Apr 13, 2022

evan-forbes commented Dec 19, 2022

evan-forbes commented Dec 19, 2022

Initial SMT store type #12

Initial SMT store type #12

Conversation

tzdybal commented Jan 13, 2021 • edited Loading

Description

codecov-io commented Jan 14, 2021 • edited Loading

Codecov Report

tzdybal Jan 18, 2021

Choose a reason for hiding this comment

tzdybal Jan 18, 2021

Choose a reason for hiding this comment

liamsi left a comment

Choose a reason for hiding this comment

liamsi Jan 28, 2021

Choose a reason for hiding this comment

liamsi commented Jan 28, 2021

tzdybal commented Jan 28, 2021

robert-zaremba Jan 28, 2021

Choose a reason for hiding this comment

tzdybal Jan 29, 2021

Choose a reason for hiding this comment

liamsi left a comment

Choose a reason for hiding this comment

robert-zaremba commented Feb 3, 2021

liamsi commented Feb 3, 2021

robert-zaremba commented Feb 3, 2021

liamsi commented Feb 3, 2021

tzdybal commented Feb 3, 2021

This comment has been minimized.

adlerjohn commented Aug 31, 2021

adlerjohn commented Apr 12, 2022

adlerjohn commented Apr 13, 2022

evan-forbes commented Dec 19, 2022

evan-forbes commented Dec 19, 2022

tzdybal commented Jan 13, 2021 •

edited

Loading

codecov-io commented Jan 14, 2021 •

edited

Loading