commands: swap the outpoints and destinations arguments to createspend

The outpoints argument could eventually be made optional in order to
introduce some automated coin selection. So it makes more sense for it
to be after a required parameter, the destinations.
This commit is contained in:
Antoine Poinsot 2022-11-17 15:15:22 +01:00
parent 838daa0c93
commit 4bd65213e2
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
5 changed files with 40 additions and 40 deletions

View File

@ -265,8 +265,8 @@ impl DaemonControl {
pub fn create_spend(
&self,
coins_outpoints: &[bitcoin::OutPoint],
destinations: &HashMap<bitcoin::Address, u64>,
coins_outpoints: &[bitcoin::OutPoint],
feerate_vb: u64,
) -> Result<CreateSpendResult, CommandError> {
if coins_outpoints.is_empty() {
@ -677,22 +677,22 @@ mod tests {
.cloned()
.collect();
assert_eq!(
control.create_spend(&[], &destinations, 1),
control.create_spend(&destinations, &[], 1),
Err(CommandError::NoOutpoint)
);
assert_eq!(
control.create_spend(&[dummy_op], &HashMap::new(), 1),
control.create_spend(&HashMap::new(), &[dummy_op], 1),
Err(CommandError::NoDestination)
);
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 0),
control.create_spend(&destinations, &[dummy_op], 0),
Err(CommandError::InvalidFeerate(0))
);
// The coin doesn't exist. If we create a new unspent one at this outpoint with a much
// higher value, we'll get a Spend transaction with a change output.
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 1),
control.create_spend(&destinations, &[dummy_op], 1),
Err(CommandError::UnknownOutpoint(dummy_op))
);
let mut db_conn = control.db().lock().unwrap().connection();
@ -706,7 +706,7 @@ mod tests {
spend_txid: None,
spend_block: None,
}]);
let res = control.create_spend(&[dummy_op], &destinations, 1).unwrap();
let res = control.create_spend(&destinations, &[dummy_op], 1).unwrap();
let tx = res.psbt.unsigned_tx;
assert_eq!(tx.input.len(), 1);
assert_eq!(tx.input[0].previous_output, dummy_op);
@ -717,13 +717,13 @@ 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, 89_830);
let res = control.create_spend(&[dummy_op], &destinations, 2).unwrap();
let res = control.create_spend(&destinations, &[dummy_op], 2).unwrap();
let tx = res.psbt.unsigned_tx;
assert_eq!(tx.output[1].value, 89_660);
// If we ask for a too high feerate, or a too large/too small output, it'll fail.
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 10_000),
control.create_spend(&destinations, &[dummy_op], 10_000),
Err(CommandError::InsufficientFunds(
bitcoin::Amount::from_sat(100_000),
bitcoin::Amount::from_sat(10_000),
@ -732,7 +732,7 @@ mod tests {
);
*destinations.get_mut(&dummy_addr).unwrap() = 100_001;
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 1),
control.create_spend(&destinations, &[dummy_op], 1),
Err(CommandError::InsufficientFunds(
bitcoin::Amount::from_sat(100_000),
bitcoin::Amount::from_sat(100_001),
@ -741,7 +741,7 @@ mod tests {
);
*destinations.get_mut(&dummy_addr).unwrap() = 4_500;
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 1),
control.create_spend(&destinations, &[dummy_op], 1),
Err(CommandError::InvalidOutputValue(bitcoin::Amount::from_sat(
4_500
)))
@ -750,7 +750,7 @@ 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.create_spend(&[dummy_op], &destinations, 1).unwrap();
let res = control.create_spend(&destinations, &[dummy_op], 1).unwrap();
let tx = res.psbt.unsigned_tx;
assert_eq!(tx.input.len(), 1);
assert_eq!(tx.input[0].previous_output, dummy_op);
@ -768,7 +768,7 @@ mod tests {
.unwrap(),
)]);
assert_eq!(
control.create_spend(&[dummy_op], &destinations, 1),
control.create_spend(&destinations, &[dummy_op], 1),
Err(CommandError::AlreadySpent(dummy_op))
);
@ -836,17 +836,17 @@ mod tests {
.cloned()
.collect();
let mut psbt_a = control
.create_spend(&[dummy_op_a], &destinations_a, 1)
.create_spend(&destinations_a, &[dummy_op_a], 1)
.unwrap()
.psbt;
let txid_a = psbt_a.unsigned_tx.txid();
let psbt_b = control
.create_spend(&[dummy_op_b], &destinations_b, 10)
.create_spend(&destinations_b, &[dummy_op_b], 10)
.unwrap()
.psbt;
let txid_b = psbt_b.unsigned_tx.txid();
let psbt_c = control
.create_spend(&[dummy_op_a, dummy_op_b], &destinations_c, 100)
.create_spend(&destinations_c, &[dummy_op_a, dummy_op_b], 100)
.unwrap()
.psbt;
let txid_c = psbt_c.unsigned_tx.txid();

View File

@ -8,22 +8,8 @@ use std::{collections::HashMap, convert::TryInto, str::FromStr};
use miniscript::bitcoin::{self, consensus, util::psbt::PartiallySignedTransaction as Psbt};
fn create_spend(control: &DaemonControl, params: Params) -> Result<serde_json::Value, Error> {
let outpoints = params
.get(0, "outpoints")
.ok_or_else(|| Error::invalid_params("Missing 'outpoints' parameter."))?
.as_array()
.and_then(|arr| {
arr.iter()
.map(|entry| {
entry
.as_str()
.and_then(|e| bitcoin::OutPoint::from_str(e).ok())
})
.collect::<Option<Vec<bitcoin::OutPoint>>>()
})
.ok_or_else(|| Error::invalid_params("Invalid 'outpoints' parameter."))?;
let destinations = params
.get(1, "destinations")
.get(0, "destinations")
.ok_or_else(|| Error::invalid_params("Missing 'destinations' parameter."))?
.as_object()
.and_then(|obj| {
@ -36,6 +22,20 @@ fn create_spend(control: &DaemonControl, params: Params) -> Result<serde_json::V
.collect::<Option<HashMap<bitcoin::Address, u64>>>()
})
.ok_or_else(|| Error::invalid_params("Invalid 'destinations' parameter."))?;
let outpoints = params
.get(1, "outpoints")
.ok_or_else(|| Error::invalid_params("Missing 'outpoints' parameter."))?
.as_array()
.and_then(|arr| {
arr.iter()
.map(|entry| {
entry
.as_str()
.and_then(|e| bitcoin::OutPoint::from_str(e).ok())
})
.collect::<Option<Vec<bitcoin::OutPoint>>>()
})
.ok_or_else(|| Error::invalid_params("Invalid 'outpoints' parameter."))?;
let feerate: u64 = params
.get(2, "feerate")
.ok_or_else(|| Error::invalid_params("Missing 'feerate' parameter."))?
@ -43,7 +43,7 @@ fn create_spend(control: &DaemonControl, params: Params) -> Result<serde_json::V
.and_then(|i| i.try_into().ok())
.ok_or_else(|| Error::invalid_params("Invalid 'feerate' parameter."))?;
let res = control.create_spend(&outpoints, &destinations, feerate)?;
let res = control.create_spend(&destinations, &outpoints, feerate)?;
Ok(serde_json::json!(&res))
}

View File

@ -65,7 +65,7 @@ def spend_coins(minisafed, bitcoind, coins):
destinations = {
bitcoind.rpc.getnewaddress(): total_value - 11 - 31 - 300 * len(coins)
}
res = minisafed.rpc.createspend([c["outpoint"] for c in coins], destinations, 1)
res = minisafed.rpc.createspend(destinations, [c["outpoint"] for c in coins], 1)
signed_psbt = minisafed.sign_psbt(PSBT.from_base64(res["psbt"]))
finalized_psbt = minisafed.finalize_psbt(signed_psbt)

View File

@ -77,7 +77,7 @@ def test_jsonrpc_server(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): 20_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 18)
res = minisafed.rpc.createspend(destinations, outpoints, 18)
assert "psbt" in res
res = minisafed.rpc.createspend(
outpoints=outpoints, destinations=destinations, feerate=18
@ -107,7 +107,7 @@ def test_create_spend(minisafed, bitcoind):
bitcoind.rpc.getnewaddress(): 400_000,
bitcoind.rpc.getnewaddress(): 1_000_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 18)
res = minisafed.rpc.createspend(destinations, outpoints, 18)
assert "psbt" in res
# The transaction must contain a change output.
@ -133,7 +133,7 @@ def test_list_spend(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): int(value_a * COIN // 2),
}
res = minisafed.rpc.createspend(outpoints, destinations, 6)
res = minisafed.rpc.createspend(destinations, outpoints, 6)
assert "psbt" in res
addr = minisafed.rpc.getnewaddress()["address"]
@ -144,7 +144,7 @@ def test_list_spend(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): int((value_a + value_b) * COIN - 1_000),
}
res_b = minisafed.rpc.createspend(outpoints, destinations, 2)
res_b = minisafed.rpc.createspend(destinations, outpoints, 2)
assert "psbt" in res_b
# Store them both in DB.
@ -184,7 +184,7 @@ def test_update_spend(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): 200_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 6)
res = minisafed.rpc.createspend(destinations, outpoints, 6)
assert "psbt" in res
# Now update it
@ -241,7 +241,7 @@ def test_broadcast_spend(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): 200_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 6)
res = minisafed.rpc.createspend(destinations, outpoints, 6)
psbt = PSBT.from_base64(res["psbt"])
txid = psbt.tx.txid().hex()

View File

@ -18,7 +18,7 @@ def test_spend_change(minisafed, bitcoind):
bitcoind.rpc.getnewaddress(): 100_000,
minisafed.rpc.getnewaddress()["address"]: 100_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 2)
res = minisafed.rpc.createspend(destinations, outpoints, 2)
assert "psbt" in res
# The transaction must contain a change output.
@ -44,7 +44,7 @@ def test_spend_change(minisafed, bitcoind):
destinations = {
bitcoind.rpc.getnewaddress(): 100_000,
}
res = minisafed.rpc.createspend(outpoints, destinations, 2)
res = minisafed.rpc.createspend(destinations, outpoints, 2)
spend_psbt = PSBT.from_base64(res["psbt"])
# We can sign and broadcast it.