Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cherrypick precompiles fixes #148

Merged
merged 13 commits into from
Jan 16, 2023
Merged

Cherrypick precompiles fixes #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cd30e82
v can only be 27 or 28 prefixed only with zeros (#964)
nanocryk Jan 7, 2023
c378c7a
check input size is a multiple of 192 (like in geth) (#963)
nanocryk Jan 7, 2023
1270c4f
pad input if too short
nanocryk Jan 4, 2023
61e76f4
pad input in modexp
nanocryk Jan 6, 2023
bfc955f
pad all input fields + fix wrong test
nanocryk Jan 6, 2023
750f52d
fix modexp gas cost
nanocryk Jan 6, 2023
16abc4e
remove dbg!
nanocryk Jan 6, 2023
ac68b77
remove redundant arg + import vec!
nanocryk Jan 6, 2023
b77ebce
use copy_from_slice
nanocryk Jan 9, 2023
4960882
clippy
nanocryk Jan 9, 2023
84e13b4
clippy
nanocryk Jan 12, 2023
83d2f5a
fix template compilation
librelois Jan 16, 2023
192b63b
more ecrecover tests
nanocryk Jan 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 31 additions & 19 deletions frame/evm/precompile/bn128/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,37 +26,43 @@ use fp_evm::{
};
use sp_core::U256;

fn read_fr(input: &[u8], start_inx: usize) -> Result<bn::Fr, PrecompileFailure> {
if input.len() < start_inx + 32 {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("Input not long enough".into()),
});
/// Copy bytes from input to target.
fn read_input(source: &[u8], target: &mut [u8], offset: usize) {
// Out of bounds, nothing to copy.
if source.len() <= offset {
return;
}

bn::Fr::from_slice(&input[start_inx..(start_inx + 32)]).map_err(|_| PrecompileFailure::Error {
// Find len to copy up to target len, but not out of bounds.
let len = core::cmp::min(target.len(), source.len() - offset);
target[..len].copy_from_slice(&source[offset..][..len]);
}

fn read_fr(input: &[u8], start_inx: usize) -> Result<bn::Fr, PrecompileFailure> {
let mut buf = [0u8; 32];
read_input(input, &mut buf, start_inx);

bn::Fr::from_slice(&buf).map_err(|_| PrecompileFailure::Error {
exit_status: ExitError::Other("Invalid field element".into()),
})
}

fn read_point(input: &[u8], start_inx: usize) -> Result<bn::G1, PrecompileFailure> {
use bn::{AffineG1, Fq, Group, G1};

if input.len() < start_inx + 64 {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("Input not long enough".into()),
});
}
let mut px_buf = [0u8; 32];
let mut py_buf = [0u8; 32];
read_input(input, &mut px_buf, start_inx);
read_input(input, &mut py_buf, start_inx + 32);

let px = Fq::from_slice(&input[start_inx..(start_inx + 32)]).map_err(|_| {
PrecompileFailure::Error {
exit_status: ExitError::Other("Invalid point x coordinate".into()),
}
let px = Fq::from_slice(&px_buf).map_err(|_| PrecompileFailure::Error {
exit_status: ExitError::Other("Invalid point x coordinate".into()),
})?;
let py = Fq::from_slice(&input[(start_inx + 32)..(start_inx + 64)]).map_err(|_| {
PrecompileFailure::Error {
exit_status: ExitError::Other("Invalid point y coordinate".into()),
}

let py = Fq::from_slice(&py_buf).map_err(|_| PrecompileFailure::Error {
exit_status: ExitError::Other("Invalid point y coordinate".into()),
})?;

Ok(if px == Fq::zero() && py == Fq::zero() {
G1::zero()
} else {
Expand Down Expand Up @@ -173,6 +179,12 @@ impl Precompile for Bn128Pairing {
handle.record_cost(Bn128Pairing::BASE_GAS_COST)?;
U256::one()
} else {
if handle.input().len() % 192 > 0 {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("bad elliptic curve pairing size".into()),
});
}

// (a, b_a, b_b - each 64-byte affine coordinates)
let elements = handle.input().len() / 192;

Expand Down
162 changes: 100 additions & 62 deletions frame/evm/precompile/modexp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

extern crate alloc;

use alloc::vec::Vec;
use core::{cmp::max, ops::BitAnd};
use alloc::{vec, vec::Vec};
use core::cmp::max;

use num::{BigUint, FromPrimitive, One, ToPrimitive, Zero};

Expand All @@ -38,9 +38,9 @@ const MIN_GAS_COST: u64 = 200;
// https://eips.ethereum.org/EIPS/eip-2565
fn calculate_gas_cost(
base_length: u64,
exp_length: u64,
mod_length: u64,
exponent: &BigUint,
exponent_bytes: &[u8],
) -> u64 {
fn calculate_multiplication_complexity(base_length: u64, mod_length: u64) -> u64 {
let max_length = max(base_length, mod_length);
Expand All @@ -56,18 +56,15 @@ fn calculate_gas_cost(
words * words
}

fn calculate_iteration_count(exp_length: u64, exponent: &BigUint) -> u64 {
fn calculate_iteration_count(exponent: &BigUint, exponent_bytes: &[u8]) -> u64 {
let mut iteration_count: u64 = 0;
let exp_length = exponent_bytes.len() as u64;

if exp_length <= 32 && exponent.is_zero() {
iteration_count = 0;
} else if exp_length <= 32 {
iteration_count = exponent.bits() - 1;
} else if exp_length > 32 {
// construct BigUint to represent (2^256) - 1
let bytes: [u8; 32] = [0xFF; 32];
let max_256_bit_uint = BigUint::from_bytes_be(&bytes);

// from the EIP spec:
// (8 * (exp_length - 32)) + ((exponent & (2**256 - 1)).bit_length() - 1)
//
Expand All @@ -77,21 +74,42 @@ fn calculate_gas_cost(
// must be > 0)
// * the addition can't overflow because the terms are both capped at roughly
// 8 * max size of exp_length (1024)
iteration_count =
(8 * (exp_length - 32)) + exponent.bitand(max_256_bit_uint).bits() - 1;
// * the EIP spec is written in python, in which (exponent & (2**256 - 1)) takes the
// FIRST 32 bytes. However this `BigUint` `&` operator takes the LAST 32 bytes.
// We thus instead take the bytes manually.
let exponent_head = BigUint::from_bytes_be(&exponent_bytes[..32]);

iteration_count = (8 * (exp_length - 32)) + exponent_head.bits() - 1;
}

max(iteration_count, 1)
}

let multiplication_complexity = calculate_multiplication_complexity(base_length, mod_length);
let iteration_count = calculate_iteration_count(exp_length, exponent);
let iteration_count = calculate_iteration_count(exponent, exponent_bytes);
max(
MIN_GAS_COST,
multiplication_complexity * iteration_count / 3,
)
}

/// Copy bytes from input to target.
fn read_input(source: &[u8], target: &mut [u8], source_offset: &mut usize) {
// We move the offset by the len of the target, regardless of what we
// actually copy.
let offset = *source_offset;
*source_offset += target.len();

// Out of bounds, nothing to copy.
if source.len() <= offset {
return;
}

// Find len to copy up to target len, but not out of bounds.
let len = core::cmp::min(target.len(), source.len() - offset);
target[..len].copy_from_slice(&source[offset..][..len]);
}

// ModExp expects the following as inputs:
// 1) 32 bytes expressing the length of base
// 2) 32 bytes expressing the length of exponent
Expand All @@ -111,35 +129,35 @@ fn calculate_gas_cost(
impl Precompile for Modexp {
fn execute(handle: &mut impl PrecompileHandle) -> PrecompileResult {
let input = handle.input();
let mut input_offset = 0;

if input.len() < 96 {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("input must contain at least 96 bytes".into()),
});
};
// Yellowpaper: whenever the input is too short, the missing bytes are
// considered to be zero.
let mut base_len_buf = [0u8; 32];
read_input(input, &mut base_len_buf, &mut input_offset);
let mut exp_len_buf = [0u8; 32];
read_input(input, &mut exp_len_buf, &mut input_offset);
let mut mod_len_buf = [0u8; 32];
read_input(input, &mut mod_len_buf, &mut input_offset);

// reasonable assumption: this must fit within the Ethereum EVM's max stack size
let max_size_big = BigUint::from_u32(1024).expect("can't create BigUint");

let mut buf = [0; 32];
buf.copy_from_slice(&input[0..32]);
let base_len_big = BigUint::from_bytes_be(&buf);
let base_len_big = BigUint::from_bytes_be(&base_len_buf);
if base_len_big > max_size_big {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("unreasonably large base length".into()),
});
}

buf.copy_from_slice(&input[32..64]);
let exp_len_big = BigUint::from_bytes_be(&buf);
let exp_len_big = BigUint::from_bytes_be(&exp_len_buf);
if exp_len_big > max_size_big {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("unreasonably large exponent length".into()),
});
}

buf.copy_from_slice(&input[64..96]);
let mod_len_big = BigUint::from_bytes_be(&buf);
let mod_len_big = BigUint::from_bytes_be(&mod_len_buf);
if mod_len_big > max_size_big {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("unreasonably large modulus length".into()),
Expand All @@ -151,11 +169,11 @@ impl Precompile for Modexp {
let exp_len = exp_len_big.to_usize().expect("exp_len out of bounds");
let mod_len = mod_len_big.to_usize().expect("mod_len out of bounds");

// input length should be at least 96 + user-specified length of base + exp + mod
let total_len = base_len + exp_len + mod_len + 96;
if input.len() < total_len {
return Err(PrecompileFailure::Error {
exit_status: ExitError::Other("insufficient input size".into()),
// if mod_len is 0 output must be empty
if mod_len == 0 {
return Ok(PrecompileOutput {
exit_status: ExitSucceed::Returned,
output: vec![],
});
}

Expand All @@ -165,21 +183,22 @@ impl Precompile for Modexp {
BigUint::zero()
} else {
// read the numbers themselves.
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];
read_input(input, &mut base_buf, &mut input_offset);
let base = BigUint::from_bytes_be(&base_buf);

let exp_start = base_start + base_len;
let exponent = BigUint::from_bytes_be(&input[exp_start..exp_start + exp_len]);
let mut exp_buf = vec![0u8; exp_len];
read_input(input, &mut exp_buf, &mut input_offset);
let exponent = BigUint::from_bytes_be(&exp_buf);

let mut mod_buf = vec![0u8; mod_len];
read_input(input, &mut mod_buf, &mut input_offset);
let modulus = BigUint::from_bytes_be(&mod_buf);

// do our gas accounting
let gas_cost =
calculate_gas_cost(base_len as u64, exp_len as u64, mod_len as u64, &exponent);
let gas_cost = calculate_gas_cost(base_len as u64, mod_len as u64, &exponent, &exp_buf);

handle.record_cost(gas_cost)?;
let input = handle.input();

let mod_start = exp_start + exp_len;
let modulus = BigUint::from_bytes_be(&input[mod_start..mod_start + mod_len]);

if modulus.is_zero() || modulus.is_one() {
BigUint::zero()
Expand Down Expand Up @@ -228,7 +247,7 @@ mod tests {
}

#[test]
fn test_empty_input() -> Result<(), PrecompileFailure> {
fn test_empty_input() {
let input = Vec::new();

let cost: u64 = 1;
Expand All @@ -242,25 +261,17 @@ mod tests {
let mut handle = MockHandle::new(input, Some(cost), context);

match Modexp::execute(&mut handle) {
Ok(_) => {
panic!("Test not expected to pass");
Ok(precompile_result) => {
assert_eq!(precompile_result.output.len(), 0);
}
Err(e) => {
assert_eq!(
e,
PrecompileFailure::Error {
exit_status: ExitError::Other(
"input must contain at least 96 bytes".into()
)
}
);
Ok(())
Err(_) => {
panic!("Modexp::execute() returned error"); // TODO: how to pass error on?
}
}
}

#[test]
fn test_insufficient_input() -> Result<(), PrecompileFailure> {
fn test_insufficient_input() {
let input = hex::decode(
"0000000000000000000000000000000000000000000000000000000000000001\
0000000000000000000000000000000000000000000000000000000000000001\
Expand All @@ -279,17 +290,12 @@ mod tests {
let mut handle = MockHandle::new(input, Some(cost), context);

match Modexp::execute(&mut handle) {
Ok(_) => {
panic!("Test not expected to pass");
Ok(precompile_result) => {
assert_eq!(precompile_result.output.len(), 1);
assert_eq!(precompile_result.output, vec![0x00]);
}
Err(e) => {
assert_eq!(
e,
PrecompileFailure::Error {
exit_status: ExitError::Other("insufficient input size".into())
}
);
Ok(())
Err(_) => {
panic!("Modexp::execute() returned error"); // TODO: how to pass error on?
}
}
}
Expand Down Expand Up @@ -474,4 +480,36 @@ mod tests {
let expected = BigUint::parse_bytes(b"0", 10).unwrap();
assert_eq!(result, expected);
}

#[test]
fn test_long_exp_gas_cost_matches_specs() {
let input = vec![
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 38, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 96, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
16, 0, 0, 0, 255, 255, 255, 2, 0, 0, 179, 0, 0, 2, 0, 0, 122, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 255, 251, 0, 0, 0, 0, 4, 38, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 96, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 16, 0, 0, 0, 255, 255, 255, 2, 0, 0, 179, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255,
255, 255, 255, 249,
];

let context: Context = Context {
address: Default::default(),
caller: Default::default(),
apparent_value: From::from(0),
};

let mut handle = MockHandle::new(input, Some(100_000), context);

let _ = Modexp::execute(&mut handle).expect("Modexp::execute() returned error");

assert_eq!(handle.gas_used, 7104); // gas used when ran in geth
}
}
12 changes: 9 additions & 3 deletions frame/evm/precompile/simple/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,15 @@ impl LinearCostPrecompile for ECRecover {
let mut sig = [0u8; 65];

msg[0..32].copy_from_slice(&input[0..32]);
sig[0..32].copy_from_slice(&input[64..96]);
sig[32..64].copy_from_slice(&input[96..128]);
sig[64] = input[63];
sig[0..32].copy_from_slice(&input[64..96]); // r
sig[32..64].copy_from_slice(&input[96..128]); // s
sig[64] = input[63]; // v

// v can only be 27 or 28 on the full 32 bytes value.
// https://github.com/ethereum/go-ethereum/blob/a907d7e81aaeea15d80b2d3209ad8e08e3bf49e0/core/vm/contracts.go#L177
if input[32..63] != [0u8; 31] || ![27, 28].contains(&input[63]) {
return Ok((ExitSucceed::Returned, [0u8; 0].to_vec()));
}

let result = match sp_io::crypto::secp256k1_ecdsa_recover(&sig, &msg) {
Ok(pubkey) => {
Expand Down
Loading