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 12 commits
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion bouncer/commands/go_bananas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ async function playLp(asset: Asset, price: number, liquidity: number) {

async function launchTornado() {
const chainflip = await getChainflipApi();
const epoch = (await chainflip.query.bitcoinVault.currentVaultEpochAndState()).toJSON()!
const epoch = (await chainflip.query.bitcoinVault.currentVaultEpoch()).toJSON()!
.epochIndex as number;
const pubkey = (
(await chainflip.query.bitcoinVault.vaults(epoch)).toJSON()!.publicKey.current as string
Expand Down
2 changes: 1 addition & 1 deletion bouncer/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions engine/src/witness/eth/key_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,6 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
{
info!("Handling event: {event}");
let call: state_chain_runtime::RuntimeCall = match event.event_parameters {
KeyManagerEvents::AggKeySetByAggKeyFilter(_) => pallet_cf_vaults::Call::<
_,
<Inner::Chain as PalletInstanceAlias>::Instance,
>::vault_key_rotated {
block_number: header.index,
tx_id: event.tx_hash,
}
.into(),
KeyManagerEvents::AggKeySetByGovKeyFilter(AggKeySetByGovKeyFilter {
new_agg_key,
..
Expand Down
29 changes: 11 additions & 18 deletions state-chain/cf-integration-tests/src/authorities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ use sp_runtime::AccountId32;
use std::collections::{BTreeSet, HashMap};

use cf_primitives::{AuthorityCount, FlipBalance, GENESIS_EPOCH};
use cf_traits::{AsyncResult, EpochInfo, SafeMode, VaultRotator, VaultStatus};
use cf_traits::{AsyncResult, EpochInfo, VaultRotator, VaultStatus};
use pallet_cf_environment::SafeModeUpdate;
use pallet_cf_validator::{CurrentRotationPhase, RotationPhase};
use state_chain_runtime::{
safe_mode::RuntimeSafeMode, BitcoinVault, Environment, EthereumInstance, EthereumVault, Flip,
PolkadotInstance, PolkadotVault, Runtime, RuntimeOrigin, Validator,
BitcoinVault, Environment, EthereumInstance, EthereumVault, Flip, PolkadotInstance,
PolkadotVault, Runtime, RuntimeOrigin, Validator,
};

// Helper function that creates a network, funds backup nodes, and have them join the auction.
Expand Down Expand Up @@ -97,11 +97,13 @@ fn authority_rotates_with_correct_sequence() {
assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeyHandoverComplete));

// Activate new key.
testnet.move_forward_blocks(2);
// The key is immediately activated in the next block
testnet.move_forward_blocks(1);
assert!(matches!(
Validator::current_rotation_phase(),
RotationPhase::ActivatingKeys(..)
));

assert_eq!(
AllVaults::status(),
AsyncResult::Ready(VaultStatus::RotationComplete),
Expand Down Expand Up @@ -294,7 +296,8 @@ fn authority_rotation_can_succeed_after_aborted_by_safe_mode() {
}

#[test]
fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mode() {
ramizhasan111 marked this conversation as resolved.
Show resolved Hide resolved
fn authority_rotation_cannot_be_aborted_after_key_handover_and_completes_even_on_safe_mode_enabled()
{
const EPOCH_BLOCKS: u32 = 1000;
const MAX_AUTHORITIES: AuthorityCount = 10;
super::genesis::default()
Expand Down Expand Up @@ -324,20 +327,10 @@ fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mo

// Authority rotation is stalled while in Code Red because of disabling dispatching
// witness extrinsics and so witnessing vault rotation will be stalled.
assert!(matches!(AllVaults::status(), AsyncResult::Pending));

// We activate witnessing calls by setting safe mode to code green just for the
// witnesser pallet.
let mut runtime_safe_mode_with_witnessing = RuntimeSafeMode::CODE_RED;
runtime_safe_mode_with_witnessing.witnesser =
pallet_cf_witnesser::PalletSafeMode::CODE_GREEN;

assert_ok!(Environment::update_safe_mode(
pallet_cf_governance::RawOrigin::GovernanceApproval.into(),
SafeModeUpdate::CodeAmber(runtime_safe_mode_with_witnessing)
assert!(matches!(
AllVaults::status(),
AsyncResult::Ready(VaultStatus::RotationComplete)
));

// rotation should now complete since the witness calls are now dispatched.
testnet.move_forward_blocks(3);
assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index(), "We should be in a new epoch");
});
Expand Down
25 changes: 2 additions & 23 deletions state-chain/cf-integration-tests/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ use frame_support::{
traits::{IntegrityTest, OnFinalize, OnIdle},
};
use pallet_cf_funding::{MinimumFunding, RedemptionAmount};
use pallet_cf_validator::RotationPhase;
use sp_consensus_aura::SlotDuration;
use sp_std::collections::btree_set::BTreeSet;
use state_chain_runtime::{
AccountRoles, AllPalletsWithSystem, BitcoinInstance, EthereumInstance, PolkadotInstance,
Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Weight,
AccountRoles, AllPalletsWithSystem, BitcoinInstance, PolkadotInstance, Runtime, RuntimeCall,
RuntimeEvent, RuntimeOrigin, Weight,
};
use std::{
cell::RefCell,
Expand Down Expand Up @@ -282,26 +281,6 @@ impl Engine {
), RuntimeOrigin::none()
);
}
RuntimeEvent::Validator(pallet_cf_validator::Event::RotationPhaseUpdated { new_phase: RotationPhase::ActivatingKeys(_) }) => {
// NOTE: This is a little inaccurate a representation of how it actually works. An event is emitted
// which contains the transaction to broadcast for the rotation tx, which the CFE then broadcasts.
// This is a simpler way to represent this in the tests. Representing in this way in the tests also means
// that for dot, given we don't have a key to sign with initially, it will work without extra test boilerplate.

// If we rotating let's witness the keys being rotated on the contract
queue_dispatch_extrinsic(
RuntimeCall::Witnesser(
pallet_cf_witnesser::Call::witness_at_epoch {
call: Box::new(pallet_cf_vaults::Call::<_, EthereumInstance>::vault_key_rotated {
block_number: 100,
tx_id: [1u8; 32].into(),
}.into()),
epoch_index: Validator::epoch_index(),
}),
RuntimeOrigin::signed(self.node_id.clone())
);
}

RuntimeEvent::PolkadotVault(pallet_cf_vaults::Event::<_, PolkadotInstance>::AwaitingGovernanceActivation { .. }) => {
queue_dispatch_extrinsic(
RuntimeCall::Environment(pallet_cf_environment::Call::witness_polkadot_vault_creation {
Expand Down
7 changes: 7 additions & 0 deletions state-chain/chains/src/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,13 @@ impl ChainCrypto for BitcoinCrypto {
fn handover_key_matches(current_key: &Self::AggKey, new_key: &Self::AggKey) -> bool {
new_key.previous.is_some_and(|previous| current_key.current == previous)
}

fn maybe_broadcast_barriers_on_rotation(
_rotation_broadcast_id: cf_primitives::BroadcastId,
) -> Vec<cf_primitives::BroadcastId> {
// we dont need to put broadcast barriers for Bitcoin
vec![]
}
}

fn verify_single_threshold_signature(
Expand Down
9 changes: 6 additions & 3 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,12 @@ impl ChainCrypto for PolkadotCrypto {
true
}

/// We sign with a specific key, so we can optimistically activate the next one.
fn optimistic_activation() -> bool {
true
fn maybe_broadcast_barriers_on_rotation(
rotation_broadcast_id: BroadcastId,
) -> Vec<BroadcastId> {
// For polkadot, we need to pause future epoch broadcasts until all the previous epoch
// broadcasts (including the rotation tx) has successfully broadcasted.
vec![rotation_broadcast_id]
}
}

Expand Down
17 changes: 17 additions & 0 deletions state-chain/chains/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ impl ChainCrypto for EvmCrypto {
fn agg_key_to_payload(agg_key: Self::AggKey, _for_handover: bool) -> Self::Payload {
H256(Blake2_256::hash(&agg_key.to_pubkey_compressed()))
}

fn maybe_broadcast_barriers_on_rotation(
rotation_broadcast_id: BroadcastId,
) -> Vec<BroadcastId> {
// For Ethereum, we need to put 2 barriers, the first on the last non-rotation tx of the
// previous epoch, the second on the rotation tx itself. This is because before we execute
// the rotation tx for eth, we need to make sure all previous tx have successfully
// broadcast. Also, we need to pause future new epoch tx from broadcast until the rotation
// broadcast has successfully completed.
//
// If the rotation tx is the first broadcast ever, we dont need the first barrier.
if rotation_broadcast_id > 1 {
vec![rotation_broadcast_id - 1, rotation_broadcast_id]
} else {
vec![rotation_broadcast_id]
}
}
}

#[derive(Copy, Clone, RuntimeDebug, PartialEq, Eq)]
Expand Down
14 changes: 6 additions & 8 deletions state-chain/chains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use core::{fmt::Display, iter::Step};
use crate::benchmarking_value::{BenchmarkValue, BenchmarkValueExtended};
pub use address::ForeignChainAddress;
use address::{AddressDerivationApi, ToHumanreadableAddress};
use cf_primitives::{AssetAmount, ChannelId, EgressId, EthAmount, TransactionHash};
use cf_primitives::{AssetAmount, BroadcastId, ChannelId, EgressId, EthAmount, TransactionHash};
use codec::{Decode, Encode, FullCodec, MaxEncodedLen};
use frame_support::{
pallet_prelude::{MaybeSerializeDeserialize, Member},
Expand Down Expand Up @@ -187,19 +187,17 @@ pub trait ChainCrypto {
Self::UtxoChain::get()
}

/// Determines whether the chain crypto allows for optimistic activation of new aggregate keys.
///
/// By default, this is true for Utxo-based chains, false otherwise.
fn optimistic_activation() -> bool {
Self::UtxoChain::get()
}

/// Determines whether the chain crypto supports key handover.
///
/// By default, this is true for Utxo-based chains, false otherwise.
fn key_handover_is_required() -> bool {
Self::UtxoChain::get()
}

/// Provides chain specific functionality for providing the broadcast barriers on rotation tx
/// broadcast
fn maybe_broadcast_barriers_on_rotation(rotation_broadcast_id: BroadcastId)
-> Vec<BroadcastId>;
}

/// Provides chain-specific replay protection data.
Expand Down
25 changes: 6 additions & 19 deletions state-chain/chains/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub type MockEthereumChannelId = u128;

thread_local! {
static MOCK_KEY_HANDOVER_IS_REQUIRED: RefCell<bool> = RefCell::new(true);
static MOCK_OPTIMISTIC_ACTIVATION: RefCell<bool> = RefCell::new(false);
static MOCK_SIGN_WITH_SPECIFIC_KEY: RefCell<bool> = RefCell::new(false);
static MOCK_VALID_METADATA: RefCell<bool> = RefCell::new(true);
}
Expand All @@ -35,20 +34,6 @@ impl Get<bool> for MockKeyHandoverIsRequired {
}
}

pub struct MockOptimisticActivation;

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

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

pub struct MockFixedKeySigningRequests;

impl MockFixedKeySigningRequests {
Expand Down Expand Up @@ -264,13 +249,15 @@ impl ChainCrypto for MockEthereumChainCrypto {
MockFixedKeySigningRequests::get()
}

fn optimistic_activation() -> bool {
MockOptimisticActivation::get()
}

fn key_handover_is_required() -> bool {
MockKeyHandoverIsRequired::get()
}

fn maybe_broadcast_barriers_on_rotation(
_rotation_broadcast_id: BroadcastId,
) -> Vec<BroadcastId> {
unimplemented!()
}
}

impl_default_benchmark_value!(MockAggKey);
Expand Down
6 changes: 6 additions & 0 deletions state-chain/chains/src/none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,10 @@ impl ChainCrypto for NoneChainCrypto {
fn agg_key_to_payload(_agg_key: Self::AggKey, _for_handover: bool) -> Self::Payload {
unimplemented!()
}

fn maybe_broadcast_barriers_on_rotation(
_rotation_broadcast_id: BroadcastId,
) -> Vec<BroadcastId> {
unimplemented!()
}
}
4 changes: 2 additions & 2 deletions state-chain/pallets/cf-broadcast/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn generate_on_signature_ready_call<T: pallet::Config<I>, I>() -> pallet::Call<T
threshold_request_id,
threshold_signature_payload: PayloadFor::<T, I>::benchmark_value(),
api_call: Box::new(ApiCallFor::<T, I>::benchmark_value()),
broadcast_id: 1,
broadcast_attempt_id: BroadcastAttemptId { broadcast_id: 1, attempt_count: 0 },
initiated_at: INITIATED_AT.into(),
}
}
Expand Down Expand Up @@ -115,7 +115,7 @@ benchmarks_instance_pallet! {
BenchmarkValue::benchmark_value(),
signed_api_call,
BenchmarkValue::benchmark_value(),
1,
BroadcastAttemptId{broadcast_id: 1, attempt_count: 0},
INITIATED_AT.into(),
);

Expand Down