Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Introduce genericism around the entry-point of the execute queue in pvf validation. #3226

Closed

Conversation

Lldenaurois
Copy link
Contributor

This PR introduces genericism around the entry-point of the execute queue in pvf validation. That is, currently it is assumed that every PVF exports a function validate_block, and every worker in the PVF execute queue hard-codes this entry-point. In order to support additional function entry-points in the PVF, we modify the queue to accept the entry-point function as an argument.

This PR is currently blocked by: paritytech/substrate#9093.

Once unblocked, the following modifications will be made:

  1. executor_intf::prevalidate should test that the validate_block is exported in the RuntimeBlob
  2. Candidate Validation subsystem should support messages from the CollatorProtocol
  3. CollatorProtocol should request a validation under the PVF validate_collator by submitting a CandidateValidationMessage
  4. Should the validate_collator not be exported in the PVF, CandidateValidation should notify the CollatorProtocol that they needn't encforce it
  5. If the Parachain's PVF exports the validate_collator function, then the CollatorProtocol will enforce that CollatorProtocolMessage::Declare should include all input data to the validate_collator function and submit these
  6. Upon every Declare message, the validator will disconnect peers for which CandidateValidation has notified of a failed Collator Validation

@Lldenaurois Lldenaurois added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jun 13, 2021
@Lldenaurois Lldenaurois force-pushed the pvf-generic-executor branch from ed89425 to 50d5a5e Compare June 13, 2021 01:48
@Lldenaurois Lldenaurois force-pushed the pvf-generic-executor branch from 389bcdd to 2178a44 Compare June 13, 2021 03:05
) => {
let (code_tx, code_rx) = oneshot::channel();
// TODO ensure that we needn't call this every time
let validation_code = match runtime_api_request(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the best option to ensure the validation_code is only fetched if it has not yet been prepared?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's actually a feature in itself.

See https://github.com/paritytech/polkadot/issues/2866

}

async fn recv_request(stream: &mut UnixStream) -> io::Result<(PathBuf, Vec<u8>)> {
async fn recv_request(stream: &mut UnixStream) -> io::Result<(PathBuf, Vec<u8>, String)> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess at this point we should introduce a nominal type

/// This function enables the implementation of the Candidate Validation subsystem to delineate
/// the entry-point of the PVF and the oneshot channel passed to the execution of any function
/// exported in the pvf.
///
Copy link
Contributor

Choose a reason for hiding this comment

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

There is quite some trailing whitespaces.

Could you please clean them up in this PR. Also it would be great if you can configure your editor to remove trailing whitespaces automatically on save

/// situations this function should return immediately.
///
/// Returns an error if the request cannot be sent to the validation host, i.e. if it shut down.
pub async fn execute_pprevf(
Copy link
Contributor

Choose a reason for hiding this comment

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

oh no

This is why I proposed to think of another name for this feature xD

) => {
let (code_tx, code_rx) = oneshot::channel();
// TODO ensure that we needn't call this every time
let validation_code = match runtime_api_request(
Copy link
Contributor

Choose a reason for hiding this comment

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

That's actually a feature in itself.

See https://github.com/paritytech/polkadot/issues/2866

@louismerlin louismerlin removed the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants