Fix ALT_BN128_MULTIPLICATION_INPUT_LEN constant

[LStan]

Problem

Input for alt_bn128_multiplication (Scalar multiplication in G1) consist of 1 point in G1 (64 bytes), 1 scalar (BigInteger256 - 32 bytes). So 96 bytes in total, not 128.

Summary of Changes

Changed ALT_BN128_MULTIPLICATION_INPUT_LEN to 96

[samkim-crypto]

Sorry for the delay on this and thanks for the contribution! This is indeed something that I missed.

Unfortunately, this change will technically incur a breaking change to the behavior of the runtime, which can lead to break in consensus. These type of changes will generally need to be feature gated (example) Unfortunately, the functions in the bn254 module itself do not have access to the invoke context, which contains information about which feature gates that are activated. This makes the required changes slightly more involved:

1. Define two versions of the alt_bn128_multiplication functions: one with ALT_BN128_MULTIPLICATION_INPUT_LEN = 128 and the other with ALT_BN128_MULTIPLICATION_INPUT_LEN = 96.
2. Define a new feature gate in the feature-set crate.
3. In this line, we should update the function to be either of the two multiplication functions defined above depending on whether the new feature gate is active or not.

Since the changes are somewhat involved, if you are okay with it, then I can commit the changes outlined above. If you are willing to take a shot, then that is fine as well, so please let me know!

[steviez]

2. Define a new feature gate in the feature-set crate.

More so than this, changes to runtime behavior need to go through a SIMD so other client teams (Firedancer, etc) can be aware of changes and implement corresponding changes as well

[LStan]

I want to try to do it, but I have some questions about naming:

1. How to name the second function? alt_bn128_multiplication_fix or alt_bn128_multiplication_fix_input_len or something else?
2. How to name the feature? enable_alt_bn128_multiplication_fix?
3. Where to get id for the feature?

About SIMD. I don't know, how to do it. Firedancer also checks for 128: https://github.com/firedancer-io/firedancer/blob/513e3250c6154390c070920d0d76ef6876154820/src/ballet/bn254/fd_bn254.c#L192-L194

[samkim-crypto]

1. How to name the second function? alt_bn128_multiplication_fix or alt_bn128_multiplication_fix_input_len or something else?

For this, how about we create a non-public function alt_bn128_apply_multiplication(input: &[u8], expected_length: usize) -> Result<Vec<u8>, AltBn128Error> that is identical to the original alt_bn128_multiplication(input: &[u8]), but takes in the expected length as input.

Then, let's make alt_bn128_multiplication(input: &[u8]) invoke alt_bn128_apply_multiplication with the correct length, and define a new function alt_bn128_multiplication_128(input: &[u8]) that invokes alt_bn128_apply_multiplication on the original input of 128.

Technically, we are making a breaking change to alt_bn128_multiplication, which is a public function, but it is not used anywhere else in the run time, so it should be okay.

[samkim-crypto]

2. How to name the feature? enable_alt_bn128_multiplication_fix?

Let's name it fix_alt_bn128_multiplication_input_length.

For the SIMD, I'll go ahead and create it.

3. Where to get id for the feature?

Usually, one of the core contributors generate a keypair and use the corresponding public key. For this, let's use bn2puAyxUx6JUabAxYdKdJ5QHbNNmKw8dCGuGCyRrFN.

[mergify]

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

[LStan]

I've added the feature. Please check

[samkim-crypto]

Looks great! Just some minor comments below. Let me create a SIMD and let you know.

[samkim-crypto]

Let me work on the SIMD. Once the SIMD is merged, let's plug the SIMD number in here.