fix: deflake //rs/bitcoin/adapter:adapter_integration_test

[basvandijk]

Why

//rs/bitcoin/adapter:adapter_integration_test is one of the most flaky tests. All 4 flaky runs in the last week failed identically, in the single sub-test doge_test_send_tx:

thread 'doge_test_send_tx' panicked at rs/bitcoin/adapter/tests/integration.rs:806:9:
assertion `left == right` failed
  left: 0 SAT
 right: 100000000 SAT

i.e. the 1 DOGE sent from Alice to Bob never showed up within the test's 30-second polling window. Since Alice and Bob are labelled addresses in the same dogecoind wallet, the transaction is "trusted" and counts towards Bob's balance as soon as it enters dogecoind's mempool — so the failure means the transaction never reached dogecoind's mempool at all.

Root cause: the adapter's transaction relay is a one-shot with no retry

The relay path is:

test --gRPC SendTransaction--> adapter --P2P inv--> dogecoind
                               adapter <--getdata-- dogecoind
                               adapter --tx-------> dogecoind --> mempool

TransactionStore::advertise_txids advertises a cached transaction to each peer exactly once: the peer's socket address is recorded in TransactionInfo.advertised and never advertised to again until the transaction's 10-minute TTL expires. That advertised mark:

• is set even when the underlying send fails (send_to swallows connection write errors), and
• survives a connection discard + reconnect, because it is keyed by the peer's stable configured socket address and lives independently of the ConnectionManager.

The peer (dogecoind) only requests the transaction (getdata) in reaction to the inv. So if any single step of the unacknowledged inv → getdata → tx handshake is lost — e.g. the inv is written to a connection that is being torn down (5s incomplete-handshake timeout, ping timeout, invalid-message discard), or the getdata arrives after the connection is gone — nobody ever retries and the transaction is silently lost for the rest of the test.

The test binary runs 18 tests concurrently, each spawning bitcoind/dogecoind daemons, so CI runs see heavy CPU contention that provokes exactly this kind of connection churn and event-loop starvation. The DOGE variant flakes first because dogecoind v1.14.9 is based on the much older Bitcoin Core 0.14 P2P stack, which is slower under load; the BTC variant runs identical test/adapter code and has not been observed failing.

Prior deflake attempts (#9629, #8929, #8559, #7892, #7338, #7105) addressed other root causes (idleness, handshake waits, timeouts) but left the one-shot relay in place, which is why this test keeps recurring.

What

Make the transaction relay robust instead of a silent one-shot: TransactionInfo.advertised now records when the transaction was last advertised to each peer (HashMap<SocketAddr, Instant> instead of HashSet<SocketAddr>), and advertise_txids re-advertises the transaction after TX_READVERTISE_PERIOD_SECS (10s) for as long as it stays in the cache. The timestamps use Instant (a monotonic clock) rather than SystemTime, so wall-clock jumps (NTP corrections, VM clock adjustments) cannot perturb the re-advertisement schedule.

This retries the relay regardless of which message of the inv → getdata → tx exchange was lost — including across connection discard + reconnect. It is also safe on mainnet: peers that already received the transaction simply ignore the repeated inv announcement (a few dozen bytes per peer every 10s per still-cached transaction, worst case).

While touching advertise_txids, also fixed a latent bug in the MAXIMUM_TRANSACTION_PER_INV overflow branch, where a shadowed loop variable broadcast the partially-built inventory of one peer to all peers (currently unreachable: the cache holds at most 250 transactions, far below the 50,000 threshold).

Unit tests: added test_adapter_readvertise_after_period, and adjusted the existing test_adapter_dont_readvertise* tests, which still verify that transactions are not re-advertised within the re-advertisement period.

Verification

• bazel test --test_output=errors --runs_per_test=3 --jobs=3 //rs/bitcoin/adapter:adapter_integration_test — 3 parallel runs (reproducing the CI contention) all pass.
• bazel test //rs/bitcoin/adapter:adapter_test //rs/bitcoin/adapter:adapter_integration_test — pass.

---

This PR was created following the steps in .claude/skills/fix-flaky-tests/SKILL.md.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR deflakes //rs/bitcoin/adapter:adapter_integration_test by making the adapter’s transaction relay resilient to dropped inv → getdata → tx handshakes, via periodic re-advertisement while a transaction remains cached.

Changes:

• Change per-peer transaction advertisement tracking from a one-shot set to a timestamped map, and re-advertise after TX_READVERTISE_PERIOD_SECS (10s).
• Fix an inventory overflow-path bug where a partially-built inv could be broadcast to the wrong peers.
• Update/add unit tests to validate “no re-advertise within period” and “re-advertise after period” behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

rs/bitcoin/adapter/src/transaction_store.rs:348

• The test doc comment has duplicate step numbering ("3." appears twice). This makes the steps harder to follow when reading failures.

    /// This function tests that we don't readvertise transactions that were recently advertised.
    /// Test Steps:
    /// 1. Add transaction to manager.
    /// 2. Advertise that transaction and create requests from peer.
    /// 3. Check that this transaction does not get advertised again during manager tick.
    /// 3. Check transaction advertisement is correctly tracked.

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 61825bc.

Security Overview

• 🔎 Scanned files: 1 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/14c3f6d7-8c8e-4a33-9b18-2f289f25b557?scanId=ad3bb78c-1344-40c8-a40a-a3a1e81158e2&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type

Relevant files

Enhancement

► rs/bitcoin/adapter/src/transaction_store.rs     Add TX_READVERTISE_PERIOD_SECS constant and update re-advertisement logic to use per-peer HashMap of last advertisement times ► rs/bitcoin/adapter/src/transaction_store.rs     Replace advertised from HashSet to HashMap<SocketAddr, Instant> with comments explaining re-advertisement behavior ► rs/bitcoin/adapter/src/transaction_store.rs     Update new() to initialize advertised as HashMap<SocketAddr, Instant> ► rs/bitcoin/adapter/src/transaction_store.rs     Modify advertise_txids to readvertise based on per-peer last advertisement and current time ► rs/bitcoin/adapter/src/transaction_store.rs     Adjust broadcasting logic to send Inv for each address with updated advertising condition ► rs/bitcoin/adapter/src/transaction_store.rs     Update unit tests to reflect readvertising behavior, including a new test for readvertise-after-period