chore(trie): proof code refactoring and fixes #2604

qdm12 · 2022-06-15T15:20:01Z

Changes

New package lib/trie/proof containing:
- lib/trie/proof.go (refactored)
- internal/trie/record files (then removed)
- lib/trie/database.go (moved since it had nothing to do with database)
Proof Verify function:
- ⚠️ returns an error when root is not found in proof encoded nodes
- ⚠️ fix buildTrie to detect root node for roots with encoding smaller than 32 bytes
- ⚠️ Removes child with no matching hash from proof trie
- ⚠️ Only decode root node, and then lazy decode nodes. This is to allow for 'values' (not node encodings) to be together node encodings with v1 generated proofs.
- Only verify for a single key and value instead of pairs (no usage at all)
- Produce ordered (from root to leaf) proof encoded nodes
- Returns only an error and no boolean (was unneeded before)
Proof Generate function:
- ⚠️ returns an error when one key is not found in trie when generating proof
- Remove internal/trie/recorder code (now unneeded)
- Generate encoded nodes ordered from root to leaf (to reduce verify hash computations)
- Do not add inlined nodes
Add full coverage unit tests and remove older tests
Minor changes
- Better error wrapping
- Database arguments uses a smaller locally defined Database interface in proof and trie packages
- rename proofEncodedNodes to encodedProofNodes
- rename decProofs to encodedProofNodes

Tests

go test -count 1 ./internal/trie/... ./lib/trie/...

Issues

One more step towards #2418

Primary Reviewer

@EclesioMeloJunior

dot/state/storage.go

lib/trie/proof/generate.go

qdm12 · 2022-06-15T19:57:13Z

lib/trie/proof/verify.go

+// Note this is exported because it is imported and used by:
+// https://github.com/ComposableFi/ibc-go/blob/6d62edaa1a3cb0768c430dab81bb195e0b0c72db/modules/light-clients/11-beefy/types/client_state.go#L78


Notify them of the fixes/changes once this gets merged

lib/trie/proof/verify.go

lib/trie/proof/verify_test.go

lib/trie/proof/generate_test.go

codecov · 2022-06-16T15:52:39Z

Codecov Report

Merging #2604 (25b4e57) into development (e7749cf) will increase coverage by 0.15%.
The diff coverage is 93.51%.

@@               Coverage Diff               @@
##           development    #2604      +/-   ##
===============================================
+ Coverage        61.97%   62.12%   +0.15%     
===============================================
  Files              215      214       -1     
  Lines            28453    28556     +103     
===============================================
+ Hits             17634    17741     +107     
- Misses            9067     9068       +1     
+ Partials          1752     1747       -5

