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

Add chain-info Subcommand #11250

Merged
merged 13 commits into from
Apr 25, 2022
Merged

Add chain-info Subcommand #11250

merged 13 commits into from
Apr 25, 2022

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented Apr 20, 2022 •
edited
Loading

Adds a chain-info Subcommand to stdout DB meta column information serialized as JSON, including:

  • best_hash
  • best_number
  • genesis_hash
  • finalized_hash
  • finalized_number

Usage and output example

./binary chain-info --base-path=PATH_TO_DB --chain=CHAIN --pruning=PRUNING

{"best_hash":"0xbfc95e0d9864d8fe7a2cc2fd2af298d68fb5f9fe6bad8529545ec981ba677efa","best_number":10,"genesis_hash":"0x3a1beb9351d4d004e0c19f92dac659c2a356e1c0dcd4f38a93ee02dc1acd98f5","finalized_hash":"0x3a1beb9351d4d004e0c19f92dac659c2a356e1c0dcd4f38a93ee02dc1acd98f5","finalized_number":0,"finalized_state":["0x3a1beb9351d4d004e0c19f92dac659c2a356e1c0dcd4f38a93ee02dc1acd98f5",0]}

Rel https://substrate.stackexchange.com/questions/1949/get-block-height-from-database-snapshot-without-running-a-node

polkadot companion: paritytech/polkadot#5370

@tgmichel tgmichel mentioned this pull request Apr 20, 2022
Closed
ggwpez
ggwpez reviewed Apr 20, 2022
View reviewed changes
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.

Very good idea, I was looking for something like this!

I think for the future we should think of a structure similar to the benchmark commands, instead of having dashed commands.

client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
bin/node/cli/src/command.rs Outdated Show resolved Hide resolved
bin/node/cli/src/command.rs Outdated Show resolved Hide resolved
@ggwpez ggwpez requested a review from cheme April 20, 2022 15:35
bkchr
bkchr reviewed Apr 20, 2022
View reviewed changes
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/blockchain_info_cmd.rs Outdated Show resolved Hide resolved
@tgmichel
Copy link
Contributor Author

tgmichel commented Apr 21, 2022 •
edited
Loading

Thank you @ggwpez and @bkchr for your review. I pushed the change for using sc_client_db utility functions instead of spawning the service tasks. For that some visibility changes are required, please let me know if that's a concern.

cheme
cheme reviewed Apr 21, 2022
View reviewed changes
client/db/src/lib.rs Outdated Show resolved Hide resolved
ggwpez
ggwpez approved these changes Apr 21, 2022
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just one nitpick, feel free to ignore.

client/cli/src/commands/blockchain_info_cmd.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 Apr 21, 2022
@tgmichel tgmichel mentioned this pull request Apr 22, 2022
Merged
cheme
cheme approved these changes Apr 22, 2022
View reviewed changes
Copy link
Contributor

@cheme cheme left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.
Maybe I would use just chain-info instead of blockchain-info for the command (we got purge-chain in existing command and it is shorter to type).
CI seems in need of a cargo +nightly --fmt all.

@tgmichel tgmichel changed the title Add blockchain-info Subcommand Add chain-info Subcommand Apr 22, 2022
bkchr
bkchr approved these changes Apr 25, 2022
View reviewed changes
client/cli/src/commands/chain_info_cmd.rs Outdated Show resolved Hide resolved
@tgmichel @bkchr
Copyright year
f13751c 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@bkchr
Copy link
Member

bkchr commented Apr 25, 2022

bot merge

@paritytech-processbot
Copy link

Error: Github API says "Allow edits from maintainers" is not enabled for paritytech/polkadot#5370. The bot would use that permission to push the lockfile update after merging this PR. Please check https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork.

@bkchr bkchr merged commit 59a2150 into paritytech:master Apr 25, 2022
@Dengjianping Dengjianping mentioned this pull request Apr 28, 2022
Closed
@github-actions github-actions bot mentioned this pull request May 31, 2022
Closed
@kamilsa kamilsa mentioned this pull request Jun 7, 2022
Closed
godcodehunter pushed a commit to sensoriumxr/substrate that referenced this pull request Jun 22, 2022
@tgmichel @bkchr @godcodehunter
Add chain-info Subcommand (paritytech#11250)
14d965b 
* Add `blockchain-info` Subcommand

* Update comment

* Cleanup

* Cleanup

* Use `sync_run`

* Use `sc_client_db` utility fns instead service backend

* Use service `Backend` builder

* Impl `From<sp_blockchain::Info>`

* Rename to `chain-info`

* fmt

* Copyright year

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Expose `DatabaseParams`

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jun 22, 2022
Closed
@Dengjianping Dengjianping mentioned this pull request Jul 7, 2022
Merged
10 tasks
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
@tgmichel @bkchr @DaviRain-Su
Add chain-info Subcommand (paritytech#11250)
b8c4678 
* Add `blockchain-info` Subcommand

* Update comment

* Cleanup

* Cleanup

* Use `sync_run`

* Use `sc_client_db` utility fns instead service backend

* Use service `Backend` builder

* Impl `From<sp_blockchain::Info>`

* Rename to `chain-info`

* fmt

* Copyright year

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Expose `DatabaseParams`

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@tgmichel @bkchr @ark0f
Add chain-info Subcommand (paritytech#11250)
b1b80dd 
* Add `blockchain-info` Subcommand

* Update comment

* Cleanup

* Cleanup

* Use `sync_run`

* Use `sc_client_db` utility fns instead service backend

* Use service `Backend` builder

* Impl `From<sp_blockchain::Info>`

* Rename to `chain-info`

* fmt

* Copyright year

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Expose `DatabaseParams`

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@cheme cheme cheme approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
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
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tgmichel @bkchr @cheme @ggwpez