Fix: Wallet sync may decrement address index
This bug seems to be Electrum-specific. The fix is to check the proposed changes against the current state of the database. Ensure newly suggested indexes are not smaller than indexes already in database. Changes: * Check index updates before they are applied to database during Electrum Blockchain sync (Thank you @rajarshimaitra for providing an elegant solution). Tests added: * bdk_blockchain_tests!::test_sync_address_index_should_not_decrement * bdk_blockchain_tests!::test_sync_address_index_should_increment These tests ensure there will be no unexpected address reuse when grabbing a new address via `Wallet::get_address` with `AddressIndex::New`. Other changes: * Tweak `rpc.rs` so that clippy is happy.
This commit is contained in:
@@ -372,6 +372,7 @@ macro_rules! bdk_blockchain_tests {
|
||||
use $crate::blockchain::Blockchain;
|
||||
use $crate::database::MemoryDatabase;
|
||||
use $crate::types::KeychainKind;
|
||||
use $crate::wallet::AddressIndex;
|
||||
use $crate::{Wallet, FeeRate, SyncOptions};
|
||||
use $crate::testutils;
|
||||
|
||||
@@ -651,6 +652,60 @@ macro_rules! bdk_blockchain_tests {
|
||||
assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect number of unspents");
|
||||
}
|
||||
|
||||
// Syncing wallet should not result in wallet address index to decrement.
|
||||
// This is critical as we should always ensure to not reuse addresses.
|
||||
#[test]
|
||||
fn test_sync_address_index_should_not_decrement() {
|
||||
let (wallet, blockchain, _descriptors, mut test_client) = init_single_sig();
|
||||
|
||||
const ADDRS_TO_FUND: u32 = 7;
|
||||
const ADDRS_TO_IGNORE: u32 = 11;
|
||||
|
||||
let mut first_addr_index: u32 = 0;
|
||||
|
||||
(0..ADDRS_TO_FUND + ADDRS_TO_IGNORE).for_each(|i| {
|
||||
let new_addr = wallet.get_address(AddressIndex::New).unwrap();
|
||||
|
||||
if i == 0 {
|
||||
first_addr_index = new_addr.index;
|
||||
}
|
||||
assert_eq!(new_addr.index, i+first_addr_index, "unexpected new address index (before sync)");
|
||||
|
||||
if i < ADDRS_TO_FUND {
|
||||
test_client.receive(testutils! {
|
||||
@tx ((@addr new_addr.address) => 50_000)
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
wallet.sync(&blockchain, SyncOptions::default()).unwrap();
|
||||
|
||||
let new_addr = wallet.get_address(AddressIndex::New).unwrap();
|
||||
assert_eq!(new_addr.index, ADDRS_TO_FUND+ADDRS_TO_IGNORE+first_addr_index, "unexpected new address index (after sync)");
|
||||
}
|
||||
|
||||
// Even if user does not explicitly grab new addresses, the address index should
|
||||
// increment after sync (if wallet has a balance).
|
||||
#[test]
|
||||
fn test_sync_address_index_should_increment() {
|
||||
let (wallet, blockchain, descriptors, mut test_client) = init_single_sig();
|
||||
|
||||
const START_FUND: u32 = 4;
|
||||
const END_FUND: u32 = 20;
|
||||
|
||||
// "secretly" fund wallet via given range
|
||||
(START_FUND..END_FUND).for_each(|addr_index| {
|
||||
test_client.receive(testutils! {
|
||||
@tx ((@external descriptors, addr_index) => 50_000)
|
||||
});
|
||||
});
|
||||
|
||||
wallet.sync(&blockchain, SyncOptions::default()).unwrap();
|
||||
|
||||
let address = wallet.get_address(AddressIndex::New).unwrap();
|
||||
assert_eq!(address.index, END_FUND, "unexpected new address index (after sync)");
|
||||
}
|
||||
|
||||
/// Send two conflicting transactions to the same address twice in a row.
|
||||
/// The coins should only be received once!
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user