fix(message_sender): validate outbound messages statelessly

[shane-moore]

Problem, Evidence, and Context (Required)

NetworkMessageSender::do_send reused the inbound Validator, so outbound self-validation mutated shared duty and duplicate state before slot routing and network enqueue. If routing or enqueue then failed, a later re-emission could be suppressed even though the original message never reached the wire.

Closes #1192.

Change Overview (Required)

• Add validate_outbound, a stateless preflight that decodes the envelope, checks SignedSSVMessage structural invariants, parses the role, and returns the nested routing slot.
• Remove the stateful Validator and DutiesProvider coupling from NetworkMessageSender.
• Keep incoming validation behavior and pre-decode versus post-decode classification unchanged.
• Deliberately leave network, duty, timing, fork-role, signature-verification, and validation-state checks to outbound producers and the incoming validator.

Risks, Trade-offs, and Mitigations (Required)

• A semantically invalid but structurally valid locally produced message is no longer stopped by sender-side inbound validation. Peers still run the full incoming validator.
• The known pre-Boole contribution packaging bug in fix: batch pre-Boole sync contribution proofs #1052 can therefore reach the wire. Any build containing this change that is deployed while the active SSV fork is Alan must also include fix: batch pre-Boole sync contribution proofs #1052 or an equivalent fix. If every target is already Boole, fix: batch pre-Boole sync contribution proofs #1052 is not required.
• Structurally malformed messages are still dropped before routing or enqueue.

Validation (Required)

• cargo test -p message_validator -p message_sender, 112 validator tests passed.
• cargo check -p client.
• make cargo-fmt-check.
• make lint, full workspace clippy with warnings denied.

Rollback (Required for behavior or runtime changes; optional otherwise)

Revert the commit. There are no data migrations, configuration changes, or persistent state changes.

Blockers / Dependencies (Optional)

#1052, or an equivalent fix, is conditionally required only for deployments whose active SSV fork is still Alan.

[shane-moore]

@claude, please take a look

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 3m 52s —— View job

---

Review: fix(message_sender): validate outbound messages statelessly

• Read changed files and diff against origin/epbs
• Analyze validate_outbound and structural checks
• Review NetworkMessageSender changes
• Assess correctness, safety, and risk trade-offs
• Post review feedback

---

Overall this is a solid, well-motivated fix. Removing the shared, stateful Validator from the outbound path eliminates a real correctness bug: outbound self-validation was mutating the same duty/duplicate state used for incoming validation, so a routing/enqueue failure could poison state and suppress a later legitimate re-emission. The replacement validate_outbound is stateless, easy to reason about, and the new unit tests cover the meaningful branches (nested-slot extraction, undecodable bytes, unknown slot, duplicate signers, invalid role). No other call sites break — client/src/lib.rs is the only consumer and it's updated.

Correctness — no new false rejections ✅

The old outbound path called validator.validate(.., TopicContext::default()), which ran structure + role + committee/duty/signature checks (topic skipped). validate_outbound performs a strict subset — structure, role, slot extraction — so it cannot newly reject a message the previous code would have sent. The relaxation is exactly the intended direction.

Key trade-off for reviewers to weigh (documented in the PR body)

