Overlay v2 mempool fix

docs/RUST_OVERLAY_DESIGN.md

@@ -97,9 +97,16 @@ dedicated doc under [`docs/rust-overlay/`](rust-overlay/):
   announcements and per-peer/total timeouts. See
   [scp-flooding.md](rust-overlay/scp-flooding.md) and
   [tx-propagation.md](rust-overlay/tx-propagation.md).
-- **Mempool lives in the overlay**. Fee-ordered, capacity 100,000,
-  300-second max age. Core queries it for nomination via `GetTopTxs`.
-  See [mempool.md](rust-overlay/mempool.md).
+- **Mempool lives in the overlay**. Fee-ordered per tx set phase
+  (classic / Soroban), capacity 100,000, 300-second max age. Core pulls
+  twice what fits in the next ledger (plus one), per phase, via
+  `GetTopTxs`, and builds the nominated set with lazy validation: only
+  candidates about to be included are validated, unreached candidates
+  stay in the mempool. See [mempool.md](rust-overlay/mempool.md).
+- **TX set fetches are retried**. A fetch moves to another peer on
+  `DontHave`, disconnect, or a 2 s silence, and is abandoned only once
+  its slot has aged out of every peer's cache. See
+  [txset-fetching.md](rust-overlay/txset-fetching.md).
 - **Backpressure asymmetry**. SCP and TxSet events to Core are on an
   unbounded channel and never drop. TX events are on a bounded channel
   (10,000) and may drop under load — TXs are re-fetchable via the same
@@ -169,6 +176,7 @@ overlay/
 │   │   ├── inv_batcher.rs   # Per-peer INV batching
 │   │   ├── inv_tracker.rs   # Peer→TX advertisement tracking
 │   │   ├── pending_requests.rs  # GETDATA timeout/retry
+│   │   ├── txset_fetch.rs   # TX set fetch retry bookkeeping
 │   │   └── tx_buffer.rs     # TX storage for GETDATA responses
 │   └── http/
 │       └── mod.rs           # HTTP server (TX submission, status)
@@ -186,9 +194,6 @@ relevant subsystem doc.
   (`wire.rs`), so fee ordering and outbound INV `fee_per_op` are
   correct, but source account and sequence number are not tracked and
   per-account queries are not possible.
-- **TX-set fetch retry is commented out** (`libp2p_overlay.rs:1829-1916`).
-  A pending fetch to a silent peer leaks until the peer disconnects;
-  Core's own retry policy is the only safety net.
 - **The mempool size (100,000 entries) and the peer/connection fanout
   are hardcoded** — there are no config knobs for them, and the overlay
   does not enforce inbound or outbound connection limits.

docs/metrics.md

@@ -63,6 +63,7 @@ crypto.verify.total                       | meter     | sum of both hits and mis
 crypto.verify.tx-valid-hit                | meter     | signature cache hits that occurred while validating transactions (outside of background signature validation)
 crypto.verify.tx-valid-total              | meter     | sum of both hits and misses during transaction validation (outside of background signature validation)
 herder.txset.validate                     | timer     | time spent turning a received tx set into an applicable tx set and validating it on a validity-cache miss
+herder.txset.candidates-validated         | counter   | candidate transactions validated while building a tx set for nomination (with lazy validation: the included ones plus the excess-demand probes)
 herder.pending[-soroban]-txs.age0         | counter   | number of gen0 pending transactions
 herder.pending[-soroban]-txs.age1         | counter   | number of gen1 pending transactions
 herder.pending[-soroban]-txs.age2         | counter   | number of gen2 pending transactions
@@ -130,6 +131,9 @@ overlay.delay.write-queue                 | timer     | time between each messag
 overlay.error.read                        | meter     | error while receiving a message
 overlay.error.write                       | meter     | error while sending a message
 overlay.fetch.txset                       | timer     | time to complete fetching of a txset
+overlay.fetch.txset-request               | meter     | tx set fetch requests core issued to the overlay (first request or safety-net re-request)
+overlay.fetch.txset-retry                 | meter     | tx set fetches the overlay re-dispatched to another peer (timeout, DontHave, or disconnect)
+overlay.fetch.txset-dont-have             | meter     | DontHave answers received for a tx set we were waiting on from that peer
 overlay.fetch.qset                        | timer     | time to complete fetching of a qset
 overlay.fetch.unique-recv                 | meter     | number of bytes of fetched messages that have not yet been received
 overlay.fetch.duplicate-recv              | meter     | number of bytes of fetched messages that have already been received

docs/rust-overlay/ipc.md