Flag	Coverage Δ
unit-tests	`62.12% <93.51%> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dot/state/storage.go	`49.72% <0.00%> (ø)`
internal/trie/node/encode.go	`93.33% <ø> (ø)`
internal/trie/node/node.go	`100.00% <ø> (ø)`
lib/runtime/wasmer/imports.go	`48.49% <75.00%> (+0.09%)`	⬆️
internal/trie/node/decode.go	`91.11% <78.12%> (-4.81%)`	⬇️
lib/trie/proof/generate.go	`92.59% <92.59%> (ø)`
lib/trie/proof/verify.go	`94.54% <94.54%> (ø)`
internal/trie/node/children.go	`100.00% <100.00%> (ø)`
internal/trie/node/header.go	`100.00% <100.00%> (ø)`
internal/trie/node/key.go	`100.00% <100.00%> (ø)`
... and 9 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 d3282f7...25b4e57. Read the comment docs.

seunlanlege · 2022-06-22T08:54:11Z

Only verify for a single key and value instead of pairs (no usage at all)

Hi @qdm12 I'm currently using the trie lib in gossamer to implement a light client in golang here: composableFi/ibc-go

We actually depend on this functionality and wondering if it'll be possible to keep it?

seunlanlege · 2022-06-22T08:55:29Z

Also with these changes, would you say that the lib can be used to verify proofs from paritytech/trie

qdm12 · 2022-06-29T12:56:03Z

Hi @seunlanlege sorry for the delay, I was off the last few days.

Only verify for a single key and value instead of pairs (no usage at all)
We actually depend on this functionality and wondering if it'll be possible to keep it?

I have checked your codebase, you only check a single key-value pair in every of your calls so far, so it won't affect your codebase. The functions you use are still exported as well, although their signature and import path do change a bit, but Gossamer is still in its v0.x.x versions so this is expected.

Also with these changes, would you say that the lib can be used to verify proofs from paritytech/trie

This is a work in progress to be able to verify proofs generated by the v1 trie state.
Together with #2530 and an additional PR adding headers (got a local branch with it already), it should allow to verify proofs at the very least. I'll notify you once it's ready 😉

- `lib/trie` proof files - `internal/trie/record` package

- Only verify for a single key and value - Produce ordered proof encoded nodes from root to leaves - Node deduplication logic removed (now unneeded) - Remove recorder code (now unneeded) - Generate uses a `Database` smaller interface - Better error wrapping - Error when key is not found in trie when generating proof - Generate only generates proof encoded nodes for a single given key

- `buildTrie` returns a `*trie.Trie` - `loadProof` only acts on a parent node pointer - Older `LoadFromProof` and `loadProof` functions removed from `trie/database.go`

…han 32 bytes

- More context to error wrapping - Verify function only returns an error

…ature

- `proofHashToNode` to `merkleValueToNode` - `proofHash` to `merkleValue`

- Load trie only once from database - Move StorageState `GenerateTrieProof` to `lib/trie/proof` package - Update tests

timwu20 · 2022-07-04T19:21:52Z

lib/trie/database.go

 // Load reconstructs the trie from the database from the given root hash.
 // It is used when restarting the node to load the current state trie.
-func (t *Trie) Load(db chaindb.Database, rootHash common.Hash) error {
+func (t *Trie) Load(db Database, rootHash common.Hash) error {


I would like to remove the chaindb dependency completely at some point.

Agreed, although not a priority, but a not-too-complex task either.

Also we should use locally defined interfaces exported so that they get injected (accept interfaces), it should be up to the caller to define a (narrow as possible) interface for a concrete implementation. I guess one step closer now 😉

lib/trie/proof/generate_test.go

I am Quentin Mc Gaw, a software engineer working the Go Polkadot host **Gossamer**. I have been working full time on Gossamer since October 2021, mostly on the state trie and storage. I have also made a [few minor pull requests](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12) to the Polkadot specification repository. I am requesting to join the Fellowship at rank 1. ## Main contributions ### Gossamer - Fix memory leaks - Trie encoding buffer pools usage fixed [#2009](ChainSafe/gossamer#2009) - Fix state map of tries memory leak [#2286](ChainSafe/gossamer#2286) - Fix sync benchmark [#2234](ChainSafe/gossamer#2234) - Trie proof fixes ([#2604](ChainSafe/gossamer#2604), [#2661](ChainSafe/gossamer#2661)) - Fix end to end tests orchestration ([#2470](ChainSafe/gossamer#2470), [#2452](ChainSafe/gossamer#2452), [#2385](ChainSafe/gossamer#2385), [#2370](ChainSafe/gossamer#2370)) - State trie statistics ([#2378](ChainSafe/gossamer#2378), [#2310](ChainSafe/gossamer#2310), [#2272](ChainSafe/gossamer#2272)) - State trie fixes and improvements - Only deep copy nodes when mutation is certain [#2352](ChainSafe/gossamer#2352) and [#2223](ChainSafe/gossamer#2223) - Only deep copy necessary fields of a node [#2384](ChainSafe/gossamer#2384) - Use Merkle values for database keys instead of always hash [#2725](ChainSafe/gossamer#2725) - Opportunistic parallel Merkle value commputing [#2081](ChainSafe/gossamer#2081) - Grandpa capped number of tracked messages ([#2490](ChainSafe/gossamer#2490), [#2485](ChainSafe/gossamer#2485)) - Add pprof HTTP service for profiling [#1991](ChainSafe/gossamer#1991) Ongoing work: - State trie lazy loading and caching - State trie v1 support ([#2736](ChainSafe/gossamer#2736), [#2747](ChainSafe/gossamer#2747), [#2687](ChainSafe/gossamer#2687), [#2686](ChainSafe/gossamer#2686), [#2685](ChainSafe/gossamer#2685), [#2673](ChainSafe/gossamer#2673), [#2611](ChainSafe/gossamer#2611), [#2530](ChainSafe/gossamer#2530)) ### Polkadot specification ➡️ [Pull requests from qdm12](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12)

github-actions · 2022-11-23T18:30:47Z

🎉 This PR is included in version 0.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

qdm12 changed the title ~~Qdm12/trie/proofs refactor~~ chore(trie): proof code refactoring and fixes Jun 15, 2022

qdm12 force-pushed the qdm12/trie/proofs-refactor branch 2 times, most recently from 7b06cf8 to af7791a Compare June 15, 2022 17:29

qdm12 changed the base branch from development to qdm12/trie/split-encodeandhash June 15, 2022 17:30

qdm12 force-pushed the qdm12/trie/proofs-refactor branch 2 times, most recently from 8a738e6 to b897c7c Compare June 15, 2022 19:25

qdm12 commented Jun 15, 2022

View reviewed changes

lib/trie/proof/generate_test.go Outdated Show resolved Hide resolved

qdm12 force-pushed the qdm12/trie/proofs-refactor branch from 2132ac2 to e9bc295 Compare June 15, 2022 20:19

qdm12 mentioned this pull request Jun 15, 2022

lib/trie: split into subpackages and move to internal/trie #2350

Closed

qdm12 marked this pull request as ready for review June 16, 2022 13:00

qdm12 requested review from noot, edwardmack, timwu20, EclesioMeloJunior, jimjbrettj and kishansagathiya as code owners June 16, 2022 13:00

qdm12 force-pushed the qdm12/trie/proofs-refactor branch from e9bc295 to 67f6383 Compare June 16, 2022 15:25

qdm12 marked this pull request as draft June 17, 2022 15:35

qdm12 force-pushed the qdm12/trie/proofs-refactor branch 2 times, most recently from 0bdb189 to ad08ea1 Compare June 30, 2022 17:47

qdm12 force-pushed the qdm12/trie/split-encodeandhash branch from 9d55d9c to 9691124 Compare June 30, 2022 17:48

qdm12 force-pushed the qdm12/trie/proofs-refactor branch from ad08ea1 to 58dbc47 Compare June 30, 2022 19:38

qdm12 changed the base branch from qdm12/trie/split-encodeandhash to development June 30, 2022 20:45

qdm12 marked this pull request as ready for review June 30, 2022 20:45

Merge in internal/trie/proof

47e2d0b

- `lib/trie` proof files - `internal/trie/record` package

qdm12 added 20 commits July 4, 2022 17:53

Move proof trie build functions to internal/trie/proof

90d2546

- `buildTrie` returns a `*trie.Trie` - `loadProof` only acts on a parent node pointer - Older `LoadFromProof` and `loadProof` functions removed from `trie/database.go`

Fix buildTrie to detect root node for roots with encoding smaller t…

7285e16

…han 32 bytes

Add check for root not found

31ccd8e

Fix buildTrie trigger sizes for root hash check

6882bb6

Add tests with full coverage

ce04e19

- More context to error wrapping - Verify function only returns an error

Remove older tests

e8d88b4

Generate takes []byte instead of common.Hash to match Verify sign…

b64e74b

…ature

Move internal/trie/proof -> lib/trie/proof

b5b33c2

Add Verify export comment

2c89af8

Deduplicate encoded nodes (as before)

ea84414

Use encodeNode helper in Test_walk

ee988d5

Review some error wrappings

da0c760

Set all temporary proof trie nodes as dirty

84b5a57

Comments fixes

9a5bb95

Add walk benchmark

9f70103

Rename variables

99541b9

- `proofHashToNode` to `merkleValueToNode` - `proofHash` to `merkleValue`

WIP

69cd49f

Generate generates deduplicated encoded proof nodes for N full keys

53e4a64

- Load trie only once from database - Move StorageState `GenerateTrieProof` to `lib/trie/proof` package - Update tests

Do not add inlined nodes to proof nodes

25b4e57

qdm12 force-pushed the qdm12/trie/proofs-refactor branch from b3faafd to 25b4e57 Compare July 4, 2022 17:54

qdm12 mentioned this pull request Jul 4, 2022

chore(trie): add MerkleValue method and functions #2602

Merged

timwu20 approved these changes Jul 4, 2022

View reviewed changes

EclesioMeloJunior reviewed Jul 5, 2022

View reviewed changes

lib/trie/proof/generate_test.go Show resolved Hide resolved

EclesioMeloJunior approved these changes Jul 5, 2022

View reviewed changes

qdm12 merged commit d2da7fb into development Jul 7, 2022

qdm12 deleted the qdm12/trie/proofs-refactor branch July 7, 2022 01:19

qdm12 mentioned this pull request Sep 29, 2022

Member Request polkadot-fellows/seeding#28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(trie): proof code refactoring and fixes #2604

chore(trie): proof code refactoring and fixes #2604

qdm12 commented Jun 15, 2022 •

edited

Loading

qdm12 Jun 15, 2022

codecov bot commented Jun 16, 2022 •

edited

Loading

seunlanlege commented Jun 22, 2022

seunlanlege commented Jun 22, 2022

qdm12 commented Jun 29, 2022 •

edited

Loading

timwu20 Jul 4, 2022

qdm12 Jul 4, 2022

github-actions bot commented Nov 23, 2022

		// Note this is exported because it is imported and used by:
		// https://github.com/ComposableFi/ibc-go/blob/6d62edaa1a3cb0768c430dab81bb195e0b0c72db/modules/light-clients/11-beefy/types/client_state.go#L78

chore(trie): proof code refactoring and fixes #2604

chore(trie): proof code refactoring and fixes #2604

Conversation

qdm12 commented Jun 15, 2022 • edited Loading

Changes

Tests

Issues

Primary Reviewer

qdm12 Jun 15, 2022

Choose a reason for hiding this comment

codecov bot commented Jun 16, 2022 • edited Loading

Codecov Report

seunlanlege commented Jun 22, 2022

seunlanlege commented Jun 22, 2022

qdm12 commented Jun 29, 2022 • edited Loading

timwu20 Jul 4, 2022

Choose a reason for hiding this comment

qdm12 Jul 4, 2022

Choose a reason for hiding this comment

github-actions bot commented Nov 23, 2022

qdm12 commented Jun 15, 2022 •

edited

Loading

codecov bot commented Jun 16, 2022 •

edited

Loading

qdm12 commented Jun 29, 2022 •

edited

Loading