-
Notifications
You must be signed in to change notification settings - Fork 827
chore: handling of zero point in bls point mapping G2 #4000
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
base: master
Are you sure you want to change the base?
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 |
---|---|---|
|
@@ -47,6 +47,18 @@ | |
if (opts._debug !== undefined) { | ||
opts._debug(`${pName} failed: ${e.message}`) | ||
} | ||
// noble‑curves throws this for inputs that map to the point at infinity | ||
if (e.message === 'bad point: ZERO') { | ||
// return two zeroed field elements (x & y), each the same length as the input | ||
const zeroPoint = new Uint8Array(opts.data.length * 2) | ||
if (opts._debug !== undefined) { | ||
opts._debug(`${pName} mapping to ZERO point, returning zero-filled output`) | ||
} | ||
return { | ||
executionGasUsed: gasUsed, | ||
returnValue: zeroPoint, | ||
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. Can it be explicitly checked that this should return a value and not "throw" at the EVM level? If it would throw at the EVM level the return buffer is cleared (RETURNDATASIZE would push 0 to stack) and the CALL to this precompile would push 0 to stack (instead of 1, signalling no-error for the call) 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. Will come back to this. This is only throwing from within noble-curves (not for the wasm library), and aside from hardcoding results and pre-returning (therefore bypassing noble instead of waiting for it to throw) I don't see what we could do differently. |
||
} | ||
} | ||
return EvmErrorResult(e, opts.gasLimit) | ||
} | ||
|
||
|
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 string is hardcoded, can we read this specific string from an export from BLS? Can a comment be added here that this lacks the necessary test input? Can we somehow help here to generate this input? We know which error string we should hit, so can we somehow reverse-engineer the input to trigger this?
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.
From the chat with the testing team, I understood that they will publish updated test vectors that will include the zero point test for G1 and G2. We have reverse engineered the G1 scalar from the BigInt value that Paul used in noble-curves (which from I understand he obtained from someone at the EF), but he mentioned having no idea how to generate the equivalent scalar for G2. I would just wait for the testing team to share the test vectors (this doesn't have to be merged before if we feel strongly about this)