Skip to content

fix(p2p): validate BLOCK_TXS in BatchTxRequester#23371

Merged
spalladino merged 1 commit into
merge-train/spartanfrom
fc/batch-tx-validation
May 20, 2026
Merged

fix(p2p): validate BLOCK_TXS in BatchTxRequester#23371
spalladino merged 1 commit into
merge-train/spartanfrom
fc/batch-tx-validation

Conversation

@fcarreiro
Copy link
Copy Markdown
Contributor

@fcarreiro fcarreiro commented May 18, 2026

This PR modifies and clarifies what is validated where:

  • sendRequestToPeer (from reqresp) is considered a low level method. It does not validate transactions and does only very minimal checks on the request and response objects. It does penalize peers under some circumstances.
  • The concept of subprotocol validator is removed from reqresp.
  • BatchTxRequester now (1) validates the req and response objects; (2) validates the TXs when the response is valid.
  • Unused validateRequestedTx is dropped in this PR.

Closes https://linear.app/aztec-labs/issue/A-1014/block-txs-reqresp-validator-validaterequestedblocktxs-is-never-invoked .

Copy link
Copy Markdown
Contributor Author

fcarreiro commented May 18, 2026

}
}

/**
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused.

@fcarreiro fcarreiro force-pushed the fc/batch-tx-validation branch from 0138e81 to 4210082 Compare May 18, 2026 13:55
return;
}

await this.handleSuccessResponseFromPeer(peerId, blockResponse);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TXs themselves are validated inside here.

@fcarreiro fcarreiro force-pushed the fc/batch-tx-validation branch from 4210082 to a28872a Compare May 19, 2026 16:04
@AztecBot
Copy link
Copy Markdown
Collaborator

Flakey Tests

🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/2e0946c90218c335�2e0946c90218c3358;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_epochs/epochs_mbps.parallel.test.ts "builds multiple blocks per slot with L2 to L1 messages" (330s) (code: 0) group:e2e-p2p-epoch-flakes

Comment on lines 1558 to 1559
// Given proposal (should have locally), ensure returned txs are valid subset and match request indices
const proposal = await this.mempools.attestationPool.getBlockProposalByArchive(request.archiveRoot.toString());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to tweak this validation: a malicious proposer who sends two different proposals with a different set of txs but same archive root, will cause two honest nodes to fail at this exchange, since the responder will return a tx that's not in the proposal from the requester's perspective. Fix is to request the txs by proposal hash vs proposal archive.

But that's not concern of this PR.

@spalladino spalladino merged commit a2baf79 into merge-train/spartan May 20, 2026
15 checks passed
@spalladino spalladino deleted the fc/batch-tx-validation branch May 20, 2026 17:03
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.

3 participants