Chore: Disallow as_conversions (#561)

Fix feature gated import in hash precompile Change missed featured `into` to `try_into` birchmd fixes Cleaner usize conversion error Fix missing vec in hash Fix tests Fix V check Fix testnet env EVM account Change hex feature from alloc to std Improve u64 to u8 conversion in JSON Cargo fmt
aurora-is-near · Aug 18, 2022 · c4d659b · c4d659b
1 parent d13c3cd
commit c4d659b
Show file tree

Hide file tree

Showing 32 changed files with 261 additions and 166 deletions.
diff --git a/.env/local.env b/.env/local.env
@@ -1,6 +1,6 @@
-CARGO_FEATURES_BUILD = "testnet"
-RUSTC_FLAGS_BUILD = "-C link-arg=-s"
-NEAR_EVM_ACCOUNT = "aurora.test.near"
-WASM_FILE = "aurora-local.wasm"
-NEAR_CLI = "local-near"
-IS_PROD = false
+CARGO_FEATURES_BUILD="testnet"
+RUSTC_FLAGS_BUILD="-C link-arg=-s"
+NEAR_EVM_ACCOUNT="aurora.test.near"
+WASM_FILE="aurora-local.wasm"
+NEAR_CLI="local_near"
+IS_PROD=false
diff --git a/.env/mainnet.env b/.env/mainnet.env
@@ -1,9 +1,9 @@
-CARGO_FEATURES_BUILD = "mainnet"
-CARGO_FEATURES_BUILD_TEST = "mainnet,integration-test"
-CARGO_FEATURES_TEST = "mainnet-test"
-RUSTC_FLAGS_BUILD = "-C link-arg=-s"
-NEAR_EVM_ACCOUNT = "aurora"
-WASM_FILE = "aurora-mainnet.wasm"
-WASM_FILE_TEST = "aurora-mainnet-test.wasm"
-NEAR_CLI = "near"
-IS_PROD = true
+CARGO_FEATURES_BUILD="mainnet"
+CARGO_FEATURES_BUILD_TEST="mainnet,integration-test"
+CARGO_FEATURES_TEST="mainnet-test"
+RUSTC_FLAGS_BUILD="-C link-arg=-s"
+NEAR_EVM_ACCOUNT="aurora"
+WASM_FILE="aurora-mainnet.wasm"
+WASM_FILE_TEST="aurora-mainnet-test.wasm"
+NEAR_CLI="near"
+IS_PROD=true
diff --git a/.env/testnet.env b/.env/testnet.env
@@ -1,9 +1,9 @@
-CARGO_FEATURES_BUILD = "testnet"
-CARGO_FEATURES_BUILD_TEST = "testnet,integration-test"
-CARGO_FEATURES_TEST = "testnet-test"
-RUSTC_FLAGS_BUILD = "-C link-arg=-s"
-NEAR_EVM_ACCOUNT = "aurora"
-WASM_FILE = "aurora-testnet.wasm"
-WASM_FILE_TEST = "aurora-testnet-test.wasm"
-NEAR_CLI = "near"
-IS_PROD = false
+CARGO_FEATURES_BUILD="testnet"
+CARGO_FEATURES_BUILD_TEST="testnet,integration-test"
+CARGO_FEATURES_TEST="testnet-test"
+RUSTC_FLAGS_BUILD="-C link-arg=-s"
+NEAR_EVM_ACCOUNT="aurora"
+WASM_FILE="aurora-testnet.wasm"
+WASM_FILE_TEST="aurora-testnet-test.wasm"
+NEAR_CLI="near"
+IS_PROD=false
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/engine-precompiles/src/blake2.rs b/engine-precompiles/src/blake2.rs
@@ -103,7 +103,7 @@ fn f(mut h: [u64; 8], m: [u64; 16], t: [u64; 2], f: bool, rounds: u32) -> Vec<u8
     for i in 0..rounds {
         // Typically twelve rounds for blake2b.
         // Message word selection permutation for this round.
-        let s = &consts::SIGMA[i as usize % 10];
+        let s = &consts::SIGMA[usize::try_from(i).expect("Round can convert to usize") % 10];
         g(&mut v, 0, 4, 8, 12, m[s[0]], m[s[1]]);
         g(&mut v, 1, 5, 9, 13, m[s[2]], m[s[3]]);
         g(&mut v, 2, 6, 10, 14, m[s[4]], m[s[5]]);

diff --git a/engine-precompiles/src/bn128.rs b/engine-precompiles/src/bn128.rs
@@ -1,5 +1,7 @@
+use crate::bn128::consts::PAIR_ELEMENT_LEN;
 use crate::prelude::types::{Address, EthGas};
 use crate::prelude::{Borrowed, PhantomData, Vec};
+use crate::utils;
 use crate::{Byzantium, EvmPrecompileResult, HardFork, Istanbul, Precompile, PrecompileOutput};
 use evm::{Context, ExitError};
 
@@ -353,8 +355,10 @@ impl<HF: HardFork> Bn128Pair<HF> {
 
 impl Precompile for Bn128Pair<Byzantium> {
     fn required_gas(input: &[u8]) -> Result<EthGas, ExitError> {
+        let input_len = u64::try_from(input.len()).map_err(utils::err_usize_conv)?;
+        let pair_element_len = u64::try_from(PAIR_ELEMENT_LEN).map_err(utils::err_usize_conv)?;
         Ok(
-            costs::BYZANTIUM_PAIR_PER_POINT * input.len() / consts::PAIR_ELEMENT_LEN
+            costs::BYZANTIUM_PAIR_PER_POINT * input_len / pair_element_len
                 + costs::BYZANTIUM_PAIR_BASE,
         )
     }
@@ -384,8 +388,10 @@ impl Precompile for Bn128Pair<Byzantium> {
 
 impl Precompile for Bn128Pair<Istanbul> {
     fn required_gas(input: &[u8]) -> Result<EthGas, ExitError> {
+        let input_len = u64::try_from(input.len()).map_err(utils::err_usize_conv)?;
+        let pair_element_len = u64::try_from(PAIR_ELEMENT_LEN).map_err(utils::err_usize_conv)?;
         Ok(
-            costs::ISTANBUL_PAIR_PER_POINT * input.len() / consts::PAIR_ELEMENT_LEN
+            costs::ISTANBUL_PAIR_PER_POINT * input_len / pair_element_len
                 + costs::ISTANBUL_PAIR_BASE,
         )
     }

diff --git a/engine-precompiles/src/hash.rs b/engine-precompiles/src/hash.rs
@@ -2,7 +2,7 @@
 use crate::prelude::sdk;
 use crate::prelude::types::{Address, EthGas};
 use crate::prelude::vec;
-use crate::{EvmPrecompileResult, Precompile, PrecompileOutput};
+use crate::{utils, EvmPrecompileResult, Precompile, PrecompileOutput};
 use evm::{Context, ExitError};
 
 mod costs {
@@ -32,8 +32,9 @@ impl SHA256 {
 
 impl Precompile for SHA256 {
     fn required_gas(input: &[u8]) -> Result<EthGas, ExitError> {
+        let input_len = u64::try_from(input.len()).map_err(utils::err_usize_conv)?;
         Ok(
-            (input.len() as u64 + consts::SHA256_WORD_LEN - 1) / consts::SHA256_WORD_LEN
+            (input_len + consts::SHA256_WORD_LEN - 1) / consts::SHA256_WORD_LEN
                 * costs::SHA256_PER_WORD
                 + costs::SHA256_BASE,
         )
@@ -104,8 +105,9 @@ impl RIPEMD160 {
 
 impl Precompile for RIPEMD160 {
     fn required_gas(input: &[u8]) -> Result<EthGas, ExitError> {
+        let input_len = u64::try_from(input.len()).map_err(utils::err_usize_conv)?;
         Ok(
-            (input.len() as u64 + consts::RIPEMD_WORD_LEN - 1) / consts::RIPEMD_WORD_LEN
+            (input_len + consts::RIPEMD_WORD_LEN - 1) / consts::RIPEMD_WORD_LEN
                 * costs::RIPEMD160_PER_WORD
                 + costs::RIPEMD160_BASE,
         )

diff --git a/engine-precompiles/src/identity.rs b/engine-precompiles/src/identity.rs
@@ -1,5 +1,5 @@
 use crate::prelude::types::{Address, EthGas};
-use crate::{EvmPrecompileResult, Precompile, PrecompileOutput};
+use crate::{utils, EvmPrecompileResult, Precompile, PrecompileOutput};
 use evm::{Context, ExitError};
 
 /// Identity precompile costs.
@@ -26,8 +26,9 @@ impl Identity {
 
 impl Precompile for Identity {
     fn required_gas(input: &[u8]) -> Result<EthGas, ExitError> {
+        let input_len = u64::try_from(input.len()).map_err(utils::err_usize_conv)?;
         Ok(
-            (input.len() as u64 + consts::IDENTITY_WORD_LEN - 1) / consts::IDENTITY_WORD_LEN
+            (input_len + consts::IDENTITY_WORD_LEN - 1) / consts::IDENTITY_WORD_LEN
                 * costs::IDENTITY_PER_WORD
                 + costs::IDENTITY_BASE,
         )

diff --git a/engine-precompiles/src/lib.rs b/engine-precompiles/src/lib.rs
@@ -1,6 +1,7 @@
 #![allow(dead_code)]
 #![cfg_attr(not(feature = "std"), no_std)]
 #![cfg_attr(not(feature = "std"), feature(alloc_error_handler))]
+#![deny(clippy::as_conversions)]
 
 pub mod account_ids;
 pub mod blake2;
@@ -13,7 +14,6 @@ mod prelude;
 pub mod prepaid_gas;
 pub mod random;
 pub mod secp256k1;
-#[cfg(test)]
 mod utils;
 
 use crate::account_ids::{predecessor_account, CurrentAccount, PredecessorAccount};

diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs
@@ -1,7 +1,8 @@
-use crate::prelude::{PhantomData, Vec, U256};
-use crate::{Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput};
-
 use crate::prelude::types::{Address, EthGas};
+use crate::prelude::{PhantomData, Vec, U256};
+use crate::{
+    utils, Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput,
+};
 use evm::{Context, ExitError};
 use num::{BigUint, Integer};
 
@@ -17,31 +18,33 @@ impl<HF: HardFork> ModExp<HF> {
 
 impl<HF: HardFork> ModExp<HF> {
     // Note: the output of this function is bounded by 2^67
-    fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> U256 {
-        #[allow(clippy::redundant_closure)]
+    fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> Result<U256, ExitError> {
+        let start = usize::try_from(base_len).map_err(utils::err_usize_conv)?;
+        let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?;
+        // #[allow(clippy::redundant_closure)]
         let exp = parse_bytes(
             bytes,
-            (base_len as usize).saturating_add(96),
-            core::cmp::min(32, exp_len as usize),
+            start.saturating_add(96),
+            core::cmp::min(32, exp_len),
             // I don't understand why I need a closure here, but doesn't compile without one
             |x| U256::from(x),
         );
 
         if exp_len <= 32 && exp.is_zero() {
-            U256::zero()
+            Ok(U256::zero())
         } else if exp_len <= 32 {
-            U256::from(exp.bits()) - U256::from(1)
+            Ok(U256::from(exp.bits()) - U256::from(1))
         } else {
             // else > 32
-            U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1)
+            Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1))
         }
     }
 
     fn run_inner(input: &[u8]) -> Result<Vec<u8>, ExitError> {
         let (base_len, exp_len, mod_len) = parse_lengths(input);
-        let base_len = base_len as usize;
-        let exp_len = exp_len as usize;
-        let mod_len = mod_len as usize;
+        let base_len = usize::try_from(base_len).map_err(utils::err_usize_conv)?;
+        let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?;
+        let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?;
 
         let base_start = 96;
         let base_end = base_len.saturating_add(base_start);
@@ -99,7 +102,7 @@ impl Precompile for ModExp<Byzantium> {
         let (base_len, exp_len, mod_len) = parse_lengths(input);
 
         let mul = Self::mul_complexity(core::cmp::max(mod_len, base_len));
-        let iter_count = Self::calc_iter_count(exp_len, base_len, input);
+        let iter_count = Self::calc_iter_count(exp_len, base_len, input)?;
         // mul * iter_count bounded by 2^195 < 2^256 (no overflow)
         let gas = mul * core::cmp::max(iter_count, U256::one()) / U256::from(20);
 
@@ -141,7 +144,7 @@ impl Precompile for ModExp<Berlin> {
         let (base_len, exp_len, mod_len) = parse_lengths(input);
 
         let mul = Self::mul_complexity(base_len, mod_len);
-        let iter_count = Self::calc_iter_count(exp_len, base_len, input);
+        let iter_count = Self::calc_iter_count(exp_len, base_len, input)?;
         // mul * iter_count bounded by 2^189 (so no overflow)
         let gas = mul * iter_count / U256::from(3);
 

diff --git a/engine-precompiles/src/utils.rs b/engine-precompiles/src/utils.rs
@@ -1,9 +1,18 @@
+use crate::prelude::Borrowed;
+use core::num::TryFromIntError;
+#[cfg(test)]
 use evm::Context;
+use evm_core::ExitError;
 
+#[cfg(test)]
 pub fn new_context() -> Context {
     Context {
         address: Default::default(),
         caller: Default::default(),
         apparent_value: Default::default(),
     }
 }
+
+pub fn err_usize_conv(_e: TryFromIntError) -> ExitError {
+    ExitError::Other(Borrowed("ERR_USIZE_CONVERSION"))
+}
diff --git a/engine-sdk/src/near_runtime.rs b/engine-sdk/src/near_runtime.rs
@@ -6,7 +6,7 @@ use aurora_engine_types::parameters::{PromiseAction, PromiseBatchAction, Promise
 use aurora_engine_types::types::PromiseResult;
 use aurora_engine_types::H256;
 
-#[cfg(feature = "mainnet")]
+#[cfg(all(feature = "mainnet", not(feature = "testnet")))]
 /// The mainnet eth_custodian address 0x6BFaD42cFC4EfC96f529D786D643Ff4A8B89FA52
 const CUSTODIAN_ADDRESS: &[u8] = &[
     107, 250, 212, 44, 252, 78, 252, 150, 245, 41, 215, 134, 214, 67, 255, 74, 139, 137, 250, 82,

diff --git a/engine-standalone-storage/src/sync/types.rs b/engine-standalone-storage/src/sync/types.rs
@@ -142,7 +142,7 @@ impl<'a> From<&'a TransactionMessage> for BorshableTransactionMessage<'a> {
 }
 
 impl<'a> TryFrom<BorshableTransactionMessage<'a>> for TransactionMessage {
-    type Error = aurora_engine_transactions::ParseTransactionError;
+    type Error = aurora_engine_transactions::Error;
 
     fn try_from(t: BorshableTransactionMessage<'a>) -> Result<Self, Self::Error> {
         Ok(Self {
@@ -223,7 +223,7 @@ impl<'a> From<&'a TransactionKind> for BorshableTransactionKind<'a> {
 }
 
 impl<'a> TryFrom<BorshableTransactionKind<'a>> for TransactionKind {
-    type Error = aurora_engine_transactions::ParseTransactionError;
+    type Error = aurora_engine_transactions::Error;
 
     fn try_from(t: BorshableTransactionKind<'a>) -> Result<Self, Self::Error> {
         match t {

diff --git a/engine-standalone-tracing/Cargo.toml b/engine-standalone-tracing/Cargo.toml
@@ -21,6 +21,7 @@ evm-core = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.
 evm = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.36.0-aurora", default-features = false, features = ["std", "tracing"] }
 evm-runtime = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.36.0-aurora", default-features = false, features = ["std", "tracing"] }
 evm-gasometer = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.36.0-aurora", default-features = false, features = ["std", "tracing"] }
+hex = { version = "0.4", default-features = false, features = ["std"] }
 serde = { version = "1", features = ["derive"], optional = true }
 
 [features]

diff --git a/engine-tests/src/test_utils/erc20.rs b/engine-tests/src/test_utils/erc20.rs
@@ -1,5 +1,6 @@
 use crate::prelude::{transactions::legacy::TransactionLegacy, Address, U256};
 use crate::test_utils::solidity;
+use aurora_engine_transactions::NormalizedEthTransaction;
 use std::path::{Path, PathBuf};
 use std::sync::Once;
 
@@ -153,3 +154,18 @@ impl ERC20 {
         }
     }
 }
+
+pub(crate) fn legacy_into_normalized_tx(tx: TransactionLegacy) -> NormalizedEthTransaction {
+    NormalizedEthTransaction {
+        address: Default::default(),
+        chain_id: None,
+        nonce: tx.nonce,
+        gas_limit: tx.gas_limit,
+        max_priority_fee_per_gas: tx.gas_price,
+        max_fee_per_gas: tx.gas_price,
+        to: tx.to,
+        value: tx.value,
+        data: tx.data,
+        access_list: Vec::new(),
+    }
+}
diff --git a/engine-tests/src/tests/erc20.rs b/engine-tests/src/tests/erc20.rs
@@ -54,9 +54,7 @@ fn erc20_mint_out_of_gas() {
     let mut mint_tx = contract.mint(dest_address, mint_amount.into(), nonce.into());
 
     // not enough gas to cover intrinsic cost
-    let intrinsic_gas = mint_tx
-        .clone()
-        .normalize()
+    let intrinsic_gas = test_utils::erc20::legacy_into_normalized_tx(mint_tx.clone())
         .intrinsic_gas(&evm::Config::istanbul())
         .unwrap();
     mint_tx.gas_limit = (intrinsic_gas - 1).into();
@@ -213,9 +211,7 @@ fn deploy_erc_20_out_of_gas() {
     let mut deploy_transaction = constructor.deploy("OutOfGas", "OOG", INITIAL_NONCE.into());
 
     // not enough gas to cover intrinsic cost
-    let intrinsic_gas = deploy_transaction
-        .clone()
-        .normalize()
+    let intrinsic_gas = test_utils::erc20::legacy_into_normalized_tx(deploy_transaction.clone())
         .intrinsic_gas(&evm::Config::istanbul())
         .unwrap();
     deploy_transaction.gas_limit = (intrinsic_gas - 1).into();

diff --git a/engine-tests/src/tests/sanity.rs b/engine-tests/src/tests/sanity.rs
@@ -96,8 +96,8 @@ fn test_transaction_to_zero_address() {
     let tx_hex = "f8648080836691b79400000000000000000000000000000000000000008080849c8a82caa0464cada9d6a907f5537dcc0f95274a30ddaeff33276e9b3993815586293a2010a07626bd794381ba59f30e26ec6f3448d19f63bb12dcda19acda429b2fb7d3dfba";
     let tx_bytes = hex::decode(tx_hex).unwrap();
     let tx = aurora_engine_transactions::EthTransactionKind::try_from(tx_bytes.as_slice()).unwrap();
-    let normalized_tx = aurora_engine_transactions::NormalizedEthTransaction::from(tx);
-    let address = normalized_tx.address.as_ref().unwrap();
+    let normalized_tx = aurora_engine_transactions::NormalizedEthTransaction::try_from(tx).unwrap();
+    let address = normalized_tx.address;
     let sender = hex::encode(address.as_bytes());
     assert_eq!(sender.as_str(), "63eafba871e0bda44be3cde19df5aa1c0f078142");
 
@@ -113,14 +113,14 @@ fn test_transaction_to_zero_address() {
     let result = runner.submit_raw(test_utils::SUBMIT, &context).unwrap();
     assert_eq!(result.gas_used, 53_000);
     runner.env.block_height = aurora_engine::engine::ZERO_ADDRESS_FIX_HEIGHT;
-    assert_eq!(runner.get_nonce(address), U256::zero());
+    assert_eq!(runner.get_nonce(&address), U256::zero());
 
     // After the fix this transaction is simply a transfer of 0 ETH to the zero address
     context.block_index = aurora_engine::engine::ZERO_ADDRESS_FIX_HEIGHT;
     let result = runner.submit_raw(test_utils::SUBMIT, &context).unwrap();
     assert_eq!(result.gas_used, 21_000);
     runner.env.block_height = aurora_engine::engine::ZERO_ADDRESS_FIX_HEIGHT + 1;
-    assert_eq!(runner.get_nonce(address), U256::one());
+    assert_eq!(runner.get_nonce(&address), U256::one());
 }
 
 #[test]

diff --git a/engine-transactions/src/backwards_compatibility.rs b/engine-transactions/src/backwards_compatibility.rs
@@ -4,7 +4,7 @@
 //! https://github.com/aurora-is-near/aurora-engine/pull/458 for more details, but external
 //! users of this library should _never_ use the adapter in this module.
 
-use crate::{EthTransactionKind, ParseTransactionError};
+use crate::{Error, EthTransactionKind};
 use aurora_engine_types::{types::Address, H160};
 
 const ZERO_ADDRESS: Option<Address> = Some(Address::new(H160::zero()));
@@ -26,7 +26,7 @@ impl EthTransactionKindAdapter {
         &self,
         bytes: &[u8],
         block_height: u64,
-    ) -> Result<EthTransactionKind, ParseTransactionError> {
+    ) -> Result<EthTransactionKind, Error> {
         let mut result = EthTransactionKind::try_from(bytes)?;
 
         // Prior to the bug fix, the zero address was always parsed as None if