Skip to content

Conversation

@evilrobot-01
Copy link
Contributor

@evilrobot-01 evilrobot-01 commented Apr 8, 2025

Based on https://www.shawntabrizi.com/substrate-js-utilities/ and https://github.com/hack-ink/subalfred, adds a simple hash command as an additional utility to the CLI. It is feature gated behind experimental for now. It supports all the hashers typically used by the SDK (blake2, keccack, sha2, twox), without taking on any new dependencies.

Installation

cargo install --path ./crates/pop-cli --locked --features experimental

Usage

There is a subcommand for each supported hasher, which expects the required length of the output along with input data in the form of a string, hex or path to a file to be hashed. The command structure is intended to be as simple as possible, whilst also allowing easier future extensibility. Whilst the commands could be combined with the output length (e.g. keccak-256), its the same amount of characters to type and providing the length separately makes it more natural in terms of how a hasher works imo. Possible values for length are included in --help.

Some of the commands also support an additional --concat option which can be used for storage key hashing.

pop hash blake2 256 ./runtime.wasm
pop hash twox 128 some_text
pop hash keccak 256 0x68656c6c6f

TODOs:

  • potentially include the length, in bytes, with the output
  • docs - a separate pr to add to pop-docs once approved
  • tests

[sc-3512]

@evilrobot-01 evilrobot-01 added the ready-for-high-level-review The PR needs a high level review label Apr 8, 2025
@evilrobot-01 evilrobot-01 requested a review from AlexD10S April 8, 2025 06:47
@codecov
Copy link

codecov bot commented Apr 8, 2025

Codecov Report

Attention: Patch coverage is 88.43284% with 31 lines in your changes missing coverage. Please review.

Project coverage is 78.89%. Comparing base (054c5f3) to head (18b1408).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/pop-cli/src/commands/hash.rs 89.23% 20 Missing and 8 partials ⚠️
crates/pop-cli/src/commands/mod.rs 62.50% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main     #517      +/-   ##
==========================================
+ Coverage   78.78%   78.89%   +0.11%     
==========================================
  Files          99      100       +1     
  Lines       23184    23452     +268     
  Branches    23184    23452     +268     
==========================================
+ Hits        18265    18502     +237     
- Misses       2719     2742      +23     
- Partials     2200     2208       +8     
Files with missing lines Coverage Δ
crates/pop-cli/src/commands/mod.rs 68.44% <62.50%> (-0.22%) ⬇️
crates/pop-cli/src/commands/hash.rs 89.23% <89.23%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

Just tested it, not a code review. Really nice and clean feature to use.
I like that there's no new dependency introduced, so I'm not sure there's much benefit to putting it behind an experimental flag.

@evilrobot-01
Copy link
Contributor Author

It's experimental until some of the other useful commands in the listed resources land and the overall command structure can be stabilised.

@evilrobot-01 evilrobot-01 changed the title feat: hash feat: hashing Apr 9, 2025
@evilrobot-01 evilrobot-01 marked this pull request as ready for review April 10, 2025 05:22
@evilrobot-01 evilrobot-01 added ready-for-final-review The PR is ready for final review and removed ready-for-high-level-review The PR needs a high level review labels Apr 10, 2025
@chungquantin chungquantin self-requested a review April 10, 2025 06:49
Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

Nice work! The code is clean, well-organized, and very easy to review. The only change I’d propose is to replace the println! with our CLI module for the output, just to keep things consistent. Other than that, this looks good to approve and merge!

@evilrobot-01 evilrobot-01 merged commit a0f8242 into main Apr 23, 2025
33 of 34 checks passed
@evilrobot-01 evilrobot-01 deleted the frank/feat-hash branch April 23, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-final-review The PR is ready for final review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants