Skip to content

Commit

Permalink
engine-modexp bug fixes and performance improvements with unusual e…
Browse files Browse the repository at this point in the history
…xponents (#814)

## Description

- Fixes a panic with empty exponent
- Ensure correct result with zero exponent
- Fix performance issues with exponents with large zero padding

## Performance / NEAR gas cost considerations

The performance is increased for some cases (those with exponents with a
large amount of leading zeroes) and remains the same for all other
cases.

## Testing

Fuzzing and tests added.

## How should this be reviewed

## Additional information

---------

Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
  • Loading branch information
guidovranken and aleksuss authored Aug 7, 2023
1 parent 2e1f1a0 commit 0cfda46
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
45 changes: 43 additions & 2 deletions engine-modexp/src/mpnat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ pub struct MPNat {
}

impl MPNat {
fn strip_leading_zeroes(a: &[u8]) -> (&[u8], bool) {
let len = a.len();
let end = a.iter().position(|&x| x != 0).unwrap_or(len);

if end == len {
(&[], true)
} else {
(&a[end..], false)
}
}

// Koç's algorithm for inversion mod 2^k
// https://eprint.iacr.org/2017/411.pdf
fn koc_2017_inverse(aa: &Self, k: usize) -> Self {
Expand Down Expand Up @@ -143,8 +154,24 @@ impl MPNat {

/// Computes `self ^ exp mod modulus`. `exp` must be given as big-endian bytes.
pub fn modpow(&mut self, exp: &[u8], modulus: &Self) -> Self {
if exp.iter().all(|x| x == &0) {
return Self { digits: vec![1] };
// exp must be stripped because it is iterated over in
// big_wrapping_pow and modpow_montgomery, and a large
// zero-padded exp leads to performance issues.
let (exp, exp_is_zero) = Self::strip_leading_zeroes(exp);

// base^0 is always 1, regardless of base.
// Hence the result is 0 for (base^0) % 1, and 1
// for every modulus larger than 1.
//
// The case of modulus being 0 should have already been
// handled in modexp().
debug_assert!(!(modulus.digits.len() == 1 && modulus.digits[0] == 0));
if exp_is_zero {
if modulus.digits.len() == 1 && modulus.digits[0] == 1 {
return Self { digits: vec![0] };
} else {
return Self { digits: vec![1] };
}
}

if exp.len() <= core::mem::size_of::<usize>() {
Expand Down Expand Up @@ -604,6 +631,20 @@ fn test_modpow_even() {
"023f4f762936eb0973d46b6eadb59d68d06101"
);

// Test empty exp
let base = hex::decode("00").unwrap();
let exponent = hex::decode("").unwrap();
let modulus = hex::decode("02").unwrap();
let result = crate::modexp(&base, &exponent, &modulus);
assert_eq!(hex::encode(result), "01");

// Test zero exp
let base = hex::decode("00").unwrap();
let exponent = hex::decode("00").unwrap();
let modulus = hex::decode("02").unwrap();
let result = crate::modexp(&base, &exponent, &modulus);
assert_eq!(hex::encode(result), "01");

fn check_modpow_even(base: u128, exp: u128, modulus: u128, expected: u128) {
let mut x = MPNat::from_big_endian(&base.to_be_bytes());
let m = MPNat::from_big_endian(&modulus.to_be_bytes());
Expand Down
2 changes: 1 addition & 1 deletion engine-tests/src/tests/repro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn repro_Emufid2() {
block_timestamp: 1_662_118_048_636_713_538,
input_path: "src/tests/res/input_Emufid2.hex",
evm_gas_used: 1_156_364,
near_gas_used: 296,
near_gas_used: 282,
});
}

Expand Down

0 comments on commit 0cfda46

Please sign in to comment.