Reject re-created outpoint keys and zero-input transactions

app/gui/bitassets/reserve_register.rs

@@ -1,7 +1,7 @@
 use std::borrow::Cow;
 
 use eframe::egui;
-use plain_bitassets::types::BitAssetData;
+use plain_bitassets::types::{BitAssetData, Transaction};
 
 use crate::{
     app::App,
@@ -25,7 +25,13 @@ fn register_bitasset(
     bitasset_data: Cow<BitAssetData>,
     fee: bitcoin::Amount,
 ) -> anyhow::Result<()> {
-    let mut tx = app.wallet.create_regular_transaction(fee)?;
+    // The reservation input funds a zero-fee registration, so only select
+    // Bitcoin inputs if there is a fee to pay.
+    let mut tx = if fee == bitcoin::Amount::ZERO {
+        Transaction::default()
+    } else {
+        app.wallet.create_regular_transaction(fee)?
+    };
     let () = app.wallet.register_bitasset(
         &mut tx,
         plaintext_name,

app/rpc_server.rs

@@ -599,7 +599,13 @@ impl RpcServer for RpcServerImpl {
     }
 
     async fn reserve_bitasset(&self, plain_name: String) -> RpcResult<Txid> {
-        let mut tx = Transaction::default();
+        // Consensus requires at least one input, so fund the reservation with
+        // a zero-fee regular transaction.
+        let mut tx = self
+            .app
+            .wallet
+            .create_regular_transaction(Amount::ZERO)
+            .map_err(custom_err)?;
         let () = match self.app.wallet.reserve_bitasset(&mut tx, &plain_name) {
             Ok(()) => (),
             Err(err) => return Err(custom_err(err)),

lib/state/block.rs

@@ -8,8 +8,8 @@ use crate::{
     types::{
         AmountOverflowError, Authorization, BitAssetId, Body, FilledOutput,
         FilledOutputContent, GetAddress as _, GetBitcoinValue as _, Header,
-        InPoint, OutPoint, OutPointKey, OutputContent, SpentOutput, TxData,
-        Verify as _,
+        InPoint, MerkleRoot, OutPoint, OutPointKey, OutputContent, SpentOutput,
+        TxData, Txid, Verify as _,
     },
 };
 
@@ -18,6 +18,44 @@ fn calculate_total_inputs(body: &Body) -> usize {
     body.transactions.iter().map(|t| t.inputs.len()).sum()
 }
 
+/// Check that a block body does not create any outpoint key twice, and that
+/// its coinbase outpoints do not already exist. Must run after the merkle root
+/// check, against the pre-block state.
+///
+/// * A body may not contain the same txid twice. With unique txids, every
+///   `Regular { txid, vout }` key created by the body is distinct, coinbase
+///   keys differ by `vout`, and keys of different `OutPoint` variants never
+///   collide, so no body creates the same key twice. Transaction output keys
+///   are checked against existing state per transaction in
+///   `State::validate_filled_transaction`.
+/// * Coinbase outpoint keys (`Coinbase { merkle_root, vout }`) must not
+///   already exist in `utxos` or `stxos`. The merkle root commits only to
+///   `(coinbase, transactions)`, not to the parent block, so two blocks with
+///   identical bodies (e.g. no transactions and the same zero-value coinbase
+///   output, which is allowed since coinbase value 0 <= fees 0) would
+///   otherwise re-create the same coinbase keys.
+fn validate_outpoint_uniqueness(
+    state: &State,
+    rotxn: &RoTxn,
+    merkle_root: MerkleRoot,
+    body: &Body,
+) -> Result<(), Error> {
+    let mut txids: Vec<Txid> =
+        body.transactions.iter().map(|tx| tx.txid()).collect();
+    txids.par_sort_unstable();
+    if let Some(dup) = txids.windows(2).find(|w| w[0] == w[1]) {
+        return Err(Error::DuplicateTransaction { txid: dup[0] });
+    }
+    for vout in 0..body.coinbase.len() {
+        let outpoint = OutPoint::Coinbase {
+            merkle_root,
+            vout: vout as u32,
+        };
+        let () = state.ensure_outpoint_is_new(rotxn, &outpoint)?;
+    }
+    Ok(())
+}
+
 /// Validate a block, returning fees
 pub fn validate(
     state: &State,
@@ -48,6 +86,7 @@ pub fn validate(
         };
         return Err(err);
     }
+    let () = validate_outpoint_uniqueness(state, rotxn, merkle_root, body)?;
     let mut coinbase_value = bitcoin::Amount::ZERO;
     for output in &body.coinbase {
         coinbase_value = coinbase_value
@@ -128,6 +167,9 @@ pub fn prevalidate(
         return Err(err);
     }
 
+    let () =
+        validate_outpoint_uniqueness(state, rotxn, computed_merkle_root, body)?;
+
     let mut coinbase_value = bitcoin::Amount::ZERO;
     for output in &body.coinbase {
         coinbase_value = coinbase_value

lib/state/error.rs

@@ -366,6 +366,8 @@ pub enum Error {
     Db(Box<sneed::Error>),
     #[error(transparent)]
     DutchAuction(#[from] DutchAuction),
+    #[error("block body contains transaction {txid} more than once")]
+    DuplicateTransaction { txid: Txid },
     #[error(transparent)]
     FillTxOutputContents(#[from] FillTxOutputContents),
     #[error(
@@ -385,6 +387,8 @@ pub enum Error {
     MissingBitAssetInput { name_hash: Hash },
     #[error("deposit block doesn't exist")]
     NoDepositBlock,
+    #[error("transaction {txid} has no inputs")]
+    NoInputs { txid: Txid },
     #[error("total fees less than coinbase value")]
     NotEnoughFees,
     #[error("no tip")]
@@ -397,6 +401,8 @@ pub enum Error {
     SpendWithdrawalOutput { outpoint: OutPoint },
     #[error("Withdrawal bundle event block doesn't exist")]
     NoWithdrawalBundleEventBlock,
+    #[error("output {outpoint} already exists")]
+    OutPointAlreadyExists { outpoint: OutPoint },
     #[error(transparent)]
     PendingWithdrawalBundleUnknown(#[from] PendingWithdrawalBundleUnknown),
     #[error(

lib/state/mod.rs

@@ -27,6 +27,8 @@ mod dutch_auction;
 pub mod error;
 mod rollback;
 mod two_way_peg_data;
+#[cfg(test)]
+mod zero_input_replay_tests;
 
 pub use amm::{AmmPair, PoolState as AmmPoolState};
 pub use bitassets::SeqId as BitAssetSeqId;
@@ -603,12 +605,72 @@ impl State {
         }
     }
 
+    /// Check that an outpoint that is about to be created does not already
+    /// exist, either unspent (`utxos`) or spent (`stxos`).
+    ///
+    /// Block connection writes created outputs with an unchecked `put`, and
+    /// `disconnect_tip` deletes created outputs by key and restores spent
+    /// outputs from `stxos`. If an outpoint key could be created a second
+    /// time, disconnecting either creating block would delete or fail to find
+    /// the other's output, so the UTXO set would depend on reorg history
+    /// (`NoUtxo` / `NoStxo` on a later disconnect, and nodes that saw a
+    /// different reorg history would diverge on a later spend).
+    /// Keys of different `OutPoint` variants never collide.
+    pub(crate) fn ensure_outpoint_is_new(
+        &self,
+        rotxn: &RoTxn,
+        outpoint: &OutPoint,
+    ) -> Result<(), Error> {
+        let key = OutPointKey::from_outpoint(outpoint);
+        if self.utxos.contains_key(rotxn, &key)?
+            || self.stxos.contains_key(rotxn, &key)?
+        {
+            return Err(Error::OutPointAlreadyExists {
+                outpoint: *outpoint,
+            });
+        }
+        Ok(())
+    }
+
     /// Validates a filled transaction, and returns the fee
     pub fn validate_filled_transaction(
         &self,
         rotxn: &RoTxn,
         tx: &FilledTransaction,
     ) -> Result<bitcoin::Amount, Error> {
+        // A transaction must spend at least one input. The coinbase is not a
+        // `Transaction` and is not validated here. With this rule every txid
+        // commits to an outpoint that can be spent only once, so a
+        // transaction cannot be mined twice on one chain, and a zero-output
+        // transaction (which creates no keys for the check below to catch)
+        // cannot be replayed either.
+        if tx.transaction.inputs.is_empty() {
+            return Err(Error::NoInputs { txid: tx.txid() });
+        }
+        // The outpoints created by this transaction must not already exist
+        // (see `ensure_outpoint_is_new`). A transaction's txid is a pure
+        // function of its contents, so an identical transaction can be mined
+        // again once it has left the mempool if it has no inputs (it carries
+        // no authorization, so anyone can re-broadcast it), which would
+        // re-create its `Regular { txid, vout }` outpoints. A transaction that
+        // spends inputs cannot be mined again (its inputs are gone), and this
+        // check rejects the replay either way.
+        //
+        // `FilledTransaction::filled_outputs` maps `transaction.outputs`
+        // one-to-one and `connect_prevalidated` keys every filled output as
+        // `Regular { txid, vout }`, so this covers outputs of every content
+        // kind, independent of `tx.data`: Bitcoin, withdrawal, BitAsset,
+        // BitAsset control, BitAsset reservation, AMM LP token and Dutch
+        // auction receipt outputs. This runs on the mempool path and, via
+        // `block::prevalidate`, against the pre-block state on connect.
+        let txid = tx.txid();
+        for vout in 0..tx.transaction.outputs.len() {
+            let outpoint = OutPoint::Regular {
+                txid,
+                vout: vout as u32,
+            };
+            let () = self.ensure_outpoint_is_new(rotxn, &outpoint)?;
+        }
         let () = self.validate_reservations(tx)?;
         let () = self.validate_bitassets(rotxn, tx)?;
         let fee = tx.bitcoin_fee()?;