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

Remove ValidateFromChainState #5707

Merged
merged 12 commits into from
Oct 1, 2024
Merged

Remove ValidateFromChainState #5707

merged 12 commits into from
Oct 1, 2024

Conversation

AndreiEres
Copy link
Contributor

@AndreiEres AndreiEres commented Sep 13, 2024

Description

This PR removes the CandidateValidationMessage::ValidateFromChainState, which was previously used by backing, but is no longer relevant since initial async backing implementation paritytech/polkadot#5557.

Fixes #5643

Integration

This change should not affect downstream projects since ValidateFromChainState was already unused.

Review Notes

  • Removed all occurrences of ValidateFromChainState.
  • Moved utility functions, previously used in candidate validation tests and malus, exclusively to candidate validation tests as they are no longer used in malus.
  • Deleted the polkadot_parachain_candidate_validation_validate_from_chain_state metric from Prometheus.
  • Removed Spawner from ReplaceValidationResult in malus’ interceptors.
  • fake_validation_error was only used for ValidateFromChainState handling, while other cases directly used InvalidCandidate::InvalidOutputs. It has been replaced with fake_validation_error, with a fallback to InvalidCandidate::InvalidOutputs.
  • Updated overseer’s minimal example to replace ValidateFromChainState with ValidateFromExhaustive.

@AndreiEres AndreiEres marked this pull request as ready for review September 24, 2024 16:45
@AndreiEres AndreiEres changed the title [WIP] Remove ValidateFromChainState Remove ValidateFromChainState Sep 24, 2024
@AndreiEres AndreiEres added T8-polkadot This PR/Issue is related to/affects the Polkadot network. T0-node This PR/Issue is related to the topic “node”. labels Sep 24, 2024
Copy link
Contributor

@s0me0ne-unkn0wn s0me0ne-unkn0wn left a comment

Choose a reason for hiding this comment

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

I love where it's going, first we removed jaeger, then a subsystem message variant, now I eagerly await when we start to remove whole subsystems :)

Copy link
Contributor

@sandreim sandreim left a comment

Choose a reason for hiding this comment

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

Nice! LGTM!

@AndreiEres AndreiEres added this pull request to the merge queue Oct 1, 2024
Merged via the queue into master with commit 1617852 Oct 1, 2024
224 of 226 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/issue5643 branch October 1, 2024 13:55
paritytech-ci pushed a commit that referenced this pull request Oct 1, 2024
# Description

This PR removes the
`CandidateValidationMessage::ValidateFromChainState`, which was
previously used by backing, but is no longer relevant since initial
async backing implementation
paritytech/polkadot#5557.

Fixes #5643

## Integration

This change should not affect downstream projects since
`ValidateFromChainState` was already unused.

## Review Notes

- Removed all occurrences of `ValidateFromChainState`.
- Moved utility functions, previously used in candidate validation tests
and malus, exclusively to candidate validation tests as they are no
longer used in malus.
- Deleted the
`polkadot_parachain_candidate_validation_validate_from_chain_state`
metric from Prometheus.
- Removed `Spawner` from `ReplaceValidationResult` in malus’
interceptors.
- `fake_validation_error` was only used for `ValidateFromChainState`
handling, while other cases directly used
`InvalidCandidate::InvalidOutputs`. It has been replaced with
`fake_validation_error`, with a fallback to
`InvalidCandidate::InvalidOutputs`.
- Updated overseer’s minimal example to replace `ValidateFromChainState`
with `ValidateFromExhaustive`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CandidateValidationMessage::ValidateFromChainState isn't used in production
4 participants