From 0cfda4686dbdb7a57b2dc18dddc5106ec8e24a38 Mon Sep 17 00:00:00 2001
From: Guido Vranken <guidovranken@users.noreply.github.com>
Date: Mon, 7 Aug 2023 08:41:01 +0200
Subject: [PATCH] `engine-modexp` bug fixes and performance improvements with
 unusual exponents (#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>
---
 engine-modexp/src/mpnat.rs      | 45 +++++++++++++++++++++++++++++++--
 engine-tests/src/tests/repro.rs |  2 +-
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/engine-modexp/src/mpnat.rs b/engine-modexp/src/mpnat.rs
index 76c523017..917030a6c 100644
--- a/engine-modexp/src/mpnat.rs
+++ b/engine-modexp/src/mpnat.rs
@@ -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 {
@@ -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>() {
@@ -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());
diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs
index fbc58722b..a113826fb 100644
--- a/engine-tests/src/tests/repro.rs
+++ b/engine-tests/src/tests/repro.rs
@@ -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,
     });
 }