-
Notifications
You must be signed in to change notification settings - Fork 372
Refactor signing functions to use blake2AsU8a for hashing instead of registry.hash in conditional sign of extrinsic payload #6183
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
base: master
Are you sure you want to change the base?
Conversation
…d of registry.hash in conditional sign of extrinsic payload
The CI checks are currently failing. Please review the errors and address the issues. |
Thanks, it seems lint error. |
|
|
sorry, I missed place empty line. |
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.
Hey! I'm not sure this change is really needed. The current setup already uses registry.hash
, and if no custom hasher is provided, it falls back to the default — which is blake2AsU8a(256)
.
So from what I see, this PR might not actually change anything in practice. In fact, it could introduce new issues by always forcing blake2_256
, even when a different hasher was intentionally set at the API level.
https://github.com/polkadot-js/api/blob/master/packages/types/src/create/registry.ts#L469
#hasher: (data: Uint8Array) => Uint8Array = blake2AsU8a;
public hash (data: Uint8Array): IU8a {
return this.createType('CodecHash', this.#hasher(data));
}
CC: @TarikGul
Hey! Sooo... I got myself confused there — my bad 😅 The node just hardcodes it to |
Okay, then this is available to merge to master branch? |
I'll test it soon locally. |
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.
Just tested it out — looks good to me. Let @TarikGul review it as well.
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.
There is a greater underlying issue here and not the hasher itself.
registry.hash
is using blake2AsU8a
as its default hasher unless a different hasher is passed in. This PR breaks critical signing functionality because it no longer allows the TypeRegistry to setHasher
. It's important with PR's like this to look out from a wider lense and see how it can affect multiple facets. In this case substrate chains that use a different hasher will inevitably fail.
I was working on new blockchain with using SHA3 as hash function. So I set sha3 hasher into api instance in frontend, so everything is working well, but for big payload extrinsic, I am getting bad signature message.
I presume the issue is you are setting the hasher to SHA3 but want blake2 for when the payload is above 256 bytes. In this case that doesn't fit the design of PJS since it only has support for a single hasher per instance.
Thanks for sharing your insight.
Substrate is always using blake2 for payload hashing, so we always have to use blake2 for payload hashing. And this PR doesnt disturb other hash logic(This PR only changes payload hash).
This is only applied into payload signing, so other hash logic will not be changed.
This is not my desire, substrate is using blake2 with hard coded code, so we have to hard code for payload hash. |
Polkadot api is signing extrinsic payload with registry.hash if payload size is bigger than 256 byte.
But in substrate it is hard coded as blake2 for conditional payload hash.
So when I sign big payload extrinsic with UI, I am getting bad signature message(when small size payload, it is working well).
https://github.com/paritytech/polkadot-sdk/blob/436b4935b52562f79a83b6ecadeac7dcbc1c2367/substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs#L559-L574
Context

I was working on new blockchain with using SHA3 as hash function. So I set sha3 hasher into api instance in frontend, so everything is working well, but for big payload extrinsic, I am getting bad signature message.
After changing api with same changes of this PR, I was able to get success messsage.