[solana-bn254-syscall] Bn254 opt

[samkim-crypto]

Summary of Changes

Updated the syscall function API to take in PodG1 and PodG2 types directly. I think this results in a much cleaner interface (more consistent with the bls12-381 implementation) and simpler implementation since we don't need to worry about byte slicing and length checks.

One disadvantage is that length checks like this has to be taken care of by the caller, which will be in solana-syscalls. We will have to be careful to preserve the original behavior. For example:

• For G1 big-endian addition and multiplication, if input length is smaller than 128, there must be padding.

I also thought a bit about implementing Pod and Zeroable. I think (please correctly if I am wrong) if we didn't implement Pod, then we need to do something like:

let p_bytes: [u8; 64] = input[0..64].try_into().unwrap();
let q_bytes: [u8; 64] = input[64..128].try_into().unwrap();

let p = PodG1(p_bytes);
let q = PodG1(q_bytes);

But if we do implement Pod, then we can do:

let points: &[PodG1] = bytemuck::cast_slice(input);
let p = &points[0];
let q = &points[1];

In syscall, instead of the above, we do use translate_type for the conversion, but this is an unsafe function and it inherently assumes the Rust struct we are casting into has a perfectly predictable, contiguous byte layout with zero padding, which Pod and Zeroable guarantees. bytemuck is a pretty light dependency, so I don't think it hurts to have it.

But if you have strong feelings to remove Pod or Zeroable, then let me know @LStan. If we do remove it, then we probably want to do the same for bls12-381 as well.

[LStan]

One disadvantage is that length checks like this has to be taken care of by the caller, which will be in solana-syscalls. We will have to be careful to preserve the original behavior. For example:

* For G1 big-endian addition and multiplication, if input length is smaller than 128, there must be padding.

I have some doubts about this. I remember you planned to change input length check for BE from <= to ==. This will be a new variant in the Version enum. However, the logic for it will need to be implemented in solana-syscalls crate. Doesn't this kind of break encapsulation?

In syscall, instead of the above, we do use translate_type for the conversion, but this is an unsafe function and it inherently assumes the Rust struct we are casting into has a perfectly predictable, contiguous byte layout with zero padding, which Pod and Zeroable guarantees. bytemuck is a pretty light dependency, so I don't think it hurts to have it.

Why does the code for bls12-381 use translate_type instead of cast_slice? If translate_type is unsafe, why isn't it marked as unsafe?

[zz-sol]

LGTM

[zz-sol]

i think it may be more efficient to build two vectors here; so later on you don't need to iterate it twice to build iters.