Skip to content

feat: sender-affinity load balancing and direct observer forwarding - #125

Draft
bronxyz wants to merge 4 commits into
feature/txpool-in-flight-tracker-builder-pipelinefrom
feature/txpool-in-flight-tracker-forwarding-affinity
Draft

feat: sender-affinity load balancing and direct observer forwarding#125
bronxyz wants to merge 4 commits into
feature/txpool-in-flight-tracker-builder-pipelinefrom
feature/txpool-in-flight-tracker-forwarding-affinity

Conversation

@bronxyz

@bronxyz bronxyz commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add the SenderAffinityLoadBalancing hardfork (0x0f; Never on real networks, block 0 on local). Under the fork the committee-slot digest keys on the recovered sender, so one validator owns a sender's whole nonce chain instead of consecutive ranges scattering across pools and parking nonce-gapped. CommitteeSlots adds a ring-walk covers() predicate so a down owner's senders fail over to the next live slot, agreed across nodes by the sorted committee order.
  • Forward an observer's transactions directly to the committee: a per-epoch forwarder submits pending transactions by request-response to the validator owning each sender's slot (gossip before the fork), gates re-sends behind the catch-up watermark while first sends still flow, and ring-walks to the next live validator on failover. The ack carries the hashes the owner rejected as nonce-too-low, which are suppressed as acked-stale. An observer no longer runs a batch builder.
  • Stickiness in Observer mode now keys on the operator's --observer flag rather than on the node's current mode, so a staked validator that booted into Observer to catch up can be promoted again; an empty slot table mid-transition drops the message instead of reaching the modulo (division by zero aborts the node).

Stack 6/9 of the txpool in-flight tracker and observer-forwarder series.

Surface areas touched

  • Consensus protocol (primary / worker / network / state-sync)
  • Execution / EVM
  • JSON-RPC (eth_*, rayls_*, faucet)
  • Middleware (orchestrator / processor / bridge)
  • Infrastructure (types / storage / config / network-cli)
  • On-chain contracts (rayls-contracts/)
  • Operations (etc/, scripts, Docker, compose)
  • CI / build (.github/workflows/, Makefile)
  • Documentation only (doc/, in-crate READMEs, root docs)
  • Tests only

Breaking / compatibility

  • Wire: WorkerRequest::SubmitTxns is appended last and WorkerResponse::SubmitTxns after Error, so every existing variant keeps its bcs index; an un-upgraded peer answers the new request with Error and the sender retries the next validator. WorkerGossip::Txn moves to Vec<Bytes>, which bcs-encodes identically to Vec<Vec<u8>>.
  • Hardfork: SenderAffinityLoadBalancing is Never on every real network until an activation block is scheduled. The fork gate reads the local canonical tip, so validators can briefly disagree across the fork block; the worst case is a duplicate include that fails nonce-too-low at execution, never a consensus fork.
  • Behavior: observers stop producing batches and forward instead. Inbound submit fan-out is bounded by a semaphore.

Test plan

  • bcs-index pins: Error stays variant index 3 on WorkerResponse and round-trips as itself; SubmitTxns lands at index 4 (response) and 3 (request).
  • save_mark_backup / load_mark_backup round trip for forwarding marks, and the pin that a sealing snapshot writes no file.
  • Forwarder unit tests: is_caught_up_gates_on_local_lag, should_send_flows_first_sends_but_gates_resends, validate_stale_keeps_only_hashes_this_node_sent.
  • The observer-stickiness and zero-slot guards need a mid-transition committee fault to demonstrate red and land as hardening without failing-first tests.
  • make check on the stack tip; CI on this branch.

@bronxyz bronxyz changed the title feature/txpool in flight tracker forwarding affinity feat: sender-affinity load balancing and direct observer forwarding Aug 20, 2026
@bronxyz
bronxyz force-pushed the feature/txpool-in-flight-tracker-forwarding-affinity branch from d2d1908 to cdfc97c Compare August 24, 2026 11:30
@github-actions

