Fix padding and gas cost computation issues in precompiles #961
Conversation
nanocryk
changed the title
nanocryk
changed the title
| }); | ||
| } | ||
| let mut buf = [0u8; 32]; | ||
| let mut input_iter = input[start_inx..].iter().copied(); |
There was a problem hiding this comment.
| let mut input_iter = input[start_inx..].iter().copied(); | |
| buf[..input.len()].copy_from_slice(&input[..]); |
It also needs a check above that the input length is <= 32.
| let mut px_buf = [0u8; 32]; | ||
| let mut py_buf = [0u8; 32]; | ||
| let mut input_iter = input[start_inx..].iter().copied(); | ||
| px_buf.fill_with(|| input_iter.next().unwrap_or(0)); |
There was a problem hiding this comment.
Same as above, please use copy_with_slice.
| let mut mod_len_buf = [0u8; 32]; | ||
|
|
||
| let mut input_iter = input.iter().copied(); | ||
| base_len_buf.fill_with(|| input_iter.next().unwrap_or(0)); |
There was a problem hiding this comment.
Also same as above, use copy_with_slice.
| if mod_len == 0 { | ||
| return Ok(PrecompileOutput { | ||
| exit_status: ExitSucceed::Returned, | ||
| output: vec![], |
There was a problem hiding this comment.
Is this actually right? EIP-198 states:
returns an output (BASE**EXPONENT) % MODULUS as a byte array with the same length as the modulus.
So if I'm to understand the spec, this should be a number of zeros, instead of an empty array.
Also is this special case actually needed? L166 already has:
if base_len == 0 && mod_len == 0 {
There was a problem hiding this comment.
Base len is encoded before mod len, so a truncated input could have non-zero base len but zero mod len.
As the EIP states that it returns a byte array with the same length as the modulus, then if mod len is 0 the output should be zero len too.
I'm not too familiar with Go but this seems to return zero len array:
https://github.com/ethereum/go-ethereum/blob/v1.10.26/core/vm/contracts.go#L386
| let base_start = 96; // previous 3 32-byte fields | ||
| let base = BigUint::from_bytes_be(&input[base_start..base_start + base_len]); | ||
| let mut base_buf = vec![0u8; base_len]; | ||
| base_buf.fill_with(|| input_iter.next().unwrap_or(0)); |
There was a problem hiding this comment.
Same as above, please use copy_with_slice.
|
@sorpaas can we get an additional review from you? |
Precompiles modexp(5), ecadd(6) and ecmul(7) have inconsistencies compared to the yellowpaper and geth implementation.
&operator in the EIP2565. The correct behavior, also expressed in the yellowpaper, is to take the first 32 bytes of the exp input, while in Rust(exponent & (2**256 - 1))will keep the 32 last bytes. To avoid any confusion, I do like in the yellowpaper by looking directly at the first 32 bytes in the input, which is what is done in geth.