Skip to content

fix(pxe): enforce full field consumption at oracle boundaries#23802

Merged
nchamo merged 2 commits into
merge-train/fairiesfrom
nchamo/deser-field-reader-check
Jun 2, 2026
Merged

fix(pxe): enforce full field consumption at oracle boundaries#23802
nchamo merged 2 commits into
merge-train/fairiesfrom
nchamo/deser-field-reader-check

Conversation

@nchamo

@nchamo nchamo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Problem

The EphemeralArray<T> refactor in #23649 moved deserialization into the oracle registry but dropped the per-row field-consumption check. Extra trailing fields in an ephemeral-array row, and in oracle parameter slots generally, were silently ignored. A malformed oracle input that should be rejected deserialized as if it were well-formed, weakening the oracle ABI.

Fix

Adds assertReadersConsumed and calls it at the two boundaries where each FieldReader wraps exactly one logical value: oracle parameter slots (in makeEntry) and ephemeral-array rows (via a wrapped element in EPHEMERAL_ARRAY). Trailing fields there now throw. Streaming combinators that deliberately under-consume a shared reader keep working: BOUNDED_VEC and OPTION drain their trailing zero-padding so the boundary check sees a fully-consumed reader.

@nchamo nchamo self-assigned this Jun 2, 2026
@nchamo nchamo requested a review from vezenovm June 2, 2026 16:30
deserialization: {
fn: readers => {
const value = element.deserialization!.fn(readers);
assertReadersConsumed(readers);

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.

can we elaborate on why we need to assert here and the assert in makeEntry is not enough?

}
// Drain the trailing zero-padding (maxLength - length unused element slots) so the storage reader is
// fully consumed.
storageReader.skip(storageReader.remainingFields());

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.

Would be good to have a BoundedVec test where capacity > length, I didn't see one.

@nchamo nchamo enabled auto-merge (squash) June 2, 2026 18:24
@AztecBot

AztecBot commented Jun 2, 2026

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/d58126f5f29bbc20�d58126f5f29bbc208;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_epochs/epochs_invalidate_block.parallel.test.ts "proposer invalidates multiple checkpoints" (489s) (code: 0) group:e2e-p2p-epoch-flakes

@nchamo nchamo merged commit 93f756d into merge-train/fairies Jun 2, 2026
12 checks passed
@nchamo nchamo deleted the nchamo/deser-field-reader-check branch June 2, 2026 18:50
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jun 4, 2026
BEGIN_COMMIT_OVERRIDE
feat(aztec-nr): Add a delivery mode to handshake notes (AztecProtocol#23783)
fix(pxe): enforce full field consumption at oracle boundaries (AztecProtocol#23802)
chore(ci): Static oracle version check (AztecProtocol#23805)
END_COMMIT_OVERRIDE
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