[paschal533]

Description

Follow-up to the discussion in #308.

Right now, if one record in a NODES response fails to decode, the whole response is dropped. That's the right call for records we know are broken, but it also catches records that are just signed with an identity scheme we don't support yet (anything other than v4). Those aren't invalid, we just can't check them, and a peer sending one may be perfectly correct and simply ahead of us.

This makes the decoder skip records with an unsupported identity scheme and keep the rest of the response. Every other decode error (bad signature, unsorted keys, oversized record, etc.) still fails the whole message like before, so the strict behaviour stays for anything we know is bad.

Changes, all in src/rpc.rs:

• in the NODES decode loop, a record that fails with enr's "Unsupported identity scheme" error is skipped by advancing past it using the length from its RLP header, which is already bounds checked above
• three tests: a skipped record in the middle of a batch, a batch where every record is skipped, and a batch with a bad signature that's still rejected

Notes & open questions

• enr reports this case as DecoderError::Custom("Unsupported identity scheme") and DecoderError is alloy-rlp's type, so there's no dedicated variant to match on. I matched on the message through a constant. If enr ever changes the text, the new tests fail, so it can't drift silently. If you'd prefer enr to expose a small helper for this instead, I'm happy to open that PR and switch over.
• enr stops parsing a record when it hits the id check, so the rest of a skipped record is never looked at. That's fine since the record is thrown away and never reaches the routing table, but it does mean a peer can put junk after id in one record without the message failing.
• Records over 300 bytes are still rejected whatever their scheme, because enr checks the size first. EIP-778 caps every record at 300 bytes, so that seemed right to keep.
• The skip is logged at trace rather than debug, since a peer can fit a lot of small records into one packet.
• Unrelated, but cargo clippy --workspace --tests --all-features -- -D warnings currently fails on master with stable 1.95 because of nonminimal_bool in src/ipmode.rs:57. I left it out of this PR to keep it focused.

Change checklist

• Self-review
• Documentation updates if relevant
• Tests if relevant

[ackintosh]

FYI, I'm working on https://github.com/sigp/enr/pull/93 to make error handling cleaner.

[paschal533]

Hi @ackintosh, Thanks, that's the kind of helper. I'll switch over once it lands. One thing for the switch: Message::decode still returns DecoderError, so the other enr::Errors need mapping back (InvalidRlpData(e) to e, the rest to a Custom). And since https://github.com/sigp/enr/pull/93 drops Decodable, discv5 needs an enr bump anyway. I can do both here once there's a release, or leave this PR as is and do the switch in the bump PR, whichever you prefer.