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

Fix and refactor trie proof logics #2252

Merged
merged 32 commits into from
Dec 12, 2024
Merged

Fix and refactor trie proof logics #2252

merged 32 commits into from
Dec 12, 2024

Conversation

weiihann
Copy link
Contributor

@weiihann weiihann commented Nov 5, 2024

This PR primarily does the following:

  • Refactor VerifyProof as it fails on some edge cases
  • Refactor VerifyRangeProof to cover most edge cases

A new data structure OrderedSet is used for proof generation and verification. The previous implementation used slices to contain proof nodes, which introduced complexity, unnecessary duplication, and inefficiency. The OrderedSet structure acts as a key-value database, simplifying the proof logic and improving readability.

VerifyRangeProof is not fully completed. For some specific missing key in the range (i.e. gapped range), the function would deem the edge case as valid but it's not supposed to be. The root cause still needs further investigation, but it's most likely due to the Trie structure and how we perform the root hash calculation. A separate PR will be raised to tackle it.

Credits: some implementation details are based on go-ethereum

Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 66.85934% with 172 lines in your changes missing coverage. Please review.

Project coverage is 74.56%. Comparing base (2b1b219) to head (ed7d0f5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
core/trie/proof.go 65.48% 78 Missing and 29 partials ⚠️
core/trie/trie.go 56.04% 34 Missing and 6 partials ⚠️
core/trie/node.go 51.42% 12 Missing and 5 partials ⚠️
core/trie/key.go 82.22% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2252      +/-   ##
==========================================
- Coverage   75.24%   74.56%   -0.68%     
==========================================
  Files         108      109       +1     
  Lines       11520    11559      +39     
==========================================
- Hits         8668     8619      -49     
- Misses       2185     2270      +85     
- Partials      667      670       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@weiihann weiihann force-pushed the weiihann/refactor-proof branch 2 times, most recently from 44f9ae6 to 59f2edd Compare December 2, 2024 12:11
@weiihann weiihann marked this pull request as ready for review December 2, 2024 12:11
@weiihann weiihann changed the title Fixes get proof and verify proof Fix and refactor trie proof logics Dec 2, 2024
@pnowosie pnowosie self-requested a review December 3, 2024 13:00
Copy link
Contributor

@pnowosie pnowosie left a comment

Choose a reason for hiding this comment

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

1st part of review

core/trie/key.go Outdated Show resolved Hide resolved
core/trie/key.go Outdated Show resolved Hide resolved
core/trie/key.go Outdated Show resolved Hide resolved
core/trie/proof.go Outdated Show resolved Hide resolved
core/trie/proof.go Outdated Show resolved Hide resolved
core/trie/proof.go Outdated Show resolved Hide resolved
core/trie/proof.go Show resolved Hide resolved
core/trie/proof.go Outdated Show resolved Hide resolved
core/trie/proof.go Show resolved Hide resolved
core/trie/trie.go Show resolved Hide resolved
@weiihann weiihann force-pushed the weiihann/refactor-proof branch from 9d37fdf to d1b26ec Compare December 5, 2024 09:19
@pnowosie pnowosie self-requested a review December 6, 2024 11:18
Copy link
Contributor

@pnowosie pnowosie left a comment

Choose a reason for hiding this comment

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

Nice work! 👍

core/trie/proof.go Show resolved Hide resolved
core/trie/node.go Outdated Show resolved Hide resolved
core/trie/proof.go Show resolved Hide resolved
core/trie/proof.go Outdated Show resolved Hide resolved
utils/orderedset.go Outdated Show resolved Hide resolved
@weiihann weiihann force-pushed the weiihann/refactor-proof branch from 4dc6386 to 2ea3e78 Compare December 9, 2024 12:03
core/trie/key.go Outdated Show resolved Hide resolved
core/trie/trie.go Outdated Show resolved Hide resolved
core/trie/trie.go Outdated Show resolved Hide resolved
@weiihann weiihann force-pushed the weiihann/refactor-proof branch from 64fbf96 to cfb39ff Compare December 10, 2024 09:43
@rianhughes
Copy link
Contributor

Very nice work :)

@weiihann weiihann force-pushed the weiihann/refactor-proof branch from 3524b18 to fd0879a Compare December 11, 2024 11:32
verify proof passes all test cases

range proof but has issues

prove non-membership

add non-membership tests

doesnt work

some fixes

more tests

doesn't work fully

pass 4 key trie D

pass trie c and d

pass all 4 keys

tidy up tests

fix shiftright bug

pass 9 keys non-existent

proof to path pass all test cases

tidy up tests

storageNodeSet's Put will merge with existing one
@weiihann weiihann force-pushed the weiihann/refactor-proof branch from b7e10ce to ed7d0f5 Compare December 12, 2024 09:22
@kirugan kirugan merged commit 65b7507 into main Dec 12, 2024
13 checks passed
@kirugan kirugan deleted the weiihann/refactor-proof branch December 12, 2024 10:20
// Insert new entry
ps.itemPos[key] = len(ps.items)
ps.items = append(ps.items, value)
ps.size++
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need size? can we use len(ps.items) instead ?

weiihann added a commit that referenced this pull request Dec 12, 2024
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
weiihann added a commit that referenced this pull request Dec 13, 2024
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
weiihann added a commit that referenced this pull request Dec 16, 2024
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
weiihann added a commit that referenced this pull request Dec 18, 2024
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
weiihann added a commit that referenced this pull request Dec 19, 2024
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
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.

4 participants