-
Notifications
You must be signed in to change notification settings - Fork 322
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.
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
Undetected double-spent #1740
Comments
|
I've edited the script and applied Using Is there any method to remove PS: This is related to #1743. I tried to explicitly cancel the transaction as a work-around |
Thanks for sharing the example code. You're right that // We also add tx1b to the wallet
let tx_update = TxUpdate {
txs: vec![std::sync::Arc::new(tx1b.clone())],
..Default::default()
};
let update = bdk_wallet::Update {
tx_update,
..Default::default()
};
let _ = alice.apply_update_at(update, Some(101)).unwrap();
println!("After applying tx1b");
println!("- alice.list_unspent(): {:?}", alice.list_unspent().map(|o| o.outpoint).collect::<Vec<_>>());
println!("- alice.list_transaction(): {:?}", alice.transactions().map(|t| t.tx_node.txid).collect::<Vec<_>>());
println!("- alice.list_transaction(): {:?}", alice.transactions().map(|t| t.chain_position).collect::<Vec<_>>());
println!("- alice.balance: {:?}", alice.balance());
// I expect list_unspent to be empty.
// (tx1a) was double-spent
assert_eq!(alice.list_unspent().collect::<Vec<_>>(), vec![]);
assert_eq!(alice.transactions().collect::<Vec<_>>().len(), 1);
let tx = alice.transactions().next().unwrap().tx_node;
assert_eq!(tx.txid, tx1b.compute_txid()); |
I'm not sure if that's the ideal solution long term, depending on the use case. One thing that's unclear from the example is whether alice and Bitcoin Core represent distinct entities or if there's reason to think alice should be aware of the other wallet. If so, I can imagine a feature that would let you add extra keychains to the wallet for the purpose of watching other transactions. I agree that Also I seem to recall that doing a sync via electrum or esplora would allow you to watch for the status of a txid of interest, in this case the unconfirmed *digging deeper it appears for esplora you would actually watch for the spend status of an outpoint of interest, in this case |
I discovered the issue when testing the unilateral exit procedure of our Ark implementation. This issue contains a write-up of what the issue is and how it affects us. However, to my understanding the issue is broader.
|
It seems we may need to monitor during a sync/scan not just the histories of our wallet's descriptors SPKs but also the UTXOs that are used as inputs to one of our unconfirmed transactions. If we knew that a UTXO was now invalid (ie. spent and confirmed in a different TX) then we could mark our TX as "double spent" or something like that so they could be filtered out. Have you seen #1748? the redesign that @LLFourn has in mind should probably take into account this situation from also. One question, do you know how other wallets handle this situation? If you setup the same situation in Core's wallet or in Sparrow do you know how they handle it? |
I have no idea what other wallets do, I'm sorry. I'm not sure many wallets allow foreign inputs. (And we are eternally grateful that BDK does!) |
Thanks for taking the time to report this. This is a big problem (and I'm also not sure if other wallets/libraries would handle this properly). @LLFourn talked to me briefly this morning about how to fix this. This is my recollection + some of my own thoughts thrown in. To fix this problem, we need to redefine what is considered relevant by our receiving structures, and make sure our chain sources emit the replacing tx. Step 1: Redefine what is considered "relevant" in
|
@LagginTimes agreed to help out on this. He will implement the |
idea: #1764 |
Describe the bug
tx1a
) from Bob and observes it in the mempool.tx1a
creatingtx1b
tx1b
tx1b
in a block.Alice will still consider
tx1a
as a pending transaction.To Reproduce
See this https://github.com/ErikDeSmedt/bdk-gists/blob/master/tests/wallet.rs
Expected behavior
I would expect that Alice her wallet wouldn't use
tx1a
anymore. The outputBuild environment
1.0.0-beta.5
Additional context
I discovered this bug in a test-case where the entire wallet was unusable.
The text was updated successfully, but these errors were encountered: