Fix resubmit checkpoint bug (#547)

* func-tests: add func test to resubmit published checkpoints

* consensus-logic: fix bug introduced by resending previous checkpoint transaction

* consensus-logic: change invalid comparison for tsn for proof verification and formatting

* func-tests: move common function to utilities and add more asserts

* func-tests: formatting

* func-tests: formatting fix

* func-tests: ruff format

* func-test: change logic for verifying tx for submit_da_blob

crates/consensus-logic/src/csm/client_transition.rs

@@ -162,24 +162,26 @@ pub fn process_event<D: Database>(
 
                 // When DABatch appears, it is only confirmed at the moment. These will be finalized
                 // only when the corresponding L1 block is buried enough
-                writes.push(ClientStateWrite::CheckpointsReceived(
-                    checkpoints
-                        .iter()
-                        .map(|x| {
-                            L1Checkpoint::new(
-                                x.batch_info().clone(),
-                                x.bootstrap_state().clone(),
-                                !x.proof().is_empty(),
-                                *height,
-                            )
-                        })
-                        .collect(),
-                ));
-
-                actions.push(SyncAction::WriteCheckpoints(
-                    *height,
-                    proof_verified_checkpoints,
-                ));
+                if !proof_verified_checkpoints.is_empty() {
+                    writes.push(ClientStateWrite::CheckpointsReceived(
+                        proof_verified_checkpoints
+                            .iter()
+                            .map(|x| {
+                                L1Checkpoint::new(
+                                    x.batch_info().clone(),
+                                    x.bootstrap_state().clone(),
+                                    !x.proof().is_empty(),
+                                    *height,
+                                )
+                            })
+                            .collect(),
+                    ));
+
+                    actions.push(SyncAction::WriteCheckpoints(
+                        *height,
+                        proof_verified_checkpoints,
+                    ));
+                }
             } else {
                 // TODO we can expand this later to make more sense
                 return Err(Error::MissingClientSyncState);
@@ -429,11 +431,11 @@ pub fn filter_verified_checkpoints(
             let l1_tsn = checkpoint.batch_info().l1_transition;
             let l2_tsn = checkpoint.batch_info().l2_transition;
 
-            if l1_tsn.0 == last_l1_tsn.1 {
+            if l1_tsn.0 != last_l1_tsn.1 {
                 warn!(obtained = ?l1_tsn.0, expected = ?last_l1_tsn.1, "Received invalid checkpoint l1 transition, ignoring.");
                 continue;
             }
-            if l2_tsn.0 == last_l2_tsn.1 {
+            if l2_tsn.0 != last_l2_tsn.1 {
                 warn!(obtained = ?l2_tsn.0, expected = ?last_l2_tsn.1, "Received invalid checkpoint l2 transition, ignoring.");
                 continue;
             }

functional-tests/fn_btcio_inscriber.py

@@ -1,11 +1,8 @@
-import time
-
 import flexitest
 from bitcoinlib.services.bitcoind import BitcoindClient
 
 import testenv
-from constants import SEQ_PUBLISH_BATCH_INTERVAL_SECS
-from utils import generate_n_blocks, wait_until
+from utils import generate_n_blocks, submit_da_blob, wait_until
 
 
 @flexitest.register
@@ -33,21 +30,11 @@ def main(self, ctx: flexitest.RunContext):
 
         # Submit blob
         blobdata = "2c4253d512da5bb4223f10e8e6017ede69cc63d6e6126916f4b68a1830b7f805"
-        _ = seqrpc.strataadmin_submitDABlob(blobdata)
-
-        # Allow some time for sequencer to publish blob
-        time.sleep(SEQ_PUBLISH_BATCH_INTERVAL_SECS)
-
-        l1_status = seqrpc.strata_l1status()
-        txid = l1_status["last_published_txid"]
-
+        tx = submit_da_blob(btcrpc, seqrpc, blobdata)
         # Calculate scriptbpubkey for sequencer address
         addrdata = btcrpc.proxy.validateaddress(seqaddr)
         scriptpubkey = addrdata["scriptPubKey"]
 
-        # Check if txn is present in mempool/blockchain and is spent to sequencer address
-        tx = btcrpc.gettransaction(txid)
-
         # NOTE: could have just compared address
         # but bitcoinlib is somehow giving bc1* addr even though network is regtest
         assert (

functional-tests/fn_btcio_resubmit_checkpoint.py

@@ -0,0 +1,77 @@
+import flexitest
+from bitcoinlib.services.bitcoind import BitcoindClient
+
+import testenv
+from utils import (
+    generate_n_blocks,
+    get_envelope_pushdata,
+    submit_da_blob,
+    wait_until,
+    wait_until_with_value,
+)
+
+
+@flexitest.register
+class ResubmitCheckpointTest(testenv.StrataTester):
+    def __init__(self, ctx: flexitest.InitContext):
+        ctx.set_env("basic")
+
+    def main(self, ctx: flexitest.RunContext):
+        btc = ctx.get_service("bitcoin")
+        seq = ctx.get_service("sequencer")
+        btcrpc: BitcoindClient = btc.create_rpc()
+        seqrpc = seq.create_rpc()
+
+        # generate 5 btc blocks
+        generate_n_blocks(btcrpc, 5)
+
+        # Generate some funds to sequencer
+        seqaddr = seq.get_prop("address")
+
+        # Wait for seq
+        wait_until(
+            lambda: seqrpc.strata_protocolVersion() is not None,
+            error_with="Sequencer did not start on time",
+        )
+
+        verified_on = wait_until_with_value(
+            lambda: seqrpc.strata_getL2BlockStatus(1),
+            predicate=lambda val: isinstance(val, dict) and "Finalized" in val,
+            error_with="transactions are not being Finalized",
+            timeout=10,
+        )
+        verified_block_hash = btcrpc.proxy.getblockhash(verified_on["Finalized"])
+        block_data = btcrpc.getblock(verified_block_hash)
+        envelope_data = ""
+        for tx in block_data["txs"]:
+            try:
+                envelope_data = get_envelope_pushdata(tx.witness_data().hex())
+            except ValueError:
+                print("Not an envelope transaction")
+                continue
+
+        tx = submit_da_blob(btcrpc, seqrpc, envelope_data)
+        # Calculate scriptbpubkey for sequencer address
+        addrdata = btcrpc.proxy.validateaddress(seqaddr)
+        scriptpubkey = addrdata["scriptPubKey"]
+
+        # NOTE: could have just compared address
+        # but bitcoinlib is somehow giving bc1* addr even though network is regtest
+        assert (
+            tx.outputs[0].lock_script.hex() == scriptpubkey
+        ), "Output should be locked to sequencer's scriptpubkey"
+
+        # ensure that client is still up and running
+        wait_until(
+            lambda: seqrpc.strata_protocolVersion() is not None,
+            error_with="sequencer rpc is not working",
+        )
+
+        # check if chain tip is being increased
+        cur_chain_tip = seqrpc.strata_clientStatus()["chain_tip_slot"]
+        wait_until(
+            lambda: seqrpc.strata_clientStatus()["chain_tip_slot"] > cur_chain_tip,
+            "chain tip slot hasn't changed since resubmit of checkpoint blob",
+        )
+
+        return True

functional-tests/utils.py

@@ -11,6 +11,7 @@
 from strata_utils import convert_to_xonly_pk, musig_aggregate_pks
 
 from constants import *
+from seqrpc import JsonrpcClient
 
 
 def generate_jwt_secret() -> str:
@@ -464,3 +465,29 @@ def setup_test_logger(datadir_root: str, test_name: str) -> logging.Logger:
     logger.addHandler(stream_handler)
 
     return logger
+
+
+def get_envelope_pushdata(inp: str):
+    op_if = "63"
+    op_endif = "68"
+    op_pushbytes_33 = "21"
+    op_false = "00"
+    start_position = inp.index(f"{op_false}{op_if}")
+    end_position = inp.index(f"{op_endif}{op_pushbytes_33}", start_position)
+    op_if_block = inp[start_position + 3 : end_position]
+    op_pushdata = "4d"
+    pushdata_position = op_if_block.index(f"{op_pushdata}")
+    # we don't want PUSHDATA + num bytes b401
+    return op_if_block[pushdata_position + 2 + 4 :]
+
+
+def submit_da_blob(btcrpc: BitcoindClient, seqrpc: JsonrpcClient, blobdata: str):
+    _ = seqrpc.strataadmin_submitDABlob(blobdata)
+
+    # if blob data is present in tx witness then return the transaction
+    tx = wait_until_with_value(
+        lambda: btcrpc.gettransaction(seqrpc.strata_l1status()["last_published_txid"]),
+        predicate=lambda tx: blobdata in tx.witness_data().hex(),
+        timeout=10,
+    )
+    return tx