-
Notifications
You must be signed in to change notification settings - Fork 598
feat(pxe)!: pass BoundedVec sizes as oracle params #21176
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,15 +19,15 @@ describe('NoteValidationRequest', () => { | |
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 8) | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', // content end (8 storage fields) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's this 8? Wouldn't this be higher, up to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now, if you look at aztec-nr, the max length/capacity for notes is 8. That's why this test was using this particular value This test could test other values though, but I thought it made sense to test what we would receive from contracts today (8) |
||
| '0x0000000000000000000000000000000000000000000000000000000000000002', // content length | ||
| '0x0000000000000000000000000000000000000000000000000000000000000006', // note hash | ||
| '0x0000000000000000000000000000000000000000000000000000000000000007', // nullifier | ||
| '0x0000000000000000000000000000000000000000000000000000000000000008', // tx hash | ||
| '0x0000000000000000000000000000000000000000000000000000000000000009', // recipient | ||
| ].map(Fr.fromHexString); | ||
|
|
||
| const request = NoteValidationRequest.fromFields(serialized); | ||
| const request = NoteValidationRequest.fromFields(serialized, 8); | ||
|
|
||
| expect(request.contractAddress).toEqual(AztecAddress.fromBigInt(1n)); | ||
| expect(request.owner).toEqual(AztecAddress.fromBigInt(50n)); | ||
|
|
@@ -56,17 +56,16 @@ describe('NoteValidationRequest', () => { | |
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 8) | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', // extra field beyond MAX_NOTE_PACKED_LEN, this is a malformed serialization | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000', // content end (9 storage fields, but maxNotePackedLen=8) | ||
| '0x0000000000000000000000000000000000000000000000000000000000000002', // content length | ||
| '0x0000000000000000000000000000000000000000000000000000000000000006', // note hash | ||
| '0x0000000000000000000000000000000000000000000000000000000000000007', // nullifier | ||
| '0x0000000000000000000000000000000000000000000000000000000000000008', // tx hash | ||
| '0x0000000000000000000000000000000000000000000000000000000000000009', // recipient | ||
| ].map(Fr.fromHexString); | ||
|
|
||
| expect(() => NoteValidationRequest.fromFields(serialized)).toThrow( | ||
| /Error converting array of fields to NoteValidationRequest/, | ||
| expect(() => NoteValidationRequest.fromFields(serialized, 8)).toThrow( | ||
| 'Error converting array of fields to NoteValidationRequest: expected 18 fields but received 19 (maxNotePackedLen=8).', | ||
| ); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,9 @@ | |
| /// | ||
| /// @dev Whenever a contract function or Noir test is run, the `aztec_utl_assertCompatibleOracleVersion` oracle is called | ||
| /// and if the oracle version is incompatible an error is thrown. | ||
| export const ORACLE_VERSION = 13; | ||
| export const ORACLE_VERSION = 14; | ||
|
|
||
| /// This hash is computed as by hashing the Oracle interface and it is used to detect when the Oracle interface changes, | ||
| /// which in turn implies that you need to update the ORACLE_VERSION constant in this file and in | ||
| /// `noir-projects/aztec-nr/aztec/src/oracle/version.nr`. | ||
| export const ORACLE_INTERFACE_HASH = '9fb918682455c164ce8dd3acb71c751e2b9b2fc48913604069c9ea885fa378ca'; | ||
| export const ORACLE_INTERFACE_HASH = '34238ab576c377b4ffc1ddbf557560f07a66baf879920191897bc784fa5a97ee'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing version bump?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, was left unstaged 😅 |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implicitly assumes that notes will not be longer than logs no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I wrote this on the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nial mentioned that this limit might not make sense as there is no reason for notes to not be longer in case they are delivered via offchain messages.
Could you expand on why this change was done here? I didn't review your other PRs that I assumed led to this so I don't have context here. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change here because:
MAX_NOTE_PACKED_LENandPRIVATE_LOG_CIPHERTEXT_LENare actually calculated on the aztec-nr side of things, and the values are just copied to typescriptPRIVATE_LOG_CIPHERTEXT_LEN= 15, whileMAX_NOTE_PACKED_LEN= 8. This is because: `So if
MAX_NOTE_PACKED_LENisPRIVATE_LOG_CIPHERTEXT_LENminus some fields,max(PRIVATE_LOG_CIPHERTEXT_LEN, MAX_NOTE_PACKED_LEN)would always bePRIVATE_LOG_CIPHERTEXT_LENThat's why I thought it didn't make much sense to do that, and asked if I was missing something. About off-chain messages, would they go though this path? Do they emit logs somehow? Not very familiar with how they work today