fix(pxe): backward-compatible deserialization of validation requests#20957
Draft
nchamo wants to merge 3 commits intomerge-train/fairiesfrom
Draft
fix(pxe): backward-compatible deserialization of validation requests#20957nchamo wants to merge 3 commits intomerge-train/fairiesfrom
nchamo wants to merge 3 commits intomerge-train/fairiesfrom
Conversation
nchamo
commented
Feb 27, 2026
| import { MAX_NOTE_PACKED_LEN } from './note_validation_request.js'; | ||
|
|
||
| const MAX_PUBLIC_LOG_LEN_FOR_NOTE_COMPLETION = MAX_NOTE_PACKED_LEN; | ||
| const MAX_LOG_CONTENT_LEN = Math.max(MAX_PUBLIC_LOG_LEN_FOR_NOTE_COMPLETION, PRIVATE_LOG_CIPHERTEXT_LEN); |
Contributor
Author
There was a problem hiding this comment.
I believe it didn't make sense to have this. MAX_NOTE_PACKED_LEN is calculated in aztec-nr from PRIVATE_LOG_CIPHERTEXT_LEN, and it should always be lower for it. But please correct me if I'm wrong
nchamo
commented
Feb 27, 2026
| if (eventLen > MAX_EVENT_CONTENT_LEN) { | ||
| throw new Error(`Event content length ${eventLen} exceeds MAX_EVENT_CONTENT_LEN ${MAX_EVENT_CONTENT_LEN}.`); | ||
| } | ||
| if (eventLen > arraySize) { |
Contributor
Author
There was a problem hiding this comment.
This shouldn't happen, but it doesn't hurt to check
nchamo
commented
Feb 27, 2026
| // Event content is serialized as a BoundedVec: `storage[capacity] ++ len`. The storage capacity may differ across | ||
| // aztec-nr versions (e.g. old contracts used 11, current ones use 10), so we infer it dynamically from the total field | ||
| // count. This constant is only used to validate the *content length* (the BoundedVec `.len()`), not the storage size. | ||
| const MAX_EVENT_CONTENT_LEN = 10; |
Contributor
Author
There was a problem hiding this comment.
Added this so we can still validate something from PXE's side but we can remove it if we want to fully trust what we receive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In #20840, we introduced a change to reflect the actual max length used for messages and notes, we were not calculating it correctly beforehand. The main issue is that aztec-nr used those notes/events max lengths as the max size for
BoundedVecwhen communicating with PXE. And, on PXE's side, we also used that value to deserialize theBoundedVec.But when we updated those values, old contracts were no longer compatible with the new version of PXE and new contracts were no longer compatible with the old version of PXE.
I can think of quick two ways to prevent this from happening again:
BoundedVecas a parameter, from aztec-nr to PXE, so that it knows how to deserialize itThis PR is a small implementation of option (2). It:
BoundedVecbased on the known structure of the requestsBoundedVecsize) doesn't exceed a maximum allowed by PXE, while continuing to supportBoundedVecs with higher sizeFixes #14617
Fixes F-375