diff --git a/crates/subprotocols/admin/subprotocol/src/subprotocol.rs b/crates/subprotocols/admin/subprotocol/src/subprotocol.rs index 947e634b..f977da34 100644 --- a/crates/subprotocols/admin/subprotocol/src/subprotocol.rs +++ b/crates/subprotocols/admin/subprotocol/src/subprotocol.rs @@ -54,20 +54,14 @@ impl Subprotocol for AdministrationSubprotocol { // Phase 1: Execute any pending updates that have reached their activation height handle_pending_updates(state, relayer, current_height); - // Phase 2: Process incoming administration transactions + // Phase 2: Process incoming administration transactions. Unparseable txs are + // logged and skipped inside `parse_tx` to maintain system resilience. for tx in txs { - match parse_tx(tx) { - Ok(signed_payload) => { - if let Err(e) = handle_action(state, signed_payload, current_height, relayer) { - warn!(tx_id = %tx.tx().compute_txid(), error = %e, "Failed to handle admin action"); - } - } - // Parsing failures are skipped to maintain system resilience, but warned so a - // malformed governance tx isn't completely invisible. Admin txs are rare and - // security-sensitive, so a malformed one is worth surfacing. - Err(e) => { - warn!(tx_id = %tx.tx().compute_txid(), error = %e, "Skipping unparseable admin tx"); - } + let Some(signed_payload) = parse_tx(tx) else { + continue; + }; + if let Err(e) = handle_action(state, signed_payload, current_height, relayer) { + warn!(tx_id = %tx.tx().compute_txid(), error = %e, "Failed to handle admin action"); } } } diff --git a/crates/subprotocols/admin/txs/src/actions/updates/mod.rs b/crates/subprotocols/admin/txs/src/actions/updates/mod.rs index 49e9cc17..e8d53fdf 100644 --- a/crates/subprotocols/admin/txs/src/actions/updates/mod.rs +++ b/crates/subprotocols/admin/txs/src/actions/updates/mod.rs @@ -118,6 +118,30 @@ impl RenderSigningMessage for UpdateAction { } } +impl From for UpdateAction { + fn from(update: StrataAdminMultisigUpdate) -> Self { + UpdateAction::StrataAdminMultisig(update) + } +} + +impl From for UpdateAction { + fn from(update: StrataSeqManagerMultisigUpdate) -> Self { + UpdateAction::StrataSeqManagerMultisig(update) + } +} + +impl From for UpdateAction { + fn from(update: AlpenAdminMultisigUpdate) -> Self { + UpdateAction::AlpenAdminMultisig(update) + } +} + +impl From for UpdateAction { + fn from(update: StrataSecurityCouncilMultisigUpdate) -> Self { + UpdateAction::StrataSecurityCouncilMultisig(update) + } +} + impl From for UpdateAction { fn from(update: OperatorSetUpdate) -> Self { UpdateAction::OperatorSet(update) @@ -129,3 +153,39 @@ impl From for UpdateAction { UpdateAction::Sequencer(update) } } + +impl From for UpdateAction { + fn from(update: OlStfVkUpdate) -> Self { + UpdateAction::OlStfVk(update) + } +} + +impl From for UpdateAction { + fn from(update: AsmStfVkUpdate) -> Self { + UpdateAction::AsmStfVk(update) + } +} + +impl From for UpdateAction { + fn from(update: EeStfVkUpdate) -> Self { + UpdateAction::EeStfVk(update) + } +} + +impl From for UpdateAction { + fn from(update: Defcon1Update) -> Self { + UpdateAction::Defcon1(update) + } +} + +impl From for UpdateAction { + fn from(update: Defcon3Update) -> Self { + UpdateAction::Defcon3(update) + } +} + +impl From for UpdateAction { + fn from(update: SafeHarbourAddressUpdate) -> Self { + UpdateAction::SafeHarbourAddress(update) + } +} diff --git a/crates/subprotocols/admin/txs/src/errors.rs b/crates/subprotocols/admin/txs/src/errors.rs index 7a4309e0..06b78245 100644 --- a/crates/subprotocols/admin/txs/src/errors.rs +++ b/crates/subprotocols/admin/txs/src/errors.rs @@ -16,8 +16,4 @@ pub enum AdministrationTxParseError { /// Failed to parse the transaction envelope. #[error("failed to parse transaction envelope: {0}")] MalformedEnvelope(#[from] EnvelopeParseError), - - /// Failed to deserialize the transaction payload for the given transaction type. - #[error("tx type is not defined")] - UnknownTxType, } diff --git a/crates/subprotocols/admin/txs/src/parser.rs b/crates/subprotocols/admin/txs/src/parser.rs index 4827bddc..0e92c4cc 100644 --- a/crates/subprotocols/admin/txs/src/parser.rs +++ b/crates/subprotocols/admin/txs/src/parser.rs @@ -1,16 +1,32 @@ -use ssz::Decode; +use ssz::{Decode, Encode}; use ssz_derive::{Decode as DeriveDecode, Encode as DeriveEncode}; -use strata_asm_common::TxInputRef; +use strata_asm_admin_types::{AdminTxType, UpdateTxType}; +use strata_asm_common::{TxInputRef, logging::warn}; use strata_crypto::threshold_signature::SignatureSet; use strata_l1_envelope_fmt::parser::parse_envelope_payload; +use strata_l1_txfmt::TxType; -use crate::{actions::MultisigAction, errors::AdministrationTxParseError}; +use crate::{ + actions::{ + CancelAction, MultisigAction, UpdateAction, + updates::{ + AlpenAdminMultisigUpdate, AsmStfVkUpdate, Defcon1Update, Defcon3Update, EeStfVkUpdate, + OlStfVkUpdate, OperatorSetUpdate, SafeHarbourAddressUpdate, SequencerUpdate, + StrataAdminMultisigUpdate, StrataSecurityCouncilMultisigUpdate, + StrataSeqManagerMultisigUpdate, + }, + }, + errors::AdministrationTxParseError, +}; /// A signed administration payload containing both the action and its signatures. /// -/// This structure is serialized with SSZ and embedded in the witness envelope. -/// The OP_RETURN only contains the SPS-50 tag (magic bytes, subprotocol ID, tx type). -#[derive(Clone, Debug, Eq, PartialEq, DeriveEncode, DeriveDecode)] +/// In-memory representation handed to the subprotocol handler. On the wire the action is +/// encoded *without* enum discriminants: the SPS-50 tag's tx type selects the concrete +/// action type, and the envelope carries the corresponding [`SignedActionPayload`]. Use +/// [`into_envelope_bytes`](Self::into_envelope_bytes) / [`parse_tx`] to cross that +/// boundary. +#[derive(Clone, Debug, Eq, PartialEq)] pub struct SignedPayload { /// Sequence number used to prevent replay attacks and enforce ordering. pub seqno: u64, @@ -29,24 +45,109 @@ impl SignedPayload { signatures, } } + + /// Encodes this payload into the envelope wire format. + /// + /// The action is encoded without its enum discriminants; the receiver re-derives the + /// concrete type from the SPS-50 tag's tx type, so the tag (see + /// [`MultisigAction::tag`]) must be built from the same action. + pub fn into_envelope_bytes(self) -> Vec { + let Self { + seqno, + action, + signatures, + } = self; + match action { + MultisigAction::Cancel(action) => encode_wire(seqno, action, signatures), + MultisigAction::Update(update) => match update { + UpdateAction::StrataAdminMultisig(u) => encode_wire(seqno, u, signatures), + UpdateAction::StrataSeqManagerMultisig(u) => encode_wire(seqno, u, signatures), + UpdateAction::AlpenAdminMultisig(u) => encode_wire(seqno, u, signatures), + UpdateAction::StrataSecurityCouncilMultisig(u) => encode_wire(seqno, u, signatures), + UpdateAction::OperatorSet(u) => encode_wire(seqno, u, signatures), + UpdateAction::Sequencer(u) => encode_wire(seqno, u, signatures), + UpdateAction::OlStfVk(u) => encode_wire(seqno, u, signatures), + UpdateAction::AsmStfVk(u) => encode_wire(seqno, u, signatures), + UpdateAction::EeStfVk(u) => encode_wire(seqno, u, signatures), + UpdateAction::Defcon1(u) => encode_wire(seqno, u, signatures), + UpdateAction::Defcon3(u) => encode_wire(seqno, u, signatures), + UpdateAction::SafeHarbourAddress(u) => encode_wire(seqno, u, signatures), + }, + } + } +} + +/// Wire-format container embedded in the envelope: the signed payload for one concrete +/// action type `A`. +/// +/// The SPS-50 tag's tx type byte — not an encoded enum discriminant — determines `A`, so +/// the envelope carries no redundant selector bytes and each tx type has a flat, +/// self-describing SSZ schema that's easy to construct outside this codebase. +#[derive(DeriveEncode, DeriveDecode)] +struct SignedActionPayload { + seqno: u64, + action: A, + signatures: SignatureSet, +} + +fn encode_wire(seqno: u64, action: A, signatures: SignatureSet) -> Vec { + SignedActionPayload { + seqno, + action, + signatures, + } + .as_ssz_bytes() } -/// Parses a transaction to extract both the multisig action and the signature set. +/// Parses a transaction into a [`SignedPayload`] based on its SPS-50 tx type. /// -/// This function extracts the signed payload from the taproot leaf script embedded -/// in the transaction's witness data. The payload contains both the administrative -/// action and its authorizing signatures. +/// The tag's tx type selects the concrete action type to decode from the taproot leaf +/// script envelope in the transaction's witness; the payload itself carries no enum +/// discriminants. /// -/// # Arguments -/// * `tx` - A reference to the transaction input to parse +/// # Returns /// -/// # Errors -/// Returns `AdministrationTxParseError` if: -/// - The transaction lacks a taproot leaf script in its witness -/// - The envelope payload cannot be parsed -/// - The signed payload cannot be deserialized -// TODO(STR-2366): Update L1Payload to minimize DA footprint -pub fn parse_tx(tx: &TxInputRef<'_>) -> Result { +/// Returns `Some(SignedPayload)` when the tx type is known and the envelope payload is +/// well-formed, returns None (with a warning logged) otherwise. +pub fn parse_tx(tx: &TxInputRef<'_>) -> Option { + // Decode the SPS-50 tag's tx type byte into a known `AdminTxType`. An unknown + // discriminant means the tx was tagged for the administration subprotocol but with a + // type this build doesn't recognize — likely a protocol/version mismatch. + let raw_tx_type = tx.tag().tx_type(); + let admin_tx_type: AdminTxType = match raw_tx_type.try_into() { + Ok(t) => t, + Err(_) => { + // `txid` is computed inside the macro, because logging is compiled to noop in ZkVM. + warn!( + txid = %tx.tx().compute_txid(), + raw_tx_type, + "Skipping tx with unsupported admin tx type", + ); + return None; + } + }; + + // Funnel structural failures through one shared log site. Admin txs are rare and + // security-sensitive, so a malformed one is worth surfacing. + match extract_signed_payload(tx, admin_tx_type) { + Ok(payload) => Some(payload), + Err(e) => { + warn!( + txid = %tx.tx().compute_txid(), + error = %e, + "Failed to parse admin tx; skipping", + ); + None + } + } +} + +/// Extracts the envelope payload from the witness and decodes it as the concrete action +/// type implied by `admin_tx_type`. +fn extract_signed_payload( + tx: &TxInputRef<'_>, + admin_tx_type: AdminTxType, +) -> Result { let tx_type = tx.tag().tx_type(); // Extract the taproot leaf script from the first input's witness @@ -59,14 +160,130 @@ pub fn parse_tx(tx: &TxInputRef<'_>) -> Result Result { + match admin_tx_type { + AdminTxType::Cancel => { + let wire = decode_wire::(bytes, tx_type)?; + Ok(SignedPayload::new( + wire.seqno, + MultisigAction::Cancel(wire.action), + wire.signatures, + )) + } + AdminTxType::Update(update_type) => match update_type { + UpdateTxType::StrataAdminMultisigUpdate => { + decode_update::(bytes, tx_type) + } + UpdateTxType::StrataSeqManagerMultisigUpdate => { + decode_update::(bytes, tx_type) + } + UpdateTxType::AlpenAdminMultisigUpdate => { + decode_update::(bytes, tx_type) + } + UpdateTxType::StrataSecurityCouncilMultisigUpdate => { + decode_update::(bytes, tx_type) + } + UpdateTxType::OperatorUpdate => decode_update::(bytes, tx_type), + UpdateTxType::SequencerUpdate => decode_update::(bytes, tx_type), + UpdateTxType::OlStfVkUpdate => decode_update::(bytes, tx_type), + UpdateTxType::AsmStfVkUpdate => decode_update::(bytes, tx_type), + UpdateTxType::EeStfVkUpdate => decode_update::(bytes, tx_type), + UpdateTxType::Defcon1 => decode_update::(bytes, tx_type), + UpdateTxType::Defcon3 => decode_update::(bytes, tx_type), + UpdateTxType::SafeHarbourAddressUpdate => { + decode_update::(bytes, tx_type) + } + }, + } +} + +fn decode_update( + bytes: &[u8], + tx_type: TxType, +) -> Result +where + A: Encode + Decode + Into, +{ + let wire = decode_wire::(bytes, tx_type)?; + Ok(SignedPayload::new( + wire.seqno, + MultisigAction::Update(wire.action.into()), + wire.signatures, + )) +} + +fn decode_wire( + bytes: &[u8], + tx_type: TxType, +) -> Result, AdministrationTxParseError> { + // Preserve the underlying decode error so a malformed governance tx is diagnosable + // from the logs. + SignedActionPayload::from_ssz_bytes(bytes).map_err(|e| { AdministrationTxParseError::MalformedPayload { tx_type, reason: format!("{e:?}"), } - })?; + }) +} + +#[cfg(test)] +mod tests { + use strata_crypto::threshold_signature::IndexedSignature; + use strata_test_utils_arb::ArbitraryGenerator; + + use super::*; + use crate::actions::RenderSigningMessage; - Ok(signed_payload) + fn dummy_signatures() -> SignatureSet { + let sigs = vec![ + IndexedSignature::new(0, [1u8; 65]), + IndexedSignature::new(2, [2u8; 65]), + ]; + SignatureSet::new(sigs).expect("valid signature set") + } + + /// Round-trips arbitrary actions through the discriminant-free wire format, keyed by + /// the same tx type the SPS-50 tag would carry. + #[test] + fn envelope_bytes_roundtrip() { + let mut arb = ArbitraryGenerator::new(); + for _ in 0..64 { + let action: MultisigAction = arb.generate(); + let admin_tx_type = action.tx_type(); + + let original = SignedPayload::new(7, action, dummy_signatures()); + let bytes = original.clone().into_envelope_bytes(); + + let decoded = decode_signed_payload(admin_tx_type, &bytes, admin_tx_type.into()) + .expect("wire payload must decode under its own tx type"); + assert_eq!(decoded, original); + } + } + + #[test] + fn malformed_payload_is_rejected() { + let garbage = [0xffu8; 7]; + for admin_tx_type in [ + AdminTxType::Cancel, + AdminTxType::Update(UpdateTxType::SequencerUpdate), + ] { + let res = decode_signed_payload(admin_tx_type, &garbage, admin_tx_type.into()); + assert!(matches!( + res, + Err(AdministrationTxParseError::MalformedPayload { .. }) + )); + } + } } diff --git a/crates/subprotocols/admin/txs/src/test_utils/mod.rs b/crates/subprotocols/admin/txs/src/test_utils/mod.rs index 4d8e740f..e5c19679 100644 --- a/crates/subprotocols/admin/txs/src/test_utils/mod.rs +++ b/crates/subprotocols/admin/txs/src/test_utils/mod.rs @@ -3,7 +3,6 @@ use bitcoin::{ secp256k1::{Message, SECP256K1, SecretKey}, sign_message::MessageSignature, }; -use ssz::Encode; use strata_asm_proto_txs_test_utils::create_reveal_transaction_stub; use strata_crypto::threshold_signature::{IndexedSignature, SignatureSet}; @@ -91,7 +90,7 @@ pub fn create_test_admin_tx( // Create the signed payload (action + signatures) for the envelope let signed_payload = SignedPayload::new(seqno, action.clone(), signature_set); - let envelope_payload = signed_payload.as_ssz_bytes(); + let envelope_payload = signed_payload.into_envelope_bytes(); // Create a minimal reveal transaction structure // This is a simplified version - in practice, this would be created as part of diff --git a/tests/asm/admin.rs b/tests/asm/admin.rs index f5d87221..55d61142 100644 --- a/tests/asm/admin.rs +++ b/tests/asm/admin.rs @@ -32,7 +32,6 @@ use harness::{ }; use integration_tests::harness; use rand::rngs::OsRng; -use ssz::Encode; use strata_asm_admin_types::Role; use strata_asm_proto_admin_txs::{ constants::ADMINISTRATION_SUBPROTOCOL_ID, parser::SignedPayload, @@ -329,7 +328,7 @@ async fn test_wrong_key_rejected() { let seqno = 1; let sig_set = create_signature_set(&[wrong_privkey], &[0u8], &action, seqno); let signed = SignedPayload::new(seqno, action.clone(), sig_set); - let payload = signed.as_ssz_bytes(); + let payload = signed.into_envelope_bytes(); let tx = harness .build_envelope_tx(action.tag(), payload) @@ -380,7 +379,7 @@ async fn test_corrupted_signature_rejected() { let corrupted_sig_set = SignatureSet::new(indexed_sigs).unwrap(); let signed = SignedPayload::new(seqno, action.clone(), corrupted_sig_set); - let payload = signed.as_ssz_bytes(); + let payload = signed.into_envelope_bytes(); let tx = harness .build_envelope_tx(action.tag(), payload) diff --git a/tests/harness/admin.rs b/tests/harness/admin.rs index 5492e2f5..695639a3 100644 --- a/tests/harness/admin.rs +++ b/tests/harness/admin.rs @@ -18,7 +18,6 @@ use bitcoin::{ secp256k1::{PublicKey, Secp256k1, SecretKey}, BlockHash, Transaction, }; -use ssz::Encode; use strata_asm_admin_types::{AdministrationInitConfig, ConfirmationDepths, Role}; use strata_asm_bridge_types::SafeHarbourAddress; use strata_asm_common::{AnchorState, SectionStateExt, Subprotocol}; @@ -210,7 +209,7 @@ impl AdminContext { fn sign_impl(&self, action: &MultisigAction, signing_role: Role, seqno: u64) -> Vec { let keys = self.role_keys(signing_role); let sig_set = create_signature_set(&keys.privkeys, &keys.signer_indices, action, seqno); - SignedPayload::new(seqno, action.clone(), sig_set).as_ssz_bytes() + SignedPayload::new(seqno, action.clone(), sig_set).into_envelope_bytes() } }