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

[Proof] Relay signature & Merkle proof validation #406

Merged
merged 27 commits into from
Mar 22, 2024
Merged

[Proof] Relay signature & Merkle proof validation #406

merged 27 commits into from
Mar 22, 2024

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented Mar 1, 2024

Summary

Human Summary

Validate relay signatures and and merkle proof:

  1. Ensure that both relay and request session headers match the (already validated) proof session header
  2. Unmarshal the SparseMerkleClosestProof and extract the Relay that is present in the ClosestValueHash property
  3. Verify the RelayResponse signature
  4. Verify the RelayRequest signature
  5. Ensure that proof.Path matches the earliest proof submission block hash
  6. Ensure that the mining difficulty meets the required difficulty
  7. Ensure that the proof verifies against the expected root hash

AI Summary

reviewpad:summary

Issue

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_develop_and_test
  • Run E2E tests locally: make test_e2e
  • Run E2E tests on DevNet: Add the devnet-test-e2e label to the PR. This is VERY expensive, only do it after all the reviews are complete.

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@red-0ne red-0ne added supplier Changes related to the Supplier actor on-chain On-chain business logic labels Mar 1, 2024
@red-0ne red-0ne added this to the Shannon TestNet milestone Mar 1, 2024
@red-0ne red-0ne self-assigned this Mar 1, 2024
@bryanchriswhite bryanchriswhite mentioned this pull request Mar 5, 2024
Merged
11 tasks
bryanchriswhite
bryanchriswhite reviewed Mar 5, 2024
View reviewed changes
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! 😎

I had a couple questions, but otherwise this is looking great. 👌

x/proof/keeper/msg_server_submit_proof.go Outdated Show resolved Hide resolved
x/proof/keeper/msg_server_submit_proof.go Show resolved Hide resolved
x/service/types/relay.go Outdated Show resolved Hide resolved
x/proof/keeper/msg_server_submit_proof.go Outdated Show resolved Hide resolved
x/proof/keeper/msg_server_submit_proof.go Outdated Show resolved Hide resolved
@red-0ne red-0ne requested a review from bryanchriswhite March 7, 2024 15:50
bryanchriswhite
bryanchriswhite reviewed Mar 7, 2024
View reviewed changes
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

I have to run away for a bit. Will resume later today.

image

@red-0ne red-0ne requested a review from bryanchriswhite March 21, 2024 17:00
@github-actions GitHub Actions
Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. If you just created a pull request, you might need to push another commit to produce a container image DevNet can utilize to spin up infrastructure. You can use make trigger_ci to push an empty commit.

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Mar 21, 2024
Olshansk
Olshansk approved these changes Mar 21, 2024
View reviewed changes
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

My update:

  1. Reviewed the changes: looks good
  2. Added a tiny comment update: linking to our discussion
  3. Merged with main
  4. Pushed
  5. Enabled E2E tests

Let's get everything 🟢 and then :shipit: !!!

Olshansk
Olshansk approved these changes Mar 21, 2024
View reviewed changes
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

My update:

  1. Reviewed the changes: looks good
  2. Added a tiny comment update: linking to our discussion
  3. Merged with main
  4. Pushed
  5. Enabled E2E tests

Let's get everything 🟢 and then :shipit: !!!

@github-actions github-actions bot removed the devnet label Mar 21, 2024
@github-actions GitHub Actions
Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. If you just created a pull request, you might need to push another commit to produce a container image DevNet can utilize to spin up infrastructure. You can use make trigger_ci to push an empty commit.

@red-0ne red-0ne dismissed stale reviews from h5law and bryanchriswhite March 22, 2024 04:05

All change requests have been addressed

@red-0ne red-0ne merged commit 18d66b0 into main Mar 22, 2024
9 checks passed
bryanchriswhite added a commit that referenced this pull request Mar 22, 2024
@bryanchriswhite
Merge remote-tracking branch 'pokt/main' into test/proof-integration
61d83ea 
* pokt/main:
  [AppGateServer] feat: Use round-robin endpoint selection strategy (#443)
  [RelayMiner] feat: Make the RelayMiner wait for its corresponding Supllier to stake (#439)
  [Proof] Relay signature & Merkle proof validation (#406)
  [Redelegation] fix: EventRedelegation unmarshaling failure (#435)
@red-0ne red-0ne mentioned this pull request Mar 27, 2024
Closed
8 tasks
@bryanchriswhite bryanchriswhite removed push-image CI related - pushes images to ghcr.io devnet-test-e2e labels May 16, 2024
@github-actions github-actions bot removed the devnet label May 16, 2024
@Olshansk Olshansk deleted the feat/proof-validation branch May 29, 2024 16:44
okdas pushed a commit that referenced this pull request Nov 14, 2024
@red-0ne @bryanchriswhite @Olshansk
[Proof] Relay signature & Merkle proof validation (#406)
158e893 
Co-authored-by: Bryan White <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Olshansk Olshansk Olshansk approved these changes

@h5law h5law Awaiting requested review from h5law

@bryanchriswhite bryanchriswhite Awaiting requested review from bryanchriswhite

Assignees

@red-0ne red-0ne

Labels
on-chain On-chain business logic supplier Changes related to the Supplier actor
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Private TestNet
Development

Successfully merging this pull request may close these issues.
4 participants
@red-0ne @bryanchriswhite @Olshansk @h5law