@@ -35,8 +35,17 @@ Source:
 | `length` | 4     | Payload length in bytes. Native-endian.              |
 | `payload`| `length` | Type-specific bytes. May be empty.                |
 
-Maximum payload: **16 MB** (`messages.rs:154`). Frames larger than that
-are rejected with `InvalidData`.
+Maximum payload: **256 MiB** (`MAX_PAYLOAD_SIZE` in `messages.rs`,
+`IPC_MAX_PAYLOAD_SIZE` in `src/overlay/IPC.h`). This is a corruption guard
+for a trusted local channel, not a resource budget: if the stream ever
+desynchronised, the next "length" would be garbage, and an unbounded
+reader would allocate gigabytes and then wait forever for bytes that
+never come. The limit sits far above anything a well-formed message
+reaches; the size of the top-txs reply is governed by Core's per-phase
+pull budgets (see [mempool.md](mempool.md)), never by this limit. A frame
+announcing a larger payload is drained and reported as truncated on both
+sides (the connection survives; the message is dropped), and the overlay
+clamps `TOP_TXS_RESPONSE` to the limit as a backstop.
 
 > Native-endian is unusual for a wire protocol but safe here because
 > Core and Overlay always run on the same host. The libp2p network
@@ -55,14 +64,14 @@ are rejected with `InvalidData`.
 | ID | Name                  | Payload                                      | Purpose                                          |
 |---:|-----------------------|----------------------------------------------|--------------------------------------------------|
 |  1 | `BroadcastScp`        | `[scp_envelope]`                             | Flood SCP envelope to peers                       |
-|  2 | `GetTopTxs`           | `[count:u32]`                                | Get top-N TXs by fee for nomination               |
+|  2 | `GetTopTxs`           | `[classicCount:u32][classicBytes:u32][sorobanCount:u32][sorobanBytes:u32]` | Get the top TXs by fee for nomination, one (count, bytes) budget per tx set phase |
 |  3 | `RequestScpState`     | `[ledger_seq:u32]`                           | Ask peers for SCP state at a ledger seq           |
 |  4 | `LedgerClosed`        | `[ledger_seq:u32][ledger_hash:32]`           | Notify ledger advancement; triggers cache eviction |
 |  5 | `TxSetExternalized`   | `[txset_hash:32][num:u32][tx_hash:32]…`      | TX set was applied; remove TXs from mempool       |
 |  6 | `ScpStateResponse`    | `[count:u32][env_len:u32][env]…`             | SCP state for a peer that asked us                |
 |  7 | `Shutdown`            | (empty)                                      | Graceful shutdown                                 |
 |  8 | `SetPeerConfig`       | UTF-8 JSON (see below)                       | Configure peer addresses                          |
-| 10 | `SubmitTx`            | `[fee:i64 LE][num_ops:u32 LE][tx_xdr]`       | Submit TX for flood + mempool                     |
+| 10 | `SubmitTx`            | `[fee:i64 LE][num_ops:u32 LE][flags:u32 LE][tx_xdr]` | Submit TX for flood + mempool (flags bit 0 = Soroban) |
 | 11 | `RequestTxSet`        | `[hash:32]`                                  | Fetch TX set body by hash                         |
 | 12 | `CacheTxSet`          | `[hash:32][txset_xdr]`                       | Tell overlay to cache a locally-built TX set      |
 | 13 | `RequestOverlayMetrics` | (empty)                                    | Request metrics snapshot                          |
@@ -82,8 +91,8 @@ are rejected with `InvalidData`.
 
 - **Endianness inside the payload**: the *frame* header is native-endian,
   but several payloads use explicit little- or big-endian fields:
-  - `SubmitTx`: `fee` and `num_ops` are little-endian
-    (`main.rs:1102-1103`).
+  - `SubmitTx`: `fee`, `num_ops` and `flags` are little-endian.
+  - `GetTopTxs`: all four budget fields are little-endian.
   - `LedgerClosed`, `RequestScpState`, `TxSetExternalized` ledger seq
     fields: little-endian (`main.rs:1121, 1143, 1172`).
   - `RequestScpState` peer-side wire frame on `/stellar/scp/1.0.0`:
@@ -130,9 +139,12 @@ Two consequences:
 
 - **Socket closed** (Core process exits): the receiver returns `None`,
   the overlay's main loop exits. The overlay process terminates.
