[move][natives] added batch bulletproof natives #15832

dovgopoly · 2025-01-28T13:20:12Z

Description

Adds Move native functions for batch verification of Bulletproof ZK range proofs.

What

This pull request introduces native Move functions that wrap the batch operations provided by the underlying dalek-cryptography's Bulletproofs library.

Why

The primary goal is to reduce gas overhead when multiple range proofs of the same structure are needed to be verified at once. By batching the proofs together, the system minimizes repetitive computational costs, thereby enhancing performance.

How

The implementation leverages the existing Rust logic from the Bulletproofs library by wrapping its prove_multiple and verify_multiple functions as native functions in Move.

Other fixes/contributions:

Fixes an outstanding issue in aptos-crypto that this PR was about to fix.
Adds a TEST_FILTER environment variable to filter unit tests when testing the Move framework code with cargo test inside aptos-move/framework

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2025-01-28T13:20:17Z

⏱️ 9m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
check-dynamic-deps	3m	🟩
permission-check	2m	🟥 🟥 🟩 🟩 🟩 (+28 more)
rust-cargo-deny	2m	🟩
permission-check	2m	🟥 🟥 🟩 🟩 🟩 (+28 more)
general-lints	27s	🟩
semgrep/ci	21s	🟩
file_change_determinator	13s	🟩

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

aptos-move/aptos-release-builder/src/components/feature_flags.rs

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move

aptos-move/framework/src/natives/cryptography/bulletproofs.rs

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move

aptos-move/framework/src/natives/cryptography/bulletproofs.rs

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move

alinush · 2025-01-31T11:20:08Z

aptos-move/framework/src/natives/cryptography/bulletproofs.rs

+) -> SafeNativeResult<SmallVec<[Value; 1]>> {
+    charge_gas(context, comm_points.len(), bit_length)?;
+
+    let range_proof = match bulletproofs::RangeProof::from_bytes(proof_bytes) {


Abort here with the same error code as before.

Also, in the native for individual proof verification, we charge for the deserialization.

We should do it here too.

Added error code. Need to discuss charging.

alinush · 2025-01-31T11:21:31Z

aptos-move/framework/src/natives/cryptography/bulletproofs.rs

@@ -91,6 +104,54 @@ fn native_verify_range_proof(
    verify_range_proof(context, &comm_point, &pg, &proof_bytes[..], num_bits, dst)
 }

+fn native_verify_batch_range_proof(


This needs to also charge the BULLETPROOFS_BASE base cost (see the native for individual proof verification).

@zjma do you know why this is missing / what happened here?

the batch verification implementation is multivariate and non-linear. I suggested we use a value table for gas charging, so BULLETPROOFS_BASE is not needed.

Can you double check with @vgao1996. I recall he always advises to have a base cost.

I think @vgao1996's point was: every native needs to have a non-zero cost, but that's it.

crates/aptos-crypto/Cargo.toml

alinush · 2025-01-31T11:24:34Z

crates/aptos-crypto/benches/bulletproofs_batch_vrfy.rs

Rename to *_verify.rs

crates/aptos-crypto/benches/bulletproofs_batch_vrfy.rs

crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs

scripts/algebra-gas/README.md

alinush · 2025-01-31T11:30:31Z

aptos-move/aptos-gas-schedule/src/gas_schedule/aptos_framework.rs

+        // Generated at time 1738278510.3867955 by `scripts/algebra-gas/update_bulletproofs_batch_vrfy_gas_params.py` with gas_per_ns=295.16.
+        [bulletproofs_verify_base_batch_1_bits_8: InternalGas, { RELEASE_V1_27.. => "bulletproofs.verify.base_batch_1_bits_8" }, 133_731_503],
+        [bulletproofs_verify_base_batch_1_bits_16: InternalGas, { RELEASE_V1_27.. => "bulletproofs.verify.base_batch_1_bits_16" }, 195_542_132],
+        [bulletproofs_verify_base_batch_1_bits_32: InternalGas, { RELEASE_V1_27.. => "bulletproofs.verify.base_batch_1_bits_32" }, 310_976_813],


This needs to be (somewhat) consistent with the cost of individually verifying a Bulletproof for a 32-bit committed value via verify_range_proof_internal, no?

There, the cost would be:

bulletproofs_base + 32 * bulletproofs_per_bit_rangeproof_verify + (some very small deserialization cost) = 11_794_651 + 32 * 1_004_253 = 43_930_747

But here the cost is 310_976_813 which is 7x larger.

alinush

Thank you for this PR!

I requested a bunch of changes (see comments), amongst which:

Please revert back the error code numbers. Otherwise, backwards compatibility issues.
Please simplify the Move code. There is unnecessarily repeated code (some of it ours).
Please make sure the batch verification native aborts when proof deserialization fails with the same code as the native for individual proof verification.
Please make sure the new batch verification native always charges BULLETPROOFS_BASE like the individual one (e.g., all the BLS12-381 natives charge the same base cost; see here)

Most importantly, this is a non-trivial change that requires a proper PR description. Please describe/include:

The what
The why
The how
Explain how gas costs were calibrated.
Benchmarks for the (Rust) batch proving times and verification times, for all possible batch-size / num-bits combinations. (e.g., see "Benchmarks" here)

zjma · 2025-02-03T20:41:31Z

aptos-move/framework/tests/move_unit_test.rs

+    let test_filter = std::env::var("TEST_FILTER").ok();
+    run_tests_for_pkg("aptos-framework", test_filter);
 }

 #[test]
 fn move_aptos_stdlib_unit_tests() {
-    run_tests_for_pkg("aptos-stdlib");
+    let test_filter = std::env::var("TEST_FILTER").ok();
+    run_tests_for_pkg("aptos-stdlib", test_filter);
 }

 #[test]
 fn move_stdlib_unit_tests() {
-    run_tests_for_pkg("move-stdlib");
+    let test_filter = std::env::var("TEST_FILTER").ok();
+    run_tests_for_pkg("move-stdlib", test_filter);
 }

 #[test]
 fn move_token_unit_tests() {
-    run_tests_for_pkg("aptos-token");
+    let test_filter = std::env::var("TEST_FILTER").ok();
+    run_tests_for_pkg("aptos-token", test_filter);
 }

 #[test]
 fn move_token_objects_unit_tests() {
-    run_tests_for_pkg("aptos-token-objects");
+    let test_filter = std::env::var("TEST_FILTER").ok();
+    run_tests_for_pkg("aptos-token-objects", test_filter);


better load the env inside run_tests_for_pkg then?

zjma · 2025-02-03T20:42:59Z

aptos-move/framework/tests/move_unit_test.rs

@@ -28,11 +28,14 @@ fn run_tests_for_pkg(path_to_pkg: impl Into<String>) {
        ..Default::default()
    };

+    let mut utc = UnitTestingConfig::default();
+    utc.filter = test_filter;


@alinush
utc.report_statistics, if set to true, prints the amount of gas used. Can also be very useful and worth a separate envvar.

zjma · 2025-02-04T01:58:02Z

i can take care of the rebase.

zjma reviewed Jan 28, 2025

View reviewed changes

aptos-move/aptos-release-builder/src/components/feature_flags.rs Outdated Show resolved Hide resolved

zjma reviewed Jan 28, 2025

View reviewed changes

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move Show resolved Hide resolved

zjma reviewed Jan 28, 2025

View reviewed changes

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move Outdated Show resolved Hide resolved

dovgopoly force-pushed the distributedlab/batch-rp-natives branch from cc4a57b to b07b143 Compare January 29, 2025 16:56

dovgopoly changed the title ~~[move][natives] added move bulletproof natives and rust wrappers (draft, wip)~~ [move][natives] added batch bulletproof natives Jan 29, 2025

dovgopoly marked this pull request as ready for review January 29, 2025 16:57

dovgopoly requested review from alinush, rex1fernando, davidiw, movekevin and wrwg as code owners January 29, 2025 16:57

[move][natives] added batch bulletproof natives

49abefc

dovgopoly force-pushed the distributedlab/batch-rp-natives branch from b07b143 to 49abefc Compare January 29, 2025 18:20

zjma reviewed Jan 29, 2025

View reviewed changes

aptos-move/framework/src/natives/cryptography/bulletproofs.rs Outdated Show resolved Hide resolved

dovgopoly added 3 commits January 29, 2025 21:59

fixes

f519f82

fixed gas charge

a2d075d

reordered

7701322

zjma reviewed Jan 30, 2025

View reviewed changes

aptos-move/framework/src/natives/cryptography/bulletproofs.rs Outdated Show resolved Hide resolved

zjma added 7 commits January 30, 2025 08:58

revert unexpected changes to existing natives

426641a

update

a3cb025

bench update

2c12006

update

40d7276

update

eaf5b1e

update

a9327b1

gas param update

7a5d527

alinush reviewed Jan 30, 2025

View reviewed changes

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move Outdated Show resolved Hide resolved

alinush reviewed Jan 30, 2025

View reviewed changes

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move Outdated Show resolved Hide resolved

alinush reviewed Jan 30, 2025

View reviewed changes

aptos-move/framework/aptos-stdlib/sources/cryptography/ristretto255_bulletproofs.move Show resolved Hide resolved

alinush reviewed Jan 30, 2025

View reviewed changes

aptos-move/framework/src/natives/cryptography/bulletproofs.rs Outdated Show resolved Hide resolved