Copy link
Copy Markdown
Contributor

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

@raylsnetwork raylsnetwork deleted a comment from github-actions Bot Aug 24, 2026
- add the SenderAffinityLoadBalancing hardfork, Never on real networks and Block(0) on local, layered above TransactionLoadBalancing
- key the committee-slot digest on the recovered sender under the fork, so one validator owns a sender's whole nonce chain instead of consecutive ranges scattering across pools and parking nonce-gapped
- add CommitteeSlots with a ring-walk covers() predicate so a down owner's senders fail over to the next live slot, agreed across nodes by the sorted committee order
- the gossip handler builds per-slot liveness from connected peers (own slot always live) and dispatches batches through covers()

- the fork gate reads the local canonical tip, so validators can briefly disagree across the fork block; worst case is a duplicate include that fails nonce-too-low at execution, never a consensus fork
- on local the fork's version byte (0x0f) outranks HybridRewards from block 0; real networks are unaffected since the fork stays Never
- add a per-epoch observer forwarder that submits pending transactions by request-response to the validator owning each sender's slot, falling back to gossip before the fork
- append WorkerRequest::SubmitTxns last and WorkerResponse::SubmitTxns after Error so every existing variant keeps its bcs index across a rolling upgrade; the ack carries the hashes the owner rejected as nonce-too-low
- share one node-scoped in-flight tracker between the pool and its role via init_txn_pool_with_in_flight, bringing the dormant forwarding marks alive
- an observer is no longer batch-producing: it forwards instead of running a batch builder that disburses
- gate re-sends behind the catch-up watermark while first sends of new transactions still flow, and ring-walk to the next live validator on failover, marking accepted hashes and suppressing acked-stale ones
- change WorkerGossip::Txn, publish_txn, and submit_batch_if_mine to Vec<Bytes> (bcs-identical to Vec<Vec<u8>>) so a payload is encoded once for both the direct and gossip paths
- move fxhash_slot_digest and legacy_slot_digest to the types crate so the forwarder and validator compute the identical owner slot

- bound inbound submit fan-out with a semaphore, shedding via the existing Error variant so the sender retries the next validator
- assert save_mark_backup persists a forwarding snapshot and load_mark_backup reloads it on a fresh pool, coming live once forwarding re-arms
- assert a sealing snapshot writes no file, guarding the contract that restoring sealing marks would wedge the builder
- the mode-write skip treated ANY node currently in Observer mode as sticky: a staked
  validator that merely booted into Observer (catching up) could then never be promoted
  back to the committee; stickiness now keys on the operator's --observer flag
  (is_observer_sticky), the contract the skip was meant to honor
- an empty committee-slot table (a gap mid-transition) reached the dispatch modulo, and
  division by zero aborts the node - a gossip message must never be able to do that;
  submit_batch_if_mine now drops the message instead

Hardening: both paths need a mid-transition committee fault to demonstrate red, so this
lands without failing-first tests.
@bronxyz
bronxyz force-pushed the feature/txpool-in-flight-tracker-forwarding-affinity branch from cdfc97c to 2661bdd Compare August 26, 2026 11:43
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bronxyz's task in 3m 13s —— View job


PR Review: feat: sender-affinity load balancing and direct observer forwarding

  • Gathering context and reading repository guidelines
  • Analyzing commits and diff since base branch (4 commits, 24 files)
  • Reviewing key changed files
  • Checking status of prior review findings
  • Posting final review feedback

Overview

This review covers all 4 commits on the branch. The latest two commits (test: cover the forwarding mark backup restart round trip and fix: let a non-configured observer promote and guard the slot modulo) addressed items noted in the prior review. Both are solid. The issues below are those that remain open from the prior pass, with their current file locations confirmed.


Issues Remaining From Prior Review

Bug — ok_or_eyre literal string (both call sites, still unfixed)

File: crates/middleware/orchestrator/src/engine/node_inner.rs:169 and :224