Sender-side validation previously acted as a self-protection net: if our own code produced a semantically-invalid-but-structurally-valid message (e.g. the #1052 pre-Boole contribution packaging bug), the Reject branch suppressed it locally so we didn't broadcast a message peers would penalize us for. That net is now gone — such a message reaches the wire and peers run the full incoming validator against it.

This is an explicit, reasonable choice given Safety > Correctness > Simplicity, and the PR body correctly flags the conditional #1052 dependency for Alan-fork deployments. Worth a second reviewer confirming the deployment constraint (must ship with #1052 unless all targets are already Boole) is tracked wherever release gating lives, since the code no longer enforces it.

Minor (non-blocking) — redundant re-decode on the hot path

In do_send (network.rs:134-136) the caller already holds the typed SignedSSVMessage, serializes it to message_bytes, and then validate_outbound(&message_bytes) immediately from_ssz_bytes-decodes those same bytes back into a SignedSSVMessage. The serialization is needed regardless (it's what gets sent to network_tx), but the re-decode is pure overhead on the urgent_consensus path, and for a message we just constructed the UndecodableMessageData branch is effectively unreachable. Consider exposing a sibling that takes &SignedSSVMessage and having validate_outbound(&[u8]) decode then delegate. Low priority — flagging for consideration, not as a blocker. Fix this →

Nits

• validate_structure_and_role returns Role, which validate_outbound discards via ?. That's fine — role is still validated for parity with the incoming path even though slot routing keys off msg_type only. No change needed.
• The log level for outbound failure went from warn/debug to error. Appropriate, since a structurally-invalid message we produced ourselves indicates a real bug.

No changes made — this is review feedback only.
· branch fix/1192-stateless-outbound-validation

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.59664% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.10%. Comparing base (09b302d) to head (469598c).

Files with missing lines	Patch %	Lines
anchor/message_sender/src/network.rs	0.00%	9 Missing ⚠️
anchor/message_validator/src/lib.rs	99.09%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             epbs    #1193      +/-   ##
==========================================
+ Coverage   71.95%   72.10%   +0.15%     
==========================================
  Files         168      168              
  Lines       33797    33892      +95     
==========================================
+ Hits        24317    24439     +122     
+ Misses       9480     9453      -27     

Flag	Coverage Δ
rust	72.10% <91.59%> (+0.15%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shane-moore]

@claude, thanks, good catch. Addressed in 4387e802.

do_send now validates the existing typed SignedSSVMessage before serialization. The byte-oriented validate_outbound remains as the raw-input wrapper and delegates to validate_outbound_message, preserving outer decode errors without paying the encode/decode round trip on the sender path. Structural validation is retained for aggregated messages.

The #1052 deployment condition is tracked in milestone 9 and this PR’s Blockers section: any Alan deployment containing #1192 must include #1052 or an equivalent fix; Boole-only ePBS deployments do not require it.

Validation: 112 message_validator tests, cargo check -p client, make cargo-fmt-check, and make lint.

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 1m 13s —— View job

---

Follow-up review — commit 4387e802

• Read the changed files and diff against origin/epbs
• Verify validate_outbound_message / validate_outbound split
• Verify do_send uses the typed variant (no re-decode)
• Confirm structural validation retained for aggregated messages
• Post review feedback

---

The re-decode is gone and the split is exactly what I suggested. Confirmed against the code:

do_send now validates the typed value directly (message_sender/src/network.rs:134): validate_outbound_message(&message) runs validate_structure_and_role + extract_slot on the SignedSSVMessage the caller already holds. The as_ssz_bytes() serialization moved after validation (:148), so the hot urgent_consensus path no longer pays an encode→decode round trip, and the UndecodableMessageData branch — unreachable for a value we just constructed — is no longer on that path.

The byte wrapper is preserved for raw input (message_validator/src/lib.rs:389-394): validate_outbound(&[u8]) decodes, mapping decode failure to UndecodableMessageData, then delegates to validate_outbound_message. This keeps outer decode-error semantics intact for genuinely untyped input. Grep confirms the only remaining callers of the byte variant are the unit tests (:1296, :1318, :1326, :1337, :1354, :1374) — including the validate_outbound(&[]) empty-bytes case that still exercises the decode-error branch. Production goes exclusively through the typed variant.

No behavior change: both entry points funnel into the same validate_structure_and_role → extract_slot sequence, so structural validation (including aggregated-message signer checks in SignedSSVMessage::validate) is unchanged. Nice touch keeping the doc comment on the shared validate_outbound_message and a short one on the wrapper.

Notes

• Small ordering detail worth confirming intent: the error! log in the failure branch reads message.ssv_message()... fields directly. Since validation now happens before as_ssz_bytes() and takes &message, message is still fully owned/borrowed here — no move-after-use issue. ✅
• Coverage delta (Codecov) shows the new byte-wrapper's decode-error line and the sender path as partially uncovered, but that's the trivially-delegating wrapper and the network-enqueue glue; the meaningful branches in validate_outbound_message are covered by the existing 112 tests.

LGTM. This resolves the only non-blocking item from the prior review, and the earlier correctness assessment (stateless preflight is a strict subset of the old checks, so no new false rejections) still holds. The #1052 Alan-deployment gate remains a release-tracking concern, not a code one — good that it's captured in milestone 9 and the Blockers section.

No changes made — review feedback only. Compilation was not re-run here (sandbox restriction); the assessment is from source reading, and the author reports cargo check -p client + 112 validator tests passing.
· branch fix/1192-stateless-outbound-validation

[shane-moore]

local-testnet is failing cuz of #1050, so we need to resolve that first

[jnhsigmap]

this is a nice simplification, with the trade-offs as you have mentioned. all good if this is the desired direction 👍

[jnhsigmap]

merge conflicts are trivial