Merge pull request #583 from logos-blockchain/arjentix/fix-zero-deposits

fix(sequencer): don't accept zero deposits & remove unreachable

Cargo.lock

@@ -206,9 +206,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.102"
+version = "1.0.103"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c"
+checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3"
 
 [[package]]
 name = "arbitrary"

integration_tests/tests/bridge.rs

@@ -98,6 +98,66 @@ async fn public_bridge_deposit_invocation_is_dropped() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+async fn public_bridge_deposit_with_zero_amount_is_rejected() -> anyhow::Result<()> {
+    let ctx = TestContext::new().await?;
+
+    let recipient_id = ctx.existing_public_accounts()[0];
+    let bridge_account_id = system_accounts::bridge_account_id();
+    let vault_program_id = programs::vault().id();
+    let recipient_vault_id = vault_core::compute_vault_account_id(vault_program_id, recipient_id);
+
+    let message = public_transaction::Message::try_new(
+        programs::bridge().id(),
+        vec![bridge_account_id, recipient_vault_id],
+        vec![],
+        bridge_core::Instruction::Deposit {
+            l1_deposit_op_id: [0_u8; 32],
+            vault_program_id,
+            recipient_id,
+            amount: 0,
+        },
+    )
+    .context("Failed to build zero-amount public bridge deposit transaction")?;
+
+    let attack_tx = LeeTransaction::Public(lee::PublicTransaction::new(
+        message,
+        lee::public_transaction::WitnessSet::from_raw_parts(vec![]),
+    ));
+
+    let bridge_balance_before = ctx
+        .sequencer_client()
+        .get_account_balance(bridge_account_id)
+        .await?;
+    let vault_balance_before = ctx
+        .sequencer_client()
+        .get_account_balance(recipient_vault_id)
+        .await?;
+
+    let tx_hash = ctx.sequencer_client().send_transaction(attack_tx).await?;
+
+    tokio::time::sleep(Duration::from_secs(TIME_TO_WAIT_FOR_BLOCK_SECONDS)).await;
+
+    let bridge_balance_after = ctx
+        .sequencer_client()
+        .get_account_balance(bridge_account_id)
+        .await?;
+    let vault_balance_after = ctx
+        .sequencer_client()
+        .get_account_balance(recipient_vault_id)
+        .await?;
+    let tx_on_chain = ctx.sequencer_client().get_transaction(tx_hash).await?;
+
+    assert_eq!(bridge_balance_after, bridge_balance_before);
+    assert_eq!(vault_balance_after, vault_balance_before);
+    assert!(
+        tx_on_chain.is_none(),
+        "Public bridge::Deposit with zero amount should be rejected"
+    );
+
+    Ok(())
+}
+
 #[test]
 async fn private_bridge_deposit_invocation_is_dropped() -> anyhow::Result<()> {
     let ctx = TestContext::new().await?;

lez/common/src/transaction.rs

@@ -173,7 +173,7 @@ impl LeeTransaction {
                 balance: pre.balance,
                 ..post.clone()
             };
-            (expected_pre == pre) && (pre.balance <= post.balance)
+            (expected_pre == pre) && (pre.balance < post.balance)
         };
 
         if only_balance_increased {

lez/sequencer/core/src/lib.rs

@@ -729,26 +729,23 @@ fn extract_bridge_withdraw_data(tx: &LeeTransaction) -> Option<WithdrawArg> {
         risc0_zkvm::serde::from_slice::<bridge_core::Instruction, u32>(&message.instruction_data)
             .ok()?;
 
-    match instruction {
-        bridge_core::Instruction::Withdraw {
-            amount,
-            bedrock_account_pk,
-        } => {
-            let recipient_pk =
-                logos_blockchain_key_management_system_service::keys::ZkPublicKey::from(
-                    BigUint::from_bytes_le(&bedrock_account_pk),
-                );
+    let bridge_core::Instruction::Withdraw {
+        amount,
+        bedrock_account_pk,
+    } = instruction
+    else {
+        return None;
+    };
 
-            Some(WithdrawArg {
-                outputs: logos_blockchain_core::mantle::ledger::Outputs::new(
-                    logos_blockchain_core::mantle::Note::new(amount, recipient_pk),
-                ),
-            })
-        }
-        bridge_core::Instruction::Deposit { .. } => unreachable!(
-            "Deposit instructions from users should never pass validation, and thus should never be seen here"
+    let recipient_pk = logos_blockchain_key_management_system_service::keys::ZkPublicKey::from(
+        BigUint::from_bytes_le(&bedrock_account_pk),
+    );
+
+    Some(WithdrawArg {
+        outputs: logos_blockchain_core::mantle::ledger::Outputs::new(
+            logos_blockchain_core::mantle::Note::new(amount, recipient_pk),
         ),
-    }
+    })
 }
 
 fn withdraw_event_reconciliation_key(