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 all 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

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
20 changes: 9 additions & 11 deletions state-chain/cf-integration-tests/src/funding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,33 +156,31 @@ fn backup_reward_is_calculated_linearly() {

// 3 backup will split the backup reward.
assert_eq!(Validator::highest_funded_qualified_backup_node_bids().count(), 3);
const N: u128 = 100;

let rewards_per_block = &calculate_backup_rewards::<AccountId, FlipBalance>(
let rewards_per_heartbeat = &calculate_backup_rewards::<AccountId, FlipBalance>(
Validator::highest_funded_qualified_backup_node_bids().collect::<Vec<_>>(),
Validator::bond(),
1u128,
HEARTBEAT_BLOCK_INTERVAL as u128,
Emissions::backup_node_emission_per_block(),
Emissions::current_authority_emission_per_block(),
Validator::current_authority_count() as u128,
);

let rewards_per_heartbeat = &calculate_backup_rewards::<AccountId, FlipBalance>(
let rewards_per_n_heartbeats = &calculate_backup_rewards::<AccountId, FlipBalance>(
Validator::highest_funded_qualified_backup_node_bids().collect::<Vec<_>>(),
Validator::bond(),
HEARTBEAT_BLOCK_INTERVAL as u128,
HEARTBEAT_BLOCK_INTERVAL as u128 * N,
Emissions::backup_node_emission_per_block(),
Emissions::current_authority_emission_per_block(),
Validator::current_authority_count() as u128,
);

for i in 0..rewards_per_block.len() {
for i in 0..rewards_per_heartbeat.len() {
// Validator account should match
assert_eq!(rewards_per_block[i].0, rewards_per_heartbeat[i].0);
assert_eq!(rewards_per_heartbeat[i].0, rewards_per_heartbeat[i].0);
// Reward per heartbeat should be scaled linearly.
assert_eq!(
rewards_per_heartbeat[i].1,
rewards_per_block[i].1 * HEARTBEAT_BLOCK_INTERVAL as u128
);
assert_eq!(rewards_per_n_heartbeats[i].1, rewards_per_heartbeat[i].1 * N);
}
});
}
Expand Down Expand Up @@ -270,7 +268,7 @@ fn apy_can_be_above_100_percent() {
MAX_AUTHORITIES as u128;
let apy_basis_point =
FixedU64::from_rational(reward, total).checked_mul_int(10_000u32).unwrap();
assert_eq!(apy_basis_point, 241_377_726u32);
assert_eq!(apy_basis_point, 241_377_727u32);
assert_eq!(calculate_account_apy(&validator), Some(apy_basis_point));
});
}
28 changes: 3 additions & 25 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 @@ -295,26 +294,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 Expand Up @@ -596,8 +575,7 @@ impl Network {
true => true,
false =>
engine.auto_submit_heartbeat &&
engine.last_heartbeat + Validator::blocks_per_epoch() - 1 <=
current_block,
engine.last_heartbeat + HEARTBEAT_BLOCK_INTERVAL - 1 <= current_block,
} {
assert_ok!(Reputation::heartbeat(RuntimeOrigin::signed(engine.node_id.clone())));
engine.last_heartbeat = current_block;
Expand Down
54 changes: 41 additions & 13 deletions state-chain/cf-integration-tests/src/swapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,23 @@ use cf_chains::{
address::{AddressConverter, AddressDerivationApi, EncodedAddress},
assets::eth::Asset as EthAsset,
eth::{api::EthereumApi, EthereumTrackedData},
evm::TransactionFee,
CcmChannelMetadata, CcmDepositMetadata, Chain, ChainState, Ethereum, ExecutexSwapAndCall,
ForeignChain, ForeignChainAddress, SwapOrigin, TransactionBuilder, TransferAssetParams,
};
use cf_primitives::{
AccountId, AccountRole, Asset, AssetAmount, AuthorityCount, GENESIS_EPOCH, STABLE_ASSET,
};
use cf_test_utilities::{assert_events_eq, assert_events_match};
use cf_traits::{AccountRoleRegistry, EpochInfo, LpBalanceApi};
use cf_traits::{AccountRoleRegistry, Chainflip, EpochInfo, LpBalanceApi};
use frame_support::{
assert_ok,
instances::Instance1,
traits::{OnFinalize, OnIdle, OnNewAccount},
};
use pallet_cf_broadcast::{
AwaitingBroadcast, BroadcastAttemptId, RequestFailureCallbacks, RequestSuccessCallbacks,
ThresholdSignatureData, TransactionSigningAttempt,
AwaitingBroadcast, BroadcastAttemptId, BroadcastIdCounter, RequestFailureCallbacks,
RequestSuccessCallbacks, ThresholdSignatureData, TransactionSigningAttempt,
};
use pallet_cf_ingress_egress::{DepositWitness, FailedCcm};
use pallet_cf_pools::{OrderId, RangeOrderSize};
Expand Down Expand Up @@ -770,6 +771,33 @@ fn can_resign_failed_ccm() {
}

testnet.move_to_the_next_epoch();
let tx_out_id = AwaitingBroadcast::<Runtime, Instance1>::get(BroadcastAttemptId {
broadcast_id: 1,
attempt_count: 0,
})
.unwrap()
.broadcast_attempt
.transaction_out_id;

for node in Validator::current_authorities() {
// Broadcast success for id 1, which is the rotation transaction.
// This needs to succeed because it's a barrier broadcast.
assert_ok!(Witnesser::witness_at_epoch(
RuntimeOrigin::signed(node),
Box::new(RuntimeCall::EthereumBroadcaster(
pallet_cf_broadcast::Call::transaction_succeeded {
tx_out_id,
signer_id: Default::default(),
tx_fee: TransactionFee {
effective_gas_price: Default::default(),
gas_used: Default::default()
},
tx_metadata: Default::default(),
}
)),
<Runtime as Chainflip>::EpochInfo::current_epoch()
));
}
setup_pool_and_accounts(vec![Asset::Eth, Asset::Flip]);

// Deposit CCM and process the swap
Expand Down Expand Up @@ -801,27 +829,27 @@ fn can_resign_failed_ccm() {
}

// Process the swap -> egress -> threshold sign -> broadcast
let broadcast_id = 2;
assert!(EthereumBroadcaster::threshold_signature_data(broadcast_id).is_none());
testnet.move_forward_blocks(3);

// Threshold signature is ready and the call is ready to be broadcasted.
assert!(EthereumBroadcaster::threshold_signature_data(broadcast_id).is_some());

let validators = Validator::current_authorities();
let broadcast_id = BroadcastIdCounter::<Runtime, Instance1>::get();
let mut broadcast_attempt_id = BroadcastAttemptId { broadcast_id, attempt_count: 0 };

// Fail the broadcast
for _ in 0..validators.len() {
for _ in Validator::current_authorities() {
let TransactionSigningAttempt { broadcast_attempt: _attempt, nominee } =
AwaitingBroadcast::<Runtime, Instance1>::get(broadcast_attempt_id).unwrap();
AwaitingBroadcast::<Runtime, Instance1>::get(broadcast_attempt_id)
.unwrap_or_else(|| {
panic!(
"Failed to get the transaction signing attempt for {:?}.",
broadcast_attempt_id,
)
});

assert_ok!(EthereumBroadcaster::transaction_signing_failure(
RuntimeOrigin::signed(nominee),
broadcast_attempt_id,
));
testnet.move_forward_blocks(1);
broadcast_attempt_id = broadcast_attempt_id.next_attempt();
broadcast_attempt_id = broadcast_attempt_id.peek_next();
}

// Upon broadcast failure, the Failure callback is called, and failed CCM is stored.
Expand Down
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 @@ -133,6 +141,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 @@ -147,6 +156,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 @@ -184,6 +194,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
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
15 changes: 6 additions & 9 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,12 @@ 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
}

/// 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
Loading