From c1271c094f115f3904cddf2a50953a0951322949 Mon Sep 17 00:00:00 2001 From: Marcello Date: Mon, 26 Feb 2024 14:04:29 +0100 Subject: [PATCH] fix: Wait for ThresholdSignature success before switching to NewKeysActivated (#4534) * track tss request id before completing rotation * add unit test to check the correct flow is followed when tss is required --- .../cf-integration-tests/src/authorities.rs | 3 ++ .../cf-integration-tests/src/broadcasting.rs | 2 +- .../pallets/cf-broadcast/src/benchmarking.rs | 2 +- state-chain/pallets/cf-broadcast/src/lib.rs | 22 ++++---- state-chain/pallets/cf-broadcast/src/tests.rs | 6 +-- state-chain/pallets/cf-emissions/src/mock.rs | 10 ++-- .../pallets/cf-environment/src/mock.rs | 8 ++- state-chain/pallets/cf-funding/src/lib.rs | 2 +- state-chain/pallets/cf-funding/src/mock.rs | 10 ++-- .../pallets/cf-ingress-egress/src/lib.rs | 3 +- .../cf-threshold-signature/src/key_rotator.rs | 51 +++++++++++++++---- .../pallets/cf-threshold-signature/src/lib.rs | 1 + .../cf-threshold-signature/src/mock.rs | 18 ++++++- .../cf-threshold-signature/src/tests.rs | 35 ++++++++++++- state-chain/pallets/cf-vaults/src/mock.rs | 10 ++-- state-chain/pallets/cf-vaults/src/tests.rs | 8 +-- .../pallets/cf-vaults/src/vault_activator.rs | 26 ++++++++-- state-chain/traits/src/lib.rs | 18 +++++-- state-chain/traits/src/mocks/broadcaster.rs | 25 +++++---- 19 files changed, 200 insertions(+), 60 deletions(-) diff --git a/state-chain/cf-integration-tests/src/authorities.rs b/state-chain/cf-integration-tests/src/authorities.rs index 3948b6f11a..aa9bb76397 100644 --- a/state-chain/cf-integration-tests/src/authorities.rs +++ b/state-chain/cf-integration-tests/src/authorities.rs @@ -113,6 +113,9 @@ fn authority_rotates_with_correct_sequence() { RotationPhase::ActivatingKeys(..) )); + // Wait for an extra block to allow TSS to complete, we switch to RotationComplete once + // that's done + testnet.move_forward_blocks(1); assert_eq!( AllVaults::status(), AsyncResult::Ready(KeyRotationStatusOuter::RotationComplete), diff --git a/state-chain/cf-integration-tests/src/broadcasting.rs b/state-chain/cf-integration-tests/src/broadcasting.rs index 3e0ebfd369..e10a7d1314 100644 --- a/state-chain/cf-integration-tests/src/broadcasting.rs +++ b/state-chain/cf-integration-tests/src/broadcasting.rs @@ -42,7 +42,7 @@ fn bitcoin_broadcast_delay_works() { ) .unwrap(); - let broadcast_id = + let (broadcast_id, _) = >::threshold_sign_and_broadcast( bitcoin_call, ); diff --git a/state-chain/pallets/cf-broadcast/src/benchmarking.rs b/state-chain/pallets/cf-broadcast/src/benchmarking.rs index 4ee9967437..5f8e8652ab 100644 --- a/state-chain/pallets/cf-broadcast/src/benchmarking.rs +++ b/state-chain/pallets/cf-broadcast/src/benchmarking.rs @@ -128,7 +128,7 @@ mod benchmarks { #[benchmark] fn start_next_broadcast_attempt() { - let broadcast_id = Pallet::::threshold_sign_and_broadcast( + let (broadcast_id, _) = Pallet::::threshold_sign_and_broadcast( BenchmarkValue::benchmark_value(), None, |_| None, diff --git a/state-chain/pallets/cf-broadcast/src/lib.rs b/state-chain/pallets/cf-broadcast/src/lib.rs index c133e62429..88ec02df18 100644 --- a/state-chain/pallets/cf-broadcast/src/lib.rs +++ b/state-chain/pallets/cf-broadcast/src/lib.rs @@ -664,7 +664,7 @@ impl, I: 'static> Pallet { maybe_failed_callback_generator: impl FnOnce( BroadcastId, ) -> Option<>::BroadcastCallable>, - ) -> BroadcastId { + ) -> (BroadcastId, ThresholdSignatureRequestId) { let broadcast_id = Self::next_broadcast_id(); PendingBroadcasts::::append(broadcast_id); @@ -676,9 +676,7 @@ impl, I: 'static> Pallet { RequestFailureCallbacks::::insert(broadcast_id, callback); } - let _threshold_signature_id = Self::threshold_sign(api_call, broadcast_id, true); - - broadcast_id + (broadcast_id, Self::threshold_sign(api_call, broadcast_id, true)) } /// Signs a API call, use `Call::on_signature_ready` as the callback, and returns the signature @@ -922,7 +920,9 @@ impl, I: 'static> Broadcaster for Pallet { type ApiCall = T::ApiCall; type Callback = >::BroadcastCallable; - fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { Self::threshold_sign_and_broadcast(api_call, None, |_| None) } @@ -931,7 +931,7 @@ impl, I: 'static> Broadcaster for Pallet { success_callback: Option, failed_callback_generator: impl FnOnce(BroadcastId) -> Option, ) -> BroadcastId { - Self::threshold_sign_and_broadcast(api_call, success_callback, failed_callback_generator) + Self::threshold_sign_and_broadcast(api_call, success_callback, failed_callback_generator).0 } fn threshold_sign(api_call: Self::ApiCall) -> (BroadcastId, ThresholdSignatureRequestId) { @@ -949,8 +949,11 @@ impl, I: 'static> Broadcaster for Pallet { Self::clean_up_broadcast_storage(broadcast_id); } - fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { - let broadcast_id = >::threshold_sign_and_broadcast(api_call); + fn threshold_sign_and_broadcast_rotation_tx( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { + let (broadcast_id, request_id) = + >::threshold_sign_and_broadcast(api_call); if let Some(earliest_pending_broadcast_id) = PendingBroadcasts::::get() .first() @@ -962,6 +965,7 @@ impl, I: 'static> Broadcaster for Pallet { } } } - broadcast_id + + (broadcast_id, request_id) } } diff --git a/state-chain/pallets/cf-broadcast/src/tests.rs b/state-chain/pallets/cf-broadcast/src/tests.rs index 153793fb94..35c0cc57c8 100644 --- a/state-chain/pallets/cf-broadcast/src/tests.rs +++ b/state-chain/pallets/cf-broadcast/src/tests.rs @@ -435,7 +435,7 @@ fn threshold_sign_and_broadcast_with_callback() { tx_out_id: MOCK_TRANSACTION_OUT_ID, }; - let broadcast_id = + let (broadcast_id, _) = Broadcaster::threshold_sign_and_broadcast(api_call.clone(), Some(MockCallback), |_| { None }); @@ -544,7 +544,7 @@ fn callback_is_called_upon_broadcast_failure() { sig: Default::default(), tx_out_id: MOCK_TRANSACTION_OUT_ID, }; - let broadcast_id = + let (broadcast_id, _) = Broadcaster::threshold_sign_and_broadcast(api_call.clone(), None, |_| { Some(MockCallback) }); @@ -883,7 +883,7 @@ fn initiate_and_sign_broadcast( api_call: &MockApiCall, tx_type: TxType, ) -> BroadcastId { - let broadcast_id = match tx_type { + let (broadcast_id, _) = match tx_type { TxType::Normal => >::TargetChain, >>::threshold_sign_and_broadcast((*api_call).clone()), diff --git a/state-chain/pallets/cf-emissions/src/mock.rs b/state-chain/pallets/cf-emissions/src/mock.rs index cc6d495ab6..af410e1732 100644 --- a/state-chain/pallets/cf-emissions/src/mock.rs +++ b/state-chain/pallets/cf-emissions/src/mock.rs @@ -169,9 +169,11 @@ impl Broadcaster for MockBroadcast { type ApiCall = MockUpdateFlipSupply; type Callback = MockCallback; - fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { Self::call(api_call); - 1 + (1, 1) } fn threshold_sign_and_broadcast_with_callback( @@ -182,7 +184,9 @@ impl Broadcaster for MockBroadcast { unimplemented!() } - fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast_rotation_tx( + _api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { unimplemented!() } diff --git a/state-chain/pallets/cf-environment/src/mock.rs b/state-chain/pallets/cf-environment/src/mock.rs index 9e0d657d1b..584772405c 100644 --- a/state-chain/pallets/cf-environment/src/mock.rs +++ b/state-chain/pallets/cf-environment/src/mock.rs @@ -101,7 +101,9 @@ impl Broadcaster for MockPolkadotBroadcaster { type ApiCall = MockCreatePolkadotVault; type Callback = MockCallback; - fn threshold_sign_and_broadcast(_api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast( + _api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { unimplemented!() } @@ -113,7 +115,9 @@ impl Broadcaster for MockPolkadotBroadcaster { unimplemented!() } - fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast_rotation_tx( + _api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { unimplemented!() } diff --git a/state-chain/pallets/cf-funding/src/lib.rs b/state-chain/pallets/cf-funding/src/lib.rs index a855772e3b..c83ff12baf 100644 --- a/state-chain/pallets/cf-funding/src/lib.rs +++ b/state-chain/pallets/cf-funding/src/lib.rs @@ -483,7 +483,7 @@ pub mod pallet { Self::deposit_event(Event::RedemptionRequested { account_id, amount: redeem_amount, - broadcast_id: T::Broadcaster::threshold_sign_and_broadcast(call), + broadcast_id: T::Broadcaster::threshold_sign_and_broadcast(call).0, expiry_time: contract_expiry, }); } else { diff --git a/state-chain/pallets/cf-funding/src/mock.rs b/state-chain/pallets/cf-funding/src/mock.rs index 396ec52a82..424f20beeb 100644 --- a/state-chain/pallets/cf-funding/src/mock.rs +++ b/state-chain/pallets/cf-funding/src/mock.rs @@ -156,11 +156,13 @@ impl Broadcaster for MockBroadcaster { type ApiCall = MockRegisterRedemption; type Callback = MockCallback; - fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { REDEMPTION_BROADCAST_REQUESTS.with(|cell| { cell.borrow_mut().push(api_call.amount); }); - 0 + (0, 0) } fn threshold_sign_and_broadcast_with_callback( @@ -171,7 +173,9 @@ impl Broadcaster for MockBroadcaster { unimplemented!() } - fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast_rotation_tx( + _api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { unimplemented!() } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 24845c8096..ad6d1a9860 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -608,7 +608,8 @@ pub mod pallet { if let Ok(egress_transaction) = >::consolidate_utxos() { - let broadcast_id = T::Broadcaster::threshold_sign_and_broadcast(egress_transaction); + let (broadcast_id, _) = + T::Broadcaster::threshold_sign_and_broadcast(egress_transaction); Self::deposit_event(Event::::UtxoConsolidation { broadcast_id }); Self::deposit_event(Event::::BatchBroadcastRequested { broadcast_id, diff --git a/state-chain/pallets/cf-threshold-signature/src/key_rotator.rs b/state-chain/pallets/cf-threshold-signature/src/key_rotator.rs index 9477724f04..88f01f4317 100644 --- a/state-chain/pallets/cf-threshold-signature/src/key_rotator.rs +++ b/state-chain/pallets/cf-threshold-signature/src/key_rotator.rs @@ -147,14 +147,26 @@ impl, I: 'static> KeyRotator for Pallet { ), }, KeyRotationStatusVariant::AwaitingActivation => { - let new_public_key = match PendingKeyRotation::::get() { - Some(KeyRotationStatus::AwaitingActivation { new_public_key }) => + let (new_public_key, maybe_request_id) = match PendingKeyRotation::::get() + { + Some(KeyRotationStatus::AwaitingActivation { + request_id, new_public_key, + }) => (new_public_key, request_id), _ => unreachable!( "Unreachable because we are in the branch for the AwaitingActivation variant." ), }; + if let Some(request_id) = maybe_request_id { + // After the ceremony completes, it is consumed and Void is left + // behind. At this point we are sure the ceremony existed and succeded, we + // can activate the key + if Signature::::get(request_id) == AsyncResult::Void { + T::VaultActivator::activate_key(); + }; + }; + let status = T::VaultActivator::status() .replace_inner(KeyRotationStatusOuter::RotationComplete); if status.is_ready() { @@ -188,17 +200,34 @@ impl, I: 'static> KeyRotator for Pallet { PendingKeyRotation::::get() { let maybe_active_epoch_key = Self::active_epoch_key(); - T::VaultActivator::activate( + + match T::VaultActivator::start_key_activation( new_public_key, maybe_active_epoch_key.map(|EpochKey { key, .. }| key), - ); - - if maybe_active_epoch_key.is_some() { - Self::activate_new_key(new_public_key); - } else { - PendingKeyRotation::::put(KeyRotationStatus::::AwaitingActivation { - new_public_key, - }); + ) { + // If a request_id was returned we need to wait for the signing request to complete + // before ending the rotation succesfully + Some(request_id) => { + PendingKeyRotation::::put( + KeyRotationStatus::::AwaitingActivation { + request_id: Some(request_id), + new_public_key, + }, + ); + }, + // if none was returned no ceremony is required and we can already complete the + // rotation/wait for governance extrinsic to activate the vault + None => + if maybe_active_epoch_key.is_some() { + Self::activate_new_key(new_public_key); + } else { + PendingKeyRotation::::put( + KeyRotationStatus::::AwaitingActivation { + request_id: None, + new_public_key, + }, + ); + }, } } else { log::error!("Vault activation called during wrong state."); diff --git a/state-chain/pallets/cf-threshold-signature/src/lib.rs b/state-chain/pallets/cf-threshold-signature/src/lib.rs index 0e6e763538..364a3c5196 100644 --- a/state-chain/pallets/cf-threshold-signature/src/lib.rs +++ b/state-chain/pallets/cf-threshold-signature/src/lib.rs @@ -141,6 +141,7 @@ pub enum KeyRotationStatus, I: 'static = ()> { }, /// We are waiting for the key to be updated on the contract, and witnessed by the network. AwaitingActivation { + request_id: Option, new_public_key: AggKeyFor, }, /// The key has been successfully updated on the external chain, and/or funds rotated to new diff --git a/state-chain/pallets/cf-threshold-signature/src/mock.rs b/state-chain/pallets/cf-threshold-signature/src/mock.rs index 5ba973a690..6ac9390f89 100644 --- a/state-chain/pallets/cf-threshold-signature/src/mock.rs +++ b/state-chain/pallets/cf-threshold-signature/src/mock.rs @@ -8,7 +8,10 @@ use cf_chains::{ mocks::{MockAggKey, MockEthereumChainCrypto, MockThresholdSignature}, ChainCrypto, }; -use cf_primitives::{AuthorityCount, CeremonyId, FlipBalance, FLIPPERINOS_PER_FLIP, GENESIS_EPOCH}; +use cf_primitives::{ + AuthorityCount, CeremonyId, FlipBalance, ThresholdSignatureRequestId, FLIPPERINOS_PER_FLIP, + GENESIS_EPOCH, +}; use cf_traits::{ impl_mock_chainflip, impl_mock_runtime_safe_mode, mocks::{cfe_interface_mock::MockCfeInterface, signer_nomination::MockNominator}, @@ -188,7 +191,14 @@ impl pallet_cf_threshold_signature::Config for Test { pub struct MockVaultActivator; impl VaultActivator for MockVaultActivator { type ValidatorId = ::ValidatorId; - fn activate(_new_key: MockAggKey, _maybe_old_key: Option) {} + fn start_key_activation( + _new_key: MockAggKey, + _maybe_old_key: Option, + ) -> Option { + VAULT_ACTIVATION_STATUS.with(|value| *(value.borrow_mut()) = AsyncResult::Pending); + let ceremony_id = current_ceremony_id(); + Some(ceremony_id as u32) + } fn status() -> AsyncResult<()> { VAULT_ACTIVATION_STATUS.with(|value| *value.borrow()) @@ -198,6 +208,10 @@ impl VaultActivator for MockVaultActivator { fn set_status(outcome: AsyncResult<()>) { VAULT_ACTIVATION_STATUS.with(|value| *(value.borrow_mut()) = outcome) } + + fn activate_key() { + VAULT_ACTIVATION_STATUS.with(|value| *(value.borrow_mut()) = AsyncResult::Ready(())) + } } impl MockVaultActivator { diff --git a/state-chain/pallets/cf-threshold-signature/src/tests.rs b/state-chain/pallets/cf-threshold-signature/src/tests.rs index c33e0f0582..192ae8d24b 100644 --- a/state-chain/pallets/cf-threshold-signature/src/tests.rs +++ b/state-chain/pallets/cf-threshold-signature/src/tests.rs @@ -18,7 +18,7 @@ use cf_traits::{ signer_nomination::MockNominator, }, AccountRoleRegistry, AsyncResult, Chainflip, EpochInfo, EpochKey, KeyProvider, - KeyRotationStatusOuter, KeyRotator, SetSafeMode, + KeyRotationStatusOuter, KeyRotator, SetSafeMode, VaultActivator, }; pub use frame_support::traits::Get; @@ -1632,6 +1632,7 @@ mod key_rotation { BlockHeightProvider::::set_block_height(HANDOVER_ACTIVATION_BLOCK); EthereumThresholdSigner::activate_keys(); + EthereumThresholdSigner::status(); assert!(matches!( PendingKeyRotation::::get().unwrap(), @@ -1674,6 +1675,7 @@ mod key_rotation { run_cfes_on_sc_events(&cfes); EthereumThresholdSigner::activate_keys(); + EthereumThresholdSigner::status(); }); final_checks(ext); @@ -1742,6 +1744,37 @@ mod key_rotation { do_full_key_rotation(); }); } + + #[test] + fn wait_for_activating_key_tss_before_completing_rotation() { + let ext = setup(Ok(NEW_AGG_PUB_KEY_POST_HANDOVER)).execute_with(|| { + // KeyHandoverComplete + PendingKeyRotation::::put(KeyRotationStatus::KeyHandoverComplete { + new_public_key: NEW_AGG_PUB_KEY_POST_HANDOVER, + }); + // Start ActivatingKeys + EthereumThresholdSigner::activate_keys(); + assert_eq!( + PendingKeyRotation::::get().unwrap(), + KeyRotationStatus::AwaitingActivation { + request_id: Some(4), + new_public_key: NEW_AGG_PUB_KEY_POST_HANDOVER + } + ); + + // Vault activation started and it is now pending + assert_eq!(MockVaultActivator::status(), AsyncResult::Pending); + + // Request is complete + assert_eq!(EthereumThresholdSigner::signature(4), AsyncResult::Void); + + // Proceed to complete activation + EthereumThresholdSigner::status(); + + assert_eq!(MockVaultActivator::status(), AsyncResult::Ready(())); + }); + final_checks(ext); + } } #[test] diff --git a/state-chain/pallets/cf-vaults/src/mock.rs b/state-chain/pallets/cf-vaults/src/mock.rs index 022b90d36d..5044ef29bb 100644 --- a/state-chain/pallets/cf-vaults/src/mock.rs +++ b/state-chain/pallets/cf-vaults/src/mock.rs @@ -138,9 +138,11 @@ impl Broadcaster for MockBroadcaster { type ApiCall = MockSetAggKeyWithAggKey; type Callback = MockCallback; - fn threshold_sign_and_broadcast(_api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast( + _api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { Self::send_broadcast(); - 1 + (1, 1) } fn threshold_sign_and_broadcast_with_callback( @@ -151,7 +153,9 @@ impl Broadcaster for MockBroadcaster { unimplemented!() } - fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast_rotation_tx( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { Self::threshold_sign_and_broadcast(api_call) } diff --git a/state-chain/pallets/cf-vaults/src/tests.rs b/state-chain/pallets/cf-vaults/src/tests.rs index 34e52b7bf6..427ac35ffe 100644 --- a/state-chain/pallets/cf-vaults/src/tests.rs +++ b/state-chain/pallets/cf-vaults/src/tests.rs @@ -42,7 +42,7 @@ fn test_vault_key_rotated_externally_triggers_code_red() { #[test] fn key_unavailable_on_activate_returns_governance_event() { new_test_ext_no_key().execute_with(|| { - VaultsPallet::activate(NEW_AGG_PUBKEY, None); + VaultsPallet::start_key_activation(NEW_AGG_PUBKEY, None); assert_last_event!(crate::Event::AwaitingGovernanceActivation { .. }); @@ -57,7 +57,7 @@ fn when_set_agg_key_with_agg_key_not_required_we_skip_to_completion() { new_test_ext().execute_with(|| { MockSetAggKeyWithAggKey::set_required(false); - VaultsPallet::activate(NEW_AGG_PUBKEY, Some(Default::default())); + VaultsPallet::start_key_activation(NEW_AGG_PUBKEY, Some(Default::default())); assert!(matches!( PendingVaultActivation::::get().unwrap(), @@ -70,8 +70,8 @@ fn when_set_agg_key_with_agg_key_not_required_we_skip_to_completion() { fn vault_start_block_number_is_set_correctly() { new_test_ext_no_key().execute_with(|| { BlockHeightProvider::::set_block_height(1000); - VaultsPallet::activate(NEW_AGG_PUBKEY, Some(Default::default())); - + VaultsPallet::start_key_activation(NEW_AGG_PUBKEY, Some(Default::default())); + VaultsPallet::activate_key(); assert_eq!( crate::VaultStartBlockNumbers::::get( MockEpochInfo::epoch_index().saturating_add(1) diff --git a/state-chain/pallets/cf-vaults/src/vault_activator.rs b/state-chain/pallets/cf-vaults/src/vault_activator.rs index e3dfa417d8..fd30bb6f83 100644 --- a/state-chain/pallets/cf-vaults/src/vault_activator.rs +++ b/state-chain/pallets/cf-vaults/src/vault_activator.rs @@ -1,5 +1,6 @@ use super::*; use cf_chains::SetAggKeyWithAggKeyError; +use cf_primitives::ThresholdSignatureRequestId; use cf_runtime_utilities::{log_or_panic, StorageDecodeVariant}; use cf_traits::{GetBlockHeight, VaultActivator}; @@ -18,26 +19,44 @@ impl, I: 'static> VaultActivator<::ChainCrypto> } } - fn activate(new_public_key: AggKeyFor, maybe_old_public_key: Option>) { + fn activate_key() { + Self::activate_new_key_for_chain(T::ChainTracking::get_block_height()); + } + + fn start_key_activation( + new_public_key: AggKeyFor, + maybe_old_public_key: Option>, + ) -> Option { if let Some(old_key) = maybe_old_public_key { match >::new_unsigned( Some(old_key), new_public_key, ) { Ok(activation_call) => { - T::Broadcaster::threshold_sign_and_broadcast_rotation_tx(activation_call); - Self::activate_new_key_for_chain(T::ChainTracking::get_block_height()); + // we need to sign and submit the rotation call + // reporting back the request_id of the tss such that we can complete the + // rotation when that request is completed + let (_, tss_request_id) = + T::Broadcaster::threshold_sign_and_broadcast_rotation_tx(activation_call); + // since vaults are activated only when the tss completes we need to initiate + // the activation + PendingVaultActivation::::put( + VaultActivationStatus::::AwaitingActivation { new_public_key }, + ); + Some(tss_request_id) }, Err(SetAggKeyWithAggKeyError::NotRequired) => { // This can happen if, for example, on a utxo chain there are no funds that // need to be swept. Self::activate_new_key_for_chain(T::ChainTracking::get_block_height()); + None }, Err(SetAggKeyWithAggKeyError::Failed) => { log_or_panic!( "Unexpected failure during {} vault activation.", ::NAME, ); + None }, } } else { @@ -46,6 +65,7 @@ impl, I: 'static> VaultActivator<::ChainCrypto> VaultActivationStatus::::AwaitingActivation { new_public_key }, ); Self::deposit_event(Event::::AwaitingGovernanceActivation { new_public_key }); + None } } diff --git a/state-chain/traits/src/lib.rs b/state-chain/traits/src/lib.rs index 1c493ba39d..922b68c887 100644 --- a/state-chain/traits/src/lib.rs +++ b/state-chain/traits/src/lib.rs @@ -195,7 +195,15 @@ pub trait VaultActivator { /// Activate key/s on particular chain/s. For example, setting the new key /// on the contract for a smart contract chain. - fn activate(new_key: C::AggKey, maybe_old_key: Option); + /// Can also complete the activation if we don't require a signing ceremony + fn start_key_activation( + new_key: C::AggKey, + maybe_old_key: Option, + ) -> Option; + + /// Final step of key activation which result in the vault activation (in case we need to wait + /// for the signing ceremony to complete) + fn activate_key(); #[cfg(feature = "runtime-benchmarks")] fn set_status(_outcome: AsyncResult<()>); @@ -497,7 +505,9 @@ pub trait Broadcaster { type Callback: UnfilteredDispatchable; /// Request a threshold signature and then build and broadcast the outbound api call. - fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId; + fn threshold_sign_and_broadcast( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId); /// Like `threshold_sign_and_broadcast` but also registers a callback to be dispatched when the /// signature accepted event has been witnessed. @@ -509,7 +519,9 @@ pub trait Broadcaster { /// Request a threshold signature and then build and broadcast the outbound api call /// specifically for a rotation tx.. - fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId; + fn threshold_sign_and_broadcast_rotation_tx( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId); /// Resign a call, and update the signature data storage, but do not broadcast. fn threshold_resign(broadcast_id: BroadcastId) -> Option; diff --git a/state-chain/traits/src/mocks/broadcaster.rs b/state-chain/traits/src/mocks/broadcaster.rs index a250271531..ecd80037fc 100644 --- a/state-chain/traits/src/mocks/broadcaster.rs +++ b/state-chain/traits/src/mocks/broadcaster.rs @@ -27,17 +27,22 @@ impl< type ApiCall = A; type Callback = C; - fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> cf_primitives::BroadcastId { + fn threshold_sign_and_broadcast( + api_call: Self::ApiCall, + ) -> (cf_primitives::BroadcastId, ThresholdSignatureRequestId) { Self::mutate_value(b"API_CALLS", |api_calls: &mut Option>| { let api_calls = api_calls.get_or_insert(Default::default()); api_calls.push(api_call); }); - let _ = Self::next_threshold_id(); - ::mutate_value(b"BROADCAST_ID", |v: &mut Option| { - let v = v.get_or_insert(0); - *v += 1; - *v - }) + let tss_request_id = Self::next_threshold_id(); + ( + ::mutate_value(b"BROADCAST_ID", |v: &mut Option| { + let v = v.get_or_insert(0); + *v += 1; + *v + }), + tss_request_id, + ) } fn threshold_sign_and_broadcast_with_callback( @@ -45,7 +50,7 @@ impl< success_callback: Option, failed_callback_generator: impl FnOnce(BroadcastId) -> Option, ) -> BroadcastId { - let id = >::threshold_sign_and_broadcast(api_call); + let (id, _) = >::threshold_sign_and_broadcast(api_call); if let Some(callback) = success_callback { Self::put_storage(b"SUCCESS_CALLBACKS", id, callback); } @@ -74,7 +79,9 @@ impl< /// Clean up storage data related to a broadcast ID. fn clean_up_broadcast_storage(_broadcast_id: BroadcastId) {} - fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { + fn threshold_sign_and_broadcast_rotation_tx( + api_call: Self::ApiCall, + ) -> (BroadcastId, ThresholdSignatureRequestId) { >::threshold_sign_and_broadcast(api_call) } }