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

feat: broadcast barrier #4207

Merged
merged 38 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
d127712
feat: broadcast pause
ramizhasan111 Nov 6, 2023
5af74d7
feat: removed key locking and non-optimistic activation
ramizhasan111 Nov 7, 2023
d777537
fix: tests
ramizhasan111 Nov 8, 2023
078cded
Merge branch 'main' into feat/broadcast-pause
ramizhasan111 Nov 8, 2023
51c49b5
fix: bouncer prettier check
ramizhasan111 Nov 8, 2023
c2a2e20
feat: broadcast pause unit test
ramizhasan111 Nov 9, 2023
fe789b9
chore: comments
ramizhasan111 Nov 14, 2023
b42ef29
chore: test
ramizhasan111 Nov 14, 2023
06b7df2
chore: addressed comments
ramizhasan111 Nov 14, 2023
4d445bb
feat: use same broadcast_id for tx, chain-specific broadcast barriers
ramizhasan111 Nov 17, 2023
56dbec5
chore: reinstate test
ramizhasan111 Nov 20, 2023
2d3cdc5
feat: storage migration
ramizhasan111 Nov 20, 2023
1bd142b
Merge branch 'main' into feat/broadcast-pause
ramizhasan111 Nov 20, 2023
41a7f08
fix: check for all broadcasts before the barrier
ramizhasan111 Nov 21, 2023
24de28d
feat: more tests, test fixes
ramizhasan111 Nov 22, 2023
0852b66
chore: addressed comments
ramizhasan111 Nov 22, 2023
7efd2e5
feat: on_sig_ready call migration
ramizhasan111 Nov 29, 2023
dd513d1
Merge branch 'main' into feat/broadcast-pause
ramizhasan111 Nov 29, 2023
51f014b
fix: vec import for tr-runtime
ramizhasan111 Nov 29, 2023
d9a0be7
fix: use only specific key for signing
ramizhasan111 Nov 29, 2023
ba13b6b
chore: addressed comments
ramizhasan111 Nov 29, 2023
c267707
fix: only add barriers if there are already pending txs before it
ramizhasan111 Nov 30, 2023
c9daf2d
feat: remove aborted broadcasts from pending
ramizhasan111 Dec 1, 2023
20fd42a
chore: remove CurrentKey variant in RequestType
ramizhasan111 Dec 1, 2023
455ebcf
Merge branch 'main' into feat/broadcast-pause
dandanlen Dec 1, 2023
7ddeea6
fix: all tests passing (bouncer tbc)
dandanlen Dec 2, 2023
0c056b5
fix: remove defenisve_proof
dandanlen Dec 4, 2023
dd06205
fix: always remove barrier with pending broadcast
dandanlen Dec 4, 2023
6f3ff2a
chore: readability
dandanlen Dec 4, 2023
5ce1a17
fix: typo
dandanlen Dec 4, 2023
6abf06f
fix: use BTreeSet to avoid duplicates.
dandanlen Dec 4, 2023
8b46d1a
fix: check attempt count >= not ==
dandanlen Dec 4, 2023
7267a8b
test: test that the broadcast is aborted when all broadcasters attempt
kylezs Dec 4, 2023
4cb5d89
Merge branch 'main' into feat/broadcast-pause
dandanlen Dec 4, 2023
558a57a
fix: simplify signature refresh logic
dandanlen Dec 4, 2023
7a0ab45
fix: add PendingBroadcast migration
dandanlen Dec 4, 2023
43d44eb
fix: migrations
kylezs Dec 4, 2023
cd84ee0
fix: use correct spec version
kylezs Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions state-chain/cf-integration-tests/src/threshold_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl KeyUtils for EthKeyComponents {
}

