Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth/fetcher, core/txpool: surface an exported error from the pool when attempting to add a blob transaction where the computed hashes of the sidecar commitments don't match the value in the tx header. In fetcher: drop sending peer but don't remove the hashes from tracking for future request/delivery. #31210

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Feb 19, 2025

closes #31162

(WIP) I haven't reproduced the bug this aims to fix with a test case yet, but it seems plausible from reading the code.

edit: After drafting this PR up, I actually think that it should be possible, and more simple to validate blob transaction hash integrity right after we decode blob transactions at the p2p level.

This is an attempt to address the following scenario:

  • I create a valid, includable blob transaction T and propagate it to the network.
  • Block producer A becomes aware of T, requests it from some random node B.
  • B decides for some reason, to alter the sidecar such that transaction validation cannot verify the hashes of the commitments in the sidecar correspond to the blob_versioned_hashes in the transaction header:
    • If A receives a sidecar-less T from B, the transaction will fail to be added to the pool, and A will disconnect every other peer that announced T with a total size differing from the altered T that B sent. See the related issue for an explainer of how this occurs in the fetcher.
    • If A receives a sidecar-containing T whose sidecar commitments don't match blob_versioned_hashes, the tx will not make it into the pool and the hash will not be re-requested from other peers by the fetcher.

Part of the fix that this PR proposes is to surface an exported error ErrInvalidAuxiliaryData from the pool which conveys transaction validation failure from verifying the cryptographic integrity of extra-header data. Right now, this is only relevant for blob transactions where the integrity of the header field blob_versioned_hashes depends on the commitments in the sidecar.

With this PR, adding a blob transaction to the pool will return ErrInvalidAuxiliaryData if hashes of the commitments in the sidecar don't match the values in the header. The distinction is useful in the fetcher because we don't want to purge hashes from the tracker upon receive if we can't recompute the transaction hash that a peer advertises.

…d error from the pool when attempting to add a blob transaction where the computed hashes of the sidecar commitments doesn't match the value in the tx header. In fetcher: drop sending peer but don't remove the hashes from tracking for future request/delivery.
@jwasinger jwasinger changed the title eth/fetcher, core/txpool: surface an exported error from the pool when attempting to add a blob transaction where the computed hashes of the sidecar commitments doesn't match the value in the tx header. In fetcher: drop sending peer but don't remove the hashes from tracking for future request/delivery. eth/fetcher, core/txpool: surface an exported error from the pool when attempting to add a blob transaction where the computed hashes of the sidecar commitments don't match the value in the tx header. In fetcher: drop sending peer but don't remove the hashes from tracking for future request/delivery. Feb 19, 2025
@jwasinger
Copy link
Contributor Author

Closing this as I think I have a cleaner solution implemented that validates the blob_versioned_hashes<->sidecar link in the eth protocol handler.

@jwasinger jwasinger closed this Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Valid peers dropped due to blob tx validation failure from another peer
2 participants