fix(p2p)!: revamp BLOCK_TXS validations

[fcarreiro]

Summary

BLOCK_TXS request/response validation had a bug that caused us to discard perfectly good transactions.

When a peer doesn't have the block (proposal pruned, or never received) but the request carried the full tx hashes, the responder (reqRespBlockTxsHandler) still matches those hashes against its own tx pool and ships whatever it finds — it just can't produce an availability bitvector for a block it doesn't know about. This is a legitimate "I don't have the block, but here are the txs you asked for by hash" response, not misbehaviour.

Previously this case was signalled by setting archiveRoot = Fr.ZERO on the response, and validateRequestedBlockTxsConsistency treated any response that didn't echo the requested archive root (including the zero case) as a hard failure: it returned false, which routed the response through the INTERNAL_ERROR path and discarded the returned txs entirely. The intended behaviour is the opposite — we want to use the txs the peer returned and merely mark the peer as "dumb" (it can't serve index-based smart requests), without penalising it.

Changes

Drop archiveRoot from BlockTxsResponse.

• The archive root on the response only ever served as an out-of-band "I have / don't have the block" flag (and a redundant echo of the request).
• Checking if the response matches the request doesn't make sense. A cheating peer can always return the same archive root as the request, but otherwise malform the rest of the response.
• It is replaced by a peerHasBlock() helper that derives the same signal from the availability bitvector: an empty bitvector (length 0) means the peer doesn't have the block. The responder no longer special-cases the archive root.

Rework validateRequestedBlockTxsConsistency. Validation now:

• rejects + penalises (mid) duplicate txs in the response;
• resolves the block tx hashes from the proposal or the archiver — if neither is available we can't verify membership, so we reject without penalising (local-state gap, not a peer fault);
• rejects + penalises (low) any returned tx that is neither part of the block nor one we explicitly requested by hash — i.e. the returned set must be a subset of block tx hashes ∪ request.txHashes (a tx requested by hash may legitimately not belong to the block being validated);
• accepts (returns true) when the peer signals it lacks the block (!peerHasBlock()) — the returned txs are still valid and usable, which is the core of the fix;
• rejects + penalises (mid) a bitvector whose length disagrees with the block size;
• rejects + penalises (low) a peer that advertises a requested tx via its bitvector but withholds it from the response.

The previous order / strictly-increasing and maxReturnable checks are removed; membership plus the advertise-vs-deliver check cover the cases that matter.

Move dumb-marking into the smart/dumb decision. BatchTxRequester no longer inspects archive roots. decideIfPeerIsSmart marks a peer dumb (and clears its per-peer data, without penalty) whenever the response signals it lacks the block (!peerHasBlock()); penalisation for genuinely inconsistent responses is left to the validator. The old handleArchiveRootMismatch helper is removed.

Tests

• Updated the serialization, handler, validation, requester and integration tests to the peerHasBlock() model.
• Added a regression test at the validator level — a peer that signals it lacks the block via an empty bitvector but returns valid txs is now accepted instead of discarded.
• Added a regression test at the requester level — those txs are delivered (used) and the peer is marked dumb without penalty.
• Added coverage for the partial-availability case (peer returns fewer txs than bits set: we request a,b,c, peer has c,d,e, so only c comes back with three bits set) and for the by-hash case (a tx requested via request.txHashes that is not part of the block is accepted).

The regression tests fail against the former behaviour.

[spalladino]

Why not just check against the requested ones?

[fcarreiro]

Ah darn, I did indeed want to check (tx hashes by index) U (explicitly requested tx hashes). will fix in the follow up PR.

[spalladino]

Any idea why we were checking for this? Should we keep this validation?

[fcarreiro]

I think this was wrong, or in any case overspecifying. The returned indices and the returned TXs are not really linked (except that you should not withhold a TX that you have and you have been asked for.

I think we shouldn't care about the order of the returned txs. In short I care

• That you give me transactions I ask for (and not more)
• That you don't withhold
• That if you have the block, the sizes of the indices match

Makes sense? Also I think there was nothing preventing a requester to send a tx hash (explicit) that was not in the block, and then this check would fail IIUC (now it is explicit that the requester can do this).