Add support of additional crypto types and hashing algorithms for Beefy by Klapeyron · Pull Request #8707 · paritytech/polkadot-sdk

Klapeyron · 2025-05-30T08:52:43Z

We would like to use BEEFY with our custom crypto type, in this PR we would like to extend BeefyKeystore with additional code, that makes an execution to underlying keystore.

Also we added a testcases for additional crypto type that is already supported.

This PR also adds support for custom hashing.

cla-bot-2021 · 2025-05-30T08:52:51Z

User @Klapeyron, please sign the CLA here.

Klapeyron · 2025-06-03T10:44:08Z

substrate/client/consensus/beefy/src/keystore.rs

+			.keys(BEEFY_KEY_TYPE)
+			.map_err(|e| error::Error::Keystore(e.to_string()))?
+			.into_iter()
+			.filter_map(|pk| AuthorityId::try_from(pk.as_ref()).ok())


I changed the behavior here, so in case in keystore were multiple keys for beefy with different schemes, then:
Before:
it was failing on first non-compatible key
After:
it is taking keys that can be converted to AuthorityId, ignoring non-compatible ones

this doesn't look sound, it will result in duplicate authorities list if we have authorities registered with multiple types of crypto keys -- we would get an authority instance for each compatible crypto key -- which is breaking the old behavior and potentially breaking downstream assumptions

Opened a PR that should make some things more generic and should address this also: #10763

Klapeyron · 2025-06-03T10:45:13Z

substrate/client/consensus/beefy/src/keystore.rs

 			},

-			_ => Err(error::Error::Keystore("key type is not supported by BEEFY Keystore".into()))?,
+			_ => store


I am not replacing previous 2 cases with this generic call, due they perform some additional hashing, which will be not done in case I will remove them

Klapeyron · 2025-06-03T18:44:04Z

substrate/client/consensus/beefy/src/keystore.rs

 				sig_ref.to_vec()
 			},

-			_ => Err(error::Error::Keystore("key type is not supported by BEEFY Keystore".into()))?,


I am not adding a case for bls381::CRYPTO_ID, due it does not need additional hashing as I understand, please correct me if I am wrong :). So it should fall in generic call.

burdges · 2025-06-04T15:21:18Z

Any thoughts on this @drskalman ?

serban300 · 2025-06-16T13:34:38Z

Not sure if it would make sense to allow Beefy to work with custom crypto types. @acatangiu @drskalman WDYT ?

Also @Klapeyron could you give us more details about your use case please ?

Klapeyron · 2025-06-16T13:45:33Z

Also @Klapeyron could you give us more details about your use case please ?

Sure. So we have more zero knowledge related scenario, we would like to use schnorr jubjub for that. So far we implemented the cryptography traits so we can use it among substrate code, but without changes in this PR, BEEFY will not make a call to a keystore for necessary keys, due custom cryptography is not on the list of supported types.

Draft of the changes is here: input-output-hk/partner-chains#813

Also cryptography is not the only thing we would like to adjust, probably replacement of hardocded Keccak256 will require to add additional code (maybe as separate PR to not complicate).

This reverts commit 3a9ecbb.

This reverts commit 2b1172e.

acatangiu · 2025-06-19T12:28:26Z

I haven't the time now to review the code in detail. The reasoning behind this PR is good to me, maybe @drskalman and @serban300 can help with the code review.

One high-level comment I have is that this is making breaking changes to BEEFY primitives, and BEEFY and MMR runtime pallets. It means that it needs major bumps across the board and can only go in a major release.

Klapeyron · 2026-01-02T09:16:35Z

@serban300 @acatangiu may as ask for your review of this changes?

acatangiu · 2026-01-05T12:54:09Z

@serban300 @skunert please review

substrate/client/consensus/beefy/src/keystore.rs

This reverts commit 3e12fdc.

As a follow-up of the discussion #8707 (comment), I am extracting a missing forward call to a separate PR so we can deliver it independently. Context: When keystore is used by some component (like BEEFY) via Arc, then calls of `sign_with` function are forwarded to default trait implementation. It is not working, when custom keystore with custom `sign_with` implementation is used.

As a follow-up of the discussion paritytech#8707 (comment), I am extracting a missing forward call to a separate PR so we can deliver it independently. Context: When keystore is used by some component (like BEEFY) via Arc, then calls of `sign_with` function are forwarded to default trait implementation. It is not working, when custom keystore with custom `sign_with` implementation is used.

serban300 · 2026-01-29T17:54:20Z

#10763 was merged in master. Please update this PR

This PR: 1. makes some BEEFY keystore methods more generic: - `sign()` - `public_keys()` This is done by implementing the specific logic in the `BeefyAuthorityId`. 2. Removes the `BeefyAuthorityId::SignatureHasher` since for some algorithms it doesn't make sense to have a hasher. Also since now the `BeefyAuthorityId` implements both the signing and the verification logic, we should have better consistency. Related to paritytech#8707 (comment) --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Klapeyron · 2026-02-13T18:44:14Z

