feat: add WideMul128 hint #126

D240021 · 2024-08-07T03:58:27Z

Hi, I've made some progress with the hint. I haven't tested it yet but I wanted to see if you could give me some feedback.

Changes made

Added Hint name to hintName.ts enum.
I implemented hint's Zod object.
I created the Zod union in hintSchema.ts file.
I implemented the provisional logic of the hint

zmalatrax

Great overall :)

Put the file wideMul128.ts in the math folder pls

Feel free to join the telegram group of the Cairo VM ts: https://t.me/cairovmts

zmalatrax · 2024-08-07T12:50:41Z

src/hints/hintSchema.ts

@@ -33,6 +33,7 @@ const hint = z.union([
  shouldContinueSquashLoopParser,
  shouldSkipSquashLoopParser,
  testLessThanParser,
+  wideMul128,


The hint object expects the zod object you've defined to parse the hint from the compilation artifacts: wideMul128Parser

zmalatrax · 2024-08-07T12:53:10Z

src/hints/wideMul128.ts

+        lhs: lhs,
+        rhs: rhs,
+        high: high,
+        low: low


As the property names are unchanged, you can directly use the variable:

Suggested change

lhs: lhs,

rhs: rhs,

high: high,

low: low

lhs,

rhs,

high,

low

zmalatrax · 2024-08-07T12:56:57Z

src/hints/wideMul128.ts

+    low: CellRef
+): void => {
+    try {
+        const mask128 = (BigInt(1) << BigInt(128)) - BigInt(1);


Use mask rather than mask128

Use the short notation for Bigint when possible, it is more readable: 128n instead of BigInt(128)

Suggested change

const mask128 = (BigInt(1) << BigInt(128)) - BigInt(1);

const mask = (1n << 128n) - 1n

zmalatrax · 2024-08-07T13:00:03Z

src/hints/wideMul128.ts

+    try {
+        const mask128 = (BigInt(1) << BigInt(128)) - BigInt(1);
+
+        //Gets the operands' values


No need to add comments, the code is self-explanatory here (same // Do multiplication) etc.

zmalatrax · 2024-08-07T13:01:24Z

src/hints/wideMul128.ts

+import { resOperand, ResOperand } from 'hints/hintParamsSchema';
+import { VirtualMachine } from 'vm/virtualMachine';
+import { Felt } from 'primitives/felt';
+export const wideMul128Parser = z.


zmalatrax · 2024-08-07T13:03:09Z

src/hints/wideMul128.ts

+        low: low
+    }));
+
+export const wideMul128 = (


add a well-formatted JSDoc, take example on the AllocFelt252Dict one

zmalatrax · 2024-08-07T13:06:44Z

src/hints/wideMul128.ts

+        const highVal = prod >> BigInt(128);
+        const highRef = vm.cellRefToRelocatable(high);
+        vm.memory.assertEq(highRef, new Felt(highVal));


Directly create highVal as a Felt

No need to declare a variable for the address (i.e. highRef), directly call cellRefToRelocatable in assertEq()

zmalatrax · 2024-08-07T13:08:04Z

src/hints/wideMul128.ts

+    } catch (error: any) {
+        throw new Error(error.message);
+    }


Don't wrap the function in a try catch statement

zmalatrax · 2024-08-07T13:23:07Z

@D240021 You're not signing your commits ⚠️

zmalatrax · 2024-08-09T08:11:58Z

src/hints/math/wideMul128.test.ts

+      },
+      rhs: {
+        type: OpType.Immediate,
+        value: new Felt(BigInt('0x2')),


It can be simplified as new Felt(2n)

zmalatrax · 2024-08-09T08:14:56Z

src/hints/math/wideMul128.test.ts

+
+  test.each([
+    [new Felt(1n), new Felt(0n), new Felt(2n)], 
+    [new Felt(BigInt('0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF')), new Felt(1n), new Felt(BigInt('0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE'))], // max 128-bit value * 2


Hexadecimal is a bit hard to read with 128 bits, I'd rather compute it:
BigInt(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF) --> (1n << 128n) - 1
BigInt(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE) --> (1n << 128n) - 2

zmalatrax · 2024-08-09T08:17:54Z

src/hints/math/wideMul128.test.ts

+    const vm = new VirtualMachine();
+    vm.memory.addSegment();


The VirtualMachine instance comes with an empty memory: without any segment. Segments are 0-indexed

The AP register points to the segment 1, a.k.a the 2nd segment of the memory, you've got to call vm.memory.addSegment() twice

zmalatrax · 2024-08-09T08:18:58Z

src/hints/math/wideMul128.test.ts

+    const hint = wideMul128Parser.parse(WIDE_MUL_128);
+    const vm = new VirtualMachine();
+    vm.memory.addSegment();
+    vm.ap = vm.ap.add(1);


No need to increment ap for this test

Suggested change

vm.ap = vm.ap.add(1);

zmalatrax · 2024-08-09T08:22:58Z

src/hints/math/wideMul128.ts

+export type WideMul128 = z.infer<
+    typeof wideMul128Parser
+>;


Export the type directly after wideMul128Parser (before wideMul128())

zmalatrax · 2024-08-09T08:32:06Z

src/hints/math/wideMul128.ts

+    const highVal = prod >> BigInt(128);
+    vm.memory.assertEq(vm.cellRefToRelocatable(high), new Felt(highVal));
+
+    const lowVal = prod & mask;
+    const lowRef = vm.cellRefToRelocatable(low);
+    vm.memory.assertEq(lowRef, new Felt(lowVal));


use 128n instead of BigInt(128)
Directly instantiate highVal and lowVal as Felt

Group the computation and the memory assertion together: first compute highVal and lowVal
Then make assertEq calls.

Inline highRef and lowRef in the assertEq

zmalatrax · 2024-08-09T08:32:51Z

package-lock.json

+{
+  "name": "cairo-vm-ts",


Remove this file

Also bun.lockb shouldn't be modified

D240021 · 2024-08-09T20:06:07Z

Hi @zmalatrax, I've been correcting what you told me. Also, I added the cairo file for the integration test, I'm not very sure about it. I would appreciate if you could review it and give me feedback.

zmalatrax · 2024-08-14T06:45:19Z

.trunk/trunk.yaml

+  disabled:
+    - prettier


You shouldn't modify the Trunk configuration

zmalatrax · 2024-08-14T06:46:38Z

cairo_programs/cairo/hints/widemul128.cairo

+    let lhs: felt = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
+    let rhs: felt = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
+
+    let product = lhs * rhs;
+
+    let low = product & ((1 << 128) - 1);  
+    let high = product >> 128;             
+
+    return ();


You're making a multiplication of two 128 bits, stored on two limbs low and high but you're not actually using the WideMul128 hint.

You need to find a Corelib function that makes use of the WideMul128 hint:

Look at libfuncs defined in the cairo-lang-sierra-to-casm that uses the WideMul128 hint in their implementation
By searching on the cairo-lang-sierra-to-casm crate of the Cairo compiler, you can see that it is used in the u256 divmod, u512 divmod by u256 and the u128 multiplication libfuncs
The easiest libfunc directly using this hint is the u128 multiplication: build_u128_guarantee_mul which creates the u128_guarantee_mul

Search the u128_guaranteed_mul function in the Cairo corelib

Now you need to write a Cairo program that uses the function u128_guarantee_mul(), look at the function signature and do appropriately, something like this can be enough, as the U128 type operators have been overloaded:

fn main() { let _ = 0x123456_u128 * 0xFEDCBA_u128; }

When you're not sure about your program using a particular hint, compile it and look at the JSON compilation artifacts, go to the hints property and see if the wanted hint is part of it

zmalatrax · 2024-08-14T06:52:57Z

src/hints/hintSchema.ts

+import { wideMul128, wideMul128Parser } from './math/wideMul128';
 /** Zod object to parse any implemented hints */


missing blank line

zmalatrax · 2024-08-14T06:54:18Z

src/hints/math/wideMul128.ts

+ * This function calculates the product of two 128-bit operands, splits the result into high and low 128-bit parts,
+ * and stores them in the specified memory locations within the virtual machine.
+ *
+ * @param {VirtualMachine} vm - The virtual machine instance where the operation is executed.


Suggested change

* @param {VirtualMachine} vm - The virtual machine instance where the operation is executed.

* @param {VirtualMachine} vm - The virtual machine instance.

zmalatrax · 2024-10-07T15:27:34Z

Closing as the implementation has been moved to #137

Draft about initial implementation

d2e5d00

zmalatrax linked an issue Aug 7, 2024 that may be closed by this pull request

feat: add WideMul128 hint #124

Open

zmalatrax reviewed Aug 7, 2024

View reviewed changes

D240021 added 2 commits August 7, 2024 22:43

Handler and corrections added

b086616

Signed commit

1510539

zmalatrax reviewed Aug 9, 2024

View reviewed changes

D240021 added 3 commits August 9, 2024 13:52

Cairo file and corrections added

0345b36

Cairo file and corrections added

dc24035

Cairo file and corrections added

5c7f088

zmalatrax reviewed Aug 14, 2024

View reviewed changes

Cairo main function and corrections added

d2b9a27

zmalatrax closed this Oct 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add WideMul128 hint #126

feat: add WideMul128 hint #126

D240021 commented Aug 7, 2024 •

edited

Loading

zmalatrax left a comment •

edited

Loading

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax Aug 7, 2024

zmalatrax commented Aug 7, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

zmalatrax Aug 9, 2024

D240021 commented Aug 9, 2024

zmalatrax Aug 14, 2024

zmalatrax Aug 14, 2024

zmalatrax Aug 14, 2024

zmalatrax Aug 14, 2024

zmalatrax Aug 14, 2024

zmalatrax Aug 14, 2024

zmalatrax commented Oct 7, 2024

	const mask128 = (BigInt(1) << BigInt(128)) - BigInt(1);
	const mask = (1n << 128n) - 1n

		import { wideMul128, wideMul128Parser } from './math/wideMul128';
		/** Zod object to parse any implemented hints */

	* @param {VirtualMachine} vm - The virtual machine instance where the operation is executed.
	* @param {VirtualMachine} vm - The virtual machine instance.

feat: add WideMul128 hint #126

feat: add WideMul128 hint #126

Conversation

D240021 commented Aug 7, 2024 • edited Loading

zmalatrax left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmalatrax commented Aug 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

D240021 commented Aug 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmalatrax commented Oct 7, 2024

D240021 commented Aug 7, 2024 •

edited

Loading

zmalatrax left a comment •

edited

Loading