nik/liana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

commands: include missing amount in response

Browse Source 
The GUI uses the InsufficientFunds error to get the missing
amount when the user is creating a new spend.

It is not straightforward to extract this information in a
general way from the RPC error. Instead, this missing amount
will be included in the command response.

These changes are based on suggestions from darosior
and edouardparis.
This commit is contained in:
jp1ac4 2024-01-17 10:44:32 +00:00
parent 7ca6cc8dfa
commit 1339898253
No known key found for this signature in database
GPG Key ID: A7ACD32423568D7B
4 changed files with 213 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
doc/API.md
View File

@ -179,11 +179,18 @@ This command will refuse to create any output worth less than 5k sats.
#### Response
If the spend is created successfully, the following response will be received:
| Field | Type | Description |
| -------------- | ----------------- | ---------------------------------------------------- |
| `psbt` | string | PSBT of the spending transaction, encoded as base64. |
| `warnings` | list of string | Warnings, if any, generated during spend creation. |
If there are insufficient funds to create the required spend, then the following response will be received:
| Field | Type | Description |
| -------------- | ----------------- | ---------------------------------------------------- |
| `missing` | integer | Additional sats required to create the spend. |
### `updatespend`
@ -294,9 +301,7 @@ allowed in order to replace this transaction using RBF (see https://github.com/b
#### Response
| Field | Type | Description |
| -------------- | --------- | ---------------------------------------------------- |
| `psbt` | string | PSBT of the spending transaction, encoded as base64. |
The response is the same as for [`createspend`](#createspend).
### `startrescan`

245
src/commands/mod.rs
View File

@ -494,7 +494,7 @@ impl DaemonControl {
psbt,
has_change,
warnings,
} = create_spend(
} = match create_spend(
&self.config.main_descriptor,
&self.secp,
&mut tx_getter,
@ -502,7 +502,15 @@ impl DaemonControl {
&candidate_coins,
SpendTxFees::Regular(feerate_vb),
change_address,
)?;
) {
Ok(res) => res,
Err(SpendCreationError::CoinSelection(e)) => {
return Ok(CreateSpendResult::InsufficientFunds { missing: e.missing });
}
Err(e) => {
return Err(e.into());
}
};
for (addr, _) in destinations_checked {
self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info);
}
@ -510,7 +518,7 @@ impl DaemonControl {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_info);
}
Ok(CreateSpendResult {
Ok(CreateSpendResult::Success {
psbt,
warnings: warnings.iter().map(|w| w.to_string()).collect(),
})
@ -828,17 +836,19 @@ impl DaemonControl {
change_address.clone(),
) {
Ok(psbt) => psbt,
// If we get a coin selection error due to insufficient funds and we want to cancel the
// transaction, then set all previous coins as mandatory and add confirmed coins as
// optional, unless we have already done this.
Err(SpendCreationError::CoinSelection(_))
if is_cancel && candidate_coins.iter().all(|c| !c.must_select) =>
{
for cand in candidate_coins.iter_mut() {
cand.must_select = true;
Err(SpendCreationError::CoinSelection(e)) => {
// If we get a coin selection error due to insufficient funds and we want to cancel the
// transaction, then set all previous coins as mandatory and add confirmed coins as
// optional, unless we have already done this.
if is_cancel && candidate_coins.iter().all(|c| !c.must_select) {
for cand in candidate_coins.iter_mut() {
cand.must_select = true;
}
candidate_coins.extend(&confirmed_cands);
continue;
} else {
return Ok(CreateSpendResult::InsufficientFunds { missing: e.missing });
}
candidate_coins.extend(&confirmed_cands);
continue;
}
Err(e) => {
return Err(e.into());
@ -862,7 +872,7 @@ impl DaemonControl {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_address.info);
}
return Ok(CreateSpendResult {
return Ok(CreateSpendResult::Success {
psbt: rbf_psbt,
warnings: warnings.iter().map(|w| w.to_string()).collect(),
});
@ -1108,10 +1118,16 @@ pub struct ListCoinsResult {
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct CreateSpendResult {
#[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")]
pub psbt: Psbt,
pub warnings: Vec<String>,
#[serde(untagged)]
pub enum CreateSpendResult {
Success {
#[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")]
psbt: Psbt,
warnings: Vec<String>,
},
InsufficientFunds {
missing: u64,
},
}
#[derive(Debug, Clone, Serialize, Deserialize)]
@ -1150,7 +1166,6 @@ mod tests {
use super::*;
use crate::{bitcoin::Block, database::BlockInfo, spend::InsaneFeeInfo, testutils::*};
use bdk_coin_select::InsufficientFunds;
use bitcoin::{
bip32::{self, ChildNumber},
blockdata::transaction::{TxIn, TxOut, Version as TxVersion},
@ -1342,9 +1357,7 @@ mod tests {
// Insufficient funds for coin selection.
assert!(matches!(
control.create_spend(&destinations, &[], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
assert_eq!(
control.create_spend(&destinations, &[dummy_op], 0, None),
@ -1372,20 +1385,23 @@ mod tests {
// and so we get a coin selection error due to insufficient funds.
assert!(matches!(
control.create_spend(&destinations, &[], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
let res = control
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
assert!(res.psbt.inputs[0].non_witness_utxo.is_some());
let tx = res.psbt.unsigned_tx;
.unwrap()
{
(psbt, warnings)
} else {
panic!("expect successful spend creation")
};
assert!(psbt.inputs[0].non_witness_utxo.is_some());
let tx = psbt.unsigned_tx;
assert_eq!(tx.input.len(), 1);
assert_eq!(tx.input[0].previous_output, dummy_op);
assert_eq!(tx.output.len(), 2);
// It has change so no warnings expected.
assert!(res.warnings.is_empty());
assert!(warnings.is_empty());
assert_eq!(
tx.output[0].script_pubkey,
dummy_addr.payload().script_pubkey()
@ -1401,10 +1417,15 @@ mod tests {
// Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 170 sats fees.
// At 2sats/vb, it's twice that.
assert_eq!(tx.output[1].value.to_sat(), 89_830);
let res = control
let psbt = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations, &[dummy_op], 2, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
psbt
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
assert_eq!(tx.output[1].value.to_sat(), 89_660);
// A feerate of 555 won't trigger the sanity checks (they were previously not taking the
@ -1416,16 +1437,12 @@ mod tests {
// If we ask for a too high feerate, or a too large/too small output, it'll fail.
assert!(matches!(
control.create_spend(&destinations, &[dummy_op], 10_000, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
*destinations.get_mut(&dummy_addr).unwrap() = 100_001;
assert!(matches!(
control.create_spend(&destinations, &[dummy_op], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
*destinations.get_mut(&dummy_addr).unwrap() = 4_500;
assert_eq!(
@ -1450,10 +1467,15 @@ mod tests {
// If we ask for a large, but valid, output we won't get a change output. 95_000 because we
// won't create an output lower than 5k sats.
*destinations.get_mut(&dummy_addr).unwrap() = 95_000;
let res = control
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
(psbt, warnings)
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
assert_eq!(tx.input.len(), 1);
assert_eq!(tx.input[0].previous_output, dummy_op);
assert_eq!(tx.output.len(), 1);
@ -1463,64 +1485,87 @@ mod tests {
);
assert_eq!(tx.output[0].value.to_sat(), 95_000);
// change = 100_000 - 95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 = 4830
assert_eq!(res.warnings, vec!["Change amount of 4830 sats added to fee as it was too small to create a transaction output."]);
assert_eq!(warnings, vec!["Change amount of 4830 sats added to fee as it was too small to create a transaction output."]);
// Increase the target value by the change amount and the warning will disappear.
*destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_830;
let res = control
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
(psbt, warnings)
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
assert_eq!(tx.output.len(), 1);
assert!(res.warnings.is_empty());
assert!(warnings.is_empty());
// Now increase target also by the extra fee that was paying for change and we can still create the spend.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 + 4_830 + /* fee for change output */ 43;
let res = control
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
(psbt, warnings)
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
assert_eq!(tx.output.len(), 1);
assert!(res.warnings.is_empty());
assert!(warnings.is_empty());
// Now increase the target by 1 more sat and we will have insufficient funds.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 + 4_830 + /* fee for change output */ 43 + 1;
assert_eq!(
control.create_spend(&destinations, &[dummy_op], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(InsufficientFunds { missing: 1 })
))
Ok(CreateSpendResult::InsufficientFunds { missing: 1 }),
);
// Now decrease the target so that the lost change is just 1 sat.
*destinations.get_mut(&dummy_addr).unwrap() =
100_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 - 1;
let res = control
let warnings = if let CreateSpendResult::Success { warnings, .. } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
.unwrap()
{
warnings
} else {
panic!("expect successful spend creation")
};
// Message uses "sat" instead of "sats" when value is 1.
assert_eq!(res.warnings, vec!["Change amount of 1 sat added to fee as it was too small to create a transaction output."]);
assert_eq!(warnings, vec!["Change amount of 1 sat added to fee as it was too small to create a transaction output."]);
// Now decrease the target value so that we have enough for a change output.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43;
let res = control
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
(psbt, warnings)
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
assert_eq!(tx.output.len(), 2);
assert_eq!(tx.output[1].value.to_sat(), 5_000);
assert!(res.warnings.is_empty());
assert!(warnings.is_empty());
// Now increase the target by 1 and we'll get a warning again, this time for 1 less than the dust threshold.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 + 1;
let res = control
let warnings = if let CreateSpendResult::Success { warnings, .. } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap();
assert_eq!(res.warnings, vec!["Change amount of 4999 sats added to fee as it was too small to create a transaction output."]);
.unwrap()
{
warnings
} else {
panic!("expect successful spend creation")
};
assert_eq!(warnings, vec!["Change amount of 4999 sats added to fee as it was too small to create a transaction output."]);
// Now if we mark the coin as spent, we won't create another Spend transaction containing
// it.
@ -1539,9 +1584,7 @@ mod tests {
// and so we get a coin selection error due to insufficient funds.
assert!(matches!(
control.create_spend(&destinations, &[], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
// We'd bail out if they tried to create a transaction with a too high feerate.
@ -1591,18 +1634,14 @@ mod tests {
// Coin selection error due to insufficient funds.
assert!(matches!(
control.create_spend(&destinations, &[], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
// Set destination amount equal to value of confirmed coins.
*destinations.get_mut(&dummy_addr).unwrap() = 80_000;
// Coin selection error occurs due to insufficient funds to pay fee.
assert!(matches!(
control.create_spend(&destinations, &[], 1, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
let confirmed_op_2 = bitcoin::OutPoint {
txid: confirmed_op_1.txid,
@ -1623,8 +1662,14 @@ mod tests {
spend_block: None,
}]);
// First, create a transaction using auto coin selection.
let res_auto = control.create_spend(&destinations, &[], 1, None).unwrap();
let tx_auto = res_auto.psbt.unsigned_tx;
let psbt = if let CreateSpendResult::Success { psbt, .. } =
control.create_spend(&destinations, &[], 1, None).unwrap()
{
psbt
} else {
panic!("expect successful spend creation")
};
let tx_auto = psbt.unsigned_tx;
let mut tx_prev_outpoints = tx_auto
.input
.iter()
@ -1642,10 +1687,15 @@ mod tests {
assert_eq!(tx_auto.output[0].value, Amount::from_sat(80_000));
// Create a second transaction using manual coin selection.
let res_manual = control
let psbt = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations, &[confirmed_op_1, confirmed_op_2], 1, None)
.unwrap();
let tx_manual = res_manual.psbt.unsigned_tx;
.unwrap()
{
psbt
} else {
panic!("expect successful spend creation")
};
let tx_manual = psbt.unsigned_tx;
// Check that manual and auto selection give same outputs (including change).
assert_eq!(tx_auto.output, tx_manual.output);
// Check inputs are also the same. Need to sort as order is not guaranteed by `create_spend`.
@ -1677,15 +1727,18 @@ mod tests {
let empty_dest = &HashMap::<bitcoin::Address<address::NetworkUnchecked>, u64>::new();
assert!(matches!(
control.create_spend(empty_dest, &[confirmed_op_3], 5, None),
Err(CommandError::SpendCreation(
SpendCreationError::CoinSelection(..)
))
Ok(CreateSpendResult::InsufficientFunds { .. }),
));
// If we use a lower fee, the self-send will succeed.
let res = control
let psbt = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(empty_dest, &[confirmed_op_3], 1, None)
.unwrap();
let tx = res.psbt.unsigned_tx;
.unwrap()
{
psbt
} else {
panic!("expect successful spend creation")
};
let tx = psbt.unsigned_tx;
let tx_prev_outpoints = tx
.input
.iter()
@ -1789,20 +1842,32 @@ mod tests {
.iter()
.cloned()
.collect();
let mut psbt_a = control
let mut psbt_a = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations_a, &[dummy_op_a], 1, None)
.unwrap()
.psbt;
{
psbt
} else {
panic!("expect successful spend creation")
};
let txid_a = psbt_a.unsigned_tx.txid();
let psbt_b = control
let psbt_b = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations_b, &[dummy_op_b], 10, None)
.unwrap()
.psbt;
{
psbt
} else {
panic!("expect successful spend creation")
};
let txid_b = psbt_b.unsigned_tx.txid();
let psbt_c = control
let psbt_c = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations_c, &[dummy_op_a, dummy_op_b], 100, None)
.unwrap()
.psbt;
{
psbt
} else {
panic!("expect successful spend creation")
};
let txid_c = psbt_c.unsigned_tx.txid();
// We can store and query them all

43
tests/test_rpc.py
View File

@ -1151,6 +1151,49 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind):
)
def test_rbfpsbt_insufficient_funds(lianad, bitcoind):
"""Trying to increase the fee too much returns the missing funds amount."""
# Get a coin.
deposit_txid_1 = bitcoind.rpc.sendtoaddress(
lianad.rpc.getnewaddress()["address"], 30_000 / COIN
)
bitcoind.generate_block(1, wait_for_mempool=deposit_txid_1)
wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
# Create a spend that we will then attempt to replace.
destinations_1 = {
bitcoind.rpc.getnewaddress(): 29_800,
}
spend_res_1 = lianad.rpc.createspend(destinations_1, [], 1)
spend_psbt_1 = PSBT.from_base64(spend_res_1["psbt"])
spend_txid_1 = sign_and_broadcast_psbt(lianad, spend_psbt_1)
# We don't have sufficient funds to bump the fee.
assert "missing" in lianad.rpc.rbfpsbt(spend_txid_1, False, 2)
# We can still cancel it as the coin has enough value to create a single
# output at a higher feerate.
assert "psbt" in lianad.rpc.rbfpsbt(spend_txid_1, True)
wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 0)
# Get another coin.
deposit_txid_2 = bitcoind.rpc.sendtoaddress(
lianad.rpc.getnewaddress()["address"], 5_200 / COIN
)
bitcoind.generate_block(1, wait_for_mempool=deposit_txid_2)
wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
# Create a spend that we will then attempt to cancel.
destinations_2 = {
bitcoind.rpc.getnewaddress(): 5_000,
}
spend_res_2 = lianad.rpc.createspend(destinations_2, [], 1)
spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"])
spend_txid_2 = sign_and_broadcast_psbt(lianad, spend_psbt_2)
# We don't have enough to create a transaction with feerate 2 sat/vb.
assert "missing" in lianad.rpc.rbfpsbt(spend_txid_2, True)
def test_rbfpsbt_cancel(lianad, bitcoind):
"""Test the use of RBF to cancel a transaction."""

38
tests/test_spend.py
View File

@ -221,12 +221,8 @@ def test_send_to_self(lianad, bitcoind):
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="Insufficient funds. Missing \\d+ sats",
):
lianad.rpc.createspend({}, outpoints, 40500)
# Note they may ask for an impossible send-to-self. In this case we'll report missing amount.
assert "missing" in lianad.rpc.createspend({}, outpoints, 40500)
# Sign and broadcast the send-to-self transaction created above.
signed_psbt = lianad.signer.sign_psbt(spend_psbt)
@ -256,11 +252,7 @@ def test_coin_selection(lianad, bitcoind):
dest_100_000 = {bitcoind.rpc.getnewaddress(): 100_000}
# Coin selection is not possible if we have no coins.
assert len(lianad.rpc.listcoins()["coins"]) == 0
with pytest.raises(
RpcError,
match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
):
lianad.rpc.createspend(dest_100_000, [], 2)
assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
# Receive a coin in an unconfirmed deposit transaction.
recv_addr = lianad.rpc.getnewaddress()["address"]
@ -269,22 +261,14 @@ def test_coin_selection(lianad, bitcoind):
# There are still no confirmed coins to use as candidates for selection.
assert len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 0
assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
with pytest.raises(
RpcError,
match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
):
lianad.rpc.createspend(dest_100_000, [], 2)
assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
# Confirm coin.
bitcoind.generate_block(1, wait_for_mempool=deposit)
wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
# Insufficient funds for coin selection.
with pytest.raises(
RpcError,
match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
):
lianad.rpc.createspend(dest_100_000, [], 2)
assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
# Reduce spend amount.
dest_30_000 = {bitcoind.rpc.getnewaddress(): 30_000}
@ -308,22 +292,14 @@ def test_coin_selection(lianad, bitcoind):
assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1
# Check we cannot use coins as candidates if they are spending/spent or unconfirmed.
with pytest.raises(
RpcError,
match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
):
lianad.rpc.createspend(dest_30_000, [], 2)
assert "missing" in lianad.rpc.createspend(dest_30_000, [], 2)
# Now confirm the Spend.
bitcoind.generate_block(1, wait_for_mempool=spend_txid)
wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
# But its value is not enough for this Spend.
dest_60_000 = {bitcoind.rpc.getnewaddress(): 60_000}
with pytest.raises(
RpcError,
match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
):
lianad.rpc.createspend(dest_60_000, [], 2)
assert "missing" in lianad.rpc.createspend(dest_60_000, [], 2)
# Get another coin to check coin selection with more than one candidate.
recv_addr = lianad.rpc.getnewaddress()["address"]