refactor(admin): adopt SignedAction and log parse/handle failures

Author: irnb

crates/subprotocols/admin/src/authority.rs

@@ -2,7 +2,7 @@ use std::num::NonZero;
 
 use ssz_derive::{Decode, Encode};
 use strata_asm_params::Role;
-use strata_asm_txs_admin::{actions::Sighash, parser::SignedPayload};
+use strata_asm_txs_admin::{actions::Sighash, signed_action::SignedAction};
 use strata_crypto::threshold_signature::{ThresholdConfig, verify_threshold_signatures};
 
 use crate::error::AdministrationError;
@@ -65,7 +65,7 @@ impl MultisigAuthority {
     // could be added in the future if multiple signature schemes are needed.
     pub fn verify_action_signature(
         &self,
-        payload: &SignedPayload,
+        payload: &SignedAction,
         max_seqno_gap: NonZero<u8>,
     ) -> Result<SeqNoToken, AdministrationError> {
         if payload.seqno <= self.last_seqno {

crates/subprotocols/admin/src/handler.rs

@@ -5,7 +5,7 @@ use strata_asm_common::{
 };
 use strata_asm_txs_admin::{
     actions::{MultisigAction, UpdateAction, updates::predicate::ProofType},
-    parser::SignedPayload,
+    signed_action::SignedAction,
 };
 use strata_identifiers::{Buf32, L1Height};
 use strata_predicate::{PredicateKey, PredicateTypeId};
@@ -99,7 +99,7 @@ pub(crate) fn handle_pending_updates(
 /// * `Err(AdministrationError)` if validation failed or the action could not be processed
 pub(crate) fn handle_action(
     state: &mut AdministrationSubprotoState,
-    payload: SignedPayload,
+    payload: SignedAction,
     current_height: L1Height,
     relayer: &mut impl MsgRelayer,
 ) -> Result<(), AdministrationError> {
@@ -192,7 +192,7 @@ mod tests {
                 seq::SequencerUpdate,
             },
         },
-        parser::SignedPayload,
+        signed_action::SignedAction,
         test_utils::create_signature_set,
     };
     use strata_crypto::{
@@ -318,7 +318,7 @@ mod tests {
             let action = MultisigAction::Update(update.clone());
             let sighash = action.compute_sighash(seqno);
             let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-            let payload = SignedPayload::new(seqno, action, sig_set);
+            let payload = SignedAction::new(seqno, action, sig_set);
             handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
 
             // Verify state changes after processing
@@ -371,7 +371,7 @@ mod tests {
         let action = MultisigAction::Update(update.clone());
         let sighash = action.compute_sighash(valid_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(valid_seqno, action, sig_set);
+        let payload = SignedAction::new(valid_seqno, action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
         assert!(res.is_ok());
 
@@ -380,7 +380,7 @@ mod tests {
         let sighash = action.compute_sighash(1);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
 
-        let payload = SignedPayload::new(1, action, sig_set);
+        let payload = SignedAction::new(1, action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
 
         assert!(res.is_err());
@@ -397,7 +397,7 @@ mod tests {
         let action = MultisigAction::Update(update.clone());
         let sighash = action.compute_sighash(0);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(0, action, sig_set);
+        let payload = SignedAction::new(0, action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
         assert!(matches!(res, Err(AdministrationError::InvalidSeqno { .. })));
     }
@@ -438,7 +438,7 @@ mod tests {
             let sighash = action.compute_sighash(payload_seqno);
             let sig_set = create_signature_set(&seq_manager_sks, &signer_indices, sighash);
 
-            let payload = SignedPayload::new(payload_seqno, action, sig_set);
+            let payload = SignedAction::new(payload_seqno, action, sig_set);
             handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
 
             // Verify state changes after processing
@@ -531,7 +531,7 @@ mod tests {
             let sighash = update_action.compute_sighash(payload_seqno);
             let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
 
-            let payload = SignedPayload::new(payload_seqno, update_action, sig_set);
+            let payload = SignedAction::new(payload_seqno, update_action, sig_set);
             handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
         }
 
@@ -552,7 +552,7 @@ mod tests {
             let sighash = cancel_action.compute_sighash(payload_seqno);
             let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
 
-            let payload = SignedPayload::new(payload_seqno, cancel_action, sig_set);
+            let payload = SignedAction::new(payload_seqno, cancel_action, sig_set);
             handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
 
             // Verify state changes after cancellation
@@ -587,7 +587,7 @@ mod tests {
         let cancel_action: CancelAction = arb.generate();
         let cancel_action = MultisigAction::Cancel(cancel_action);
 
-        let payload = SignedPayload::new(arb.generate(), cancel_action, sig_set);
+        let payload = SignedAction::new(arb.generate(), cancel_action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
 
         assert!(matches!(res, Err(AdministrationError::UnknownAction(_))));
@@ -621,7 +621,7 @@ mod tests {
         let sighash = update_action.compute_sighash(update_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
 
-        let payload = SignedPayload::new(update_seqno, update_action, sig_set);
+        let payload = SignedAction::new(update_seqno, update_action, sig_set);
         handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
 
         // Cancel the update action (authority seqno is now 1, use seqno 2)
@@ -630,7 +630,7 @@ mod tests {
         let sighash = cancel_action.compute_sighash(cancel_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
 
-        let payload = SignedPayload::new(cancel_seqno, cancel_action, sig_set);
+        let payload = SignedAction::new(cancel_seqno, cancel_action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
 
         assert!(res.is_ok());
@@ -640,7 +640,7 @@ mod tests {
         let retry_seqno = last_seqno + 3;
         let sighash = cancel_action.compute_sighash(retry_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(retry_seqno, cancel_action, sig_set);
+        let payload = SignedAction::new(retry_seqno, cancel_action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
         assert!(res.is_err());
         assert!(matches!(res, Err(AdministrationError::UnknownAction(_))));
@@ -662,15 +662,15 @@ mod tests {
         let action = MultisigAction::Update(updates[0].clone());
         let sighash = action.compute_sighash(1);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(1, action, sig_set);
+        let payload = SignedAction::new(1, action, sig_set);
         handle_action(&mut state, payload, current_height, &mut relayer).unwrap();
 
         // Second action at seqno 11 (last_seqno is 1, gap = 10 = max_seqno_gap)
         let gap_seqno = 1 + state.max_seqno_gap().get() as u64;
         let action = MultisigAction::Update(updates[1].clone());
         let sighash = action.compute_sighash(gap_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(gap_seqno, action, sig_set);
+        let payload = SignedAction::new(gap_seqno, action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
 
         assert!(
@@ -695,7 +695,7 @@ mod tests {
         let action = MultisigAction::Update(update);
         let sighash = action.compute_sighash(too_far_seqno);
         let sig_set = create_signature_set(&admin_sks, &signer_indices, sighash);
-        let payload = SignedPayload::new(too_far_seqno, action, sig_set);
+        let payload = SignedAction::new(too_far_seqno, action, sig_set);
         let res = handle_action(&mut state, payload, current_height, &mut relayer);
 
         assert!(res.is_err());

crates/subprotocols/admin/src/subprotocol.rs

@@ -4,7 +4,7 @@
 //! with the Strata Anchor State Machine (ASM) for managing protocol governance and updates.
 
 use strata_asm_common::{
-    MsgRelayer, NullMsg, Subprotocol, SubprotocolId, TxInputRef, VerifiedAuxData,
+    MsgRelayer, NullMsg, Subprotocol, SubprotocolId, TxInputRef, VerifiedAuxData, logging::warn,
 };
 use strata_asm_params::AdministrationInitConfig;
 use strata_asm_txs_admin::{constants::ADMINISTRATION_SUBPROTOCOL_ID, parser::parse_tx};
@@ -55,10 +55,26 @@ impl Subprotocol for AdministrationSubprotocol {
 
         // Phase 2: Process incoming administration transactions
         for tx in txs {
-            if let Ok(signed_payload) = parse_tx(tx) {
-                let _ = handle_action(state, signed_payload, current_height, relayer);
+            match parse_tx(tx) {
+                Ok(signed_action) => {
+                    if let Err(error) = handle_action(state, signed_action, current_height, relayer)
+                    {
+                        warn!(
+                            tx_id = %tx.tx().compute_txid(),
+                            error = %error,
+                            "Failed to process admin tx; skipping",
+                        );
+                    }
+                }
+                Err(error) => {
+                    warn!(
+                        tx_id = %tx.tx().compute_txid(),
+                        raw_tx_type = tx.tag().tx_type(),
+                        error = %error,
+                        "Failed to parse admin tx; skipping",
+                    );
+                }
             }
-            // Transaction parsing failures are silently ignored to maintain system resilience
         }
     }
 

tests/asm/admin.rs

@@ -35,12 +35,11 @@ use harness::{
 };
 use integration_tests::harness;
 use rand::rngs::OsRng;
-use ssz::Encode;
 use strata_asm_params::Role;
 use strata_asm_txs_admin::{
     actions::{updates::predicate::ProofType, Sighash},
     constants::ADMINISTRATION_SUBPROTOCOL_ID,
-    parser::SignedPayload,
+    signed_action::SignedAction,
     test_utils::create_signature_set,
 };
 use strata_crypto::{
@@ -350,11 +349,11 @@ async fn test_wrong_key_rejected() {
     let seqno = 1;
     let sighash = action.compute_sighash(seqno);
     let sig_set = create_signature_set(&[wrong_privkey], &[0u8], sighash);
-    let signed = SignedPayload::new(seqno, action.clone(), sig_set);
-    let payload = signed.as_ssz_bytes();
+    let signed_action = SignedAction::new(seqno, action.clone(), sig_set);
+    let payload = signed_action.payload_bytes();
 
     let tx = harness
-        .build_envelope_tx(action.tag(), payload)
+        .build_envelope_tx(signed_action.tag(), payload)
         .await
         .unwrap();
 
@@ -401,11 +400,11 @@ 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 signed_action = SignedAction::new(seqno, action.clone(), corrupted_sig_set);
+    let payload = signed_action.payload_bytes();
 
     let tx = harness
-        .build_envelope_tx(action.tag(), payload)
+        .build_envelope_tx(signed_action.tag(), payload)
         .await
         .unwrap();
 
@@ -474,20 +473,20 @@ async fn test_multiple_updates_same_block() {
     let action2 = sequencer_update([8u8; 32]);
     let action3 = sequencer_update([9u8; 32]);
 
-    let payload1 = ctx.sign(&action1);
-    let payload2 = ctx.sign(&action2);
-    let payload3 = ctx.sign(&action3);
+    let signed_action_1 = ctx.sign(&action1);
+    let signed_action_2 = ctx.sign(&action2);
+    let signed_action_3 = ctx.sign(&action3);
 
     let tx1 = harness
-        .build_envelope_tx(action1.tag(), payload1)
+        .build_envelope_tx(signed_action_1.tag(), signed_action_1.payload_bytes())
         .await
         .unwrap();
     let tx2 = harness
-        .build_envelope_tx(action2.tag(), payload2)
+        .build_envelope_tx(signed_action_2.tag(), signed_action_2.payload_bytes())
         .await
         .unwrap();
     let tx3 = harness
-        .build_envelope_tx(action3.tag(), payload3)
+        .build_envelope_tx(signed_action_3.tag(), signed_action_3.payload_bytes())
         .await
         .unwrap();
 

tests/harness/admin.rs

@@ -23,7 +23,6 @@ use bitcoin::{
     secp256k1::{PublicKey, Secp256k1, SecretKey},
     BlockHash,
 };
-use ssz::Encode;
 use strata_asm_common::{AnchorState, Subprotocol};
 use strata_asm_params::{AdministrationInitConfig, Role};
 use strata_asm_proto_administration::{AdministrationSubprotoState, AdministrationSubprotocol};
@@ -37,7 +36,7 @@ use strata_asm_txs_admin::{
         },
         CancelAction, MultisigAction, Sighash, UpdateAction,
     },
-    parser::SignedPayload,
+    signed_action::SignedAction,
     test_utils::create_signature_set,
 };
 use strata_crypto::{
@@ -97,10 +96,10 @@ impl AdminContext {
         }
     }
 
-    /// Sign an action and return the serialized payload.
+    /// Sign an action and return the signed admin payload.
     ///
     /// Auto-increments the appropriate role's sequence number after signing.
-    pub fn sign(&mut self, action: &MultisigAction) -> Vec<u8> {
+    pub fn sign(&mut self, action: &MultisigAction) -> SignedAction {
         let role = Self::role_for_action(action);
         let seqno = *self.seqnos.entry(role).or_insert(1);
         let result = self.sign_impl(action, seqno);
@@ -111,7 +110,7 @@ impl AdminContext {
     /// Sign an action with a specific sequence number (for replay attack testing).
     ///
     /// Does NOT auto-increment the internal sequence number.
-    pub fn sign_with_seqno(&self, action: &MultisigAction, seqno: u64) -> Vec<u8> {
+    pub fn sign_with_seqno(&self, action: &MultisigAction, seqno: u64) -> SignedAction {
         self.sign_impl(action, seqno)
     }
 
@@ -133,10 +132,10 @@ impl AdminContext {
         }
     }
 
-    fn sign_impl(&self, action: &MultisigAction, seqno: u64) -> Vec<u8> {
+    fn sign_impl(&self, action: &MultisigAction, seqno: u64) -> SignedAction {
         let sighash = action.compute_sighash(seqno);
         let sig_set = create_signature_set(&self.privkeys, &self.signer_indices, sighash);
-        SignedPayload::new(seqno, action.clone(), sig_set).as_ssz_bytes()
+        SignedAction::new(seqno, action.clone(), sig_set)
     }
 }
 
@@ -242,8 +241,10 @@ impl AdminExt for AsmTestHarness {
         ctx: &mut AdminContext,
         action: MultisigAction,
     ) -> anyhow::Result<BlockHash> {
-        let payload = ctx.sign(&action);
-        let tx = self.build_envelope_tx(action.tag(), payload).await?;
+        let signed_action = ctx.sign(&action);
+        let tx = self
+            .build_envelope_tx(signed_action.tag(), signed_action.payload_bytes())
+            .await?;
         self.submit_and_mine_tx(&tx).await
     }
 
@@ -253,9 +254,10 @@ impl AdminExt for AsmTestHarness {
         action: MultisigAction,
         seqno: u64,
     ) -> anyhow::Result<BlockHash> {
-        let tag = action.tag();
-        let payload = ctx.sign_with_seqno(&action, seqno);
-        let tx = self.build_envelope_tx(tag, payload).await?;
+        let signed_action = ctx.sign_with_seqno(&action, seqno);
+        let tx = self
+            .build_envelope_tx(signed_action.tag(), signed_action.payload_bytes())
+            .await?;
         self.submit_and_mine_tx(&tx).await
     }
 }