Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New merkle tree that is compatible with ICS23 / IBC proofs #279

Merged
merged 12 commits into from
Oct 13, 2022

Conversation

batconjurer
Copy link
Member

@batconjurer batconjurer commented Aug 4, 2022

Closes #92

depends on #351

We are using a new implementation of merkle trees from our sparse-merkle-tree fork (this is also worth reviewing). The merkle tree uses a fixed key space (parameterized by a const set at compile time), but does not give any particular mapping from the app side key space into the tree's key space. Before, this mapping was given by a hash function. Now it is up to the application side to make this choice.

For the IBC subtree, our keys are strings, so the function we use is utf8 encodings which are right padded with 0xFF (an invalid utf8 character) to fill out a 120 byte array. (This 120 can be changed later). Then every byte is incremented by one, modulo 256. This is an order preserving, collision resistant mapping. It's not randomizing across the codomain however, so there is still reason to switch back to a tradition SMT once possible.

Our other subtrees use a Sha256 hash to map into their respective keyspaces. We have ensured that inclusion proofs works for all trees. Futhermore, non-inclusion proofs have been tested and verified for the IBC subtree.

@batconjurer batconjurer marked this pull request as draft August 4, 2022 12:15
@batconjurer batconjurer marked this pull request as ready for review August 6, 2022 01:19
@batconjurer
Copy link
Member Author

pls update wasm

tzemanovic
tzemanovic previously approved these changes Aug 8, 2022
Copy link
Member

@tzemanovic tzemanovic left a comment

Choose a reason for hiding this comment

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

LGTM! (it looks like it needs fmt, we can base this on #250 to get a working CI on this)

yito88
yito88 previously approved these changes Aug 9, 2022
Copy link
Member

@yito88 yito88 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!
The IBC packet timeout works well with this merkle tree's proof (for checking the receipt absence).

@batconjurer
Copy link
Member Author

pls update wasm

@tzemanovic tzemanovic force-pushed the bat/new-merkle-tree branch 3 times, most recently from 6752ebb to e617cf9 Compare August 11, 2022 14:09
@tzemanovic
Copy link
Member

for some reason the CI is not running on the last commit after rebase

@tzemanovic tzemanovic mentioned this pull request Aug 19, 2022
35 tasks
@yito88 yito88 mentioned this pull request Aug 21, 2022
tzemanovic added a commit that referenced this pull request Aug 29, 2022
* bat/new-merkle-tree:
  changelog: add #279
  apps/dev-deps: remove unused cargo-watch
  update checksums wasm
  [fix]: Fixed a small bug in existence proofs
  [fix]: Updated Cargo.lock files
  [feat]: Change the ibc-leaf-spec to be the same for both existence and non-existence proofs
  [fix]: Updating the .lock file
  [feat]: Downstreaming changes from arse-merkle-tree. That library needed professionalization and I made a change to minimize heap allocations and vector copies
  [fix]: Fixed map of ibc keys into fixed keyspace and fixed tests
  [fix]: Fixed new merkle trees to be compatible with existing unit tests (instead of changing the tests)
  [fix]: Fixed the proof specs. Tests now passing
  [feat]: Integrated in new merkle tree for ibc. Proofs are not correct yet
@tzemanovic tzemanovic mentioned this pull request Aug 29, 2022
5 tasks
@yito88 yito88 mentioned this pull request Oct 5, 2022
29 tasks
@tzemanovic tzemanovic dismissed stale reviews from yito88 and themself via 8a9fc4c October 6, 2022 16:10
@juped juped merged commit c5e27ed into main Oct 13, 2022
@juped juped deleted the bat/new-merkle-tree branch October 13, 2022 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-existence proof verification failed
4 participants