-
Notifications
You must be signed in to change notification settings - Fork 2
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
Pass state version for extrinsic root derivation through Config #1
Pass state version for extrinsic root derivation through Config #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a use case where, domains extrinsic root need to be derived on consensus runtime
Why? This feels wrong on the surface.
We would like to inject parent state root so that we can verify the StorageProof in the runtime using the given State root. This would avoid us doing the verification on the client side instead.
Where on the client exactly, in which specific case, does it remove all client-side verification? You can achieve the same with simple inherent extrinsic if you just want to include parent state root in the ancestor block. I don't see why it needs to be upstreamed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? This feels wrong on the surface.
We discussed this already. We want to derive extrinsic root of the domain block during fraud proof verification.
Where on the client exactly, in which specific case, does it remove all client-side verification? You can achieve the same with simple inherent extrinsic if you just want to include parent state root in the ancestor block. I don't see why it needs to be upstreamed.
Yes. you are correct. I'll try that route and remove the second commit
e504a94
to
d9bdb2b
Compare
d9bdb2b
to
fb2fd5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very reasonable.
Please update PR title and let's submit this upstream before merging (while we're working on PR that uses this in subspace/subspace) to see their response. Looks like fairly non-controversial to me.
BTW, looks like |
Any specific point where you see this implication? Anyway lets get the upstream response |
Nothing specific, I just checked and saw that the data structure is used in multiple places across the project, but didn't dig into where exactly and how. It is an assumption. |
fb2fd5d
to
4ece868
Compare
4ece868
to
1f217f3
Compare
First attempt to land this upstream: paritytech#1691 |
1. Benchmark results are collected in a single struct. 2. The output of the results is prettified. 3. The result struct used to save the output as a yaml and store it in artifacts in a CI job. ``` $ cargo run -p polkadot-subsystem-bench --release -- test-sequence --path polkadot/node/subsystem-bench/examples/availability_read.yaml | tee output.txt $ cat output.txt polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#1 Network usage, KiB total per block Received from peers 510796.000 170265.333 Sent to peers 221.000 73.667 CPU usage, s total per block availability-recovery 38.671 12.890 Test environment 0.255 0.085 polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#2 Network usage, KiB total per block Received from peers 413633.000 137877.667 Sent to peers 353.000 117.667 CPU usage, s total per block availability-recovery 52.630 17.543 Test environment 0.271 0.090 polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#3 Network usage, KiB total per block Received from peers 424379.000 141459.667 Sent to peers 703.000 234.333 CPU usage, s total per block availability-recovery 51.128 17.043 Test environment 0.502 0.167 ``` ``` $ cargo run -p polkadot-subsystem-bench --release -- --ci test-sequence --path polkadot/node/subsystem-bench/examples/availability_read.yaml | tee output.txt $ cat output.txt - benchmark_name: 'polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#1' network: - resource: Received from peers total: 509011.0 per_block: 169670.33333333334 - resource: Sent to peers total: 220.0 per_block: 73.33333333333333 cpu: - resource: availability-recovery total: 31.845848445 per_block: 10.615282815 - resource: Test environment total: 0.23582828799999941 per_block: 0.07860942933333313 - benchmark_name: 'polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#2' network: - resource: Received from peers total: 411738.0 per_block: 137246.0 - resource: Sent to peers total: 351.0 per_block: 117.0 cpu: - resource: availability-recovery total: 18.93596025099999 per_block: 6.31198675033333 - resource: Test environment total: 0.2541994199999979 per_block: 0.0847331399999993 - benchmark_name: 'polkadot/node/subsystem-bench/examples/availability_read.yaml autonomys#3' network: - resource: Received from peers total: 424548.0 per_block: 141516.0 - resource: Sent to peers total: 703.0 per_block: 234.33333333333334 cpu: - resource: availability-recovery total: 16.54178526900001 per_block: 5.513928423000003 - resource: Test environment total: 0.43960946299999537 per_block: 0.14653648766666513 ``` --------- Co-authored-by: Andrei Sandu <[email protected]>
* Initial commit. CLI which parses RPC urls. * Establish ws connections and make simple RPC requests. * Complete bridge setup. * Process subscription events. * Ctrl-C handler. * Write a bare-bones README and copy in design doc. * Modularize code a little bit. * Communicate with each chain in a separate task. * Parse headers from RPC subscription notifications. * Send (fake) extrinsics across bridge channels. And now it's deadlocked. * Fix deadlock. * Clarify in README that this is not-in-progress. * Move everything into a single folder * Move Substrate relay into appropriate folder * Get the Substrate Relay node compiling * Update Cargo.lock * Use new composite accounts from Substrate * Remove specification document It has been moved to the Wiki on the Github repo. * Update author + remove comments * Use latest master for jsonrpsee Required renaming some stuff (e.g Client -> RawClient) Co-authored-by: Jim Posen <[email protected]>
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context,
including:
ExtrinsicRootStateVersion
can be passed through runtime and I have set it to StateVersion::V0 so this should not change the extrinsic root derivation.