Implement eth/68 (EIP-5793)

[Gabriel-Trintinalia]

PR description

Implement eth/68

• https://eips.ethereum.org/EIPS/eip-5793

The NewPooledTransactionHashes message announces transaction hashes, allowing the peer to selectively fetch transactions it does not yet have.

EIP-4844 introduces a new transaction type for blob transactions. Since these blob transactions are large, naively broadcasting them to sqrt(peers) could significantly increase bandwidth requirements. Adding the transaction type and the size to the announcement message will allow nodes to select which transactions they want to fetch and also allow them to load balance or throttle peers based on past behavior.

The added metadata fields will also enable future - upgradeless - protocol tweaks to prevent certain transaction type (e.g. blob transactions) or certain transaction sizes (e.g. 128KB+) from being blindly broadcast to many peers. Enforcing announcements only and retrieval on demand would ensure a much more predictable networking behavior, limiting the amplification effect of transaction propagation DoS attack.

Modify the NewPooledTransactionHashes (0x08) message:

(eth/67): [hash_0: B_32, hash_1: B_32, ...]
(eth/68): [[type_0: B_1, type_1: B_1, ...], [size_0: B_4, size_1: B_4, ...], [hash_0: B_32, hash_1: B_32, ...]]

Fixed Issue(s)

fixes #4715

Documentation

• I thought about documentation and added the doc-change-required label to this PR if
  updates are required.

Changelog

• I thought about the changelog and included a changelog update if required.

[fab-10]

overall the implementation is good, please read the inline comments for questions, optimizations and suggestions to improve the handling of malformed messages.

Then there is another part of this implementation that should cover our new strategy on how we broadcast transactions taking in account the new type and size parameters, it is fine to do it in another PR to keep them small, have you already planned to do it?

[fab-10]

Why allowing nulls here?
type and size should never be null at this point, otherwise it means there message was malformed, and its processing should have stopped before.

[Gabriel-Trintinalia]

Hum, if the transaction is announced by a protocol version older than Eth/68, only the hash is announced

[fab-10]

if the transaction is announced by a protocol < Eth/68 then the constructor public TransactionAnnouncement(final Hash hash) is used if I am not wrong

[Gabriel-Trintinalia]

I understood your point now. I changed the constructors to not accept null and throw instead. Also added a unit test for that. Thanks again!

[Gabriel-Trintinalia]

@fab-10 Thanks for the review.

Then there is another part of this implementation that should cover our new strategy on how we broadcast transactions taking in account the new type and size parameters, it is fine to do it in another PR to keep them small, have you already planned to do it?

Yeah, that makes sense. This PR only implements the protocol specs, implementing the ability to send and receive NewPooledTransactionHashes with their sizes and types when using eth68 ensuring that we are compatible with older protocol versions.

What the client should do with the type and size and how it impacts broadcast transaction strategy are not part of the scope. I believe it would be better to handle it in a separate PR.

[fab-10]

LGTM, just a comment on the nullable arguments

[fab-10]

if the transaction is announced by a protocol < Eth/68 then the constructor public TransactionAnnouncement(final Hash hash) is used if I am not wrong

[macfarla]

LGTM just need to move the changelog entry