diff --git a/src/engine.rs b/src/engine.rs index 5cf3410d7..5b0975140 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -11,7 +11,7 @@ use crate::contract::current_address; use crate::map::{BijectionMap, LookupMap}; use crate::parameters::{ FunctionCallArgs, NEP141FtOnTransferArgs, NewCallArgs, PromiseCreateArgs, SubmitResult, - ViewCallArgs, + TransactionStatus, ViewCallArgs, }; use crate::precompiles::Precompiles; @@ -147,14 +147,18 @@ pub type EngineResult = Result; trait ExitIntoResult { /// Checks if the EVM exit is ok or an error. - fn into_result(self) -> Result<(), EngineErrorKind>; + fn into_result(self, data: Vec) -> Result; } impl ExitIntoResult for ExitReason { - fn into_result(self) -> Result<(), EngineErrorKind> { + fn into_result(self, data: Vec) -> Result { use ExitReason::*; match self { - Succeed(_) | Revert(_) => Ok(()), + Succeed(_) => Ok(TransactionStatus::Succeed(data)), + Revert(_) => Ok(TransactionStatus::Revert(data)), + Error(ExitError::OutOfOffset) => Ok(TransactionStatus::OutOfOffset), + Error(ExitError::OutOfFund) => Ok(TransactionStatus::OutOfFund), + Error(ExitError::OutOfGas) => Ok(TransactionStatus::OutOfGas), Error(e) => Err(e.into()), Fatal(e) => Err(e.into()), } @@ -555,24 +559,27 @@ impl Engine { ) -> EngineResult { let mut executor = self.make_executor(gas_limit); let address = executor.create_address(CreateScheme::Legacy { caller: origin }); - let (status, result) = ( + let (exit_reason, result) = ( executor.transact_create(origin, value.raw(), input, gas_limit, access_list), address, ); - let is_succeed = status.is_succeed(); + let used_gas = executor.used_gas(); - if let Err(e) = status.into_result() { - Engine::increment_nonce(&origin); - return Err(e.with_gas_used(used_gas)); - } + let status = match exit_reason.into_result(result.0.to_vec()) { + Ok(status) => status, + Err(e) => { + Engine::increment_nonce(&origin); + return Err(e.with_gas_used(used_gas)); + } + }; + let (values, logs, promises) = executor.into_state().deconstruct(); self.apply(values, Vec::::new(), true); Self::schedule_promises(promises); Ok(SubmitResult { - status: is_succeed, + status, gas_used: used_gas, - result: result.0.to_vec(), logs: logs.into_iter().map(Into::into).collect(), }) } @@ -594,15 +601,17 @@ impl Engine { access_list: Vec<(Address, Vec)>, // See EIP-2930 ) -> EngineResult { let mut executor = self.make_executor(gas_limit); - let (status, result) = + let (exit_reason, result) = executor.transact_call(origin, contract, value.raw(), input, gas_limit, access_list); - let is_succeed = status.is_succeed(); let used_gas = executor.used_gas(); - if let Err(e) = status.into_result() { - Engine::increment_nonce(&origin); - return Err(e.with_gas_used(used_gas)); - } + let status = match exit_reason.into_result(result) { + Ok(status) => status, + Err(e) => { + Engine::increment_nonce(&origin); + return Err(e.with_gas_used(used_gas)); + } + }; let (values, logs, promises) = executor.into_state().deconstruct(); @@ -612,9 +621,8 @@ impl Engine { Self::schedule_promises(promises); Ok(SubmitResult { - status: is_succeed, + status, gas_used: used_gas, - result, logs: logs.into_iter().map(Into::into).collect(), }) } @@ -625,7 +633,7 @@ impl Engine { Self::set_nonce(address, &new_nonce); } - pub fn view_with_args(&self, args: ViewCallArgs) -> Result, EngineErrorKind> { + pub fn view_with_args(&self, args: ViewCallArgs) -> Result { let origin = Address::from_slice(&args.sender); let contract = Address::from_slice(&args.address); let value = U256::from_big_endian(&args.amount); @@ -639,12 +647,11 @@ impl Engine { value: Wei, input: Vec, gas_limit: u64, - ) -> Result, EngineErrorKind> { + ) -> Result { let mut executor = self.make_executor(gas_limit); let (status, result) = executor.transact_call(origin, contract, value.raw(), input, gas_limit, Vec::new()); - status.into_result()?; - Ok(result) + status.into_result(result) } fn make_executor(&self, gas_limit: u64) -> StackExecutor { diff --git a/src/lib.rs b/src/lib.rs index 7ba528517..608cfb565 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,7 +83,8 @@ mod contract { use crate::parameters::{ DeployErc20TokenArgs, ExpectUtf8, FunctionCallArgs, GetErc20FromNep141CallArgs, GetStorageAtArgs, InitCallArgs, IsUsedProofCallArgs, NEP141FtOnTransferArgs, NewCallArgs, - PauseEthConnectorCallArgs, SetContractDataCallArgs, TransferCallCallArgs, ViewCallArgs, + PauseEthConnectorCallArgs, SetContractDataCallArgs, TransactionStatus, + TransferCallCallArgs, ViewCallArgs, }; use crate::json::parse_json; @@ -379,21 +380,24 @@ mod contract { ethabi::Token::Address(current_address()), ]); - Engine::deploy_code_with_input( + let address = match Engine::deploy_code_with_input( &mut engine, (&[erc20_contract, deploy_args.as_slice()].concat()).to_vec(), - ) - .map(|res| { - let address = H160(res.result.as_slice().try_into().unwrap()); - crate::log!( - crate::prelude::format!("Deployed ERC-20 in Aurora at: {:#?}", address).as_str() - ); - engine - .register_token(address.as_bytes(), &args.nep141.as_bytes()) - .sdk_unwrap(); - res.result.try_to_vec().sdk_expect("ERR_SERIALIZE") - }) - .sdk_process(); + ) { + Ok(result) => match result.status { + TransactionStatus::Succeed(ret) => H160(ret.as_slice().try_into().unwrap()), + other => sdk::panic_utf8(other.as_ref()), + }, + Err(e) => sdk::panic_utf8(e.as_ref()), + }; + + crate::log!( + crate::prelude::format!("Deployed ERC-20 in Aurora at: {:#?}", address).as_str() + ); + engine + .register_token(address.as_bytes(), &args.nep141.as_bytes()) + .sdk_unwrap(); + sdk::return_output(&address.as_bytes().try_to_vec().sdk_expect("ERR_SERIALIZE")); // TODO: charge for storage } @@ -630,6 +634,27 @@ mod contract { sdk::return_output(&data[..]); } + /// Function used to create accounts for tests + #[cfg(feature = "integration-test")] + #[no_mangle] + pub extern "C" fn mint_account() { + use evm::backend::ApplyBackend; + + let args: ([u8; 20], u64, u64) = sdk::read_input_borsh().sdk_expect("ERR_ARGS"); + let address = Address(args.0); + let nonce = U256::from(args.1); + let balance = U256::from(args.2); + let mut engine = Engine::new(address).sdk_unwrap(); + let state_change = evm::backend::Apply::Modify { + address, + basic: evm::backend::Basic { balance, nonce }, + code: None, + storage: core::iter::empty(), + reset_storage: false, + }; + engine.apply(core::iter::once(state_change), core::iter::empty(), false); + } + /// /// Utility methods. /// diff --git a/src/parameters.rs b/src/parameters.rs index 58cd1c730..b91a8f23b 100644 --- a/src/parameters.rs +++ b/src/parameters.rs @@ -61,13 +61,53 @@ impl From for ResultLog { } } +/// The status of a transaction. +#[derive(Debug, BorshSerialize, BorshDeserialize, PartialEq, Eq)] +pub enum TransactionStatus { + Succeed(Vec), + Revert(Vec), + OutOfGas, + OutOfFund, + OutOfOffset, + CallTooDeep, +} + +impl TransactionStatus { + pub fn is_ok(&self) -> bool { + matches!(*self, TransactionStatus::Succeed(_)) + } + + pub fn is_revert(&self) -> bool { + matches!(*self, TransactionStatus::Revert(_)) + } + + pub fn is_fail(&self) -> bool { + *self == TransactionStatus::OutOfGas + || *self == TransactionStatus::OutOfFund + || *self == TransactionStatus::OutOfOffset + || *self == TransactionStatus::CallTooDeep + } +} + +impl AsRef<[u8]> for TransactionStatus { + fn as_ref(&self) -> &[u8] { + match self { + Self::Succeed(_) => b"SUCCESS", + Self::Revert(_) => b"ERR_REVERT", + Self::OutOfFund => b"ERR_OUT_OF_FUNDS", + Self::OutOfGas => b"ERR_OUT_OF_GAS", + Self::OutOfOffset => b"ERR_OUT_OF_OFFSET", + Self::CallTooDeep => b"ERR_CALL_TOO_DEEP", + } + } +} + /// Borsh-encoded parameters for the `call`, `call_with_args`, `deploy_code`, /// and `deploy_with_input` methods. #[derive(Debug, BorshSerialize, BorshDeserialize)] pub struct SubmitResult { - pub status: bool, + pub status: TransactionStatus, pub gas_used: u64, - pub result: Vec, pub logs: Vec, } diff --git a/src/test_utils/exit_precompile.rs b/src/test_utils/exit_precompile.rs index 2912fbca4..11ea55bc5 100644 --- a/src/test_utils/exit_precompile.rs +++ b/src/test_utils/exit_precompile.rs @@ -1,6 +1,6 @@ use crate::parameters::SubmitResult; use crate::prelude::{Address, U256}; -use crate::test_utils::{solidity, AuroraRunner, Signer}; +use crate::test_utils::{self, solidity, AuroraRunner, Signer}; use crate::transaction::LegacyEthTransaction; pub(crate) struct TesterConstructor(pub solidity::ContractConstructor); @@ -78,8 +78,9 @@ impl Tester { let result = runner.submit_transaction(&signer.secret_key, tx).unwrap(); - if result.status { - Ok(ethabi::decode(output_type, result.result.as_slice()).unwrap()) + if result.status.is_ok() { + let result = test_utils::unwrap_success(result); + Ok(ethabi::decode(output_type, result.as_slice()).unwrap()) } else { Err(result) } diff --git a/src/test_utils/mod.rs b/src/test_utils/mod.rs index 3ab166990..ea0cf0fce 100644 --- a/src/test_utils/mod.rs +++ b/src/test_utils/mod.rs @@ -13,7 +13,7 @@ use rlp::RlpStream; use secp256k1::{self, Message, PublicKey, SecretKey}; use crate::fungible_token::FungibleToken; -use crate::parameters::{InitCallArgs, NewCallArgs, SubmitResult}; +use crate::parameters::{InitCallArgs, NewCallArgs, SubmitResult, TransactionStatus}; use crate::prelude::Address; use crate::storage; use crate::test_utils::solidity::{ContractConstructor, DeployedContract}; @@ -281,7 +281,7 @@ impl AuroraRunner { assert!(maybe_err.is_none()); let submit_result = SubmitResult::try_from_slice(&output.unwrap().return_data.as_value().unwrap()).unwrap(); - let address = Address::from_slice(&submit_result.result); + let address = Address::from_slice(&unwrap_success(submit_result)); let contract_constructor: ContractConstructor = contract_constructor.into(); DeployedContract { abi: contract_constructor.abi, @@ -512,3 +512,25 @@ pub(crate) fn address_from_hex(address: &str) -> Address { Address::from_slice(&bytes) } + +pub fn unwrap_success(result: SubmitResult) -> Vec { + match result.status { + TransactionStatus::Succeed(ret) => ret, + other => panic!("Unexpected status: {:?}", other), + } +} + +pub fn unwrap_revert(result: SubmitResult) -> Vec { + match result.status { + TransactionStatus::Revert(ret) => ret, + other => panic!("Unexpected status: {:?}", other), + } +} + +pub fn panic_on_fail(status: TransactionStatus) { + match status { + TransactionStatus::Succeed(_) => (), + TransactionStatus::Revert(message) => panic!("{}", String::from_utf8_lossy(&message)), + other => panic!("{}", String::from_utf8_lossy(other.as_ref())), + } +} diff --git a/src/test_utils/self_destruct.rs b/src/test_utils/self_destruct.rs index d8c268778..0f3745214 100644 --- a/src/test_utils/self_destruct.rs +++ b/src/test_utils/self_destruct.rs @@ -1,6 +1,6 @@ use crate::parameters::FunctionCallArgs; use crate::prelude::Address; -use crate::test_utils::{solidity, AuroraRunner, Signer}; +use crate::test_utils::{self, solidity, AuroraRunner, Signer}; use crate::transaction::LegacyEthTransaction; use borsh::BorshSerialize; use primitive_types::U256; @@ -73,8 +73,9 @@ impl SelfDestructFactory { }; let result = runner.submit_transaction(&signer.secret_key, tx).unwrap(); + let result = test_utils::unwrap_success(result); - Address::from_slice(&result.result[12..]) + Address::from_slice(&result[12..]) } } @@ -111,11 +112,10 @@ impl SelfDestruct { }; let result = runner.submit_transaction(&signer.secret_key, tx).unwrap(); + let result = test_utils::unwrap_success(result); - if result.result.len() == 32 { - Some(u128::from_be_bytes( - result.result[16..32].try_into().unwrap(), - )) + if result.len() == 32 { + Some(u128::from_be_bytes(result[16..32].try_into().unwrap())) } else { None } diff --git a/src/tests/erc20.rs b/src/tests/erc20.rs index 93aab2842..13f953a68 100644 --- a/src/tests/erc20.rs +++ b/src/tests/erc20.rs @@ -1,3 +1,4 @@ +use crate::parameters::TransactionStatus; use crate::prelude::{Address, U256}; use crate::test_utils::{ self, @@ -64,9 +65,8 @@ fn erc20_mint_out_of_gas() { mint_tx.gas = U256::from(GAS_LIMIT); mint_tx.gas_price = U256::from(GAS_PRICE); // also set non-zero gas price to check gas still charged. let outcome = runner.submit_transaction(&source_account.secret_key, mint_tx); - let error = outcome.unwrap_err(); - let error_message = format!("{:?}", error); - assert!(error_message.contains("ERR_OUT_OF_GAS")); + let error = outcome.unwrap(); + assert_eq!(error.status, TransactionStatus::OutOfGas); // Validate post-state test_utils::validate_address_balance_and_nonce( @@ -109,7 +109,7 @@ fn erc20_transfer_success() { contract.transfer(dest_address, TRANSFER_AMOUNT.into(), nonce) }) .unwrap(); - assert!(outcome.status); + assert!(outcome.status.is_ok()); // Validate post-state assert_eq!( @@ -148,8 +148,7 @@ fn erc20_transfer_insufficient_balance() { contract.transfer(dest_address, (2 * INITIAL_BALANCE).into(), nonce) }) .unwrap(); - assert!(!outcome.status); // status == false means execution error - let message = parse_erc20_error_message(&outcome.result); + let message = parse_erc20_error_message(&test_utils::unwrap_revert(outcome)); assert_eq!(&message, "&ERC20: transfer amount exceeds balance"); // Validate post-state @@ -192,9 +191,8 @@ fn deploy_erc_20_out_of_gas() { // not enough gas to complete transaction deploy_transaction.gas = U256::from(3_200_000); let outcome = runner.submit_transaction(&source_account, deploy_transaction); - let error = outcome.unwrap_err(); - let error_message = format!("{:?}", error); - assert!(error_message.contains("ERR_OUT_OF_GAS")); + let error = outcome.unwrap(); + assert_eq!(error.status, TransactionStatus::OutOfGas); // Validate post-state test_utils::validate_address_balance_and_nonce( @@ -211,9 +209,11 @@ fn get_address_erc20_balance( address: Address, contract: &ERC20, ) -> U256 { - let outcome = runner.submit_with_signer(signer, |nonce| contract.balance_of(address, nonce)); - assert!(outcome.is_ok()); - U256::from_big_endian(&outcome.unwrap().result) + let outcome = runner + .submit_with_signer(signer, |nonce| contract.balance_of(address, nonce)) + .unwrap(); + let output = test_utils::unwrap_success(outcome); + U256::from_big_endian(&output) } fn parse_erc20_error_message(result: &[u8]) -> String { diff --git a/src/tests/erc20_connector.rs b/src/tests/erc20_connector.rs index cc727dd08..79d839388 100644 --- a/src/tests/erc20_connector.rs +++ b/src/tests/erc20_connector.rs @@ -139,7 +139,8 @@ impl test_utils::AuroraRunner { let input = build_input("balanceOf(address)", &[Token::Address(target.into())]); let result = self.evm_call(token, input, origin); result.check_ok(); - U256::from_big_endian(result.submit_result().result.as_slice()) + let output = test_utils::unwrap_success(result.submit_result()); + U256::from_big_endian(output.as_slice()) } pub fn mint( diff --git a/src/tests/sanity.rs b/src/tests/sanity.rs index dbf34f2af..a33cbc99d 100644 --- a/src/tests/sanity.rs +++ b/src/tests/sanity.rs @@ -1,6 +1,8 @@ -use crate::prelude::Address; +use crate::parameters::{SubmitResult, TransactionStatus}; +use crate::prelude::{Address, U256}; use crate::test_utils; use crate::types::{self, Wei, ERC20_MINT_SELECTOR}; +use borsh::BorshSerialize; use secp256k1::SecretKey; use std::path::{Path, PathBuf}; @@ -64,14 +66,13 @@ fn test_eth_transfer_insufficient_balance() { test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into()); // attempt transfer - let err = runner + let result = runner .submit_with_signer(&mut source_account, |nonce| { // try to transfer more than we have test_utils::transfer(dest_address, INITIAL_BALANCE + INITIAL_BALANCE, nonce) }) - .unwrap_err(); - let error_message = format!("{:?}", err); - assert!(error_message.contains("ERR_OUT_OF_FUND")); + .unwrap(); + assert_eq!(result.status, TransactionStatus::OutOfFund); // validate post-state test_utils::validate_address_balance_and_nonce( @@ -314,7 +315,76 @@ fn test_block_hash_contract() { }) .unwrap(); - if !result.status { - panic!("{}", String::from_utf8_lossy(&result.result)); + test_utils::panic_on_fail(result.status); +} + +// Same as `test_eth_transfer_insufficient_balance` above, except runs through +// `near-sdk-sim` instead of `near-vm-runner`. This is important because `near-sdk-sim` +// has more production logic, in particular, state revert on contract panic. +// TODO: should be able to generalize the `call` backend of `AuroraRunner` so that this +// test does not need to be written twice. +#[test] +fn test_eth_transfer_insufficient_balance_sim() { + use crate::tests::state_migration; + + // initalize engine contract + let aurora = state_migration::deploy_evm(); + let mut signer = test_utils::Signer::random(); + let address = test_utils::address_from_secret_key(&signer.secret_key); + + let args = (address.0, INITIAL_NONCE, INITIAL_BALANCE.raw().low_u64()); + aurora + .call("mint_account", &args.try_to_vec().unwrap()) + .assert_success(); + + // validate pre-state + assert_eq!( + query_address(&address, "get_nonce", &aurora), + U256::from(INITIAL_NONCE), + ); + assert_eq!( + query_address(&address, "get_balance", &aurora), + INITIAL_BALANCE.raw(), + ); + + // Run transaction which will fail (transfer more than current balance) + let nonce = signer.use_nonce(); + let tx = test_utils::transfer( + Address([1; 20]), + INITIAL_BALANCE + INITIAL_BALANCE, + nonce.into(), + ); + let signed_tx = test_utils::sign_transaction( + tx, + Some(test_utils::AuroraRunner::default().chain_id), + &signer.secret_key, + ); + let call_result = aurora.call("submit", rlp::encode(&signed_tx).as_ref()); + let result: SubmitResult = call_result.unwrap_borsh(); + assert_eq!(result.status, TransactionStatus::OutOfFund); + + // validate post-state + assert_eq!( + query_address(&address, "get_nonce", &aurora), + U256::from(INITIAL_NONCE + 1), + ); + assert_eq!( + query_address(&address, "get_balance", &aurora), + INITIAL_BALANCE.raw(), + ); + + // helper function + fn query_address( + address: &Address, + method: &str, + aurora: &state_migration::AuroraAccount, + ) -> U256 { + let x = aurora.call(method, &address.0); + match &x.outcome().status { + near_sdk_sim::transaction::ExecutionStatus::SuccessValue(b) => { + U256::from_big_endian(&b) + } + other => panic!("Unexpected outcome: {:?}", other), + } } } diff --git a/src/tests/standard_precompiles.rs b/src/tests/standard_precompiles.rs index bf2ca483a..c6dd25f9f 100644 --- a/src/tests/standard_precompiles.rs +++ b/src/tests/standard_precompiles.rs @@ -31,8 +31,5 @@ fn standard_precompiles() { .submit_transaction(&source_account, test_all_tx) .unwrap(); - // status == false indicates failure - if !outcome.status { - panic!("{}", String::from_utf8_lossy(&outcome.result)) - } + test_utils::panic_on_fail(outcome.status); } diff --git a/src/tests/state_migration.rs b/src/tests/state_migration.rs index 297501024..6eb65ae71 100644 --- a/src/tests/state_migration.rs +++ b/src/tests/state_migration.rs @@ -1,4 +1,4 @@ -use crate::parameters::NewCallArgs; +use crate::parameters::{InitCallArgs, NewCallArgs}; use crate::prelude::U256; use crate::test_utils::AuroraRunner; use crate::types; @@ -26,7 +26,7 @@ fn test_state_migration() { assert_eq!(some_numbers, [3, 1, 4, 1, 5, 9, 2]); } -fn deploy_evm() -> AuroraAccount { +pub fn deploy_evm() -> AuroraAccount { let aurora_runner = AuroraRunner::default(); let main_account = near_sdk_sim::init_simulator(None); let contract_account = main_account.deploy( @@ -34,10 +34,11 @@ fn deploy_evm() -> AuroraAccount { aurora_runner.aurora_account_id.parse().unwrap(), 5 * near_sdk_sim::STORAGE_AMOUNT, ); + let prover_account = "prover.near".to_string(); let new_args = NewCallArgs { chain_id: types::u256_to_arr(&U256::from(aurora_runner.chain_id)), owner_id: main_account.account_id.clone().into(), - bridge_prover_id: "prover.near".to_string(), + bridge_prover_id: prover_account.clone(), upgrade_delay_blocks: 1, }; main_account @@ -49,19 +50,32 @@ fn deploy_evm() -> AuroraAccount { 0, ) .assert_success(); + let init_args = InitCallArgs { + prover_account, + eth_custodian_address: "d045f7e19B2488924B97F9c145b5E51D0D895A65".to_string(), + }; + contract_account + .call( + contract_account.account_id.clone(), + "new_eth_connector", + &init_args.try_to_vec().unwrap(), + near_sdk_sim::DEFAULT_GAS, + 0, + ) + .assert_success(); AuroraAccount { user: main_account, contract: contract_account, } } -struct AuroraAccount { +pub struct AuroraAccount { user: UserAccount, contract: UserAccount, } impl AuroraAccount { - fn call(&self, method: &str, args: &[u8]) -> ExecutionResult { + pub fn call(&self, method: &str, args: &[u8]) -> ExecutionResult { self.user.call( self.contract.account_id.clone(), method, diff --git a/src/types.rs b/src/types.rs index 597ed1608..008e59dbb 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,8 +1,6 @@ -use crate::prelude::{self, Address, String, ToString, Vec, H256, U256}; +use crate::prelude::{self, str, Address, String, ToString, Vec, H256, U256}; #[cfg(not(feature = "contract"))] use crate::prelude::{format, vec}; - -use crate::prelude::str; use borsh::{BorshDeserialize, BorshSerialize}; use ethabi::{Event, EventParam, Hash, Log, RawLog};