[bitcoind_rpc] Plans to stabilize the Emitter API?

This issue tracks the work items identified in a full review of lib.rs and test_emitter.rs on master (de7a89fd), separated into 3 broad categories: 1) Internal fixes, 2) API + Documentation, 3) Test coverage. ### 1. Bugs / Correctness - [ ] **`mempool_at` consistency loop has no backoff and no retry cap** The inner `loop` in `mempool_at` calls 5 RPCs per iteration and retries immediately with no sleep, no ceiling on retries, and the `HashSet<Txid>` allocation is repeated on every failed iteration. - [ ] **`let mut` declarations before the loop** This pattern is legal but unusual Rust. The loop is really "retry until consistent snapshot"— an `fn fetch_consistent_mempool_snapshot()` that returns a named struct would be cleaner. - [ ] **`AgreementPointNotFound` arm creates an unprotected infinite loop** `poll` responds to `AgreementPointNotFound` by resetting to genesis and looping. If genesis is somehow inaccessible or the node has a persistent inconsistency, this spins forever. Although it's a pathological case, the recommendation would be to return a hard error after a bounded number of consecutive failures. - [ ] **`connected_to()` returns itself when checkpoint has no parent** The fallback `None => self.checkpoint.block_id()` (returns itself) is unreachable via the emitter in practice, since every emitted checkpoint gets pushed onto the last cp. For correctness, should use `(block_height - 1, block.header.prev_blockhash)` directly which is consistent with the intended block-by-block behavior. --- ### 2. API Design + Documentation - [ ] **Documentation for `mempool` refers to first-seen unix timestamps** `MempoolEvent::update` originally documented the timestamp as a "first-seen unix timestamp". For transactions in `mempool_snapshot`, the current `sync_time` is used on every poll — not the time the tx was first observed. - [ ] **Confusing `start_height` semantics** `start_height` was intended as a birthday/scan-start height: "don't emit blocks before this height." PR [#2167](https://github.com/bitcoindevkit/bdk/pull/2167) exposed a bug where, after a reorg that drops the agreement point below `start_height`, the emitter skips directly to `start_height` rather than re-emitting the invalidated heights. The proposed fix modifies `start_height` internally at runtime to the agreement height, circumventing the stated purpose of the parameter and making the behavior difficult to explain. Two options: 1. Remove `start_height` entirely. The birthday/scan-start is encoded directly in `last_cp`: callers build a checkpoint containing the birthday height. The emitter scans from there naturally. 2. Add an alternate constructor that does not take `start_height` and relies solely on `last_cp` for the start height, keeping the existing constructor for backward compatibility. - [ ] **`mempool_at` doc should carry the canonical eviction guard explanation** `mempool_at` contains the actual `at_tip` check and eviction logic, but its doc just says _"no-std version of `mempool`"_. The eviction-guard semantics (withheld until emitter reaches tip, rationale) belong on `mempool_at`, with `mempool()` cross-referencing it. - [ ] **`NO_EXPECTED_MEMPOOL_TXS` constant adds friction, not clarity** The constant is a typed alias for `core::iter::empty()` hardcoded to `T = Arc<Transaction>`. This causes type-inference friction when callers pass `Transaction` directly. - [ ] **`BlockEvent<B>` generic is vestigial** `BlockEvent<B>` exists solely to support the private `poll<C, V, F>` closure abstraction. `next_block` is the only public consumer and always returns `BlockEvent<Block>`. - [ ] **`poll` / `poll_once` are undocumented** These are the algorithmic heart of the emitter. Working branch adds state-machine docs explaining entry invariants, what each `PollResponse` variant means, and the reorg-detection flow. - [ ] **CheckPoint should be generalized for any data type** #2105 - [ ] **Move away from outdated `bitcoincore_rpc::Client`** #2119 --- ### 3. Test Coverage - [ ] **Stale `mempool_avoids_re_emission` doc comment** The test `mempool_avoids_re_emission` has a doc comment that says "mempool txs should not be re-emitted" but its own assertions loop twice and assert that the same txids appear on both calls. Should be renamed to something like mempool_update_is_full_snapshot and the doc rewritten to describe what it actually tests: "every call to mempool() returns all currently-known unconfirmed transactions, including ones returned on previous calls." - [ ] **Stale `test_into_tx_graph` doc comment** The comment references `EmittedUpdate::into_tx_graph_update` — a deleted API. The test body is fine. Comment should be rewritten to describe what the test actually covers. Additionally `addr_1` and `addr_2` are generated and tracked but never funded, adding noise without coverage. - [ ] **Stale open design question as TODO in test doc** `ensure_block_emitted_after_reorg_is_at_reorg_height` contains `TODO: If the reorg height is lower than the fallback height...` — this is not a test concern, it is a design question. Should be removed from the test and tracked as a separate issue. - [ ] **Behind-tip eviction guard is untested** `test_expect_tx_evicted` tests eviction only when already at tip. The complementary scenario — evict a tx while the emitter is still behind tip, assert `evicted` is empty in that window, advance to tip, then assert the eviction is reported — is untested. - [ ] **`expected_mempool_txs` eviction path is not end-to-end tested** The unit test `test_expected_mempool_txids_accumulate_and_remove` verifies accumulation and removal on confirmation, but there is no test that: (1) passes a known-unconfirmed tx to `Emitter::new` via `expected_mempool_txs`, (2) evicts that tx from the node's mempool, and (3) asserts it appears in `MempoolEvent::evicted`. - [ ] **Missing test coverage for `start_height`** No test verifies the fundamental birthday guarantee: an emitter with `last_cp` at height N must not emit any block at or below N. - [ ] **Missing test coverage for `BlockEvent::connected_to()`** - [ ] **`mempool_at` is never called in integration tests** `mempool_at` is never called in integration tests to verify that each mempool tx carries the custom `sync_time` passed into the API.