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

lianad: ensure consistent derivation index management

Browse Source 
The database should store the last used/revealed derivation index.

Docstrings have been updated accordingly.

Getting a new address requires that the database value first be
incremented.

When listing addresses without specifying a count, all
used/revealed addresses should be returned.
This commit is contained in:
Michael Mallan 2025-03-24 15:05:50 +00:00
parent 81a5b159e5
commit e987458657
No known key found for this signature in database
GPG Key ID: 5177CDCEDB0EABEB
4 changed files with 94 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
lianad/src/commands/mod.rs
View File

@ -257,41 +257,39 @@ impl DaemonControl {
}
// Get the change address for the next derivation index.
// The spend may not have a change output, so we don't update the DB value yet.
fn next_change_addr(&self, db_conn: &mut Box<dyn DatabaseConnection>) -> SpendOutputAddress {
let index = db_conn.change_index();
let next_index = index
.increment()
.expect("Must not get into hardened territory");
let desc = self
.config
.main_descriptor
.change_descriptor()
.derive(index, &self.secp);
.derive(next_index, &self.secp);
let addr = desc.address(self.config.bitcoin_config.network);
SpendOutputAddress {
addr,
info: Some(AddrInfo {
index,
index: next_index,
is_change: true,
}),
}
}
// If we detect the given address as ours, and it has a higher derivation index than our next
// derivation index, update our next derivation index to the one after the address'.
fn maybe_increase_next_deriv_index(
// If we detect the given address as ours, and it has a higher derivation index than our last
// derivation index, update our last derivation index to the given value.
fn maybe_increase_last_deriv_index(
&self,
db_conn: &mut Box<dyn DatabaseConnection>,
addr_info: &Option<AddrInfo>,
) {
if let Some(AddrInfo { index, is_change }) = addr_info {
if *is_change && db_conn.change_index() <= *index {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_change_index(next_index, &self.secp);
} else if !is_change && db_conn.receive_index() <= *index {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_receive_index(next_index, &self.secp);
if *is_change && db_conn.change_index() < *index {
db_conn.set_change_index(*index, &self.secp);
} else if !is_change && db_conn.receive_index() < *index {
db_conn.set_receive_index(*index, &self.secp);
}
}
}
@ -352,9 +350,9 @@ impl DaemonControl {
.config
.main_descriptor
.receive_descriptor()
.derive(index, &self.secp)
.derive(new_index, &self.secp)
.address(self.config.bitcoin_config.network);
GetAddressResult::new(address, index)
GetAddressResult::new(address, new_index)
}
/// Update derivation indexes
@ -435,7 +433,11 @@ impl DaemonControl {
.checked_add(c)
.ok_or(CommandError::InvalidDerivationIndex)?
} else {
receive_index.max(change_index)
// `end_index` will not be included so add 1 in order to include the last used index.
receive_index
.max(change_index)
.checked_add(1)
.ok_or(CommandError::InvalidDerivationIndex)?
};
// Derive all receive and change addresses for the queried range.
@ -667,10 +669,10 @@ impl DaemonControl {
}
};
for (addr, _) in destinations_checked {
self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info);
self.maybe_increase_last_deriv_index(&mut db_conn, &addr.info);
}
if has_change {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_info);
self.maybe_increase_last_deriv_index(&mut db_conn, &change_info);
}
Ok(CreateSpendResult::Success {
@ -1052,10 +1054,10 @@ impl DaemonControl {
// In case of success, make sure to update our next derivation index if any address
// used in the transaction outputs was from the future.
for (addr, _) in destinations {
self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info);
self.maybe_increase_last_deriv_index(&mut db_conn, &addr.info);
}
if has_change {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_address.info);
self.maybe_increase_last_deriv_index(&mut db_conn, &change_address.info);
}
return Ok(CreateSpendResult::Success {
@ -1204,7 +1206,7 @@ impl DaemonControl {
locktime,
)?;
if has_change {
self.maybe_increase_next_deriv_index(&mut db_conn, &sweep_addr_info);
self.maybe_increase_last_deriv_index(&mut db_conn, &sweep_addr_info);
}
Ok(CreateRecoveryResult { psbt })
@ -1397,12 +1399,17 @@ mod tests {
let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new());
let control = &ms.control();
// We can get an address
// We can get an address (it will have index 1)
let addr = control.get_new_address().address;
// $ bitcoin-cli deriveaddresses "wsh(or_d(pk([aabbccdd]xpub68JJTXc1MWK8KLW4HGLXZBJknja7kDUJuFHnM424LbziEXsfkh1WQCiEjjHw4zLqSUm4rvhgyGkkuRowE9tCJSgt3TQB5J3SKAbZ2SdcKST/0/*),and_v(v:pkh([aabbccdd]xpub68JJTXc1MWK8PEQozKsRatrUHXKFNkD1Cb1BuQU9Xr5moCv87anqGyXLyUd4KpnDyZgo3gz4aN1r3NiaoweFW8UutBsBbgKHzaD5HkTkifK/0/*),older(10000))))#wx6v3mks" 1
// [
// "bc1q9ksrc647hx8zp2cewl8p5f487dgux3777yees8rjcx46t4daqzzqt7yga8",
// "bc1qm06ceyghltr8v5cmeckh6cquhy9nks626pahapc04xd98kwf478qwmhqew"
// ]
assert_eq!(
addr,
bitcoin::Address::from_str(
"bc1q9ksrc647hx8zp2cewl8p5f487dgux3777yees8rjcx46t4daqzzqt7yga8"
"bc1qm06ceyghltr8v5cmeckh6cquhy9nks626pahapc04xd98kwf478qwmhqew"
)
.unwrap()
.assume_checked()
@ -1426,39 +1433,38 @@ mod tests {
assert_eq!(list.addresses.last().unwrap().index, 6);
let addr0 = control.get_new_address().address;
let addr1 = control.get_new_address().address;
let _addr2 = control.get_new_address().address;
let addr3 = control.get_new_address().address;
let _addr1 = control.get_new_address().address;
let addr2 = control.get_new_address().address;
let _addr3 = control.get_new_address().address;
let addr4 = control.get_new_address().address;
let list = control.list_addresses(Some(0), None).unwrap();
assert_eq!(list.addresses[0].index, 0);
assert_eq!(list.addresses[0].receive, addr0);
assert_eq!(list.addresses.last().unwrap().index, 4);
assert_eq!(list.addresses[1].receive, addr0); // first address has index 1
assert_eq!(list.addresses.last().unwrap().index, 5);
assert_eq!(list.addresses.last().unwrap().receive, addr4);
let list = control.list_addresses(None, None).unwrap();
assert_eq!(list.addresses[0].index, 0);
assert_eq!(list.addresses[0].receive, addr0);
assert_eq!(list.addresses.last().unwrap().index, 4);
assert_eq!(list.addresses[1].index, 1);
assert_eq!(list.addresses[1].receive, addr0);
assert_eq!(list.addresses.last().unwrap().index, 5);
assert_eq!(list.addresses.last().unwrap().receive, addr4);
let list = control.list_addresses(Some(1), Some(3)).unwrap();
assert_eq!(list.addresses[0].index, 1);
assert_eq!(list.addresses[0].receive, addr1);
assert_eq!(list.addresses[0].receive, addr0);
assert_eq!(list.addresses.last().unwrap().index, 3);
assert_eq!(list.addresses.last().unwrap().receive, addr3);
assert_eq!(list.addresses.last().unwrap().receive, addr2);
let addr5 = control.get_new_address().address;
let list = control.list_addresses(Some(5), None).unwrap();
assert_eq!(list.addresses.len(), 1);
assert_eq!(list.addresses[0].index, 5);
assert_eq!(list.addresses[0].receive, addr5);
assert_eq!(list.addresses.last().unwrap().index, 5);
assert_eq!(list.addresses.last().unwrap().receive, addr5);
assert_eq!(list.addresses[0].receive, addr4);
// We can get no address for the last unhardened index.
let max_unhardened_index = 2u32.pow(31) - 1;

12
lianad/src/database/mod.rs
View File

@ -30,9 +30,9 @@ use miniscript::bitcoin::{self, bip32, psbt::Psbt, secp256k1, Address, Network,
pub struct Wallet {
/// Timestamp at wallet creation time.
pub timestamp: u32,
/// Derivation index for the next receiving address.
/// Derivation index for the last used/revealed receiving address.
pub receive_index: bip32::ChildNumber,
/// Derivation index for the next change address.
/// Derivation index for the last used/revealed change address.
pub change_index: bip32::ChildNumber,
/// Timestamp to start rescanning from, if any.
pub rescan_timestamp: Option<u32>,
@ -73,20 +73,20 @@ pub trait DatabaseConnection {
/// Update our best chain seen.
fn update_tip(&mut self, tip: &BlockChainTip);
/// Get the derivation index for the next receiving address
/// Get the derivation index for the last used/revealed receiving address
fn receive_index(&mut self) -> bip32::ChildNumber;
/// Set the derivation index for the next receiving address
/// Set the derivation index for the last used/revealed receiving address
fn set_receive_index(
&mut self,
index: bip32::ChildNumber,
secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
);
/// Get the derivation index for the next change address
/// Get the derivation index for the last used/revealed change address
fn change_index(&mut self) -> bip32::ChildNumber;
/// Set the derivation index for the next change address
/// Set the derivation index for the last used/revealed change address
fn set_change_index(
&mut self,
index: bip32::ChildNumber,

28
tests/test_rpc.py
View File

@ -152,29 +152,35 @@ def test_update_derivation_indexes(lianad):
def test_getaddress(lianad):
res = lianad.rpc.getnewaddress()
assert "address" in res
# The first new wallet address has index 1
assert res["derivation_index"] == 1
# We'll get a new one at every call
assert res["address"] != lianad.rpc.getnewaddress()["address"]
# new address has derivation_index higher than the previous one
assert lianad.rpc.getnewaddress()["derivation_index"] == res["derivation_index"] + 2
info = lianad.rpc.getinfo()
assert info["receive_index"] == res["derivation_index"] + 3
assert info["receive_index"] == res["derivation_index"] + 2 # 3 == 1 + 2
assert info["change_index"] == 0
def test_listaddresses(lianad):
list = lianad.rpc.listaddresses(2, 5)
list1 = lianad.rpc.listaddresses(2, 5)
list2 = lianad.rpc.listaddresses(start_index=2, count=5)
assert list == list2
assert "addresses" in list
addr = list["addresses"]
assert list1 == list2
assert "addresses" in list1
addr = list1["addresses"]
assert addr[0]["index"] == 2
assert addr[-1]["index"] == 6
list3 = lianad.rpc.listaddresses() # start_index = 0, receive_index = 0
list3 = (
lianad.rpc.listaddresses()
) # start_index = 0, receive_index = 0 (returns 1 "used" address for index 0)
_ = lianad.rpc.getnewaddress() # start_index = 0, receive_index = 1
_ = lianad.rpc.getnewaddress() # start_index = 0, receive_index = 2
# list4 returns all indexes from 0 up to last used.
# The first new address has index 1, so returned indexes are 0, 1, 2:
list4 = lianad.rpc.listaddresses()
assert len(list4["addresses"]) == len(list3["addresses"]) + 2 == 2
assert len(list4["addresses"]) == len(list3["addresses"]) + 2 == 3
list5 = lianad.rpc.listaddresses(0)
assert list4 == list5
@ -534,7 +540,13 @@ def test_create_spend(lianad, bitcoind):
assert len(spend_psbt.o) == 4
assert len(spend_psbt.tx.vout) == 4
assert lianad.rpc.getinfo()["change_index"] == 15
# 15 new receive addresses have been generated (starting at index 1),
# so last used value is 15:
assert lianad.rpc.getinfo()["receive_index"] == 15
# For each received coin, the change index has also been updated by the poller
# (see https://github.com/wizardsardine/liana/issues/1333), so is also 15.
# Then `createspend` will use the next index for change and update the DB value accordingly:
assert lianad.rpc.getinfo()["change_index"] == 16
# The transaction must contain the spent transaction for each input for P2WSH. But not for Taproot.
# We don't make assumptions about the ordering of PSBT inputs.

28
tests/test_spend.py
View File

@ -231,10 +231,21 @@ def test_send_to_self(lianad, bitcoind):
lianad.rpc.getnewaddress()["address"]: 0.04,
lianad.rpc.getnewaddress()["address"]: 0.05,
}
info = lianad.rpc.getinfo()
# We've generated 3 receive addresses, starting at index 1, so last used is 3.
assert info["receive_index"] == 3
# No change addresses used, so last index is 0.
assert info["change_index"] == 0
deposit_txid = bitcoind.rpc.sendmany("", destinations)
bitcoind.generate_block(1, wait_for_mempool=deposit_txid)
wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 3)
info = lianad.rpc.getinfo()
assert info["receive_index"] == 3
# Change index has been updated by poller, even though none used
# (see https://github.com/wizardsardine/liana/issues/1333):
assert info["change_index"] == 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"]]
@ -243,6 +254,12 @@ 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
info = lianad.rpc.getinfo()
# Send to self didn't use any receive addresses...
assert info["receive_index"] == 3
# ... but it did use a new change address:
assert info["change_index"] == 4
# Note they may ask for an impossible send-to-self. In this case we'll report missing amount.
huge_feerate = 50_000 if USE_TAPROOT else 40_500
assert "missing" in lianad.rpc.createspend({}, outpoints, huge_feerate)
@ -267,16 +284,19 @@ def test_send_to_self(lianad, bitcoind):
)
wait_for(lambda: len(list(unspent_coins())) == 1)
# We've used 3 receive addresses and so the DB receive index must be 3.
assert len(lianad.rpc.listaddresses()["addresses"]) == 3
info = lianad.rpc.getinfo()
# The poller has updated the receive index based on the change index
# (see https://github.com/wizardsardine/liana/issues/1333):
assert info["receive_index"] == 4
assert info["change_index"] == 4
# Create a new spend to the receive address with index 3.
recv_addr = lianad.rpc.listaddresses(3, 1)["addresses"][0]["receive"]
res = lianad.rpc.createspend(
{recv_addr: 11_965_000 if USE_TAPROOT else 11_955_000}, [], 2
)
assert "psbt" in res
# Max(receive_index, change_index) is now 4:
assert len(lianad.rpc.listaddresses()["addresses"]) == 4
# Max(receive_index, change_index) is now 4, so we return addresses 0, 1, 2, 3, 4:
assert len(lianad.rpc.listaddresses()["addresses"]) == 5
# But the spend has no change:
psbt = PSBT.from_base64(res["psbt"])
assert len(psbt.o) == 1