-
Notifications
You must be signed in to change notification settings - Fork 265
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
fix(avm-simulator): L1TOL2MESSAGEEXISTS opcode #5807
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 |
---|---|---|
|
@@ -172,24 +172,11 @@ export class AvmPersistableStateManager { | |
* @returns exists - whether the message exists in the L1 to L2 Messages tree | ||
*/ | ||
public async checkL1ToL2MessageExists(msgHash: Fr, msgLeafIndex: Fr): Promise<boolean> { | ||
let exists = false; | ||
try { | ||
// The following 2 values are used to compute a message nullifier. Given that here we do not care about getting | ||
// non-nullified messages we can just pass in random values and the nullifier check will effectively be ignored | ||
// (no nullifier will be found). | ||
const ignoredContractAddress = AztecAddress.random(); | ||
const ignoredSecret = Fr.random(); | ||
const gotMessage = await this.hostStorage.commitmentsDb.getL1ToL2MembershipWitness( | ||
ignoredContractAddress, | ||
msgHash, | ||
ignoredSecret, | ||
); | ||
exists = gotMessage !== undefined && gotMessage.index == msgLeafIndex.toBigInt(); | ||
} catch { | ||
// error getting message - doesn't exist! | ||
exists = false; | ||
} | ||
this.log.debug(`l1ToL2Messages(${msgHash})@${msgLeafIndex} ?? exists: ${exists}.`); | ||
const valueAtIndex = await this.hostStorage.commitmentsDb.getL1ToL2LeafValue(msgLeafIndex.toBigInt()); | ||
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. Is the value what we want from the tree? 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. Who is this comment for? Should be the same value as for the membership proof so seems fine yes? This is essentially the same as should be checked in AVM? |
||
const exists = valueAtIndex?.equals(msgHash) ?? false; | ||
this.log.debug( | ||
`l1ToL2Messages(@${msgLeafIndex}) ?? exists: ${exists}, expected: ${msgHash}, found: ${valueAtIndex}.`, | ||
); | ||
this.trace.traceL1ToL2MessageCheck(msgHash, msgLeafIndex, exists); | ||
return Promise.resolve(exists); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,6 +92,12 @@ export interface CommitmentsDB { | |
secret: Fr, | ||
): Promise<MessageLoadOracleInputs<typeof L1_TO_L2_MSG_TREE_HEIGHT>>; | ||
|
||
/** | ||
* @param leafIndex the leaf to look up | ||
* @returns The l1 to l2 leaf value or undefined if not found. | ||
*/ | ||
getL1ToL2LeafValue(leafIndex: bigint): Promise<Fr | undefined>; | ||
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. Instead of introducing |
||
|
||
/** | ||
* Gets the index of a commitment in the note hash tree. | ||
* @param commitment - The commitment. | ||
|
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.
How is this opcode supposed to handle nullified messages? See comment in #5805
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.
As mentioned, I don't think exists should necessarily take into account if it is "active", just that it is in the tree. Separately, you might not be able to even know if it is nullified because you might not know the secret 🤷