-- **Frame parse error** (e.g. bad magic, oversized length): connection
-  is treated as broken; depending on which side detected it, either
-  process logs the error and shuts down. There is no resync.
+- **Oversized frame**: drained and ignored, see above; the connection
+  survives.
+- **Frame parse error** (e.g. unknown message type, connection lost
+  mid-frame): connection is treated as broken; depending on which side
+  detected it, either process logs the error and shuts down. There is
+  no resync.
 - **Unknown message type**: returns `InvalidData`. Adding a new message
   type requires updating `MessageType::try_from` (in `messages.rs`).
 

docs/rust-overlay/mempool.md

@@ -5,6 +5,11 @@ transactions. Core queries it for nomination (`GetTopTxs`), and the
 overlay services TX-flood requests from it (`tx_buffer` is *not* the
 mempool — see [tx-propagation.md](tx-propagation.md)).
 
+The mempool does no transaction validation: Core validates the
+transactions it pulls at nomination, and pulls only as many as fit in the
+next ledger (see [Querying](#querying)) so that work is not spent on
+transactions that cannot make it into the block.
+
 Source: `flood/mempool.rs`. Owned by `Overlay` in `integrated.rs`,
 guarded by `RwLock`.
 
@@ -28,14 +33,19 @@ Mempool::new(100000, Duration::from_secs(300))
 | Index         | Type                                  | Purpose                                  |
 |---------------|---------------------------------------|------------------------------------------|
 | `by_hash`     | `HashMap<TxHash, MempoolEntry>`       | O(1) lookup, dedup                       |
-| `by_fee`      | `BTreeSet<FeePriority>`               | Ordered access (for `top_by_fee`)        |
+| `by_fee`      | `[BTreeSet<FeePriority>; 2]`, one per `TxKind` | Ordered access per tx set phase (for `top_by_fee`) |
+
+`MempoolEntry` (`flood/mempool.rs`) is an `Arc<ValidatedTx>` plus
+`received_at` (for age-based eviction). The `ValidatedTx` (`wire.rs`)
+carries the canonical envelope bytes, sha256 hash, fee, op count and
+kind (classic or Soroban) — computed once at the trust boundary where
+the transaction entered the process (or supplied by Core for `SubmitTx`),
+and shared by reference through the rest of the pipeline.
 
-`MempoolEntry` (`flood/mempool.rs:20-24`) is an
-`Arc<ValidatedTx>` plus `received_at` (for age-based eviction). The
-`ValidatedTx` (`wire.rs`) carries the canonical envelope bytes, sha256
-hash, fee, and op count — computed once at the trust boundary where the
-transaction entered the process, and shared by reference through the
-rest of the pipeline.
+Classic and Soroban transactions are indexed separately because Core
+fills the two tx set phases independently, with independent limits: a
+Soroban transaction's (much larger) fee must not crowd classic
+transactions out of a pull, and vice versa.
 
 ## Fee ordering
 
@@ -56,19 +66,20 @@ comparison (deterministic).
 
 ## Insertion
 
-`Mempool::insert` (`flood/mempool.rs:106-127`):
+`Mempool::insert` (`flood/mempool.rs`):
 
 1. **Dedup**: if `by_hash` already contains this hash, return `false`.
 2. **Capacity check**: while at `max_size`, call `evict_lowest_fee`.
-3. Add to both indexes.
+3. Add to `by_hash` and to the `by_fee` index for the tx's kind.
 4. Return `true`.
 
+
 ## Eviction
 
-- **Capacity-based** (`evict_lowest_fee`, `flood/mempool.rs:179-184`):
-  takes the last entry of `by_fee` (lowest priority) and removes it via
-  `remove`. Called from `insert` on capacity overflow — happens
-  *synchronously per insert*.
+- **Capacity-based** (`evict_lowest_fee`, `flood/mempool.rs`): compares
+  the lowest-priority entry of each kind's `by_fee` index and removes the
+  lower of the two via `remove`. Called from `insert` on capacity
+  overflow — happens *synchronously per insert*.
 - **Age-based** (`evict_expired`, `flood/mempool.rs:152-166`): scans for
   entries with `now - received_at > max_age` and removes them. Called
   only from the `RemoveTxsFromMempool` handler (i.e. piggybacked on
@@ -87,33 +98,75 @@ hash from both indexes, then runs `evict_expired`.
 ## Insertion sources
 
 Two paths into the mempool, one per trust boundary. Both produce the
-same `Arc<ValidatedTx>` currency, so every entry carries its real fee
-and op count and fee ordering is correct regardless of origin.
+same `Arc<ValidatedTx>` currency, so every entry carries its real fee,
+op count and kind, and fee ordering is correct regardless of origin.
 
-### `SubmitTx` from Core (`main.rs:1095`)
+### `SubmitTx` from Core (`main.rs`)
 
-Core submits `(data, fee, num_ops)` over IPC; the handler mints the
-entry with `ValidatedTx::from_core_trusted`, which trusts Core's
-metadata and does not re-decode (it only rejects fee-bumps via the
-envelope discriminant and hashes the bytes).
+Core submits `(data, fee, num_ops, flags)` over IPC; the handler mints the
+entry with `ValidatedTx::from_core_trusted`, which trusts Core's metadata
+(including the classic/Soroban kind in `flags`) and does not re-decode
+(it only rejects fee-bumps via the envelope discriminant and hashes the
+bytes).
 
-### TX received over the network (`main.rs:727-734`)
+### TX received over the network (`main.rs`)
 
 `LibP2pOverlayEvent::TxReceived` already carries an `Arc<ValidatedTx>`:
 the per-peer TX stream reader minted it with `ValidatedTx::from_network`
 during its single strict decode of the inbound message
-(`flood/inv_messages.rs`), reading fee/op metadata off the decoded
-envelope. The handler just forwards it to `submit_tx`.
+(`flood/inv_messages.rs`), reading fee/op metadata and the kind (from the
+envelope's Soroban `ext`) off the decoded envelope. The handler just
+forwards it to `submit_tx`.
 
 ## Querying
 
-- `Mempool::top_by_fee(count)`: returns up to `count` hashes from
-  `by_fee` in priority order. Used by `GetTopTxs` for nomination.
+- `Mempool::top_by_fee(kind, budget)`: walks that kind's `by_fee` index
+  in priority order and returns up to `budget.max_count` transactions
+  whose envelope bytes total at most `budget.max_bytes`. A transaction
+  that would overflow the byte budget is skipped and the walk continues
+  with smaller ones (mirroring Core's surge pricing). Used by
+  `GetTopTxs` for nomination.
 - `Mempool::contains(hash)`: O(1) dedup check.
 - `Mempool::get(hash)`: returns `Option<&Arc<ValidatedTx>>`.
 
-`GetTopTxs` (`integrated.rs:79-87`) returns `(hash, data)` pairs to
-Core for inclusion in a TX set.
+`GetTopTxs` (`integrated.rs`) carries a `TopTxsRequest` — one
+`(max_count, max_bytes)` budget for the classic phase and one for the
+Soroban phase. Core deliberately asks for **more than fits**, by count
+and by bytes alike (`HerderImpl::mempoolPullBudget`): each budget is
+`(what fits + one transaction) × MEMPOOL_PULL_MULTIPLIER` (currently 2;
+`HerderImpl.h`). For counts, "what fits" is `maxTxSetSizeOps` (classic)
+or `ledgerMaxTxCount` (Soroban) and one transaction is 1. For bytes,
+"what fits" is the classic byte allowance or the Soroban ledger tx byte
+limit, and one transaction is the maximum transaction size of that
+phase. The two dimensions get the same slack on purpose: whichever one
+binds first decides how much surplus Core actually receives, and a byte
+budget of just "one ledger plus one transaction" would let a few large
+invalid transactions at the top of the fee order leave a byte-bound
+ledger short. The reply lists classic transactions first, then Soroban,
+each highest fee first; it is sized by these budgets, and the IPC frame
+limit (256 MiB) sits far above them as a corruption guard.
+
+Why over-pull: the mempool does no stateful validation, so its top
+entries can be unusable (stale sequence number, drained account, bad
+signature). An exact-fit pull would then produce a partially empty
+ledger even with plenty of valid demand behind it. Over-pulling is cheap
+because Core builds the nominated set with **lazy validation**
+(`LazyTxValidator`, `TxSetFrame.cpp`; this is the only way tx sets are built): surge pricing walks the
+candidates in fee order and validates one only when it is about to be
+included, plus at most one non-fitting candidate per fee lane (that
+probe is what tells surge pricing there was excess demand, and it has to
+be a valid transaction, otherwise an invalid one could raise everyone's
+base fee). Candidates that surge pricing never reaches are neither
+validated nor reported, so Core leaves them in the mempool for the next
+ledger. Only candidates that were validated and failed are removed from
+the mempool (except those with a future sequence number, which are
+chained behind a pending transaction). The metric
+`herder.txset.candidates-validated` counts lazy validations; per ledger
+it should stay close to the number of included transactions.
+
+The "+1" matters on its own: surge pricing raises the base fee only when
+it sees a valid transaction that does not fit, so a pull with no slack
+would hide excess demand and keep fees at the minimum under overload.
 
 ## Known gaps