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

Expose validation specific host function. #2366

Closed
wants to merge 29 commits into from

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Feb 2, 2021

This PR is required for paritytech/cumulus#295

It exposes a runtime interface 'Validation'. It only contains a function 'validation_code' that return the wasm code use in a validation context.

This is use in paritytech/cumulus#295 to insert the validation code in place of the runtime code in proof (note that this only work with cumulus because currently cumulus uses the same wasm for validation and runtime).

@cheme cheme added the A0-please_review Pull request needs code review. label Feb 2, 2021
// Locking as read only to allow access to code memory from
// host validation extension.
// Note that this only work if two process are communicating only.
// Would be racy otherwhise.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure of this assertion: I did switch the lock to read to be able to acces the wasm slice directly, but this would be racy if the shared memory is consumed by multiple client.
CC\ @arkpar

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you are doing all this, when we read the code anyway.

Copy link
Contributor Author

@cheme cheme Feb 2, 2021

Choose a reason for hiding this comment

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

The code is passed as a slice and the validation host function uses an extension which require 'static lifetime, so I cannot use a slice in it.
(I could have put it behind an Arc but would be a clone)

Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to clone than doing the magic here.

Copy link
Contributor Author

@cheme cheme Feb 2, 2021

Choose a reason for hiding this comment

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

I can also try to access it with 'let validation_code = runtime_api_request( ctx, descriptor.relay_parent, RuntimeApiRequest::ValidationCode( descriptor.para_id, assumption, code_tx, ), code_rx, ).await?;', not sure it if it is accessible.
Edit: does not make sense, will just clone for now.

Copy link
Member

Choose a reason for hiding this comment

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

Multiple parallel validation are supported, but each process is run in a separate instance of ValidationHost. So aliasing code memory slice should be safe.

@cheme
Copy link
Contributor Author

cheme commented Apr 8, 2021

Closing, I am reverting cumulus PR to only compact proof (I remove the value skip to keep thing simple, so no need for this pr).

@cheme cheme closed this Apr 8, 2021
@cheme
Copy link
Contributor Author

cheme commented Jun 21, 2021

This branch and cumulus pre refactor https://github.com/paritytech/cumulus/tree/d300493f36150011437fe4e65b5eb25453eb6d85 and trie paritytech/trie#126 .
Could probably be a start in paritytech/polkadot-sdk#967 .

Here the point was to avoid the runtime code in pov, which was done by removing from proof and allowing access from validation function (current).
For 3310, the validation code from inherent would replace the one here (current become next).

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants