applySlashes can halt the chain or drain collateral on an unregistered slash entry

[grantkee]

Severity: high, but latent. Pre-existing; not introduced by fix/eip7702-delegation-signature.

Problem

applySlashes (src/consensus/ConsensusRegistry.sol:182) accepts a slash entry for an address
that holds no ConsensusNFT, or holds one but never staked. Neither case is filtered, and both
reach _consensusBurn. burn() guards exactly this case; applySlashes does not.

The function has two skips, and neither fires:

Sentinel token ids (0 and type(uint160).max) are skipped at :190. Any other address passes.

isRetired(slash.validatorAddress) at :194 returns false for an address with
currentStatus == Undefined. That is explicit in the implementation
(src/consensus/ConsensusRegistry.sol:502): a validator that "never existed in the first place"
returns false, not true. So the retired skip never fires for an unregistered address.

Control then reaches the balance comparison at :196. For an unregistered address
balances[addr] == 0, so 0 > slash.amount is false for every amount, including zero. The
entry falls to _consensusBurn at :203.

Nothing catches it earlier. Undefined is not committee-eligible, so _ejectFromCommittees finds
nothing to eject and its committee-size checks all pass.

Two variants

An address that never held a ConsensusNFT halts the chain. _consensusBurn runs to
_burnConsensusNFT (src/consensus/StakeManager.sol:215), whose _burn(_getTokenId(...)) reverts
ERC721NonexistentToken on OZ v5.2.0. applySlashes runs inside the closing block, so the revert
means the block cannot be produced. Because issuance.call at :1330 precedes the burn, the whole
transaction reverts atomically: no TEL moves, and the only consequence is the halt.

An address that was minted but never staked drains collateral silently. The NFT exists, so _burn
succeeds. But initialStakeAmt comes from getBalanceBreakdown (:1324), which reads
versions[validators[addr].stakeVersion].stakeAmount with stakeVersion defaulting to 0. The
registry therefore ships a full genesis stake amount to Issuance at :1330 for an address that
deposited nothing, drawing on other validators' collateral. Nothing on the path reverts:
_exit, _retire, and _setStatus have no status guards.

Repeat it and the registry stops backing the sum of balances[]. Once short, the Issuance
consolidation inside concludeEpoch reverts, and that halts the chain.

The events name the wrong address

mint (:903) never initializes validators[addr].validatorAddress; it only calls _mint. In the
minted-but-never-staked variant, _setStatus therefore runs with addr == address(0). The net
effect on the validator sets is neutral, since address(0) is added to the Exited set and removed
again by _retire. But ValidatorExited and ValidatorRetired both emit a struct whose
validatorAddress field is zero. The drain is not merely missing a dedicated event: the two events
that do fire name the wrong address, which defeats log-based monitoring.

Why this is latent today

Slashing is disabled for the MNO pilot (src/consensus/design.md:32-33,
src/interfaces/IConsensusRegistry.sol:243, :252, src/interfaces/IStakeManager.sol:15, :22), and
the situation is stronger than disabled: no slash producer exists in the node.
epoch_boundary_slashes() in telcoin-network/crates/tn-reth/src/evm/block.rs:575-578 is a
hardcoded Vec::new(), and the only other body is a #[cfg(test)] seam at :583-586. That is not
a feature flag, so even the e2e suite links the empty one. Nothing in the consensus, network, node,
or engine crates constructs a Slash. Equivocation is detected as
ConsensusError::CertificateEquivocation, but it terminates in errors and peer penalties and never
reaches the execution layer.

This is a gate on enabling slashing rather than an active production risk. It is rated high for the
consequence it carries at that point.

Why the contract is the right place to fix it

applySlashes([]) is invoked unconditionally every epoch
(telcoin-network/crates/tn-reth/src/evm/block.rs:417-425 has no if !slashes.is_empty() guard),
so the function is live critical-path code today even with an empty list.

The Rust side does no filtering at all: no membership check, no dedup, no zero-address check, no
length bound. The guidance comment for future implementers at block.rs:558-573 covers ordering,
amount sizing, and determinism, but never says to validate registry membership.

The live applyIncentives path avoids this class of bug by accident of design rather than by a
shared guard. RewardsCounter::get_address_counts
(telcoin-network/crates/types/src/gas_accumulator.rs:169-189) resolves each authority through
committee.authority(...) and silently drops non-members, so the reward list is committee-derived
and BTreeMap-deduped by construction. A slash producer inherits none of that scaffolding. The
contract has to be safe against a caller documented to do no filtering.

Contradicts a stated invariant

src/consensus/invariants.md claims that "applyIncentives and applySlashes skip sentinel-address
and retired entries rather than reverting, so no malformed system-call input can stall the
boundary." That does not hold for an Undefined entry.

The comment at src/consensus/ConsensusRegistry.sol:192-193 assumes a slashed validator has
necessarily signed a consensus header and is therefore whitelisted, staked, and active. That
assumption is unenforced on both sides of the boundary.

Fix

Give _consensusBurn, or applySlashes directly, the same Undefined short-circuit that burn()
already has at :919-922. One guard closes both variants.

For comparison, burn() also runs _checkConsensusNFTOwner at :917 before branching, which
rejects a never-minted address outright. applyIncentives guards the same class of input by capping
weight to the outstanding balance.

Tests to add

No test covers either variant. The applySlashes suite has ten tests covering the exact-balance
boundary, retired-in-batch, double-slash, and network-emptying cases, and none names an unregistered
or never-staked address. The asymmetry is worth noting: topUpSlashedStake, which is merely
user-callable, does have testRevert_topUpSlashedStake_unknownValidator.

Add a slash entry for an address that never held a ConsensusNFT, and one for an address that was
minted but never staked. The second should assert that the registry's native balance is unchanged.