Fix OL ignoring withdrawal fulfillments

bin/strata-client/src/errors.rs

@@ -10,6 +10,9 @@ pub(crate) enum InitError {
     #[error("missing init client state")]
     MissingInitClientState,
 
+    #[error("rollup name too long")]
+    InvalidRollupName(String),
+
     #[error("io: {0}")]
     Io(#[from] io::Error),
 

bin/strata-client/src/main.rs

@@ -80,6 +80,14 @@ fn main_inner(args: Args) -> anyhow::Result<()> {
     let params = resolve_and_validate_params(args.rollup_params.as_deref(), &config)
         .map_err(anyhow::Error::from)?;
 
+    // Sanity check to ensure we don't break our assumptions.
+    let rollup_name = &params.rollup.rollup_name;
+    if rollup_name.len() != 4 {
+        error!("aborting due to invalid rollup name, must be 4 bytes!");
+        error!("we use this as the bridge tx magic value, this will be fixed in a future version");
+        return Err(InitError::InvalidRollupName(rollup_name.to_owned()).into());
+    }
+
     // Init the task manager and logging before we do anything else.
     let runtime = tokio::runtime::Builder::new_multi_thread()
         .enable_all()

crates/asm/common/src/tx.rs

@@ -1,6 +1,6 @@
 use bitcoin::Transaction;
 
-/// A parsed SPS-50 tag payload (excluding the “ALPN” magic and subprotocol ID),
+/// A parsed SPS-50 tag payload (excluding the "ALPN" magic and subprotocol ID),
 /// containing the subprotocol-specific transaction type and any auxiliary data.
 ///
 /// This struct represents everything in the OP_RETURN after the first 6 bytes:

crates/asm/stf/src/tx_filter.rs

@@ -9,7 +9,7 @@ use strata_asm_common::{SubprotocolId, TagPayload, TxInput};
 ///
 /// The SPS-50 header MUST be encoded as an `OP_RETURN` in output index 0, with payload:
 /// ```text
-/// [0..4]   ASCII magic “ALPN”
+/// [0..4]   ASCII magic "ALPN"
 /// [4]      subprotocol type (u8)
 /// [5]      tx type (u8)
 /// [6..]    auxiliary data (ignored here)

crates/btcio/Cargo.toml

@@ -38,7 +38,7 @@ strata-state = { workspace = true, features = ["test_utils"] }
 strata-status.workspace = true
 strata-test-utils.workspace = true
 
-corepc-node = { version = "0.7.1", features = ["28_0", "download"] }
+corepc-node = { version = "0.8.0", features = ["29_0", "download"] }
 
 [features]
 test_utils = ["dep:hex", "dep:musig2"]

crates/chaintsn/src/errors.rs

@@ -55,6 +55,9 @@ pub enum OpError {
     #[error("invalid proof")]
     InvalidProof,
 
+    #[error("op referenced non-existent deposit {0}")]
+    UnknownDeposit(u32),
+
     /// Used to discard checkpoints we aren't looking for.
     #[error("operation does not advance the finalized epoch")]
     EpochNotExtend,

crates/chaintsn/src/transition.rs

@@ -162,17 +162,20 @@ fn process_l1_block(
     block_mf: &L1BlockManifest,
     params: &RollupParams,
 ) -> Result<(), TsnError> {
+    let blkid = block_mf.blkid();
+
     // Just iterate through every tx's operation and call out to the handlers for that.
     for tx in block_mf.txs() {
-        let in_blkid = block_mf.blkid();
         for op in tx.protocol_ops() {
             // Try to process it, log a warning if there's an error.
             if let Err(e) = process_proto_op(state, block_mf, op, params) {
-                warn!(?op, %in_blkid, %e, "invalid protocol operation");
+                warn!(?op, in_blkid = %blkid, %e, "invalid protocol operation");
             }
         }
     }
 
+    debug!(%blkid, "processed block manifest");
+
     Ok(())
 }
 
@@ -184,18 +187,28 @@ fn process_proto_op(
 ) -> Result<(), OpError> {
     match &op {
         ProtocolOperation::Checkpoint(ckpt) => {
+            let epoch = ckpt.checkpoint().batch_info().epoch();
+            debug!(%epoch, "processing checkpoint proto-op");
             process_l1_checkpoint(state, block_mf, ckpt, params)?;
         }
 
         ProtocolOperation::Deposit(info) => {
+            let deposit_idx = info.deposit_idx;
+            debug!(%deposit_idx, "processing deposit proto-op");
             process_l1_deposit(state, block_mf, info)?;
         }
 
         ProtocolOperation::WithdrawalFulfillment(info) => {
+            let deposit_idx = info.deposit_idx;
+            let txid = &info.txid;
+            debug!(%deposit_idx, %txid, "processing withdrawal fulfillment proto-op");
             process_withdrawal_fulfillment(state, info)?;
         }
 
         ProtocolOperation::DepositSpent(info) => {
+            let deposit_idx = info.deposit_idx;
+            let txid = &info.deposit_idx;
+            debug!(%deposit_idx, %txid, "processing reimbursement proto-op");
             process_deposit_spent(state, info)?;
         }
 
@@ -294,13 +307,20 @@ fn process_withdrawal_fulfillment(
     state: &mut StateCache,
     info: &WithdrawalFulfillmentInfo,
 ) -> Result<(), OpError> {
+    if !state.check_deposit_exists(info.deposit_idx) {
+        return Err(OpError::UnknownDeposit(info.deposit_idx));
+    }
+
     state.mark_deposit_fulfilled(info);
     Ok(())
 }
 
 /// Locked deposit on L1 has been spent.
 fn process_deposit_spent(state: &mut StateCache, info: &DepositSpendInfo) -> Result<(), OpError> {
     // Currently, we are not tracking how this was spent, only that it was.
+    if !state.check_deposit_exists(info.deposit_idx) {
+        return Err(OpError::UnknownDeposit(info.deposit_idx));
+    }
 
     state.mark_deposit_reimbursed(info.deposit_idx);
     Ok(())

crates/l1tx/src/deposit/deposit_tx.rs

@@ -234,10 +234,12 @@ mod tests {
         },
         error::DepositParseError,
     };
-    const MAGIC_BYTES: &[u8] = &[1, 2, 3, 4, 5];
+
+    const MAGIC_BYTES: &[u8] = &[1, 2, 3, 4];
     const ADDRESS: &str = "bcrt1p729l9680ht3zf7uhl6pgdrlhfp9r29cwajr5jk3k05fer62763fscz0w4s";
 
     fn dummy_config() -> DepositTxParams {
+        assert_eq!(MAGIC_BYTES.len(), 4, "test: magic not 4 bytes");
         let addr = BitcoinAddress::parse(ADDRESS, Network::Regtest).unwrap();
         DepositTxParams {
             magic_bytes: MAGIC_BYTES.to_vec(),

crates/l1tx/src/deposit/test_utils.rs

@@ -3,7 +3,7 @@ use strata_test_utils::bitcoin::test_taproot_addr;
 
 pub fn get_deposit_tx_config() -> DepositTxParams {
     DepositTxParams {
-        magic_bytes: "stratasss".to_string().as_bytes().to_vec(),
+        magic_bytes: "ALPN".to_string().as_bytes().to_vec(),
         address_length: 20,
         deposit_amount: 1_000_000_000,
         address: test_taproot_addr(),

crates/l1tx/src/filter/types.rs

@@ -55,6 +55,15 @@ impl TxFilterConfig {
             generate_taproot_address(&operator_wallet_pks, rollup_params.network)?;
 
         let rollup_name = rollup_params.rollup_name.clone();
+
+        // Make sure the magic bytes is always of length 4.
+        // TODO replace this with a more generic system
+        let magic_bytes = rollup_name
+            .bytes()
+            .chain(std::iter::repeat(0))
+            .take(4)
+            .collect::<Vec<_>>();
+
         let expected_addrs = SortedVec::new_unchecked(vec![address.clone()]);
         let sequencer_cred_rule = rollup_params.cred_rule.clone();
 
@@ -65,8 +74,9 @@ impl TxFilterConfig {
 
         let operators_pubkey =
             XOnlyPk::new(int_pubkey.serialize().into()).expect("Aggregated pubkey should be valid");
+
         let deposit_config = DepositTxParams {
-            magic_bytes: rollup_name.clone().into_bytes(),
+            magic_bytes,
             address_length: rollup_params.address_length,
             deposit_amount: rollup_params.deposit_amount,
             address,