[program] Clean up secp256k1 interface#11
Conversation
samkim-crypto
force-pushed
the
secp256k1-interface
branch
from
f2a119d to
43f5648
Compare
samkim-crypto
force-pushed
the
secp256k1-interface
branch
2 times, most recently
from
1dc26f5 to
ecf08b7
Compare
| mod verify; | ||
|
|
||
| #[cfg(any(target_os = "solana", target_arch = "bpf"))] | ||
| mod syscall; |
There was a problem hiding this comment.
I am not completely sure if we need this module or not yet. If I invoke syscalls using the syscall wrapper crates from the sdk, I keep getting a jump offset error. For now, I created a syscall module and invoke the syscalls directly within, but I'll dig into why this is causing the issue and address it in a follow-up.
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Just a couple of fly-by suggestions, looks great overall
| HASHED_PUBKEY_SERIALIZED_SIZE, SECP256K1_PRIVATE_KEY_SIZE, SECP256K1_PUBKEY_SIZE, | ||
| SECP256K1_UNCOMPRESSED_PUBKEY_SIZE, SIGNATURE_OFFSETS_SERIALIZED_SIZE, | ||
| SIGNATURE_SERIALIZED_SIZE, | ||
| #![cfg_attr(any(target_os = "solana", target_arch = "bpf"), no_std)] |
There was a problem hiding this comment.
We should be able to make this no-std in all situations, no?
| @@ -9,31 +9,20 @@ crate-type = ["cdylib", "rlib"] | |||
| [features] | |||
| default = [] | |||
| no-entrypoint = [] | |||
There was a problem hiding this comment.
Can be fixed in a follow-up, but this isn't currently used, so I don't think anyone can actually import this crate.
Either way, maybe we should just include the instruction creator in solana-secp256k1-verify?
3 participants
Summary of Changes
We are going to deprecate the precompiles, so we don't need our program to be backwards compatible with the existing precompiles any more.
I removed the lengthy offset parsing logic in the program and simplified the tests. The program is now just one file that interprets the pubkey, signatures, and message from the instruction data and just verifies the program.
I also used pinocchio framework for the program, so it should be pretty light weight.
It seems like the program is using about 25,352k CUs for a single verification. The
secp256k1-recoversyscall itself requires 25,000 CUs (here, so this is pretty close to the theoretical limit. In follow-up PRs, let me double check if we can shave off a little more from the 352 CUs.I also need to update the README for the repository, but I'll do this in a follow-up PR.