-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: add WideMul128 hint #126
Changes from 1 commit
d2e5d00
b086616
1510539
0345b36
dc24035
5c7f088
d2b9a27
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 | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,55 @@ | ||||||||||||||||||
import { z } from 'zod'; | ||||||||||||||||||
import { HintName } from 'hints/hintName'; | ||||||||||||||||||
import { cellRef, CellRef } from 'hints/hintParamsSchema'; | ||||||||||||||||||
import { resOperand, ResOperand } from 'hints/hintParamsSchema'; | ||||||||||||||||||
import { VirtualMachine } from 'vm/virtualMachine'; | ||||||||||||||||||
import { Felt } from 'primitives/felt'; | ||||||||||||||||||
export const wideMul128Parser = z. | ||||||||||||||||||
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. add JSDoc |
||||||||||||||||||
object({ | ||||||||||||||||||
WideMul128: z.object({ | ||||||||||||||||||
lhs: resOperand, | ||||||||||||||||||
rhs: resOperand, | ||||||||||||||||||
high: cellRef, | ||||||||||||||||||
low: cellRef | ||||||||||||||||||
}), | ||||||||||||||||||
}) | ||||||||||||||||||
.transform(({ WideMul128: { lhs, rhs, high, low } }) => ({ | ||||||||||||||||||
type: HintName.WideMul128, | ||||||||||||||||||
lhs: lhs, | ||||||||||||||||||
rhs: rhs, | ||||||||||||||||||
high: high, | ||||||||||||||||||
low: low | ||||||||||||||||||
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. As the property names are unchanged, you can directly use the variable:
Suggested change
|
||||||||||||||||||
})); | ||||||||||||||||||
|
||||||||||||||||||
export const wideMul128 = ( | ||||||||||||||||||
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. add a well-formatted JSDoc, take example on the |
||||||||||||||||||
vm: VirtualMachine, | ||||||||||||||||||
lhs: ResOperand, | ||||||||||||||||||
rhs: ResOperand, | ||||||||||||||||||
high: CellRef, | ||||||||||||||||||
low: CellRef | ||||||||||||||||||
): void => { | ||||||||||||||||||
try { | ||||||||||||||||||
const mask128 = (BigInt(1) << BigInt(128)) - BigInt(1); | ||||||||||||||||||
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. Use Use the short notation for Bigint when possible, it is more readable:
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
//Gets the operands' values | ||||||||||||||||||
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. No need to add comments, the code is self-explanatory here (same // Do multiplication) etc. |
||||||||||||||||||
const lhsVal = vm.getResOperandValue(lhs).toBigInt(); | ||||||||||||||||||
const rhsVal = vm.getResOperandValue(rhs).toBigInt(); | ||||||||||||||||||
|
||||||||||||||||||
// Do multiplication | ||||||||||||||||||
const prod = lhsVal * rhsVal; | ||||||||||||||||||
|
||||||||||||||||||
//Calc and storage high | ||||||||||||||||||
const highVal = prod >> BigInt(128); | ||||||||||||||||||
const highRef = vm.cellRefToRelocatable(high); | ||||||||||||||||||
vm.memory.assertEq(highRef, new Felt(highVal)); | ||||||||||||||||||
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. Directly create No need to declare a variable for the address (i.e. |
||||||||||||||||||
|
||||||||||||||||||
//Calc and storage low | ||||||||||||||||||
const lowVal = prod & mask128; | ||||||||||||||||||
const lowRef = vm.cellRefToRelocatable(low); | ||||||||||||||||||
vm.memory.assertEq(lowRef, new Felt(lowVal)); | ||||||||||||||||||
|
||||||||||||||||||
} catch (error: any) { | ||||||||||||||||||
throw new Error(error.message); | ||||||||||||||||||
} | ||||||||||||||||||
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. Don't wrap the function in a try catch statement |
||||||||||||||||||
}; | ||||||||||||||||||
|
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.
The
hint
object expects the zod object you've defined to parse the hint from the compilation artifacts:wideMul128Parser