-
Notifications
You must be signed in to change notification settings - Fork 13
fix(spv): apply InstantSend locks to self-broadcast transactions #815
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
Changes from all commits
87e7916
e995ed5
5382141
c13831e
7251b9e
d6ce0ec
c217928
4179528
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -144,6 +144,13 @@ pub(crate) struct SpvEventHandler { | |||||||||||||||
| connection_status: Option<Arc<ConnectionStatus>>, | ||||||||||||||||
| reconcile_tx: Arc<Mutex<Option<mpsc::Sender<()>>>>, | ||||||||||||||||
| finality_tx: Arc<Mutex<Option<mpsc::Sender<AssetLockFinalityEvent>>>>, | ||||||||||||||||
| /// Wallet manager reference for applying InstantSend locks directly. | ||||||||||||||||
| /// | ||||||||||||||||
| /// Self-broadcast transactions bypass the MempoolManager and are fed to | ||||||||||||||||
| /// the WalletManager via `notify_wallet_after_broadcast()`. When the IS | ||||||||||||||||
| /// lock arrives later, the MempoolManager doesn't know about the tx and | ||||||||||||||||
| /// cannot apply the lock. We apply it here directly on the WalletManager. | ||||||||||||||||
| wallet: Arc<AsyncRwLock<WalletManager<ManagedWalletInfo>>>, | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| impl EventHandler for SpvEventHandler { | ||||||||||||||||
|
|
@@ -256,6 +263,33 @@ impl EventHandler for SpvEventHandler { | |||||||||||||||
| | SyncEvent::SyncComplete { .. } | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| // TODO(workaround): Remove once dashpay/rust-dashcore#487 is fixed. | ||||||||||||||||
| // | ||||||||||||||||
| // Apply InstantSend locks directly on the WalletManager. | ||||||||||||||||
| // | ||||||||||||||||
| // Self-broadcast transactions bypass the MempoolManager (they are fed | ||||||||||||||||
| // directly to WalletManager via notify_wallet_after_broadcast — see | ||||||||||||||||
| // the other workaround in spawn_request_handler). When the IS lock | ||||||||||||||||
| // arrives from the network, the MempoolManager doesn't know about | ||||||||||||||||
| // the tx and stores it as a "pending IS lock" that is never matched. | ||||||||||||||||
| // Applying the lock here ensures self-broadcast txs transition from | ||||||||||||||||
| // unconfirmed to spendable. | ||||||||||||||||
| // | ||||||||||||||||
| // Once upstream broadcast calls handle_tx() on the MempoolManager, | ||||||||||||||||
| // both workarounds (notify_wallet_after_broadcast and this) can be | ||||||||||||||||
| // removed — the normal MempoolManager pipeline will handle everything. | ||||||||||||||||
| // | ||||||||||||||||
| // For MempoolManager-tracked txs this is a harmless no-op — the | ||||||||||||||||
| // WalletManager deduplicates via its instant_send_locks HashSet. | ||||||||||||||||
| if let SyncEvent::InstantLockReceived { instant_lock, .. } = event { | ||||||||||||||||
| let txid = instant_lock.txid; | ||||||||||||||||
| let wallet = Arc::clone(&self.wallet); | ||||||||||||||||
| tokio::spawn(async move { | ||||||||||||||||
| let mut wm = wallet.write().await; | ||||||||||||||||
| wm.process_instant_send_lock(txid); | ||||||||||||||||
| }); | ||||||||||||||||
|
Comment on lines
+286
to
+290
|
||||||||||||||||
| let wallet = Arc::clone(&self.wallet); | |
| tokio::spawn(async move { | |
| let mut wm = wallet.write().await; | |
| wm.process_instant_send_lock(txid); | |
| }); | |
| let mut wm = self.wallet.write().await; | |
| wm.process_instant_send_lock(txid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds important new behavior (applying InstantSend locks to self-broadcast transactions) but there’s no regression test coverage in src/spv/tests.rs for InstantLockReceived or for the notify_wallet_after_broadcast path. Adding a focused test that simulates a broadcast-notified tx and then an InstantLockReceived event (asserting spendable/unconfirmed transitions) would help prevent this from silently regressing.