Conversation
- Add sha2 dependency (optional) - Add 'solana' feature that enables SHA-256 instead of Keccak-256 - Conditionally select hash function based on feature flag - Default behavior (Keccak-256) unchanged for EVM compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Introduces and EVM feature flag with an optional sha3 dependency Makes it a default for testing
Now operations that bar recovery IDs above 1 or necessitate translation to 27/28 instead of usual 1, 2 for IDs under a specified flag
XuyangSong
left a comment
There was a problem hiding this comment.
thank you for the work
the changes make sense to me
| let cu_instances: Vec<Vec<u8>> = cus.iter().map(|cu| cu.instance.clone()).collect(); | ||
| let cu_instances: Vec<Vec<u8>> = cus | ||
| .iter() | ||
| .map(|cu| cu.instance.to_journal().unwrap()) |
There was a problem hiding this comment.
was there a particular reason you took out the error handler here?
There was a problem hiding this comment.
Not in particular, just to keep stable the current core types
But if you like the change, I can also handle errors properly
| #[cfg(not(feature = "evm"))] | ||
| let recid_byte = self.recid.to_byte(); | ||
|
|
There was a problem hiding this comment.
Since you've already added the solana feature, would "solana" be clearer than "not evm"?
There was a problem hiding this comment.
I was thinking about this and this is basically future-oriented. We may have more chains with more quirks and this one particularly has to be avoided for the evm and not other chains as the recovery ID trick seems to be very EVM specific. So I thought a not would be more suitable here
| #[cfg(not(feature = "evm"))] | ||
| let recid_byte = bytes[64]; |
Currently the compliance instance field is in a serialized format. This PR makes the field store the instance in a deserialized manner, minimizing dependency on the top-level zkvm package.