From d68d0e113473e59310723f805fd86ca204e5f1ed Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 13:06:23 +0200 Subject: [PATCH] commands: adapt 'createrecovery' to multiple recovery paths --- doc/API.md | 24 ++++++++++++++++-------- src/commands/mod.rs | 31 ++++++++++++++++--------------- src/jsonrpc/api.rs | 10 +++++++++- tests/test_rpc.py | 2 +- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/doc/API.md b/doc/API.md index 9c0a160f..bcb93b58 100644 --- a/doc/API.md +++ b/doc/API.md @@ -267,18 +267,26 @@ Confirmation time is based on the timestamp of blocks. ### `createrecovery` -Create a transaction that sweeps all coins whose timelocked recovery path is available to a provided -address at a provided feerate. +Create a transaction that sweeps all coins for which a timelocked recovery path is +currently available to a provided address with the provided feerate. -Will error if no such coins are available or the sum of their value is not enough to cover the -requested feerate. +The `timelock` parameter can be used to specify which recovery path to use. By default, +we'll use the first recovery path available. If created for a later timelock a recovery +transaction may be satisfied using an earlier timelock but not the opposite. + +Due to the fact coins are generally received at different block heights, not all coins may be +spendable through a single recovery path at the same time. + +This command will error if no such coins are available or the sum of their value is not enough to +cover the requested feerate. #### Request -| Field | Type | Description | -| ---------- | ----------------- | ----------------------------------------------------------------- | -| `address` | str | The Bitcoin address to sweep the coins to. | -| `feerate` | integer | Target feerate for the transaction, in satoshis per virtual byte. | +| Field | Type | Description | +| ---------- | ----------------- | ----------------------------------------------------------------------------------------- | +| `address` | str | The Bitcoin address to sweep the coins to. | +| `feerate` | integer | Target feerate for the transaction, in satoshis per virtual byte. | +| `timelock` | int or `null` | Recovery path to be used, identified by the number of blocks after which it is available. | #### Response diff --git a/src/commands/mod.rs b/src/commands/mod.rs index a86f9a95..1f54f900 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -124,8 +124,8 @@ impl fmt::Display for CommandError { Self::RescanTrigger(s) => write!(f, "Error while starting rescan: '{}'", s), Self::RecoveryNotAvailable => write!( f, - "No coin currently available through the timelocked recovery path." - ), + "No coin currently spendable through this timelocked recovery path." + ), } } } @@ -668,14 +668,18 @@ impl DaemonControl { ListTransactionsResult { transactions } } - /// Create a transaction that sweeps all coins whose timelocked recovery path is currently - /// available to a provided address with the provided feerate. + /// Create a transaction that sweeps all coins for which a timelocked recovery path is + /// currently available to a provided address with the provided feerate. /// - /// Note that not all coins may be spendable through the recovery path at the same time. + /// The `timelock` parameter can be used to specify which recovery path to use. By default, + /// we'll use the first recovery path available. + /// + /// Note that not all coins may be spendable through a single recovery path at the same time. pub fn create_recovery( &self, address: bitcoin::Address, feerate_vb: u64, + timelock: Option, ) -> Result { if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); @@ -702,27 +706,24 @@ impl DaemonControl { outputs: vec![PsbtOut::default()], }; - // Query the coins that we can spend through the recovery path from the database. + // Query the coins that we can spend through the specified recovery path (if no recovery + // path specified, use the first available one) from the database. let current_height = self.bitcoin.chain_tip().height; - let desc_timelock = self.config.main_descriptor.first_timelock_value(); - let timelock: i32 = desc_timelock - .try_into() - .expect("Must fit, it's effectively a u16"); + let timelock = + timelock.unwrap_or_else(|| self.config.main_descriptor.first_timelock_value()); + let height_delta: i32 = timelock.try_into().expect("Must fit, it's a u16"); let sweepable_coins = db_conn .coins(CoinType::Unspent) .into_iter() .filter(|(_, c)| { // We are interested in coins available at the *next* block c.block_info - .map(|b| current_height + 1 >= b.height + timelock) + .map(|b| current_height + 1 >= b.height + height_delta) .unwrap_or(false) }); // Fill-in the transaction inputs and PSBT inputs information. Record the value // that is fed to the transaction while doing so, to compute the fees afterward. - let csv_value: u16 = desc_timelock - .try_into() - .expect("Must fit, it's effectively a u16"); let mut in_value = bitcoin::Amount::from_sat(0); let txin_sat_vb = self.config.main_descriptor.max_sat_vbytes(); let mut sat_vb = 0; @@ -731,7 +732,7 @@ impl DaemonControl { in_value += coin.amount; psbt.unsigned_tx.input.push(bitcoin::TxIn { previous_output: coin.outpoint, - sequence: bitcoin::Sequence::from_height(csv_value), + sequence: bitcoin::Sequence::from_height(timelock), // TODO: once we move to Taproot, anti-fee-sniping using nSequence ..bitcoin::TxIn::default() }); diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index b7c85e3d..3e67659d 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -148,8 +148,16 @@ fn create_recovery(control: &DaemonControl, params: Params) -> Result = params + .get(2, "timelock") + .map(|tl| { + tl.as_u64() + .and_then(|tl| tl.try_into().ok()) + .ok_or_else(|| Error::invalid_params("Invalid 'timelock' parameter.")) + }) + .transpose()?; - let res = control.create_recovery(address, feerate)?; + let res = control.create_recovery(address, feerate, timelock)?; Ok(serde_json::json!(&res)) } diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 2e8cc87a..55c76858 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -554,7 +554,7 @@ def test_create_recovery(lianad, bitcoind): # There's nothing to sweep with pytest.raises( RpcError, - match="No coin currently available through the timelocked recovery path", + match="No coin currently spendable through this timelocked recovery path", ): lianad.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2)