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

Merge #96: Some tweaks to the Spend transaction creation

Browse Source 
4bd65213e2561453f7c1fee16acccdd128296406 commands: swap the outpoints and destinations arguments to createspend (Antoine Poinsot)
838daa0c93ad72adde9b5c502796f87571fb5efd commands: set the BIP32 derivation in PSBT for change outputs (Antoine Poinsot)
dbf17681a432791f58ef4cdbb3fb2ebfa4ba0742 commands: signal for opt-in RBF in Spend transactions (Antoine Poinsot)

Pull request description:

  It was missing a few things, see the linked issues for details.

  Fixes #90.
  Fixes #76.
  Fixes #33.

ACKs for top commit:
  darosior:
    self-ACK 4bd65213e2561453f7c1fee16acccdd128296406

Tree-SHA512: f7122d48afa764b68993df7e66043900408ec64aa9181a87495168476d424a19dbf6adac3885df7a0d0865f0c47b17faea246f4abcbd7c908baf26fc8d0d6269
This commit is contained in:
Antoine Poinsot 2022-11-17 16:53:18 +01:00
commit 25bbce8612
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
5 changed files with 62 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/commands/mod.rs
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() {
@ -298,6 +298,7 @@ impl DaemonControl {
in_value += coin.amount;
txins.push(bitcoin::TxIn {
previous_output: *op,
sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME,
// TODO: once we move to Taproot, anti-fee-sniping using nSequence
..bitcoin::TxIn::default()
});
@ -332,9 +333,23 @@ impl DaemonControl {
value: amount.to_sat(),
script_pubkey: address.script_pubkey(),
});
// TODO: if it's an address of ours, signal it as change to signing devices by adding
// the BIP32 derivation path to the PSBT output.
psbt_outs.push(PsbtOut::default());
// If it's an address of ours, signal it as change to signing devices by adding the
// BIP32 derivation path to the PSBT output.
let bip32_derivation =
if let Some((index, is_change)) = db_conn.derivation_index_by_address(address) {
let desc = if is_change {
self.config.main_descriptor.change_descriptor()
} else {
self.config.main_descriptor.receive_descriptor()
};
desc.derive(index, &self.secp).bip32_derivations()
} else {
Default::default()
};
psbt_outs.push(PsbtOut {
bip32_derivation,
..PsbtOut::default()
});
}
// Now create the transaction, compute its fees and already sanity check if its feerate
@ -397,7 +412,10 @@ impl DaemonControl {
// TODO: shuffle once we have Taproot
change_txo.value = change_amount.to_sat();
tx.output.push(change_txo);
psbt_outs.push(PsbtOut::default());
psbt_outs.push(PsbtOut {
bip32_derivation: change_desc.bip32_derivations(),
..PsbtOut::default()
});
}
}
}
@ -659,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();
@ -688,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);
@ -699,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),
@ -714,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),
@ -723,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
)))
@ -732,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);
@ -750,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))
);
@ -818,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();

32
src/jsonrpc/api.rs
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))
}

2
tests/test_framework/utils.py
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)

12
tests/test_rpc.py
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()

4
tests/test_spend.py
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.