Skip to content

ddk: keep own broadcasts through esplora's indexing lag - #230

Open
bennyhodl wants to merge 1 commit into
masterfrom
benny/225-broadcast-grace
Open

bennyhodl wants to merge 1 commit into
masterfrom
benny/225-broadcast-grace

Conversation

@bennyhodl

Copy link
Copy Markdown
Owner

Summary

Item 5 of #225. electrs indexes its mempool behind the node. A wallet that broadcasts and then syncs inside that lag reads its own transaction as evicted, frees its coins, and can double-spend them on the next send. Lygos works around it with a 60 s recent_broadcasts filter. This PR fixes it in DDK.

Changes

  • One owner for recent broadcasts: ddk/src/chain/broadcasts.rs (private RecentBroadcasts, 60 s grace), held by EsploraClient. The wallet and the manager share that client, so every broadcast is recorded in one place.
  • New public EsploraClient::broadcast: records the txid and returns the broadcast time. Blockchain::send_transaction goes through it, so the manager's funding, CET, refund and close broadcasts are all recorded.
  • command::sync: drops evictions of transactions broadcast within the grace window before applying the update. The comparison uses the sync's own start time, which is what bdk_esplora stamps evictions with, so it reads no clock.
  • sign_and_broadcast records the sent transaction as unconfirmed with apply_unconfirmed_txs_events and forwards the events. Before this, DDK's wallet only learned about its own sends from esplora, so its coins looked unspent until electrs indexed the send. On its own, the grace filter changed nothing.

Not changed (same exposure as master)

  • Restart: the grace map is in memory, so a restart and sync inside the 60 s window gets no grace for that transaction. Master didn't record local sends at all.
  • RBF ties: bdk orders unconfirmed conflicts by (last_seen, txid). A bump_fee in the same second as the previous sync records the replacement with the same timestamp, and bdk usually keeps the original canonical until the next sync. That is when master switched over too, so this is not a regression. Fixing it would mean stamping replacements past their conflicts.
  • Tracker sync: the contract tracker sync (ddk: make the contract UTXO tracker available without manager #227) gets evictions now, but the tracker only learns transactions from chain sync, never from local inserts, so it doesn't need the grace.

Testing

  • New chain::broadcasts::tests::evictions_of_own_broadcasts_wait_out_the_grace: an own eviction inside the window is dropped, one from a sync that started before the broadcast is dropped, one past the window is applied, and a foreign eviction is always applied.
  • New wallet::tests::a_sent_transaction_keeps_its_coins_spent_through_an_immediate_sync (regtest): the sent coin is spent right away and stays spent through an immediate sync, and exactly one TxUnconfirmed event fires across the send and the sync. With the old code it fails, on both the coin check and the event count.
  • ddk lib tests wallet:: and chain::: 36 passed. enumeration_contract and four stateless_execution close and splice tests pass on regtest.
  • cargo fmt --check, cargo clippy -- -D warnings and cargo check -p ddk --no-default-features pass.
  • Reviewed by GPT-6 Astra:
    • the timestamp comparison is correct against bdk_esplora 0.22.2 and bdk_chain 0.23.3;
    • the local insert dropped wallet events, which is fixed;
    • the restart and RBF-tie points are noted above.

A wallet sync asks esplora for every unconfirmed transaction it expects
and applies the eviction of any esplora does not return. electrs indexes
its mempool behind the node, so a sync right after a broadcast evicted
the wallet's own transaction and freed its coins for the next selection.
Worse, sign_and_broadcast never recorded the transaction at all, so its
coins looked unspent until esplora indexed it.

EsploraClient now owns the transactions this node broadcast recently.
Every broadcast (wallet sends and fee bumps, and the manager's funding,
CET, refund and close transactions through Blockchain::send_transaction)
goes through EsploraClient::broadcast, which records it. The wallet sync
drops evictions of a transaction broadcast less than a minute before the
sync started. The eviction stamp is the sync's start time, so the check
needs no clock.

sign_and_broadcast records the sent transaction as unconfirmed in the
wallet and persists it, so its coins are spent at once and stay spent
across a restart. Recording forwards the wallet events it produces, so
subscribers still hear of each send once, as they did from the sync.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant