Skip to content
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

Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto #1815

Conversation

drskalman
Copy link
Contributor

Next step in process of making BEEFY being able to generate both ECDSA and BLS signature after #1705. It allows BEEFY to use a pair of ECDSA and BLS key as a AuthorityId.

drskalman and others added 25 commits September 25, 2023 15:29
…ssfuly try to implement ByteArray for paired crypto Public
…me of aux traits for `paired_crypto::Signature`
…_crypto.rs`.

- put serialize and descerialze under `serde` feature instead of std.
- in `primitives/core/src/bls.rs`.
- fix documentation in `primitives/core/src/bls.rs`.
- cargo fmt pair_crypto.rs
…and SCALE.

Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Davide Galassi <davxy@datawok.net>
- Remove `SignaturePair` trait.
- Skip encoding redundant copy of paired::Public and Signature and implement Decode
…rypto type

- Implement ecdsa_bls37_crypto for BEEFY primitives.
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-kusama-bridge/2971/6

@Lederstrumpf Lederstrumpf added the T0-node This PR/Issue is related to the topic “node”. label Oct 17, 2023
@davxy
Copy link
Member

davxy commented Oct 19, 2023

@drskalman Can you remind me what is the condition to remove experimental? We want to switch to 381?

Copy link
Member

@davxy davxy left a comment

Choose a reason for hiding this comment

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

At some point we need to refactor all this crypto stuff. Too much duplicated code


Edit: implementations of new stuff is missing from the local keystore implementation

@@ -140,6 +140,45 @@ pub mod bls_crypto {
}
}
}

/// BEEFY cryptographic types for (ECDSA,BLS) crypto pair
Copy link
Member

Choose a reason for hiding this comment

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

Why here you've not specified BLS12-377?
This will be changed to use 381?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly.

The BLS in the BLS12 curves and BLS in the signature are not the same thing though. (I think only the L in the BLS signature and in the BLS12 curves are the same person). You could do BLS signature with any other BLS friendly curves such as BN or BLS24. Here I just wanted to say that we are generating a BLS type signature without going into the technicality of which curve we are using under the hood.

Copy link
Member

@davxy davxy Oct 23, 2023

Choose a reason for hiding this comment

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

But here we are not defining types which are bounded by a generic pairing fiendly curve to produce BLS signatures. Here we are using a specific curve (which at the moment is bls12-377). And the same applies to bls_crypto module.

So maybe the question now is: is intentional to not be explicit about the curve in the modules names?

I'm not saying that this is not the right choice. I'm just curious

@davxy davxy self-requested a review October 19, 2023 07:32
@drskalman
Copy link
Contributor Author

Edit: implementations of new stuff is missing from the local keystore implementation

Done!

@drskalman
Copy link
Contributor Author

@drskalman Can you remind me what is the condition to remove experimental? We want to switch to 381?

This requires that we generate the Web3sum proofs on BW6-767 instead of what currently is don on BW6-767:

  • The APK proof library is currently hard coded on using BLS-377 and BW6-761.
  • BW6-761 scaler field is more Fourier transform friendly. We have a good enough Fourier transform for BW6-767 but I don't think it is merged upstream, tested, benchmarked etc.
  • We need to run a power of tau cermony for BW6-767 before we are able to use it.

Alterantively we could compute the APK Proof Snarks in non-native field but that need quite a bit of re-implementation of apk proof library.

I'm not sure how long it will take for either of these approaches to be implemented and I'm hoping while waiting for these to get implemented at least we could have a test relay on a testnet by using bls12-377 for now.

Copy link
Contributor

@Lederstrumpf Lederstrumpf left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@Lederstrumpf Lederstrumpf enabled auto-merge (squash) October 24, 2023 17:59
@Lederstrumpf Lederstrumpf merged commit fbd5777 into paritytech:master Oct 24, 2023
107 of 110 checks passed
ordian added a commit that referenced this pull request Oct 26, 2023
* master:
  Removed TODO from test-case for hard-coded delivery fee estimation (#2042)
  Expose collection attributes from `Inspect` trait (#1914)
  `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897)
  [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023)
  Sort the benchmarks before listing them (#2026)
  publish pallet-root-testing (#2017)
  Contracts: Add benchmarks to include files (#2022)
  Small optimisation to `--profile dev` wasm builds (#1851)
  basic-authorship: Improve time recording and logging (#2010)
  Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815)
  [ci] Run check-rust-feature-propagation in pr and master (#2012)
  Improve features dev-ex (#1831)
  Remove obsolete comment. (#2008)
ordian added a commit that referenced this pull request Oct 26, 2023
* tsv-disabling:
  Removed TODO from test-case for hard-coded delivery fee estimation (#2042)
  Expose collection attributes from `Inspect` trait (#1914)
  `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897)
  [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023)
  Sort the benchmarks before listing them (#2026)
  publish pallet-root-testing (#2017)
  Contracts: Add benchmarks to include files (#2022)
  Small optimisation to `--profile dev` wasm builds (#1851)
  basic-authorship: Improve time recording and logging (#2010)
  Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815)
  [ci] Run check-rust-feature-propagation in pr and master (#2012)
  Improve features dev-ex (#1831)
  Remove obsolete comment. (#2008)
ordian added a commit that referenced this pull request Oct 26, 2023
* tsv-disabling: (36 commits)
  Removed TODO from test-case for hard-coded delivery fee estimation (#2042)
  Expose collection attributes from `Inspect` trait (#1914)
  `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897)
  [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023)
  Sort the benchmarks before listing them (#2026)
  publish pallet-root-testing (#2017)
  Contracts: Add benchmarks to include files (#2022)
  Small optimisation to `--profile dev` wasm builds (#1851)
  basic-authorship: Improve time recording and logging (#2010)
  Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815)
  [ci] Run check-rust-feature-propagation in pr and master (#2012)
  Improve features dev-ex (#1831)
  Remove obsolete comment. (#2008)
  Refactor candidates test in paras_inherent (#2004)
  PVF: Add worker check during tests and benches (#1771)
  Bump actions/setup-node from 3.8.1 to 4.0.0 (#1997)
  polkadot: enable tikv-jemallocator/unprefixed_malloc_on_supported_platforms (#2002)
  Make `IdentityInfo` generic in `pallet-identity` (#1661)
  Ensure correct variant count in `Runtime[Hold/Freeze]Reason` (#1900)
  `CheckWeight`: Add more logging (#1996)
  ...
s0me0ne-unkn0wn pushed a commit that referenced this pull request Oct 29, 2023
…1815)

Next step in process of making BEEFY being able to generate both ECDSA
and BLS signature after #1705. It allows BEEFY to use a pair of ECDSA
and BLS key as a AuthorityId.

---------

Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Robert Hambrock <roberthambrock@gmail.com>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-kusama-bridge/2971/7

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…aritytech#1815)

Next step in process of making BEEFY being able to generate both ECDSA
and BLS signature after paritytech#1705. It allows BEEFY to use a pair of ECDSA
and BLS key as a AuthorityId.

---------

Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Robert Hambrock <roberthambrock@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants