Skip to content

Commit

Permalink
Chore: Disallow as_conversions (#561)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
joshuajbouw committed Aug 18, 2022
1 parent ffdc22d commit c167d8d
Show file tree
Hide file tree
Showing 32 changed files with 261 additions and 166 deletions.
12 changes: 6 additions & 6 deletions .env/local.env
Original file line number Diff line number Diff line change
@@ -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
18 changes: 9 additions & 9 deletions .env/mainnet.env
Original file line number Diff line number Diff line change
@@ -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
18 changes: 9 additions & 9 deletions .env/testnet.env
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion engine-precompiles/src/blake2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]]);
Expand Down
10 changes: 8 additions & 2 deletions engine-precompiles/src/bn128.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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,
)
}
Expand Down Expand Up @@ -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,
)
}
Expand Down
8 changes: 5 additions & 3 deletions engine-precompiles/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
5 changes: 3 additions & 2 deletions engine-precompiles/src/identity.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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,
)
Expand Down
2 changes: 1 addition & 1 deletion engine-precompiles/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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};
Expand Down
33 changes: 18 additions & 15 deletions engine-precompiles/src/modexp.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
9 changes: 9 additions & 0 deletions engine-precompiles/src/utils.rs
Original file line number Diff line number Diff line change
@@ -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"))
}
2 changes: 1 addition & 1 deletion engine-sdk/src/near_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions engine-standalone-storage/src/sync/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions engine-standalone-tracing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
16 changes: 16 additions & 0 deletions engine-tests/src/test_utils/erc20.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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(),
}
}
8 changes: 2 additions & 6 deletions engine-tests/src/tests/erc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions engine-tests/src/tests/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions engine-transactions/src/backwards_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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
Expand Down
Loading

0 comments on commit c167d8d

Please sign in to comment.