Thank you @serban300, we are now happy with current state of master with your recent changes, I will close this PR. If you will be interested in this remaining part of the PR (MMR hasher adjustment), I can rebase and narrow it to this scope of changes

serban300 · 2026-02-13T18:47:26Z

Thank you @serban300, we are now happy with current state of master with your recent changes, I will close this PR. If you will be interested in this remaining part of the PR (MMR hasher adjustment), I can rebase and narrow it to this scope of changes

Nice, glad to hear that ! I'm good. Feel free to close.

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from f2c5ac1 to 22103b7 Compare June 3, 2025 10:12

Klapeyron changed the title ~~Extended pallet_beefy Config with Hashing trait~~ Add support of additional crypto types for BeefyKeeystore Jun 3, 2025

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from 8a29ef3 to 5f4738d Compare June 3, 2025 10:41

Klapeyron commented Jun 3, 2025

View reviewed changes

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from aed7013 to 26e99a8 Compare June 3, 2025 18:45

Klapeyron marked this pull request as ready for review June 3, 2025 18:45

Klapeyron mentioned this pull request Jun 4, 2025

Non-unique authorities list, impact on GRANDPA and BEEFY #8705

Open

2 tasks

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from 0f3d74e to 56dac31 Compare June 4, 2025 19:47

bkchr requested a review from serban300 June 5, 2025 09:36

serban300 requested a review from acatangiu June 16, 2025 12:38

Klapeyron added 7 commits June 16, 2025 20:34

Extended pallet_beefy Config with Hashing trait

d20dc41

Extend BeefyKeystore to non-supported types

57e5473

Revert "Extended pallet_beefy Config with Hashing trait"

0f62d30

This reverts commit 3a9ecbb.

Add tests for bls381

5fa818e

Prdoc

19cf512

Revert "Revert "Extended pallet_beefy Config with Hashing trait""

53fc8bf

This reverts commit 2b1172e.

Removed sp_consensus_beefy::MmrHashing type

9719b43

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from 5577411 to 9719b43 Compare June 16, 2025 18:34

Klapeyron requested a review from a team as a code owner June 16, 2025 18:34

Removed sp_consensus_beefy::BeefySignatureHasher

7dbcab6

Klapeyron added 2 commits June 23, 2025 10:30

Merge branch 'master' into hasher_trait_for_pallet_beefy

a3fd4db

Merge branch 'master' into hasher_trait_for_pallet_beefy

eac2769

Klapeyron changed the title ~~Add support of additional crypto types for BeefyKeeystore~~ Add support of additional crypto types and hashing algorithms for BeefyKeeystore Jun 30, 2025

Merge branch 'master' into hasher_trait_for_pallet_beefy

c919d6d

nkpar enabled auto-merge November 28, 2025 03:31

nkpar disabled auto-merge November 28, 2025 03:31

Merge branch 'paritytech:master' into hasher_trait_for_pallet_beefy

b69e08a

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from b69e08a to 579c7de Compare January 5, 2026 11:43

Klapeyron requested review from a team, cheme and koute as code owners January 5, 2026 11:43

paritytech-review-bot bot requested a review from a team January 5, 2026 11:44

Klapeyron force-pushed the hasher_trait_for_pallet_beefy branch from 579c7de to b69e08a Compare January 5, 2026 12:08

acatangiu approved these changes Jan 5, 2026

View reviewed changes

acatangiu added the T0-node This PR/Issue is related to the topic “node”. label Jan 5, 2026

serban300 reviewed Jan 8, 2026

View reviewed changes

substrate/client/consensus/beefy/src/keystore.rs Show resolved Hide resolved

Klapeyron added 2 commits January 8, 2026 20:42

Parametrization of the hasher for ecdsa

fb408c0

fixup! Parametrization of the hasher for ecdsa

d5278be

serban300 mentioned this pull request Jan 9, 2026

Make some BEEFY keystore logic more generic #10763

Merged

Added missing sign_with forward call

3e12fdc

Klapeyron mentioned this pull request Jan 13, 2026

Missing sign_with forward call #10784

Merged

Revert "Added missing sign_with forward call"

c033f3d

This reverts commit 3e12fdc.

Klapeyron closed this Feb 13, 2026

Conversation

Klapeyron commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-bot-2021 bot commented May 30, 2025

Uh oh!

Klapeyron Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acatangiu Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serban300 Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Klapeyron Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Klapeyron Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burdges commented Jun 4, 2025

Uh oh!

serban300 commented Jun 16, 2025

Uh oh!

Klapeyron commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acatangiu commented Jun 19, 2025

Uh oh!

Klapeyron commented Jan 2, 2026

Uh oh!

acatangiu commented Jan 5, 2026

Uh oh!

Uh oh!

serban300 commented Jan 29, 2026

Uh oh!

Klapeyron commented Feb 13, 2026

Uh oh!

serban300 commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Klapeyron commented May 30, 2025 •

edited

Loading

Klapeyron Jun 3, 2025 •

edited

Loading

acatangiu Jan 5, 2026 •

edited

Loading

Klapeyron Jun 3, 2025 •

edited

Loading

Klapeyron commented Jun 16, 2025 •

edited

Loading