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

feat(dot/parachain): implement candidate validation from chainstate #4067

Merged
merged 12 commits into from
Aug 12, 2024

Conversation

edwardmack
Copy link
Member

@edwardmack edwardmack commented Jul 1, 2024

Changes

Note this should be merged after PR #4036 has been merged since this PR is based on that branch.

Implement updates to candidate validation subsystem to handle validation from chainstate.

  • validateFromChainState utilizes getValidationData function to assemble persisted validation data and validation code.
  • call validateFromExhaustive with retrieved data to validate candidate.
  • Added tests to confirm validateFromChainState function is working as expected.

Tests

go test github.com/ChainSafe/gossamer/dot/parachain/candidate-validation -v -run ^TestCandidateValidation_validateFromChainState$

Issues

closes #3919

Copy link
Contributor

@timwu20 timwu20 left a comment

Choose a reason for hiding this comment

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

why are there so many commits on this PR but only 5 files changed?

lib/erasure/erasure.go Outdated Show resolved Hide resolved
@timwu20 timwu20 force-pushed the ed/feat/candidate_validation_chainstate branch from b20f628 to d3d152e Compare July 17, 2024 17:12
@edwardmack edwardmack force-pushed the ed/feat/candidate_validation_chainstate branch from 5eef681 to d3d152e Compare July 18, 2024 16:27
Copy link
Contributor

@kishansagathiya kishansagathiya left a comment

Choose a reason for hiding this comment

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

Should be good after resolving these comments.

Copy link
Contributor

@axaysagathiya axaysagathiya left a comment

Choose a reason for hiding this comment

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

LGTM!

@edwardmack edwardmack merged commit a58b1a4 into feat/parachain Aug 12, 2024
22 checks passed
@edwardmack edwardmack deleted the ed/feat/candidate_validation_chainstate branch August 12, 2024 21:06
edwardmack added a commit that referenced this pull request Oct 4, 2024
…4067)

Co-authored-by: Timothy Wu <tim.wu@chainsafe.io>

test(dot/parachain/backing): make sure parachain candidate reaches quorum (#4115)

- written integration test to check everything mentioned above
- I re-wrote the logic to convert the attested to the backed candidate. (was not working as expected)
- minor changes in dot/parachain/backing/per_relay_parent_state.go to improve the readability.

test(dot/parachain/backing): validation fail does not stop the candidate backing subsystem (#4117)

address merge conflicts

chore(dot/parachain): improve Run method of subsystems (#4113)

- I have updated the Run method of the subsystem interface to pass only necessary arguments.
- removed context, context cancel func, wait group and OverseerToSubSystem channel from the state struct of all the subsystems(example: candidateBacking{}, AvailabilityStoreSubsystem{}).
- improved Run method of some subsystems.
- removed return type from RegisterSubsystem method of overseer.
- because of these changes, some tests were failing, so I fixed those tests.
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