diff --git a/doc/API.md b/doc/API.md index bc8f037e..e120a7c5 100644 --- a/doc/API.md +++ b/doc/API.md @@ -114,6 +114,11 @@ Will error if the given coins are not sufficient to cover the transaction cost a the given feerate. If on the contrary the transaction is more than sufficiently funded, it will create a change output when economically rationale to do so. +You can create a send-to-self transaction by not specifying any destination. This command will +create a single change output. This may be useful to "refresh" coins whose timelocked recovery path +may be close to expiry without having to bear the complexity of computing the correct amount for the +change output. + This command will refuse to create any output worth less than 5k sats. #### Request diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 73b3f11d..37c6486e 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -46,7 +46,6 @@ const MAINNET_GENESIS_TIME: u32 = 1231006505; #[derive(Debug, Clone, PartialEq, Eq)] pub enum CommandError { NoOutpoint, - NoDestination, InvalidFeerate(/* sats/vb */ u64), UnknownOutpoint(bitcoin::OutPoint), AlreadySpent(bitcoin::OutPoint), @@ -54,7 +53,7 @@ pub enum CommandError { InvalidOutputValue(bitcoin::Amount), InsufficientFunds( /* in value */ bitcoin::Amount, - /* out value */ bitcoin::Amount, + /* out value */ Option, /* target feerate */ u64, ), InsaneFees(InsaneFeeInfo), @@ -75,7 +74,6 @@ impl fmt::Display for CommandError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Self::NoOutpoint => write!(f, "No provided outpoint. Need at least one."), - Self::NoDestination => write!(f, "No provided destination. Need at least one."), Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb), Self::AlreadySpent(op) => write!(f, "Coin at '{}' is already spent.", op), Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op), @@ -85,11 +83,19 @@ impl fmt::Display for CommandError { addr, expected, addr.network ), Self::InvalidOutputValue(amount) => write!(f, "Invalid output value '{}'.", amount), - Self::InsufficientFunds(in_val, out_val, feerate) => write!( - f, - "Cannot create a {} sat/vb transaction with input value {} and output value {}", - feerate, in_val, out_val - ), + Self::InsufficientFunds(in_val, out_val, feerate) => if let Some(out_val) = out_val { + write!( + f, + "Cannot create a {} sat/vb transaction with input value {} and output value {}", + feerate, in_val, out_val + ) + } else { + write!( + f, + "Not enough fund to create a {} sat/vb transaction with input value {}", + feerate, in_val + ) + }, Self::InsaneFees(info) => write!( f, "We assume transactions with a fee larger than {} sats or a feerate larger than {} sats/vb are a mistake. \ @@ -161,7 +167,10 @@ fn sanity_check_psbt( let tx = &psbt.unsigned_tx; // Must have as many in/out in the PSBT and Bitcoin tx. - if psbt.inputs.len() != tx.input.len() || psbt.outputs.len() != tx.output.len() { + if psbt.inputs.len() != tx.input.len() + || psbt.outputs.len() != tx.output.len() + || tx.output.is_empty() + { return Err(CommandError::SanityCheckFailure(psbt.clone())); } @@ -320,12 +329,10 @@ impl DaemonControl { coins_outpoints: &[bitcoin::OutPoint], feerate_vb: u64, ) -> Result { + let is_self_send = destinations.is_empty(); if coins_outpoints.is_empty() { return Err(CommandError::NoOutpoint); } - if destinations.is_empty() { - return Err(CommandError::NoDestination); - } if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); } @@ -419,6 +426,7 @@ impl DaemonControl { ..PsbtOut::default() }); } + assert_eq!(txouts.is_empty(), is_self_send); // Now create the transaction, compute its fees and already sanity check if its feerate // isn't much less than what was asked (and obviously that fees aren't negative). @@ -433,19 +441,23 @@ impl DaemonControl { in_value .checked_sub(out_value) .ok_or(CommandError::InsufficientFunds( - in_value, out_value, feerate_vb, + in_value, + Some(out_value), + feerate_vb, ))?; let nochange_feerate_vb = absolute_fee.to_sat().checked_div(nochange_vb).unwrap(); if nochange_feerate_vb.checked_mul(10).unwrap() < feerate_vb.checked_mul(9).unwrap() { return Err(CommandError::InsufficientFunds( - in_value, out_value, feerate_vb, + in_value, + Some(out_value), + feerate_vb, )); } // If necessary, add a change output. The computation here is a bit convoluted: we infer // the needed change value from the target feerate and the size of the transaction *with // an added output* (for the change). - if nochange_feerate_vb > feerate_vb { + if is_self_send || nochange_feerate_vb > feerate_vb { // Get the change address to create a dummy change txo. let change_index = db_conn.change_index(); let change_desc = self @@ -487,7 +499,11 @@ impl DaemonControl { bip32_derivation: change_desc.bip32_derivations(), ..PsbtOut::default() }); + } else if is_self_send { + return Err(CommandError::InsufficientFunds(in_value, None, feerate_vb)); } + } else if is_self_send { + return Err(CommandError::InsufficientFunds(in_value, None, feerate_vb)); } } @@ -772,9 +788,9 @@ impl DaemonControl { // Compute the value of the single output based on the requested feerate. let tx_vbytes = (psbt.unsigned_tx.vsize() + sat_vb) as u64; let absolute_fee = bitcoin::Amount::from_sat(tx_vbytes.checked_mul(feerate_vb).unwrap()); - let output_value = in_value.checked_sub(absolute_fee).ok_or({ - CommandError::InsufficientFunds(in_value, bitcoin::Amount::from_sat(0), feerate_vb) - })?; + let output_value = in_value + .checked_sub(absolute_fee) + .ok_or(CommandError::InsufficientFunds(in_value, None, feerate_vb))?; psbt.unsigned_tx.output[0].value = output_value.to_sat(); sanity_check_psbt(&self.config.main_descriptor, &psbt)?; @@ -944,10 +960,6 @@ mod tests { control.create_spend(&destinations, &[], 1), Err(CommandError::NoOutpoint) ); - assert_eq!( - control.create_spend(&HashMap::new(), &[dummy_op], 1), - Err(CommandError::NoDestination) - ); assert_eq!( control.create_spend(&destinations, &[dummy_op], 0), Err(CommandError::InvalidFeerate(0)) @@ -996,7 +1008,7 @@ mod tests { control.create_spend(&destinations, &[dummy_op], 10_000), Err(CommandError::InsufficientFunds( bitcoin::Amount::from_sat(100_000), - bitcoin::Amount::from_sat(10_000), + Some(bitcoin::Amount::from_sat(10_000)), 10_000 )) ); @@ -1005,7 +1017,7 @@ mod tests { control.create_spend(&destinations, &[dummy_op], 1), Err(CommandError::InsufficientFunds( bitcoin::Amount::from_sat(100_000), - bitcoin::Amount::from_sat(100_001), + Some(bitcoin::Amount::from_sat(100_001)), 1 )) ); diff --git a/src/jsonrpc/mod.rs b/src/jsonrpc/mod.rs index 8c0f0df3..132eca6f 100644 --- a/src/jsonrpc/mod.rs +++ b/src/jsonrpc/mod.rs @@ -152,7 +152,6 @@ impl From for Error { fn from(e: commands::CommandError) -> Error { match e { commands::CommandError::NoOutpoint - | commands::CommandError::NoDestination | commands::CommandError::UnknownOutpoint(..) | commands::CommandError::InvalidFeerate(..) | commands::CommandError::AlreadySpent(..) diff --git a/tests/test_spend.py b/tests/test_spend.py index 93fb7596..f14c077d 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -1,6 +1,6 @@ from fixtures import * from test_framework.serializations import PSBT -from test_framework.utils import wait_for, COIN +from test_framework.utils import wait_for, COIN, RpcError def test_spend_change(lianad, bitcoind): @@ -170,3 +170,52 @@ def test_coin_marked_spent(lianad, bitcoind): return True wait_for(lambda: all(is_spent(c) for c in deposited_coins())) + + +def test_send_to_self(lianad, bitcoind): + """Test we can use createspend with no destination to send to a change address.""" + # Get 3 coins. + destinations = { + lianad.rpc.getnewaddress()["address"]: 0.03, + lianad.rpc.getnewaddress()["address"]: 0.04, + lianad.rpc.getnewaddress()["address"]: 0.05, + } + deposit_txid = bitcoind.rpc.sendmany("", destinations) + bitcoind.generate_block(1, wait_for_mempool=deposit_txid) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 3) + + # Then create a send-to-self transaction (by not providing any destination) that + # sweeps them all. + outpoints = [c["outpoint"] for c in lianad.rpc.listcoins()["coins"]] + specified_feerate = 142 + res = lianad.rpc.createspend({}, outpoints, specified_feerate) + spend_psbt = PSBT.from_base64(res["psbt"]) + assert len(spend_psbt.o) == len(spend_psbt.tx.vout) == 1 + + # Note they may ask for an impossible send-to-self. In this case we'll error cleanly. + with pytest.raises( + RpcError, + match="Not enough fund to create a 40500 sat/vb transaction with input value 0.12 BTC", + ): + lianad.rpc.createspend({}, outpoints, 40500) + + # Sign and broadcast the send-to-self transaction created above. + signed_psbt = lianad.signer.sign_psbt(spend_psbt) + lianad.rpc.updatespend(signed_psbt.to_base64()) + spend_txid = signed_psbt.tx.txid().hex() + lianad.rpc.broadcastspend(spend_txid) + + # The only output is the change output so the feerate of the transaction must + # not be lower than the one provided, and only possibly slightly higher (since + # we slightly overestimate the satisfaction size). + # FIXME: a 15% increase is huge. + res = bitcoind.rpc.getmempoolentry(spend_txid) + spend_feerate = int(res["fees"]["base"] * COIN / res["vsize"]) + assert specified_feerate <= spend_feerate <= int(specified_feerate * 115 / 100) + + # We should by now only have one coin. + bitcoind.generate_block(1, wait_for_mempool=spend_txid) + unspent_coins = lambda: ( + c for c in lianad.rpc.listcoins()["coins"] if c["spend_info"] is None + ) + wait_for(lambda: len(list(unspent_coins())) == 1)