Skip to content

fix(wallet): categorize self-transfers to external addresses as trusted_pending #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dmenec wants to merge 2 commits into
base: master
Choose a base branch
from
+302 −3
Open

fix(wallet): categorize self-transfers to external addresses as trusted_pending #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3674263
fix(wallet): categorize self-transfers to external addresses as trust…
Dmenec Apr 4, 2026
3b2af6b
test(wallet): add balance categorization tests for trusted/untrusted …
Dmenec Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,12 +1115,54 @@ impl Wallet {
/// Return the balance, separated into available, trusted-pending, untrusted-pending, and
/// immature values.
pub fn balance(&self) -> Balance {
self.tx_graph.graph().balance(
let graph = self.tx_graph.graph();
let index = &self.tx_graph.index;

let owned_outpoints: HashSet<OutPoint> =
index.outpoints().iter().map(|(_, op)| *op).collect();

let mut untrusted_outpoints: HashSet<OutPoint> = HashSet::new();

let mut changed = true;
while changed {
Copy link
Copy Markdown
Contributor

@nymius nymius Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this for loop { ... } and break if conditions are met. changed doesn't hold any meaning by itself outside of this particular context.

changed = false;
for tx_node in graph.full_txs() {
if tx_node.tx.is_coinbase() {
continue;
}
let is_untrusted = tx_node.tx.input.iter().any(|txin| {
!owned_outpoints.contains(&txin.previous_output)
|| untrusted_outpoints.contains(&txin.previous_output)
Copy link
Copy Markdown
Contributor

@nymius nymius Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if I'm missing an assumption over graph here, but if txin.previous_output is confirmed, wouldn't it mean that that output should be considered trusted?

This case:

#[test]
fn test_pay_to_internal_from_not_trusted_but_confirmed() {
    let (mut wallet, _) = get_funded_wallet_wpkh();

    // Build a tx whose input comes from an unknown (external) outpoint,
    // but whose output goes to our change (internal) keychain address.
    let external_txid = Txid::from_raw_hash(Hash::all_zeros());
    let tx = Transaction {
        input: vec![TxIn {
            previous_output: OutPoint {
                txid: external_txid,
                vout: 0,
            },
            ..Default::default()
        }],
        output: vec![TxOut {
            value: Amount::from_sat(500),
            script_pubkey: wallet
                .next_unused_address(KeychainKind::Internal)
                .address
                .script_pubkey(),
        }],
        version: transaction::Version::ONE,
        lock_time: absolute::LockTime::ZERO,
    };

    insert_tx(&mut wallet, tx);

    let anchor = ConfirmationBlockTime {
        block_id: wallet.latest_checkpoint().get(2000).unwrap().block_id(),
        confirmation_time: 0,
    };
    
    // The transaction is confirmed
    insert_anchor(&mut wallet, external_txid, anchor);

    let balance = wallet.balance();

    // The output is ours but the input is not owned, so it must be untrusted_pending.
    assert!(balance.trusted_pending > Amount::ZERO);
    assert_eq!(balance.untrusted_pending, Amount::ZERO);
}

Copy link
Copy Markdown
Contributor Author

@Dmenec Dmenec Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I think there is another bug in there. Maybe another hashset of confirmed outpoints and checking if the hashet contains it on:

let is_untrusted = tx_node.tx.input.iter().any(|txin| {
    !owned_outpoints.contains(&txin.previous_output)
        || untrusted_outpoints.contains(&txin.previous_output)
});

Would solve that?

Copy link
Copy Markdown
Contributor

@nymius nymius Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, did you intend to show something in the code? I'm not getting it.
Anyways, I wouldn't be using hash sets lightly here, not without testing against large cases.

Copy link
Copy Markdown
Contributor Author

@Dmenec Dmenec Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think I lost context just with this snippet. The point was to add another condition there to check if it's confirmed or not, maybe by storing all the confirmed transactions in a separate hash set.

Agreeing with benchmarking it first.

Copy link
Copy Markdown
Contributor

@nymius nymius Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing with @ValuedMammal, we conclude the definition of trust is: all inputs must be controlled by this wallet.
I don't know if it's already in the docs, but it would be worth adding it here.
In that case, my original comment is invalid, and you shouldn't make changes based on it.

});
if is_untrusted {
for (vout, _) in tx_node.tx.output.iter().enumerate() {
let op = OutPoint {
txid: tx_node.txid,
vout: vout as u32,
};
if owned_outpoints.contains(&op) && untrusted_outpoints.insert(op) {
changed = true;
}
}
}
}
}

let trusted_outpoints: HashSet<&OutPoint> =
owned_outpoints.difference(&untrusted_outpoints).collect();

let keyed_outpoints = index
.outpoints()
.iter()
.cloned()
.map(|((k, i), op)| ((k, i, op), op));

graph.balance(
&self.chain,
self.chain.tip().block_id(),
CanonicalizationParams::default(),
self.tx_graph.index.outpoints().iter().cloned(),
|&(k, _), _| k == KeychainKind::Internal,
keyed_outpoints,
|&(_, _, op), _| trusted_outpoints.contains(&op),
)
}

Expand Down
257 changes: 257 additions & 0 deletions tests/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2961,3 +2961,260 @@ fn test_tx_ordering_untouched_preserves_insertion_ordering() {
// Check vout is sorted by recipient insertion order
assert!(txouts == vec![400, 300, 500]);
}

#[test]
fn test_trusted_pending_balance_from_owned_outpoints() {
let (mut wallet, txid) = get_funded_wallet_wpkh();
let tx = Transaction {
input: vec![TxIn {
previous_output: OutPoint { txid, vout: 0 },
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(500),
script_pubkey: wallet
.next_unused_address(KeychainKind::Internal)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

insert_tx(&mut wallet, tx.clone());

let balance = wallet.balance();

assert!(balance.trusted_pending > Amount::ZERO);
assert_eq!(balance.untrusted_pending, Amount::ZERO);
}

#[test]
fn test_untrusted_pending_balance_from_external_inputs() {
let (descriptor, change_descriptor) = get_test_wpkh_and_change_desc();
let mut wallet = Wallet::create(descriptor, change_descriptor)
.network(Network::Regtest)
.create_wallet_no_persist()
.expect("wallet");

let txid = Txid::from_raw_hash(Hash::all_zeros());

let tx = Transaction {
input: vec![TxIn {
previous_output: OutPoint { txid, vout: 0 },
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(500),
script_pubkey: wallet
.next_unused_address(KeychainKind::External)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

insert_tx(&mut wallet, tx.clone());

let balance = wallet.balance();

assert!(balance.untrusted_pending > Amount::ZERO);
assert_eq!(balance.trusted_pending, Amount::ZERO);
}

#[test]
fn test_trusted_pending_transitive_chain() {
let (mut wallet, txid) = get_funded_wallet_wpkh();

let tx_a = Transaction {
input: vec![TxIn {
previous_output: OutPoint { txid, vout: 0 },
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(500),
script_pubkey: wallet
.next_unused_address(KeychainKind::External)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

let tx_a_txid = tx_a.compute_txid();
insert_tx(&mut wallet, tx_a);

let tx_b = Transaction {
input: vec![TxIn {
previous_output: OutPoint {
txid: tx_a_txid,
vout: 0,
},
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(500),
script_pubkey: wallet
.next_unused_address(KeychainKind::External)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};
insert_tx(&mut wallet, tx_b);

let balance = wallet.balance();

assert!(balance.trusted_pending > Amount::ZERO);
assert_eq!(balance.untrusted_pending, Amount::ZERO);
}

#[test]
fn test_pay_to_internal_from_not_trusted() {
let (mut wallet, _) = get_funded_wallet_wpkh();

// Build a tx whose input comes from an unknown (external) outpoint,
// but whose output goes to our change (internal) keychain address.
let external_txid = Txid::from_raw_hash(Hash::all_zeros());
let tx = Transaction {
input: vec![TxIn {
previous_output: OutPoint {
txid: external_txid,
vout: 0,
},
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(500),
script_pubkey: wallet
.next_unused_address(KeychainKind::Internal)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

insert_tx(&mut wallet, tx);

let balance = wallet.balance();

// The output is ours but the input is not owned, so it must be untrusted_pending.
assert!(balance.untrusted_pending > Amount::ZERO);
assert_eq!(balance.trusted_pending, Amount::ZERO);
}

#[test]
fn test_trusted_pending_does_not_propagate_through_foreign_outputs() {
let (mut wallet, txid) = get_funded_wallet_wpkh();

let foreign_addr = Address::from_str("bcrt1q3qtze4ys45tgdvguj66zrk4fu6hq3a3v9pfly5")
.expect("valid address")
.require_network(Network::Regtest)
.unwrap();

let tx_a = Transaction {
input: vec![TxIn {
previous_output: OutPoint { txid, vout: 0 },
..Default::default()
}],
output: vec![
TxOut {
value: Amount::from_sat(25_000),
script_pubkey: foreign_addr.script_pubkey(),
},
TxOut {
value: Amount::from_sat(24_000),
script_pubkey: wallet
.next_unused_address(KeychainKind::Internal)
.address
.script_pubkey(),
},
],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};
let tx_a_txid = tx_a.compute_txid();
insert_tx(&mut wallet, tx_a);

let tx_b = Transaction {
input: vec![TxIn {
previous_output: OutPoint {
txid: tx_a_txid,
vout: 0,
},
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(20_000),
script_pubkey: wallet
.next_unused_address(KeychainKind::External)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};
insert_tx(&mut wallet, tx_b);

let balance = wallet.balance();

assert!(balance.trusted_pending > Amount::ZERO);
assert!(balance.untrusted_pending > Amount::ZERO);
}

#[test]
fn test_spending_untrusted_is_untrusted() {
let (mut wallet, _) = get_funded_wallet_wpkh();

let external_tx = Transaction {
input: vec![TxIn {
previous_output: OutPoint {
txid: Txid::all_zeros(), // not owned by wallet
vout: 0,
},
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(50_000),
script_pubkey: wallet
.next_unused_address(KeychainKind::External)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

let external_txid = external_tx.compute_txid();
insert_tx(&mut wallet, external_tx);

let tx_spend = Transaction {
input: vec![TxIn {
previous_output: OutPoint {
txid: external_txid,
vout: 0,
},
..Default::default()
}],
output: vec![TxOut {
value: Amount::from_sat(45_000),
script_pubkey: wallet
.next_unused_address(KeychainKind::Internal)
.address
.script_pubkey(),
}],
version: transaction::Version::ONE,
lock_time: absolute::LockTime::ZERO,
};

insert_tx(&mut wallet, tx_spend);

let balance = wallet.balance();

assert_eq!(balance.untrusted_pending, Amount::from_sat(45_000));
assert_eq!(balance.trusted_pending, Amount::ZERO);
}