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

Paired-key Crypto Scheme #14705

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

drskalman
Copy link
Contributor

BEEFY needs two cryptographic keys at the same time. Validators should sign BEEFY payload using both ECDSA and BLS key. The network will gossip a payload which contains a valid ECDSA key. The prover nodes aggregate the BLS keys if aggregation fails to verifies the validator which provided a valid ECDSA signature but an invalid BLS signature is subject to slashing.

As such BEEFY session should be initiated with both key. Currently there is no straight forward way of doing so, beside having a session with RuntimeApp corresponding to a crypto scheme contains both keys.

This pull request implement a generic paired_crypto scheme as well as implementing it for (ECDSA, BLS) pair.

Polkadot companion: (if applicable)

Cumulus companion: (if applicable)

Checklist

  • My PR includes a detailed description as outlined in the "Description" section above
  • My PR follows the labeling requirements of this project (at minimum one label for each A, B, C and D required)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • If this PR alters any external APIs or interfaces used by Polkadot, the corresponding Polkadot PR is ready as well as the corresponding Cumulus PR (optional)

✄ -----------------------------------------------------------------------------

@drskalman drskalman changed the title Skalman paired scheme crypto Paired-ky Crypto Scheme Aug 3, 2023
Comment on lines 70 to 78
pub struct PublicWithInner<LeftPublic: PublicKeyBound, RightPublic: PublicKeyBound, const LEFT_PLUS_RIGHT_SIZE: usize> {
inner: [u8; LEFT_PLUS_RIGHT_SIZE],
_phantom: PhantomData<fn() -> (LeftPublic, RightPublic)>,
}

pub struct Public<LeftPublic: PublicKeyBound, RightPublic: PublicKeyBound> {
left_public : LeftPublic,
right_public: RightPublic,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davxy : could you tell me which way of implementing 'paired_cryptodo you prefer? In my opinion the second is more clean and more expressive but it results in copying duringtryForm<[u8]>andAsMut<[u8]>operations. On the other hand, if we go with the inner approach, it could be that when we try to use the subkeys, deserializinginner` results in copying as well as we don't have control on the implementation of subkey types.

Copy link
Member

Choose a reason for hiding this comment

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

IMO the second representation is better (as you said).
In this way we abstract the internal representation of the components.

It'd be interesting to try to generalize to Tuples (using https://docs.rs/impl-trait-for-tuples/0.2.2/impl_trait_for_tuples/) instead of defining the single types.
In this way we'd have the implementation of Public, Pair, etc for any tuple of types implementing Public, Pair, etc...

@drskalman drskalman changed the title Paired-ky Crypto Scheme Paired-key Crypto Scheme Aug 3, 2023
- unsucessfuly try to implement ByteArray for paired crypto Public
Comment on lines 101 to 105
// impl<LeftPublic: PublicKeyBound, RightPublic: PublicKeyBound, const LEFT_LEN: usize, const RIGHT_LEN: usize> AsMut<[u8]> for Public<LeftPublic, RightPublic, LEFT_LEN, RIGHT_LEN> {
// fn as_mut(&mut self) -> &mut [u8] {
// &mut [self.0.as_mut(), self.1.as_mut()].concat()
// }
// }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davxy it seems that the second representation (LeftPublic: PublicBound, RightPublic: PublicBound) does not work because we can not implement AsRef<u8> and AsMut<u8> as we don't have a consecutive representation of the sub-elements as these people suggests. Should I go back to the first solution:

pub struct PublicWithInner<LeftPublic: PublicKeyBound, RightPublic: PublicKeyBound, const LEFT_PLUS_RIGHT_SIZE: usize> {
    inner: [u8; LEFT_PLUS_RIGHT_SIZE],
 	_phantom: PhantomData<fn() -> (LeftPublic, RightPublic)>,
}

or should I keep both representations at the same time:

pub struct Public<LeftPublic: PublicKeyBound, RightPublic: PublicKeyBound, const LEFT_PLUS_RIGHT_SIZE: usize> {
    left_public : LeftPublic,
    right_public: RightPublic,
    inner: [u8; LEFT_PLUS_RIGHT_SIZE],

}

or you have a solution to building non-ephemeral slice in a function that works here?

🙏

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3427364

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants