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.

Sign up for GitHub

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

Jump to bottom

Ensure key ownership proof is optimal #4699

Merged
merged 10 commits into from
Jun 27, 2024
Merged

Ensure key ownership proof is optimal #4699

merged 10 commits into from
Jun 27, 2024

Conversation

serban300
Copy link
Contributor

@serban300 serban300 commented Jun 5, 2024 •
edited
Loading

Ensure that the key ownership proof doesn't contain duplicate or unneeded nodes.

We already have these checks for the bridge messages proof. Just making them more generic and performing them also for the key ownership proof.

@serban300 serban300 added R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges. labels Jun 5, 2024
@serban300 serban300 requested a review from acatangiu June 5, 2024 08:04
@serban300 serban300 self-assigned this Jun 5, 2024
@serban300 serban300 requested a review from a team as a code owner June 5, 2024 08:04
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 5, 2024 08:04
bkchr
bkchr reviewed Jun 5, 2024
View reviewed changes
bridges/primitives/runtime/src/storage_proof.rs Outdated Show resolved Hide resolved
substrate/frame/session/src/historical/mod.rs Outdated Show resolved Hide resolved
substrate/frame/session/src/historical/mod.rs Outdated Show resolved Hide resolved
substrate/primitives/trie/src/recorder_ext.rs Outdated Show resolved Hide resolved
substrate/primitives/trie/src/recorder_ext.rs Outdated Show resolved Hide resolved
substrate/primitives/trie/src/recorder_ext.rs Outdated Show resolved Hide resolved
substrate/primitives/trie/src/recorder_ext.rs Outdated Show resolved Hide resolved
substrate/primitives/trie/src/recorder_ext.rs Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 5, 2024 14:30
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 7, 2024 14:27
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable-int
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6424610

acatangiu
acatangiu approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Looks good to me!

substrate/primitives/trie/src/recorder.rs Outdated Show resolved Hide resolved
@acatangiu
Copy link
Contributor

@serban300 the zombienet-polkadot-functional-0003-beefy-and-mmr test is failing on proof generation and verification.

I don't see how this PR would impact that path, but needs double-checking from you.

@serban300
Copy link
Contributor Author

@serban300 the zombienet-polkadot-functional-0003-beefy-and-mmr test is failing on proof generation and verification.

I don't see how this PR would impact that path, but needs double-checking from you.

I think the root cause for the zombienet-polkadot-functional-0003-beefy-and-mmr failures is #4309 (comment) . This PR shouldn't affect it. But I will run it again to confirm.

@serban300
Copy link
Contributor Author

@bkchr I think I addressed all your comments. Could you PTAL ?

bkchr
bkchr approved these changes Jun 27, 2024
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Looks good to me! Good work!

@acatangiu acatangiu added this pull request to the merge queue Jun 27, 2024
@serban300 serban300 removed this pull request from the merge queue due to a manual request Jun 27, 2024
@serban300 serban300 added this pull request to the merge queue Jun 27, 2024
Merged via the queue into paritytech:master with commit d604e84 Jun 27, 2024
154 of 158 checks passed
@serban300 serban300 deleted the historical branch June 27, 2024 11:52
@bkontur bkontur mentioned this pull request Jun 28, 2024
Draft
5 tasks
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
@serban300 @acatangiu @TarekkMA
Ensure key ownership proof is optimal (paritytech#4699)
7bfaeb3 
Ensure that the key ownership proof doesn't contain duplicate or
unneeded nodes.

We already have these checks for the bridge messages proof. Just making
them more generic and performing them also for the key ownership proof.

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>
This was referenced Aug 21, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkchr bkchr bkchr approved these changes

@acatangiu acatangiu acatangiu approved these changes

Assignees

@serban300 serban300

Labels
R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges.
Projects
Bridges + XCM
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serban300 @paritytech-cicd-pr @acatangiu @bkchr