pub struct ThresholdSigner<KeyComponents, SigVerification> {
previous_key_components: Option<KeyComponents>,
key_components: KeyComponents,
proposed_key_components: Option<KeyComponents>,
_phantom: PhantomData<SigVerification>,
Expand All @@ -90,8 +91,14 @@ where
if key == current_key {
return self.key_components.sign(message)
}
let next_key = self.proposed_key_components.as_ref().unwrap().agg_key();
if key == next_key {
if self.previous_key_components.is_some() &&
self.previous_key_components.as_ref().unwrap().agg_key() == key
{
return self.previous_key_components.as_ref().unwrap().sign(message)
}
if self.proposed_key_components.is_some() &&
self.proposed_key_components.as_ref().unwrap().agg_key() == key
{
self.proposed_key_components.as_ref().unwrap().sign(message)
} else {
panic!("Unknown key");
Expand All @@ -108,6 +115,7 @@ where
// Rotate to the current proposed key and clear the proposed key
pub fn use_proposed_key(&mut self) {
if self.proposed_key_components.is_some() {
self.previous_key_components = Some(self.key_components.clone());
self.key_components =
self.proposed_key_components.as_ref().expect("No key has been proposed").clone();
self.proposed_key_components = None;
Expand All @@ -120,6 +128,7 @@ pub type EthThresholdSigner = ThresholdSigner<EthKeyComponents, SchnorrVerificat
impl Default for EthThresholdSigner {
fn default() -> Self {
ThresholdSigner {
previous_key_components: None,
key_components: EthKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH),
proposed_key_components: None,
_phantom: PhantomData,
Expand All @@ -134,6 +143,7 @@ pub type DotThresholdSigner = ThresholdSigner<DotKeyComponents, PolkadotSignatur
impl Default for DotThresholdSigner {
fn default() -> Self {
Self {
previous_key_components: None,
key_components: DotKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH),
proposed_key_components: None,
_phantom: PhantomData,
Expand Down Expand Up @@ -171,6 +181,7 @@ pub type BtcThresholdSigner = ThresholdSigner<BtcKeyComponents, btc::Signature>;
impl Default for BtcThresholdSigner {
fn default() -> Self {
Self {
previous_key_components: None,
key_components: BtcKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH),
proposed_key_components: None,
_phantom: PhantomData,
Expand Down
6 changes: 0 additions & 6 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,6 @@ impl ChainCrypto for PolkadotCrypto {
EncodedPolkadotPayload(Blake2_256::hash(&agg_key.aliased_ref()[..]).to_vec())
}

/// Once authored, polkadot extrinsic must be signed by the key whose nonce was incorporated
/// into the extrinsic.
fn sign_with_specific_key() -> bool {
true
}

fn maybe_broadcast_barriers_on_rotation(
rotation_broadcast_id: BroadcastId,
) -> Vec<BroadcastId> {
Expand Down
8 changes: 0 additions & 8 deletions state-chain/chains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,6 @@ pub trait ChainCrypto {
true
}

/// Determines whether threshold signatures are made with a specific fixed key, or whether the
/// key is refreshed if we need to retry the signature.
///
/// By default, this is true for Utxo-based chains, false otherwise.
fn sign_with_specific_key() -> bool {
Self::UtxoChain::get()
}

/// Determines whether the chain crypto supports key handover.
///
/// By default, this is true for Utxo-based chains, false otherwise.
Expand Down
19 changes: 0 additions & 19 deletions state-chain/chains/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ pub enum ChainChoice {

thread_local! {
static MOCK_KEY_HANDOVER_IS_REQUIRED: RefCell<bool> = RefCell::new(true);
static MOCK_SIGN_WITH_SPECIFIC_KEY: RefCell<bool> = RefCell::new(false);
static MOCK_VALID_METADATA: RefCell<bool> = RefCell::new(true);
static MOCK_BROADCAST_BARRIERS: RefCell<ChainChoice> = RefCell::new(ChainChoice::Ethereum);
}
Expand Down Expand Up @@ -56,20 +55,6 @@ impl Get<ChainChoice> for MockBroadcastBarriers {
}
}

pub struct MockFixedKeySigningRequests;

impl MockFixedKeySigningRequests {
pub fn set(value: bool) {
MOCK_SIGN_WITH_SPECIFIC_KEY.with(|v| *v.borrow_mut() = value);
}
}

impl Get<bool> for MockFixedKeySigningRequests {
fn get() -> bool {
MOCK_SIGN_WITH_SPECIFIC_KEY.with(|v| *v.borrow())
}
}

#[derive(Debug, Clone, Default, PartialEq, Eq, Encode, Decode, TypeInfo)]
pub struct MockEthereumTransactionMetadata;

Expand Down Expand Up @@ -267,10 +252,6 @@ impl ChainCrypto for MockEthereumChainCrypto {
new_key != &BAD_AGG_KEY_POST_HANDOVER
}

fn sign_with_specific_key() -> bool {
MockFixedKeySigningRequests::get()
}

fn key_handover_is_required() -> bool {
MockKeyHandoverIsRequired::get()
}
Expand Down
19 changes: 6 additions & 13 deletions state-chain/pallets/cf-threshold-signature/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,7 @@ pub mod pallet {
request_id,
attempt_count.wrapping_add(1),
payload,
if <T::TargetChainCrypto as ChainCrypto>::sign_with_specific_key() {
RequestType::SpecificKey(key, epoch)
} else {
RequestType::CurrentKey
},
RequestType::SpecificKey(key, epoch),
));
Event::<T, I>::RetryRequested { request_id, ceremony_id }
},
Expand Down Expand Up @@ -810,14 +806,11 @@ where
type ValidatorId = T::ValidatorId;

fn request_signature(payload: PayloadFor<T, I>) -> RequestId {
let request_type = if <T::TargetChainCrypto as ChainCrypto>::sign_with_specific_key() {
T::KeyProvider::active_epoch_key().defensive_map_or_else(
|| RequestType::CurrentKey,
|EpochKey { key, epoch_index, .. }| RequestType::SpecificKey(key, epoch_index),
)
} else {
RequestType::CurrentKey
};
let request_type = T::KeyProvider::active_epoch_key().defensive_map_or_else(
|| RequestType::SpecificKey(Default::default(), Default::default()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we use RequestType::CurrentKey it will fail more gracefully (it will emit CurrentKeyUnavailable and retry with CurrentKey until one is available).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the CurrentKey variant in the RequestType since we dont use it anymore. The ceremonies will always be signed with key active at the time of threshold signature request since do optimistic rotation for all chains now and every ceremony has a key associated to it.

|EpochKey { key, epoch_index, .. }| RequestType::SpecificKey(key, epoch_index),
);

Self::inner_request_signature(payload, request_type)
}

Expand Down
4 changes: 1 addition & 3 deletions state-chain/pallets/cf-threshold-signature/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
self as pallet_cf_threshold_signature, mock::*, AttemptCount, CeremonyContext, CeremonyId,
Error, PalletOffence, RequestContext, RequestId, ThresholdSignatureResponseTimeout,
};
use cf_chains::mocks::{MockAggKey, MockEthereumChainCrypto, MockFixedKeySigningRequests};
use cf_chains::mocks::{MockAggKey, MockEthereumChainCrypto};
use cf_traits::{
mocks::{key_provider::MockKeyProvider, signer_nomination::MockNominator},
AsyncResult, Chainflip, EpochKey, KeyProvider, ThresholdSigner,
Expand Down Expand Up @@ -479,8 +479,6 @@ fn test_retries_for_immutable_key() {
..
} = EthereumThresholdSigner::pending_ceremonies(ceremony_id).unwrap();

MockFixedKeySigningRequests::set(true);

// Pretend the key has been updated to the next one.
MockKeyProvider::<MockEthereumChainCrypto>::add_key(MockAggKey(*b"NEXT"));

Expand Down
50 changes: 0 additions & 50 deletions state-chain/runtime/src/chainflip/broadcaster.rs

This file was deleted.

Loading