From 5c5c3fc97b73e041a48f0bdd5023dfe35f4d2101 Mon Sep 17 00:00:00 2001 From: Pia Date: Mon, 22 Apr 2024 21:54:27 +0900 Subject: [PATCH 1/5] memory location update --- contracts/perf.md | 32 +++++++++++++++++++++++++++++ contracts/src/GrandSumVerifier.sol | 26 +++++++++++------------ contracts/src/InclusionVerifier.sol | 24 +++++++++++----------- 3 files changed, 57 insertions(+), 25 deletions(-) create mode 100644 contracts/perf.md diff --git a/contracts/perf.md b/contracts/perf.md new file mode 100644 index 00000000..0f4ede24 --- /dev/null +++ b/contracts/perf.md @@ -0,0 +1,32 @@ +### Gas report + +Before: +| **Category** | **Contract/Method** | **Min** | **Max** | **Avg** | **# calls** | **usd (avg)** | **% of limit** | +|--------------|--------------------------------------------|---------|---------|---------|-------------|---------------|----------------| +| Methods | | | | | | | | +| | GrandSumVerifier - verifyProof | - | - | 271155 | 2 | - | | +| | Summa - submitCommitment | - | - | 1068958 | 5 | - | | +| | Summa - submitProofOfAddressOwnership | - | - | 700479 | 3 | - | | +| Deployments | | | | | | | | +| | GrandSumVerifier | - | - | 277127 | | - | 0.9% | +| | InclusionVerifier | - | - | 284887 | | - | 0.9% | +| | src/DummyVerifyingKey.sol:Halo2VerifyingKey | - | - | 266321 | | - | 0.9% | +| | src/VerifyingKey.sol:Halo2VerifyingKey | - | - | 350335 | | - | 1.2% | +| | Summa | - | - | 1961848 | | - | 6.5% | +| | Verifier | - | - | 1907494 | | - | 6.4% | + +after: + +| **Category** | **Contract/Method** | **Min** | **Max** | **Avg** | **# calls** | **usd (avg)** | **% of limit** | +| ------------ | ------------------------------------------- | ------- | ------- | ------- | ----------- | ------------- | -------------- | +| Methods | | | | | | | | +| | GrandSumVerifier - verifyProof | - | - | 271140 | 2 | - | | +| | Summa - submitCommitment | - | - | 1068943 | 5 | - | | +| | Summa - submitProofOfAddressOwnership | - | - | 700479 | 3 | - | | +| Deployments | | | | | | | | +| | GrandSumVerifier | - | - | 277115 | | - | 0.9% | +| | InclusionVerifier | - | - | 284923 | | - | 0.9% | +| | src/DummyVerifyingKey.sol:Halo2VerifyingKey | - | - | 266321 | | - | 0.9% | +| | src/VerifyingKey.sol:Halo2VerifyingKey | - | - | 350335 | | - | 1.2% | +| | Summa | - | - | 1961848 | | - | 6.5% | +| | Verifier | - | - | 1907494 | | - | 6.4% | diff --git a/contracts/src/GrandSumVerifier.sol b/contracts/src/GrandSumVerifier.sol index cfd2f3df..14ad16e0 100644 --- a/contracts/src/GrandSumVerifier.sol +++ b/contracts/src/GrandSumVerifier.sol @@ -9,19 +9,19 @@ contract GrandSumVerifier { // Memory positions for the verifying key. // The memory location starts at 0x200 due to the maximum operation on the ec_pairing function being 0x180, marking the maximum memory location used - uint256 internal constant N_INV_MPTR = 0x220; - uint256 internal constant LHS_X_MPTR = 0x240; - uint256 internal constant LHS_Y_MPTR = 0x260; - uint256 internal constant G1_X_MPTR = 0x280; - uint256 internal constant G1_Y_MPTR = 0x2a0; - uint256 internal constant G2_X_1_MPTR = 0x2c0; - uint256 internal constant G2_X_2_MPTR = 0x2e0; - uint256 internal constant G2_Y_1_MPTR = 0x300; - uint256 internal constant G2_Y_2_MPTR = 0x320; - uint256 internal constant NEG_S_G2_X_1_MPTR = 0x340; - uint256 internal constant NEG_S_G2_X_2_MPTR = 0x360; - uint256 internal constant NEG_S_G2_Y_1_MPTR = 0x380; - uint256 internal constant NEG_S_G2_Y_2_MPTR = 0x3a0; + uint256 internal constant N_INV_MPTR = 0x180; + uint256 internal constant LHS_X_MPTR = 0x1a0; + uint256 internal constant LHS_Y_MPTR = 0x1c0; + uint256 internal constant G1_X_MPTR = 0x1e0; + uint256 internal constant G1_Y_MPTR = 0x200; + uint256 internal constant G2_X_1_MPTR = 0x220; + uint256 internal constant G2_X_2_MPTR = 0x240; + uint256 internal constant G2_Y_1_MPTR = 0x260; + uint256 internal constant G2_Y_2_MPTR = 0x280; + uint256 internal constant NEG_S_G2_X_1_MPTR = 0x2a0; + uint256 internal constant NEG_S_G2_X_2_MPTR = 0x2c0; + uint256 internal constant NEG_S_G2_Y_1_MPTR = 0x2e0; + uint256 internal constant NEG_S_G2_Y_2_MPTR = 0x300; diff --git a/contracts/src/InclusionVerifier.sol b/contracts/src/InclusionVerifier.sol index e84b41cc..0c28301d 100644 --- a/contracts/src/InclusionVerifier.sol +++ b/contracts/src/InclusionVerifier.sol @@ -8,18 +8,18 @@ contract InclusionVerifier { // Memory positions for the verifying key. // The memory location starts at 0x200 due to the maximum operation on the ec_pairing function being 0x180. - uint256 internal constant LHS_X_MPTR = 0x200; - uint256 internal constant LHS_Y_MPTR = 0x220; - uint256 internal constant G1_X_MPTR = 0x240; - uint256 internal constant G1_Y_MPTR = 0x260; - uint256 internal constant G2_X_1_MPTR = 0x280; - uint256 internal constant G2_X_2_MPTR = 0x2a0; - uint256 internal constant G2_Y_1_MPTR = 0x2c0; - uint256 internal constant G2_Y_2_MPTR = 0x2e0; - uint256 internal constant NEG_S_G2_X_1_MPTR = 0x300; - uint256 internal constant NEG_S_G2_X_2_MPTR = 0x320; - uint256 internal constant NEG_S_G2_Y_1_MPTR = 0x340; - uint256 internal constant NEG_S_G2_Y_2_MPTR = 0x360; + uint256 internal constant LHS_X_MPTR = 0x180; + uint256 internal constant LHS_Y_MPTR = 0x1a0; + uint256 internal constant G1_X_MPTR = 0x1c0; + uint256 internal constant G1_Y_MPTR = 0x1e0; + uint256 internal constant G2_X_1_MPTR = 0x200; + uint256 internal constant G2_X_2_MPTR = 0x220; + uint256 internal constant G2_Y_1_MPTR = 0x240; + uint256 internal constant G2_Y_2_MPTR = 0x260; + uint256 internal constant NEG_S_G2_X_1_MPTR = 0x280; + uint256 internal constant NEG_S_G2_X_2_MPTR = 0x2a0; + uint256 internal constant NEG_S_G2_Y_1_MPTR = 0x2c0; + uint256 internal constant NEG_S_G2_Y_2_MPTR = 0x2e0; function verifyProof( address vk, From 4b30f06ae8bfcaf8d3373f659d4e92239b1faed4 Mon Sep 17 00:00:00 2001 From: Pia Date: Sat, 27 Apr 2024 22:16:01 +0900 Subject: [PATCH 2/5] smol fixes --- contracts/src/GrandSumVerifier.sol | 6 +++++- contracts/src/InclusionVerifier.sol | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/contracts/src/GrandSumVerifier.sol b/contracts/src/GrandSumVerifier.sol index 14ad16e0..6b232fa5 100644 --- a/contracts/src/GrandSumVerifier.sol +++ b/contracts/src/GrandSumVerifier.sol @@ -141,6 +141,10 @@ contract GrandSumVerifier { let lhs_x := calldataload(commitment_proof_pos) // C_X let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) + if iszero(success) { + mstore(0, "EC addition failed") + revert(0, 0x20) + } // Store LHS_X and LHS_Y to memory mstore(LHS_X_MPTR, mload(0x80)) @@ -152,7 +156,7 @@ contract GrandSumVerifier { let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y - success := and(success, ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y)) + success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) } // Return 1 as result if everything succeeds diff --git a/contracts/src/InclusionVerifier.sol b/contracts/src/InclusionVerifier.sol index 0c28301d..ff695257 100644 --- a/contracts/src/InclusionVerifier.sol +++ b/contracts/src/InclusionVerifier.sol @@ -115,7 +115,8 @@ contract InclusionVerifier { // The proof length should match 4 times the length of the evaluation values. success := and(success, eq(4, div(proof_length, mul(evaluation_values_length, 0x20)))) if iszero(success) { - revert(0, 0) + mstore(0, "Number of evaluation mismatch") + revert(0, 0x20) } for { let i := 0 } lt(i, evaluation_values_length) { i := add(i, 1) } { @@ -142,9 +143,13 @@ contract InclusionVerifier { let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) if iszero(success) { + // mstore(0, "EC addition failed") + // revert(0, 0x20) + // This line does not revert invalid inclusion proof. Why? revert(0, 0) } + // Store LHS_X and LHS_Y to memory mstore(LHS_X_MPTR, mload(0x80)) mstore(LHS_Y_MPTR, mload(0xa0)) @@ -154,7 +159,7 @@ contract InclusionVerifier { let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y - success := and(success, ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y)) + success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) } // Return 1 as result if everything succeeds From 5a4610bda02263472b7c03222d802c14ed5d7209 Mon Sep 17 00:00:00 2001 From: Pia Date: Sat, 27 Apr 2024 22:45:36 +0900 Subject: [PATCH 3/5] smol fixes --- contracts/src/GrandSumVerifier.sol | 17 ++++++++++++++--- contracts/src/InclusionVerifier.sol | 20 +++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/contracts/src/GrandSumVerifier.sol b/contracts/src/GrandSumVerifier.sol index 6b232fa5..05983f15 100644 --- a/contracts/src/GrandSumVerifier.sol +++ b/contracts/src/GrandSumVerifier.sol @@ -130,20 +130,25 @@ contract GrandSumVerifier { // Assign values on memory for multiplication mstore(0x80, mload(G1_X_MPTR)) mstore(0xa0, mload(G1_Y_MPTR)) - success := and(success, ec_mul_tmp(success, minus_z)) + success := ec_mul_tmp(success, minus_z) + if iszero(success) { + revert(0, 0) + } // Performaing `c_g_to_minus_z := c + g_to_minus_z` // `c` is equivalent to `commitment` as input on the `open_grand_sums` function. // the values of 'g_to_minus_z` is already located at 0x80 and 0xa0 in the previous step let commitment_proof_pos := add(add(PROOF_CPTR, div(proof_length, 2)), double_shift_pos) success := check_ec_point(success, commitment_proof_pos, q) + if iszero(success) { + revert(0, 0) + } let lhs_x := calldataload(commitment_proof_pos) // C_X let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) if iszero(success) { - mstore(0, "EC addition failed") - revert(0, 0x20) + revert(0, 0) } // Store LHS_X and LHS_Y to memory @@ -153,10 +158,16 @@ contract GrandSumVerifier { // Checking from calldata for grand sum proof let proof_pos := add(PROOF_CPTR, double_shift_pos) success := check_ec_point(success, proof_pos, q) + if iszero(success) { + revert(0, 0) + } let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) + if iszero(success) { + revert(0, 0) + } } // Return 1 as result if everything succeeds diff --git a/contracts/src/InclusionVerifier.sol b/contracts/src/InclusionVerifier.sol index ff695257..b5c1a415 100644 --- a/contracts/src/InclusionVerifier.sol +++ b/contracts/src/InclusionVerifier.sol @@ -100,7 +100,8 @@ contract InclusionVerifier { let challenges_length := calldataload(challenges_length_pos) success := and(success, eq(challenges_length, 4)) if iszero(success) { - revert(0, 0) + mstore(0, "Number of challenge mismatch") + revert(0, 0x20) } mstore(NEG_S_G2_X_1_MPTR, calldataload(add(challenges_length_pos, 0x20))) @@ -130,7 +131,10 @@ contract InclusionVerifier { mstore(0x80, mload(G1_X_MPTR)) mstore(0xa0, mload(G1_Y_MPTR)) mstore(0xc0, minus_z) - success := and(success, ec_mul_tmp(success, minus_z)) + success := ec_mul_tmp(success, minus_z) + if iszero(success) { + revert(0, 0) + } // Performaing like `c_g_to_minus_z = c + g_to_minus_z` in `verify_kzg_proof` function that is located in `amortized_kzg.rs`. // @@ -138,14 +142,14 @@ contract InclusionVerifier { // The values of 'g_to_minus_z` is already located at 0x80 and 0xa0 in the previous step let commitment_proof_pos := add(add(PROOF_CPTR, div(proof_length, 2)), double_shift_pos) success := check_ec_point(success, commitment_proof_pos, q) + if iszero(success) { + revert(0, 0) + } let lhs_x := calldataload(commitment_proof_pos) // C_X let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) if iszero(success) { - // mstore(0, "EC addition failed") - // revert(0, 0x20) - // This line does not revert invalid inclusion proof. Why? revert(0, 0) } @@ -156,10 +160,16 @@ contract InclusionVerifier { // Checking from calldata let proof_pos := add(PROOF_CPTR, double_shift_pos) success := check_ec_point(success, proof_pos, q) + if iszero(success) { + revert(0, 0) + } let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) + if iszero(success) { + revert(0, 0) + } } // Return 1 as result if everything succeeds From 486bdefa0958a137714bf26b8443bf617dcefb77 Mon Sep 17 00:00:00 2001 From: Pia Date: Sat, 27 Apr 2024 22:53:23 +0900 Subject: [PATCH 4/5] add error --- contracts/src/GrandSumVerifier.sol | 19 ++++++++++++------- contracts/src/InclusionVerifier.sol | 22 ++++++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/contracts/src/GrandSumVerifier.sol b/contracts/src/GrandSumVerifier.sol index 05983f15..19a8f026 100644 --- a/contracts/src/GrandSumVerifier.sol +++ b/contracts/src/GrandSumVerifier.sol @@ -103,7 +103,7 @@ contract GrandSumVerifier { success := and(success, eq(0, mod(proof_length, 0x80))) if iszero(success) { mstore(0, "Invalid proof length") - revert(0, 0x20) + revert(0, 0x15) } // Load the length of evaluation values, positioned after the proof data. @@ -114,7 +114,7 @@ contract GrandSumVerifier { success := and(success, eq(4, div(proof_length, mul(evaluation_values_length, 0x20)))) if iszero(success) { mstore(0, "Number of evaluation mismatch") - revert(0, 0x20) + revert(0, 0x1F) } for { let i := 0 } lt(i, evaluation_values_length) { i := add(i, 1) } { @@ -132,7 +132,8 @@ contract GrandSumVerifier { mstore(0xa0, mload(G1_Y_MPTR)) success := ec_mul_tmp(success, minus_z) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } // Performaing `c_g_to_minus_z := c + g_to_minus_z` @@ -141,14 +142,16 @@ contract GrandSumVerifier { let commitment_proof_pos := add(add(PROOF_CPTR, div(proof_length, 2)), double_shift_pos) success := check_ec_point(success, commitment_proof_pos, q) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } let lhs_x := calldataload(commitment_proof_pos) // C_X let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } // Store LHS_X and LHS_Y to memory @@ -159,14 +162,16 @@ contract GrandSumVerifier { let proof_pos := add(PROOF_CPTR, double_shift_pos) success := check_ec_point(success, proof_pos, q) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } } diff --git a/contracts/src/InclusionVerifier.sol b/contracts/src/InclusionVerifier.sol index b5c1a415..03f9fb19 100644 --- a/contracts/src/InclusionVerifier.sol +++ b/contracts/src/InclusionVerifier.sol @@ -92,7 +92,8 @@ contract InclusionVerifier { // Ensure the proof length is divisible by `0x80`, accommodating the structured data layout. success := and(success, eq(0, mod(proof_length, 0x80))) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid proof length") + revert(0, 0x15) } // Load the NEG_S_G2 point with the calculated point @@ -101,7 +102,7 @@ contract InclusionVerifier { success := and(success, eq(challenges_length, 4)) if iszero(success) { mstore(0, "Number of challenge mismatch") - revert(0, 0x20) + revert(0, 0x1E) } mstore(NEG_S_G2_X_1_MPTR, calldataload(add(challenges_length_pos, 0x20))) @@ -117,7 +118,7 @@ contract InclusionVerifier { success := and(success, eq(4, div(proof_length, mul(evaluation_values_length, 0x20)))) if iszero(success) { mstore(0, "Number of evaluation mismatch") - revert(0, 0x20) + revert(0, 0x1F) } for { let i := 0 } lt(i, evaluation_values_length) { i := add(i, 1) } { @@ -133,7 +134,8 @@ contract InclusionVerifier { mstore(0xc0, minus_z) success := ec_mul_tmp(success, minus_z) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } // Performaing like `c_g_to_minus_z = c + g_to_minus_z` in `verify_kzg_proof` function that is located in `amortized_kzg.rs`. @@ -143,14 +145,16 @@ contract InclusionVerifier { let commitment_proof_pos := add(add(PROOF_CPTR, div(proof_length, 2)), double_shift_pos) success := check_ec_point(success, commitment_proof_pos, q) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } let lhs_x := calldataload(commitment_proof_pos) // C_X let lhs_y := calldataload(add(commitment_proof_pos, 0x20)) // C_Y success := ec_add_tmp(success, lhs_x, lhs_y) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } // Store LHS_X and LHS_Y to memory @@ -161,14 +165,16 @@ contract InclusionVerifier { let proof_pos := add(PROOF_CPTR, double_shift_pos) success := check_ec_point(success, proof_pos, q) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } let rhs_x := calldataload(proof_pos) // PI_X let rhs_y := calldataload(add(proof_pos, 0x20)) // PI_Y success := ec_pairing(success, mload(LHS_X_MPTR), mload(LHS_Y_MPTR), rhs_x, rhs_y) if iszero(success) { - revert(0, 0) + mstore(0, "Invalid ec computation") + revert(0, 0x17) } } From 7c823269268f466f8f07d41cde5b9389ac73d88d Mon Sep 17 00:00:00 2001 From: Pia Date: Sat, 27 Apr 2024 22:57:38 +0900 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=A4=B7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/perf.md | 32 -------------------------------- 1 file changed, 32 deletions(-) delete mode 100644 contracts/perf.md diff --git a/contracts/perf.md b/contracts/perf.md deleted file mode 100644 index 0f4ede24..00000000 --- a/contracts/perf.md +++ /dev/null @@ -1,32 +0,0 @@ -### Gas report - -Before: -| **Category** | **Contract/Method** | **Min** | **Max** | **Avg** | **# calls** | **usd (avg)** | **% of limit** | -|--------------|--------------------------------------------|---------|---------|---------|-------------|---------------|----------------| -| Methods | | | | | | | | -| | GrandSumVerifier - verifyProof | - | - | 271155 | 2 | - | | -| | Summa - submitCommitment | - | - | 1068958 | 5 | - | | -| | Summa - submitProofOfAddressOwnership | - | - | 700479 | 3 | - | | -| Deployments | | | | | | | | -| | GrandSumVerifier | - | - | 277127 | | - | 0.9% | -| | InclusionVerifier | - | - | 284887 | | - | 0.9% | -| | src/DummyVerifyingKey.sol:Halo2VerifyingKey | - | - | 266321 | | - | 0.9% | -| | src/VerifyingKey.sol:Halo2VerifyingKey | - | - | 350335 | | - | 1.2% | -| | Summa | - | - | 1961848 | | - | 6.5% | -| | Verifier | - | - | 1907494 | | - | 6.4% | - -after: - -| **Category** | **Contract/Method** | **Min** | **Max** | **Avg** | **# calls** | **usd (avg)** | **% of limit** | -| ------------ | ------------------------------------------- | ------- | ------- | ------- | ----------- | ------------- | -------------- | -| Methods | | | | | | | | -| | GrandSumVerifier - verifyProof | - | - | 271140 | 2 | - | | -| | Summa - submitCommitment | - | - | 1068943 | 5 | - | | -| | Summa - submitProofOfAddressOwnership | - | - | 700479 | 3 | - | | -| Deployments | | | | | | | | -| | GrandSumVerifier | - | - | 277115 | | - | 0.9% | -| | InclusionVerifier | - | - | 284923 | | - | 0.9% | -| | src/DummyVerifyingKey.sol:Halo2VerifyingKey | - | - | 266321 | | - | 0.9% | -| | src/VerifyingKey.sol:Halo2VerifyingKey | - | - | 350335 | | - | 1.2% | -| | Summa | - | - | 1961848 | | - | 6.5% | -| | Verifier | - | - | 1907494 | | - | 6.4% |