[secp256k1-verify, program] Refactor out instruction constructor into solana-secp256k1-verify#16
Conversation
| default = ["keccak"] | ||
| default = ["verify", "instruction", "keccak"] | ||
| verify = ["dep:solana-secp256k1-recover", "dep:solana-define-syscall"] | ||
| instruction = ["dep:solana-instruction", "dep:solana-pubkey"] |
There was a problem hiding this comment.
If developers just wants to use the instruction constructor function (e.g. to CPI into the secp256k1 program) in their program, it doesn't make sense for their program to import crates like solana-secp256k1-recover, solana-keccak-hasher, etc, so I scoped the instruction constructor function behind instruction feature.
zz-sol
left a comment
zz-sol
left a comment
There was a problem hiding this comment.
LGTM. just two minor comments.
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks good to me overall! Just some naming nits
There was a problem hiding this comment.
nit: can we put this in a file called instruction.rs instead?
|
|
||
| /// Constructs an on-chain instruction to invoke `solana-secp256k1-program`. | ||
| pub fn secp256k1_verify_instruction( | ||
| program_id: &Address, |
There was a problem hiding this comment.
Can we define a default program id at the top-level of the repo?
There was a problem hiding this comment.
Did you mean at the top of the crate or repository? Do you have an example of how it is done at the repo level 🙏 ?
| use {alloc::vec, alloc::vec::Vec, solana_address::Address, solana_instruction::Instruction}; | ||
|
|
||
| /// Constructs an on-chain instruction to invoke `solana-secp256k1-program`. | ||
| pub fn secp256k1_verify_instruction( |
There was a problem hiding this comment.
nit: can probably just call this verify for simplicity, if it's moved into instruction.rs, the import looks pretty neat as solana_secp256k1_verify::instruction::verify
samkim-crypto
force-pushed
the
refactor-verify
branch
from
aa4e80e to
e4fded3
Compare
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great! This can go in as is, the program id bit isn't necessary.
3 participants
Addressing this, this, and this comments.
I refactored out the instruction constructor to the
solana-secp256k1-verifycrate and made the program fully no-std.Update: Also updated
PubkeywithAddress.