// line 169 (start_batch_builder)
let transaction_pool = self
    .workers
    .get(worker_id as usize)
    .ok_or_eyre("worker components missing for {worker_id}")?
    .pool();

// line 224 (start_txn_forwarder) — same pattern
let transaction_pool = self
    .workers
    .get(worker_id as usize)
    .ok_or_eyre("worker components missing for {worker_id}")?
    .pool();

ok_or_eyre is not a macro — it takes a Display value, so {worker_id} is emitted as a literal brace pair rather than the actual ID. Both sites need:

.ok_or_else(|| eyre::eyre!("worker components missing for worker {worker_id}"))?

Fix this →


Potential DoS — connected_peers() on every gossip transaction (still unfixed)

File: crates/consensus/worker/src/network/handler.rs:111–116

WorkerGossip::Txn(tx_bytes) => {
    ...
    let connected = self
        .network_handle
        .inner_handle()
        .connected_peers()
        .await
        .unwrap_or_default();

Every inbound WorkerGossip::Txn message performs a full async round-trip to the network task to fetch connected peers. Under a high-throughput gossip burst each message blocks on that round-trip, serializing handler work through the network actor. TxnForwarder::submit_to_committee (txn_forwarder.rs:347-348) correctly fetches connected peers once per tick and passes the snapshot in — the gossip path should adopt the same pattern, either by caching with a short TTL or by injecting the snapshot into the call.


Wire-safety — Vec<Bytes> BCS identity still comment-only (still unfixed)

File: crates/consensus/worker/src/network/message.rs:14–16

The wire-compatibility claim in the doc comment (Vec<Bytes> bcs-identical to Vec<Vec<u8>>) remains uncheckable by the test suite. The file now has BCS pin tests for WorkerResponse::Error and WorkerRequest::SubmitTxns, but not for WorkerGossip::Txn. A minimal test would be:

#[test]
fn worker_gossip_txn_vec_bytes_is_bcs_identical_to_vec_vec_u8() {
    let payloads = vec![vec![1u8, 2, 3], vec![4u8, 5]];
    let as_bytes: Vec<Bytes> = payloads.iter().map(|v| Bytes::copy_from_slice(v)).collect();
    let old_enc = encode(&WorkerGossip::Txn(payloads.clone().into_iter().map(Into::into).collect::<Vec<Vec<u8>>>().into()));
    let new_enc = encode(&WorkerGossip::Txn(as_bytes));
    assert_eq!(old_enc, new_enc, "Vec<Bytes> must be BCS-identical to Vec<Vec<u8>>");
}

Fix this →


Minor — connected.contains O(n) inside ring-walk (still unfixed)

File: crates/middleware/orchestrator/src/engine/txn_forwarder.rs:397–400

for slot in ring_walk(owner, self.committee.len() as u64) {
    let peer = self.committee[slot as usize];
    if !connected.contains(&peer) {
        continue;
    }

connected: &[BlsPublicKey] is a slice, so .contains is O(n). Over a full ring walk this is O(committee²) in the worst case. Converting connected to a HashSet at the call site in submit_to_committee before the join_all loop would make the intent explicit.


Minor — slot_digest silent fallback undocumented (still unfixed)

File: crates/consensus/worker/src/batch-validator/src/validator.rs:298–301

if let Ok(pooled) = recover_pooled_transaction(tx) {
    return fxhash_slot_digest(pooled.sender().as_slice());
}
fxhash_slot_digest(tx)   // <-- falls back to raw-byte hash

When sender-affinity is active but signer recovery fails, slot_digest silently falls back to fxhash_slot_digest(tx). Two transactions from the same sender whose encodings differ (e.g. different gas tips) then hash to different slots, breaking nonce-chain colocation. The happy path is unaffected (malformed transactions never pass the 8-byte gate in practice), but the fallback is undocumented. A brief comment explaining why the fallback produces a deterministic slot rather than returning an error would help future readers.


Minor — is_observer_sticky still has no unit test (still unfixed)

File: crates/middleware/orchestrator/src/epoch_manager/network.rs:71–77

is_observer_sticky was extracted in the latest fix commit but no #[test] block was added alongside it. The function is pure and its three semantically distinct cases (configured observer, unconfigured node currently in Observer, target already Observer) map cleanly to table-driven tests.

Fix this →


Minor — chunk_under_budget budget-split path remains untested (still unfixed)

File: crates/middleware/orchestrator/src/engine/txn_forwarder.rs:570–575

The in-code comment still acknowledges the gap:

// only the empty case is covered: a count split needs a real pooled transaction fixture

The byte-budget split is the more important correctness property. Even a synthetic test with fixed-size stub payloads (a struct implementing the needed interface) would be better than the empty-case-only coverage that exists today.


New Observation

Minor — recover_forwarded_txns decode failures are silent

File: crates/consensus/worker/src/batch-validator/src/validator.rs:29–33

fn recover_forwarded_txns<T: AsRef<[u8]> + Sync>(txs_bytes: &[T]) -> Vec<EthPooledTransaction> {
    if txs_bytes.len() < PARALLEL_PARSE_THRESHOLD {
        txs_bytes.iter().filter_map(|bytes| bytes_to_txn(bytes.as_ref()).ok()).collect()
    } else {
        txs_bytes.par_iter().filter_map(|bytes| bytes_to_txn(bytes.as_ref()).ok()).collect()
    }
}

Decode failures are silently dropped via .ok(). For a validator receiving observer-forwarded transactions this is the correct behavior (don't pool garbage), but there is no metric or debug! log emitted for decode failures. If an observer bug starts sending malformed encodings, this path provides no signal: the validator silently accepts fewer transactions than expected, which would be difficult to distinguish from an empty forwarding tick. A debug-level counter or a single debug! on the first failure per call would make this diagnosable.


What the Latest Two Commits Address

  • test: cover the forwarding mark backup restart round tripforwarding_marks_survive_the_backup_file_round_trip in txn_pool/backup.rs is well-structured and correctly exercises the sealing-suppression asymmetry.
  • fix: let a non-configured observer promote and guard the slot modulodecide_node_mode, is_observer_sticky, and the if slots.size() == 0 { return Ok(()); } guard in submit_batch_if_mine all look correct. The empty-slot-table guard (:129–131) is new and directly fixes the abort-on-gossip path.

Positive Highlights

  • CommitteeSlots tests (sealed_batch.rs:380–438) — ring-walk, down-owner failover, wrap-around, all-dead, and connectivity-based construction are all verified. This is thorough.
  • validate_stale peer-suppression guard — correctly filters the peer's stale claim against the locally-sent set.
  • MAX_CONCURRENT_SUBMIT_TXNS = 16 semaphore with shed — shed returns Error (no new wire variant), keeping a flooded committee member responsive.
  • is_caught_up epoch-reset guardseen >= anchor correctly handles the watch-channel reset at epoch boundaries.
  • BCS variant-index pin tests (worker_response_error_keeps_bcs_index_3, worker_request_submit_txns_is_appended_last) — lock in the wire property that must not regress.
  • SenderAffinityLoadBalancing ships Never on all production networks — dormant until explicitly scheduled.

@raylsnetwork raylsnetwork deleted a comment from github-actions Bot Aug 26, 2026
bronxyz added a commit that referenced this pull request Aug 26, 2026
- `ok_or_eyre` takes a `Display` value, so the braces were emitted literally

- addresses the PR #125 (feature/txpool-in-flight-tracker-forwarding-affinity) review:
  `ok_or_eyre` literal at both `node_inner.rs` sites
bronxyz added a commit that referenced this pull request Aug 26, 2026
- the `Vec<Bytes>` identity with `Vec<Vec<u8>>` was asserted only in a comment; the pin
  encodes both and cross-decodes the old bytes into the new variant

- addresses the PR #125 (feature/txpool-in-flight-tracker-forwarding-affinity) review: the
  `Vec<Bytes>` wire identity was comment-only
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