Skip to content

Commit

Permalink
fix: reject empty pre-sign requests (#1190)
Browse files Browse the repository at this point in the history
* Reject basically empty bitcoin transaction pre-sign requests
* Add more empty request checks
  • Loading branch information
djordon authored Jan 9, 2025
1 parent 042a093 commit 4d59099
Show file tree
Hide file tree
Showing 3 changed files with 288 additions and 92 deletions.
21 changes: 9 additions & 12 deletions signer/src/bitcoin/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ impl<'a> UnsignedTransaction<'a> {
/// Construct an unsigned transaction.
///
/// This function can fail if the output amounts are greater than the
/// input amounts.
/// input amounts or if the [`Requests`] object is empty.
///
/// The returned BTC transaction has the following properties:
/// 1. The amounts for each output has taken fees into consideration.
Expand All @@ -822,7 +822,7 @@ impl<'a> UnsignedTransaction<'a> {
/// Construct a transaction with stub witness data.
///
/// This function can fail if the output amounts are greater than the
/// input amounts.
/// input amounts or if the [`Requests`] object is empty.
///
/// The returned BTC transaction has the following properties:
/// 1. The amounts for each output has taken fees into consideration.
Expand All @@ -833,6 +833,9 @@ impl<'a> UnsignedTransaction<'a> {
/// 5. All witness data is correctly set, except for the fake
/// signatures from (4).
pub fn new_stub(requests: Requests<'a>, state: &SignerBtcState) -> Result<Self, Error> {
if requests.is_empty() {
return Err(Error::BitcoinNoRequests);
}
// Construct a transaction base. This transaction's inputs have
// witness data with dummy signatures so that our virtual size
// estimates are accurate. Later we will update the fees.
Expand Down Expand Up @@ -1806,10 +1809,9 @@ mod tests {
assert_eq!(new_utxo.public_key, requests.signer_state.public_key);
}

/// Transactions that do not service requests do not have the OP_RETURN
/// output.
/// You cannot create sweep transactions that do not service requests.
#[test]
fn no_requests_no_op_return() {
fn no_requests_no_sweep() {
let public_key = XOnlyPublicKey::from_str(X_ONLY_PUBLIC_KEY1).unwrap();
let signer_state = SignerBtcState {
utxo: SignerUtxo {
Expand All @@ -1823,14 +1825,9 @@ mod tests {
magic_bytes: [0; 2],
};

// No requests so the first output should be the signers UTXO and
// that's it. No OP_RETURN output.
let requests = Requests::new(Vec::new());
let unsigned = UnsignedTransaction::new(requests, &signer_state).unwrap();
assert_eq!(unsigned.tx.output.len(), 1);

// There is only one output and it's a P2TR output
assert!(unsigned.tx.output[0].script_pubkey.is_p2tr());
let sweep = UnsignedTransaction::new(requests, &signer_state);
assert!(sweep.is_err());
}

/// We aggregate the bitmaps to form a single one at the end. Check
Expand Down
Loading

0 comments on commit 4d59099

Please sign in to comment.