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

try-runtime: add cli option --export-proof #12539

Merged

Conversation

librelois
Copy link
Contributor

Add a cli option --export-proof for try-runtime subcommand to be able to extract the storage proof in a binary file.

The goal is to be able to analyze the content of a storage proof, obtained for example after the execution of the subcommand try-runtime on-runtime-upgrade to understand which data are inserted in the proof by a migration.

@stale
Copy link

stale bot commented Dec 10, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 10, 2022
@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 13, 2022
@librelois
Copy link
Contributor Author

librelois commented Dec 13, 2022

This PR is not stale, I'm still working on it.

@librelois librelois marked this pull request as ready for review December 13, 2022 16:19
@crystalin
Copy link
Contributor

@ggwpez can we get another review ? We just merged master again

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

I tried it on Polkadot and it seems to work alright.

@@ -31,13 +38,6 @@ sp-weights = { version = "4.0.0", path = "../../../../primitives/weights" }
frame-try-runtime = { optional = true, path = "../../../../frame/try-runtime" }
substrate-rpc-client = { path = "../../rpc/client" }

parity-scale-codec = "3.0.0"
hex = "0.4.3"
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 these deps were split off to the bottom since they are external.

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed.

utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
utils/frame/try-runtime/cli/src/lib.rs Show resolved Hide resolved
utils/frame/try-runtime/cli/src/commands/follow_chain.rs Outdated Show resolved Hide resolved
@ggwpez ggwpez added A0-please_review Pull request needs code review. B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 16, 2022
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

LGTM other than Oliver's comments.

We should really strive to add some basic CLI-style units tests to try-runtime CLI.

utils/frame/try-runtime/cli/src/commands/follow_chain.rs Outdated Show resolved Hide resolved
@ggwpez
Copy link
Member

ggwpez commented Dec 21, 2022

So can you still address the discussion with the dependencies please?
And the CI is red @librelois

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
utils/frame/try-runtime/cli/src/lib.rs Outdated Show resolved Hide resolved
librelois and others added 3 commits December 28, 2022 16:56
Co-authored-by: Anton <anton.kalyaev@gmail.com>
Co-authored-by: Anton <anton.kalyaev@gmail.com>
Co-authored-by: Anton <anton.kalyaev@gmail.com>
@librelois
Copy link
Contributor Author

So can you still address the discussion with the dependencies please? And the CI is red @librelois

@ggwpez Sorry for the delay, I was on vacation, now it's done.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

@ggwpez
Copy link
Member

ggwpez commented Dec 30, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 55c64bc into paritytech:master Dec 30, 2022
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* try-runtime: add cli option --export-proof

* extract proof in raw json format

* fix build

* fix(try-runtime execute-block): wrong block parsing

* fmt

* apply suggestions

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* split off external dependencies

* fmt

* fix try-runtime compilation

Co-authored-by: Anton <anton.kalyaev@gmail.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* try-runtime: add cli option --export-proof

* extract proof in raw json format

* fix build

* fix(try-runtime execute-block): wrong block parsing

* fmt

* apply suggestions

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update utils/frame/try-runtime/cli/src/lib.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* split off external dependencies

* fmt

* fix try-runtime compilation

Co-authored-by: Anton <anton.kalyaev@gmail.com>
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. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants