solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
12.79k stars 4.04k forks source link

zk syscalls are untested #23746

Open jackcmay opened 2 years ago

jackcmay commented 2 years ago

Problem

QSP-4

The following system calls are not tested: • sol_zk_token_elgamal_op; • sol_zk_token_elgamal_op_with_lo_hi; and • sol_zk_token_elgamal_op_with_scalar.

Proposed Solution

Add tests

jackcmay commented 2 years ago

A security auditors brought this up, @samkim-crypto. can we get these tests in?

samkim-crypto commented 2 years ago

These syscalls have been removed (26311) and replaced by the curve25519 syscalls.

jackcmay commented 2 years ago

@samkim-crypto can you point me to the tests for the curve25519 syscalls?

samkim-crypto commented 2 years ago

@jackcmay The curve operations use the dalek library implementation. Currently, there are just tests for native code in the zk-token-sdk

These syscalls have not been activated yet. If there are places that require more tests before activation, then I'd be happy to add more in.

jackcmay commented 2 years ago

Glad to hear there are tests for the curve operations themselves. We should also have tests for the syscalls themselves (in/out param handling, compute unit consumption, etc...). Take a look at the test for the other syscalls, they are located at the bottom of the file that declares/defines the syscalls

joncinque commented 12 months ago

We'll definitely want these tests if they don't exist -- @samkim-crypto do the tests exist now?