diff --git a/Cargo.lock b/Cargo.lock index 54a2c0da..fba5fe08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7099,6 +7099,7 @@ dependencies = [ "strata-asm-common", "strata-asm-txs-bridge-v1", "strata-bridge-types", + "strata-crypto", ] [[package]] @@ -7255,6 +7256,7 @@ dependencies = [ "rand 0.8.5", "ssz", "ssz_derive", + "strata-asm-bridge-msgs", "strata-asm-checkpoint-msgs", "strata-asm-common", "strata-asm-params", diff --git a/crates/msgs/bridge/Cargo.toml b/crates/msgs/bridge/Cargo.toml index 2ed5b467..58a4a7bb 100644 --- a/crates/msgs/bridge/Cargo.toml +++ b/crates/msgs/bridge/Cargo.toml @@ -13,3 +13,4 @@ ssz_derive.workspace = true strata-asm-common.workspace = true strata-asm-txs-bridge-v1.workspace = true strata-bridge-types.workspace = true +strata-crypto.workspace = true diff --git a/crates/msgs/bridge/src/lib.rs b/crates/msgs/bridge/src/lib.rs index 6c95b889..fe957d76 100644 --- a/crates/msgs/bridge/src/lib.rs +++ b/crates/msgs/bridge/src/lib.rs @@ -6,94 +6,44 @@ use std::any::Any; -use ssz::{Decode as SszDecode, DecodeError, Encode as SszEncode}; use ssz_derive::{Decode, Encode}; use strata_asm_common::{InterprotoMsg, SubprotocolId}; use strata_asm_txs_bridge_v1::BRIDGE_V1_SUBPROTOCOL_ID; -use strata_bridge_types::{OperatorSelection, WithdrawOutput}; +use strata_bridge_types::{OperatorIdx, OperatorSelection, WithdrawOutput}; +use strata_crypto::EvenPublicKey; /// Incoming message types received from other subprotocols. /// /// This enum represents all possible message types that the bridge subprotocol can /// receive from other subprotocols in the ASM. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode)] +#[ssz(enum_behaviour = "union")] pub enum BridgeIncomingMsg { /// Emitted after a checkpoint proof has been validated. Contains the withdrawal command /// specifying the destination descriptor and amount to be withdrawn. - DispatchWithdrawal { - /// The withdrawal output (destination + amount). - output: WithdrawOutput, - /// User's operator selection for withdrawal assignment. - selected_operator: OperatorSelection, - }, -} + DispatchWithdrawal(DispatchWithdrawalPayload), -#[derive(Debug, Encode, Decode)] -struct DispatchWithdrawalPayload { - output: WithdrawOutput, - selected_operator: OperatorSelection, + /// Emitted by the admin subprotocol when the operator set is updated. + /// Adds new operators by public key and removes existing operators by index. + UpdateOperatorSet(UpdateOperatorSetPayload), } -impl SszEncode for BridgeIncomingMsg { - fn is_ssz_fixed_len() -> bool { - false - } - - fn ssz_append(&self, buf: &mut Vec) { - match self { - Self::DispatchWithdrawal { - output, - selected_operator, - } => { - 0_u8.ssz_append(buf); - DispatchWithdrawalPayload { - output: output.clone(), - selected_operator: *selected_operator, - } - .ssz_append(buf); - } - } - } - - fn ssz_bytes_len(&self) -> usize { - match self { - Self::DispatchWithdrawal { - output, - selected_operator, - } => { - 1 + DispatchWithdrawalPayload { - output: output.clone(), - selected_operator: *selected_operator, - } - .ssz_bytes_len() - } - } - } +/// Payload for [`BridgeIncomingMsg::DispatchWithdrawal`]. +#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode)] +pub struct DispatchWithdrawalPayload { + /// The withdrawal output (destination + amount). + pub output: WithdrawOutput, + /// User's operator selection for withdrawal assignment. + pub selected_operator: OperatorSelection, } -impl SszDecode for BridgeIncomingMsg { - fn is_ssz_fixed_len() -> bool { - false - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - let (tag_bytes, payload_bytes) = bytes.split_first().ok_or_else(|| { - DecodeError::BytesInvalid("missing bridge message variant tag".into()) - })?; - - match *tag_bytes { - 0 => { - let payload = DispatchWithdrawalPayload::from_ssz_bytes(payload_bytes)?; - Ok(Self::DispatchWithdrawal { - output: payload.output, - selected_operator: payload.selected_operator, - }) - } - tag => Err(DecodeError::BytesInvalid(format!( - "unknown bridge message variant tag {tag}" - ))), - } - } +/// Payload for [`BridgeIncomingMsg::UpdateOperatorSet`]. +#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode)] +pub struct UpdateOperatorSetPayload { + /// Operator public keys to add to the bridge multisig. + pub add_members: Vec, + /// Operator indices to remove from the bridge multisig. + pub remove_members: Vec, } impl InterprotoMsg for BridgeIncomingMsg { diff --git a/crates/subprotocols/admin/Cargo.toml b/crates/subprotocols/admin/Cargo.toml index e5d86de9..44a95185 100644 --- a/crates/subprotocols/admin/Cargo.toml +++ b/crates/subprotocols/admin/Cargo.toml @@ -7,6 +7,7 @@ edition = "2024" workspace = true [dependencies] +strata-asm-bridge-msgs.workspace = true strata-asm-checkpoint-msgs.workspace = true strata-asm-common.workspace = true strata-asm-params.workspace = true diff --git a/crates/subprotocols/admin/src/handler.rs b/crates/subprotocols/admin/src/handler.rs index 72687f80..4e948f58 100644 --- a/crates/subprotocols/admin/src/handler.rs +++ b/crates/subprotocols/admin/src/handler.rs @@ -1,3 +1,4 @@ +use strata_asm_bridge_msgs::{BridgeIncomingMsg, UpdateOperatorSetPayload}; use strata_asm_checkpoint_msgs::CheckpointIncomingMsg; use strata_asm_common::{ MsgRelayer, @@ -67,8 +68,13 @@ pub(crate) fn handle_pending_updates( } } } - UpdateAction::OperatorSet(_update) => { - // TODO(STR-1721): Set an InterProtoMsg to the Bridge Subprotocol + UpdateAction::OperatorSet(update) => { + let (add_members, remove_members) = update.into_inner(); + relay_bridge_operator_set_update(relayer, add_members, remove_members); + info!( + update_id = update_id, + "Forwarded operator set update to bridge subprotocol", + ); } UpdateAction::Sequencer(update) => { let new_key = update.into_inner(); @@ -175,6 +181,18 @@ fn relay_checkpoint_predicate(relayer: &mut impl MsgRelayer, key: PredicateKey) relayer.relay_msg(&msg); } +fn relay_bridge_operator_set_update( + relayer: &mut impl MsgRelayer, + add_members: Vec, + remove_members: Vec, +) { + let msg = BridgeIncomingMsg::UpdateOperatorSet(UpdateOperatorSetPayload { + add_members, + remove_members, + }); + relayer.relay_msg(&msg); +} + #[cfg(test)] mod tests { use std::{any::Any, num::NonZero}; diff --git a/crates/subprotocols/bridge-v1/src/state/bridge.rs b/crates/subprotocols/bridge-v1/src/state/bridge.rs index a8ed7922..a7e47c3e 100644 --- a/crates/subprotocols/bridge-v1/src/state/bridge.rs +++ b/crates/subprotocols/bridge-v1/src/state/bridge.rs @@ -214,6 +214,16 @@ impl BridgeV1State { self.assignments.remove_assignment(deposit_idx) } + /// Applies an operator set update by adding new operators and removing existing ones. + pub fn apply_operator_set_update( + &mut self, + add_members: &[strata_crypto::EvenPublicKey], + remove_members: &[OperatorIdx], + ) { + self.operators + .apply_membership_changes(add_members, remove_members); + } + /// Removes an operator from the active multisig by deactivating them. /// /// # Panics diff --git a/crates/subprotocols/bridge-v1/src/subprotocol.rs b/crates/subprotocols/bridge-v1/src/subprotocol.rs index 4fdccc6d..a5e5dcf4 100644 --- a/crates/subprotocols/bridge-v1/src/subprotocol.rs +++ b/crates/subprotocols/bridge-v1/src/subprotocol.rs @@ -139,18 +139,27 @@ impl Subprotocol for BridgeV1Subproto { fn process_msgs(state: &mut Self::State, msgs: &[Self::Msg], l1ref: &L1BlockCommitment) { for msg in msgs { match msg { - BridgeIncomingMsg::DispatchWithdrawal { - output, - selected_operator, - } => { - if let Err(e) = - state.create_withdrawal_assignment(output, *selected_operator, l1ref) - { + BridgeIncomingMsg::DispatchWithdrawal(payload) => { + if let Err(e) = state.create_withdrawal_assignment( + &payload.output, + payload.selected_operator, + l1ref, + ) { // PANIC: Withdrawal assignment failure indicates catastrophic system // compromise. panic!("Failed to create withdrawal assignment: {e}",); } } + BridgeIncomingMsg::UpdateOperatorSet(payload) => { + let add_members = &payload.add_members; + let remove_members = &payload.remove_members; + info!( + added = add_members.len(), + removed = remove_members.len(), + "Applying operator set update from admin subprotocol", + ); + state.apply_operator_set_update(add_members, remove_members); + } } } } diff --git a/crates/subprotocols/checkpoint/src/handler.rs b/crates/subprotocols/checkpoint/src/handler.rs index 9518c63f..3db9251c 100644 --- a/crates/subprotocols/checkpoint/src/handler.rs +++ b/crates/subprotocols/checkpoint/src/handler.rs @@ -1,4 +1,4 @@ -use strata_asm_bridge_msgs::BridgeIncomingMsg; +use strata_asm_bridge_msgs::{BridgeIncomingMsg, DispatchWithdrawalPayload}; use strata_asm_common::{AsmLogEntry, MsgRelayer, TxInputRef, VerifiedAuxData, logging}; use strata_asm_logs::CheckpointTipUpdate; use strata_asm_txs_checkpoint::extract_checkpoint_from_envelope; @@ -61,10 +61,10 @@ pub(crate) fn handle_checkpoint_tx( relayer.emit_log(log_entry); for (output, selected_operator) in withdrawal_intents { - let bridge_msg = BridgeIncomingMsg::DispatchWithdrawal { + let bridge_msg = BridgeIncomingMsg::DispatchWithdrawal(DispatchWithdrawalPayload { output, selected_operator, - }; + }); relayer.relay_msg(&bridge_msg); } } diff --git a/crates/subprotocols/debug-v1/src/subprotocol.rs b/crates/subprotocols/debug-v1/src/subprotocol.rs index e0b1789f..89b4b641 100644 --- a/crates/subprotocols/debug-v1/src/subprotocol.rs +++ b/crates/subprotocols/debug-v1/src/subprotocol.rs @@ -4,7 +4,7 @@ //! with the Strata Anchor State Machine (ASM). use ssz_derive::{Decode, Encode}; -use strata_asm_bridge_msgs::BridgeIncomingMsg; +use strata_asm_bridge_msgs::{BridgeIncomingMsg, DispatchWithdrawalPayload}; use strata_asm_common::{ AsmError, AsmLogEntry, MsgRelayer, NullMsg, Subprotocol, SubprotocolId, TxInputRef, VerifiedAuxData, logging, @@ -95,10 +95,10 @@ fn process_parsed_debug_tx( ParsedDebugTx::MockWithdrawIntent((output, selected_operator)) => { logging::info!(amount = output.amt.to_sat(), "Processing mock withdrawal"); - let bridge_msg = BridgeIncomingMsg::DispatchWithdrawal { + let bridge_msg = BridgeIncomingMsg::DispatchWithdrawal(DispatchWithdrawalPayload { output, selected_operator, - }; + }); relayer.relay_msg(&bridge_msg); logging::info!("Successfully sent mock withdrawal intent to bridge"); diff --git a/crates/txs/admin/src/actions/updates/operator.rs b/crates/txs/admin/src/actions/updates/operator.rs index e0c2f824..dc00ff18 100644 --- a/crates/txs/admin/src/actions/updates/operator.rs +++ b/crates/txs/admin/src/actions/updates/operator.rs @@ -1,21 +1,21 @@ use arbitrary::Arbitrary; use ssz_derive::{Decode, Encode}; -use strata_identifiers::Buf32; +use strata_crypto::EvenPublicKey; use crate::{actions::Sighash, constants::AdminTxType}; /// An update to the Bridge Operator Set: -/// - removes the specified `remove_members` -/// - adds the specified `add_members` +/// - removes the specified `remove_members` (by operator index) +/// - adds the specified `add_members` (by public key) #[derive(Clone, Debug, Eq, PartialEq, Arbitrary, Encode, Decode)] pub struct OperatorSetUpdate { - add_members: Vec, - remove_members: Vec, + add_members: Vec, + remove_members: Vec, } impl OperatorSetUpdate { /// Creates a new `OperatorSetUpdate`. - pub fn new(add_members: Vec, remove_members: Vec) -> Self { + pub fn new(add_members: Vec, remove_members: Vec) -> Self { Self { add_members, remove_members, @@ -23,17 +23,17 @@ impl OperatorSetUpdate { } /// Borrow the list of operator public keys to add. - pub fn add_members(&self) -> &[Buf32] { + pub fn add_members(&self) -> &[EvenPublicKey] { &self.add_members } - /// Borrow the list of operator public keys to remove. - pub fn remove_members(&self) -> &[Buf32] { + /// Borrow the list of operator indices to remove. + pub fn remove_members(&self) -> &[u32] { &self.remove_members } /// Consume and return the inner vectors `(add_members, remove_members)`. - pub fn into_inner(self) -> (Vec, Vec) { + pub fn into_inner(self) -> (Vec, Vec) { (self.add_members, self.remove_members) } } @@ -44,18 +44,18 @@ impl Sighash for OperatorSetUpdate { } /// Returns `len(add) ‖ add[0] ‖ … ‖ add[n] ‖ len(rem) ‖ rem[0] ‖ … ‖ rem[m]` - /// where lengths are encoded as big-endian `u32`. + /// where lengths are encoded as big-endian `u32`, add members as 32-byte x-only keys, + /// and remove members as 4-byte big-endian operator indices. fn sighash_payload(&self) -> Vec { - let mut buf = Vec::with_capacity( - 4 + self.add_members.len() * 32 + 4 + self.remove_members.len() * 32, - ); + let mut buf = + Vec::with_capacity(4 + self.add_members.len() * 32 + 4 + self.remove_members.len() * 4); buf.extend_from_slice(&(self.add_members.len() as u32).to_be_bytes()); for member in &self.add_members { - buf.extend_from_slice(&member.0); + buf.extend_from_slice(&member.x_only_public_key().0.serialize()); } buf.extend_from_slice(&(self.remove_members.len() as u32).to_be_bytes()); for member in &self.remove_members { - buf.extend_from_slice(&member.0); + buf.extend_from_slice(&member.to_be_bytes()); } buf } diff --git a/guest-builder/sp1/guest-asm/Cargo.lock b/guest-builder/sp1/guest-asm/Cargo.lock index a79188b9..048240a8 100644 --- a/guest-builder/sp1/guest-asm/Cargo.lock +++ b/guest-builder/sp1/guest-asm/Cargo.lock @@ -1403,6 +1403,7 @@ dependencies = [ "strata-asm-common", "strata-asm-txs-bridge-v1", "strata-bridge-types", + "strata-crypto", ] [[package]] @@ -1520,6 +1521,7 @@ dependencies = [ "arbitrary", "ssz", "ssz_derive", + "strata-asm-bridge-msgs", "strata-asm-checkpoint-msgs", "strata-asm-common", "strata-asm-params", diff --git a/tests/Cargo.toml b/tests/Cargo.toml index e0199641..33b6f4c7 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -71,6 +71,10 @@ path = "asm/admin_to_checkpoint.rs" name = "asm_checkpoint" path = "asm/checkpoint.rs" +[[test]] +name = "asm_admin_to_bridge" +path = "asm/admin_to_bridge.rs" + [[test]] name = "asm_bridge_to_checkpoint" path = "asm/bridge_to_checkpoint.rs" diff --git a/tests/asm/admin.rs b/tests/asm/admin.rs index 935fe989..ba77441e 100644 --- a/tests/asm/admin.rs +++ b/tests/asm/admin.rs @@ -43,6 +43,7 @@ use strata_asm_txs_admin::{ parser::SignedPayload, test_utils::create_signature_set, }; +use strata_asm_txs_bridge_v1::test_utils::create_test_operators; use strata_crypto::{ keys::compressed::CompressedPublicKey, threshold_signature::{IndexedSignature, SignatureSet, ThresholdConfig}, @@ -100,7 +101,7 @@ async fn test_operator_update_is_queued() { harness .submit_admin_action( &mut ctx, - operator_set_update(vec![[10u8; 32], [11u8; 32]], vec![[20u8; 32]]), + operator_set_update(create_test_operators(2).1, vec![0]), ) .await .unwrap(); @@ -299,7 +300,10 @@ async fn test_cancel_removes_queued_update() { // Create an operator set update that gets queued (ID=0) harness - .submit_admin_action(&mut ctx, operator_set_update(vec![[6u8; 32]], vec![])) + .submit_admin_action( + &mut ctx, + operator_set_update(create_test_operators(1).1, vec![]), + ) .await .unwrap(); @@ -552,7 +556,10 @@ async fn test_cancel_prevents_queued_update_activation() { // Submit operator update (gets queued, will activate in current_height + 2) harness - .submit_admin_action(&mut ctx, operator_set_update(vec![[10u8; 32]], vec![])) + .submit_admin_action( + &mut ctx, + operator_set_update(create_test_operators(1).1, vec![]), + ) .await .unwrap(); diff --git a/tests/asm/admin_to_bridge.rs b/tests/asm/admin_to_bridge.rs new file mode 100644 index 00000000..219419a6 --- /dev/null +++ b/tests/asm/admin_to_bridge.rs @@ -0,0 +1,141 @@ +//! Admin → Bridge subprotocol interaction tests +//! +//! Tests the propagation of operator set updates from the admin subprotocol +//! to the bridge subprotocol via interprotocol messaging. +//! +//! Key interactions tested: +//! - Operator additions → bridge operator table gains new members +//! - Operator removals → bridge operator table deactivates members +//! - Combined add/remove → both applied atomically after activation + +#![allow( + unused_crate_dependencies, + reason = "test dependencies shared across test suite" +)] + +use harness::{ + admin::{create_test_admin_setup, operator_set_update, AdminContext, AdminExt}, + bridge::{create_test_bridge_setup, BridgeExt}, + test_harness::{AsmTestHarness, AsmTestHarnessBuilder}, +}; +use integration_tests::harness; +use strata_asm_txs_bridge_v1::test_utils::create_test_operators; +use strata_crypto::EvenPublicKey; + +const CONFIRMATION_DEPTH: u16 = 2; +const NUM_INITIAL_OPERATORS: usize = 3; + +/// Sets up an ASM harness with admin + bridge subprotocols and mines the init block. +async fn setup() -> (AsmTestHarness, AdminContext) { + let (admin_config, admin_ctx) = create_test_admin_setup(CONFIRMATION_DEPTH); + let (bridge_config, _) = create_test_bridge_setup(NUM_INITIAL_OPERATORS); + + let harness = AsmTestHarnessBuilder::default() + .with_admin_config(admin_config) + .with_bridge_config(bridge_config) + .build() + .await + .unwrap(); + + // Initialize subprotocols + harness.mine_block(None).await.unwrap(); + + (harness, admin_ctx) +} + +/// Submits an operator set update and mines enough blocks to activate it. +async fn submit_and_activate( + harness: &AsmTestHarness, + ctx: &mut AdminContext, + add: Vec, + remove: Vec, +) { + harness + .submit_admin_action(ctx, operator_set_update(add, remove)) + .await + .unwrap(); + + // Mine `CONFIRMATION_DEPTH` blocks to trigger activation + for _ in 0..CONFIRMATION_DEPTH { + harness.mine_block(None).await.unwrap(); + } +} + +// ============================================================================ +// Operator Set Updates → Bridge Operator Table +// ============================================================================ + +/// Verifies that adding an operator via admin propagates to the bridge after activation. +#[tokio::test(flavor = "multi_thread")] +async fn test_operator_add_propagates_to_bridge() { + let (harness, mut ctx) = setup().await; + + let initial_bridge = harness.bridge_state().unwrap(); + assert_eq!(initial_bridge.operators().len(), 3); + + let (_, new_pubkeys) = create_test_operators(1); + submit_and_activate(&harness, &mut ctx, vec![new_pubkeys[0]], vec![]).await; + + let bridge = harness.bridge_state().unwrap(); + assert_eq!(bridge.operators().len(), 4); + assert!(bridge.operators().is_in_current_multisig(3)); +} + +/// Verifies that removing an operator via admin propagates to the bridge after activation. +#[tokio::test(flavor = "multi_thread")] +async fn test_operator_remove_propagates_to_bridge() { + let (harness, mut ctx) = setup().await; + + let initial_agg_key = *harness.bridge_state().unwrap().operators().agg_key(); + + submit_and_activate(&harness, &mut ctx, vec![], vec![0]).await; + + let bridge = harness.bridge_state().unwrap(); + assert!(!bridge.operators().is_in_current_multisig(0)); + assert!(bridge.operators().is_in_current_multisig(1)); + assert!(bridge.operators().is_in_current_multisig(2)); + assert_ne!(*bridge.operators().agg_key(), initial_agg_key); +} + +/// Verifies combined add and remove in a single operator set update. +#[tokio::test(flavor = "multi_thread")] +async fn test_operator_add_and_remove_propagates_to_bridge() { + let (harness, mut ctx) = setup().await; + + let initial_agg_key = *harness.bridge_state().unwrap().operators().agg_key(); + + let (_, new_pubkeys) = create_test_operators(1); + submit_and_activate(&harness, &mut ctx, vec![new_pubkeys[0]], vec![1]).await; + + let bridge = harness.bridge_state().unwrap(); + assert_eq!(bridge.operators().len(), 4); + assert!(bridge.operators().is_in_current_multisig(0)); + assert!(!bridge.operators().is_in_current_multisig(1)); + assert!(bridge.operators().is_in_current_multisig(2)); + assert!(bridge.operators().is_in_current_multisig(3)); + assert_ne!(*bridge.operators().agg_key(), initial_agg_key); +} + +/// Verifies the update is queued and does not affect the bridge until activated. +#[tokio::test(flavor = "multi_thread")] +async fn test_operator_update_does_not_apply_before_activation() { + let (harness, mut ctx) = setup().await; + + let (_, new_pubkeys) = create_test_operators(1); + + // Submit but do NOT mine enough blocks to activate + harness + .submit_admin_action(&mut ctx, operator_set_update(vec![new_pubkeys[0]], vec![])) + .await + .unwrap(); + + let admin_state = harness.admin_state().unwrap(); + assert_eq!(admin_state.queued().len(), 1, "Update should be queued"); + + let bridge = harness.bridge_state().unwrap(); + assert_eq!( + bridge.operators().len(), + 3, + "Bridge should be unchanged while update is queued" + ); +} diff --git a/tests/harness/admin.rs b/tests/harness/admin.rs index 59d76970..115de940 100644 --- a/tests/harness/admin.rs +++ b/tests/harness/admin.rs @@ -43,6 +43,7 @@ use strata_asm_txs_admin::{ use strata_crypto::{ keys::compressed::CompressedPublicKey, threshold_signature::{ThresholdConfig, ThresholdConfigUpdate}, + EvenPublicKey, }; use strata_identifiers::Buf32; use strata_predicate::PredicateKey; @@ -152,10 +153,9 @@ pub fn sequencer_update(key: [u8; 32]) -> MultisigAction { } /// Create an operator set update action. -pub fn operator_set_update(add: Vec<[u8; 32]>, remove: Vec<[u8; 32]>) -> MultisigAction { +pub fn operator_set_update(add: Vec, remove: Vec) -> MultisigAction { MultisigAction::Update(UpdateAction::OperatorSet(OperatorSetUpdate::new( - add.into_iter().map(Buf32::from).collect(), - remove.into_iter().map(Buf32::from).collect(), + add, remove, ))) }