Skip to content

Commit

Permalink
Feat: refactore gas charge logic form EVM exit reason (#935)
Browse files Browse the repository at this point in the history
### Description

To be fully compatible with EVM, we should:
:arrow_right:  Success| Revert
:arrow_right:  ExitError - Execution errors should charge gas from users
:arrow_right:  ExitFatal - shouldn't charge user gas
:arrow_right:  Transactions validation errors should not charge user gas

This PR changed the logic for EVM exit reason gas charge for users,
which includes EVM execution errors gas charge. Previously transaction
just panicked, and user's gas was not charged.

#### EVM
Related to EVM changes:  aurora-is-near/sputnikvm/pull/52

---------

Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
  • Loading branch information
mrLSD and aleksuss authored Aug 27, 2024
1 parent 9c80184 commit c505f58
Show file tree
Hide file tree
Showing 13 changed files with 308 additions and 87 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock

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

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ byte-slice-cast = { version = "1", default-features = false }
criterion = "0.5"
digest = "0.10"
ethabi = { version = "18", default-features = false }
evm = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false }
evm-core = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std"] }
evm-gasometer = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std", "tracing"] }
evm-runtime = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.43.1-aurora", default-features = false, features = ["std", "tracing"] }
evm = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false }
evm-core = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std"] }
evm-gasometer = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std", "tracing"] }
evm-runtime = { git = "https://github.com/aurora-is-near/sputnikvm.git", tag = "v0.45.0-aurora", default-features = false, features = ["std", "tracing"] }
fixed-hash = { version = "0.8", default-features = false }
function_name = "0.3"
git2 = "0.19"
Expand Down
7 changes: 3 additions & 4 deletions engine-hashchain/src/hashchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ impl Hashchain {
}

/// Moves to the indicated block height if it is bigger than the current block height:
/// -Updates the previous block hashchain computing the hash.
/// -Updates the current block height.
/// -Clears the transactions.
/// -Clears the transactions.
/// - Updates the previous block hashchain computing the hash.
/// - Updates the current block height.
/// - Clears the transactions.
/// Returns an error in other case.
pub fn move_to_block(
&mut self,
Expand Down
2 changes: 1 addition & 1 deletion engine-tests/src/tests/one_inch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn test_1_inch_limit_order_deploy() {

// more than 3.5 million Ethereum gas used
assert!(result.gas_used > 3_500_000);
// less than 11 NEAR TGas used
// less than 12 NEAR TGas used
assert_gas_bound(profile.all_gas(), 12);
// at least 45% of which is from wasm execution
let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas();
Expand Down
12 changes: 6 additions & 6 deletions engine-tests/src/tests/repro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn repro_GdASJ3KESs() {
block_timestamp: 1_645_717_564_644_206_730,
input_path: "src/tests/res/input_GdASJ3KESs.hex",
evm_gas_used: 706_713,
near_gas_used: 113,
near_gas_used: 114,
});
}

Expand All @@ -51,7 +51,7 @@ fn repro_8ru7VEA() {
block_timestamp: 1_648_829_935_343_349_589,
input_path: "src/tests/res/input_8ru7VEA.hex",
evm_gas_used: 1_732_181,
near_gas_used: 204,
near_gas_used: 205,
});
}

Expand All @@ -71,7 +71,7 @@ fn repro_FRcorNv() {
block_timestamp: 1_650_960_438_774_745_116,
input_path: "src/tests/res/input_FRcorNv.hex",
evm_gas_used: 1_239_721,
near_gas_used: 165,
near_gas_used: 167,
});
}

Expand All @@ -88,7 +88,7 @@ fn repro_5bEgfRQ() {
block_timestamp: 1_651_073_772_931_594_646,
input_path: "src/tests/res/input_5bEgfRQ.hex",
evm_gas_used: 6_414_105,
near_gas_used: 645,
near_gas_used: 649,
});
}

Expand All @@ -106,7 +106,7 @@ fn repro_D98vwmi() {
block_timestamp: 1_651_753_443_421_003_245,
input_path: "src/tests/res/input_D98vwmi.hex",
evm_gas_used: 1_035_348,
near_gas_used: 167,
near_gas_used: 168,
});
}

Expand All @@ -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: 291,
near_gas_used: 293,
});
}

Expand Down
Binary file added engine-tests/src/tests/res/transaction_status.borsh
Binary file not shown.
4 changes: 3 additions & 1 deletion engine-tests/src/tests/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,8 @@ fn test_block_hash_contract() {
})
.unwrap();

utils::panic_on_fail(result.status);
let res = utils::panic_on_fail(result.status);
assert!(res.is_none(), "Status: {res:?}");
}

#[cfg(not(feature = "ext-connector"))]
Expand Down Expand Up @@ -1245,6 +1246,7 @@ mod workspace {
Some(utils::AuroraRunner::default().chain_id),
&signer.secret_key,
);

let result = aurora
.submit(rlp::encode(&signed_tx).to_vec())
.transact()
Expand Down
9 changes: 5 additions & 4 deletions engine-tests/src/tests/standalone/call_tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::prelude::{H160, H256};
use crate::utils::solidity::erc20::{ERC20Constructor, ERC20};
use crate::utils::{self, standalone, Signer};
use aurora_engine_modexp::AuroraModExp;
use aurora_engine_types::parameters::engine::TransactionStatus;
use aurora_engine_types::{
parameters::{CrossContractCallArgs, PromiseArgs, PromiseCreateArgs},
storage,
Expand Down Expand Up @@ -296,10 +297,10 @@ fn test_contract_create_too_large() {
let standalone_result = sputnik::traced_call(&mut listener, || {
runner.submit_transaction(&signer.secret_key, tx)
});
assert!(
standalone_result.is_err(),
"Expected contract too large error"
);
assert!(matches!(
standalone_result.unwrap().status,
TransactionStatus::CreateContractLimit
));
}

#[allow(clippy::too_many_lines)]
Expand Down
3 changes: 2 additions & 1 deletion engine-tests/src/tests/standard_precompiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ fn test_standard_precompiles() {
.submit_with_signer(&mut signer, |nonce| contract.call_method("test_all", nonce))
.unwrap();

utils::panic_on_fail(outcome.status);
let res = utils::panic_on_fail(outcome.status);
assert!(res.is_none(), "Status: {res:?}");
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion engine-tests/src/tests/uniswap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn test_uniswap_exact_output() {

let (_amount_in, profile) =
context.exact_output_single(&token_a, &token_b, OUTPUT_AMOUNT.into());
utils::assert_gas_bound(profile.all_gas(), 16);
utils::assert_gas_bound(profile.all_gas(), 17);
let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas();
assert!(
(40..=50).contains(&wasm_fraction),
Expand Down
8 changes: 5 additions & 3 deletions engine-tests/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,10 +1000,12 @@ pub fn unwrap_revert_slice(result: &SubmitResult) -> &[u8] {
}
}

pub fn panic_on_fail(status: TransactionStatus) {
pub fn panic_on_fail(status: TransactionStatus) -> Option<String> {
match status {
TransactionStatus::Succeed(_) => (),
TransactionStatus::Revert(message) => panic!("{}", String::from_utf8_lossy(&message)),
TransactionStatus::Succeed(_) => None,
TransactionStatus::Revert(message) => {
Some(format!("Revert: {}", String::from_utf8_lossy(&message)))
}
other => panic!("{}", String::from_utf8_lossy(other.as_ref())),
}
}
Expand Down
128 changes: 115 additions & 13 deletions engine-types/src/parameters/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,51 @@ pub struct ResultLog {
pub data: Vec<u8>,
}

/// The status of a transaction.
/// The status of a transaction representing EVM error kinds.
/// !!! THE ORDER OF VARIANTS MUSTN'T BE CHANGED FOR SAVING BACKWARD COMPATIBILITY !!!
/// !!! NEW VARIANTS SHOULD BE ADDED IN THE END OF THE ENUM ONLY !!!
#[derive(Debug, Clone, BorshSerialize, BorshDeserialize, PartialEq, Eq)]
#[cfg_attr(feature = "impl-serde", derive(Serialize, Deserialize))]
pub enum TransactionStatus {
/// The transaction succeeded.
Succeed(Vec<u8>),
/// The transaction reverted.
Revert(Vec<u8>),
/// Execution runs out of gas.
OutOfGas,
/// Not enough fund to start the execution.
OutOfFund,
/// An opcode accesses external information, but the request is off offset limit.
OutOfOffset,
/// Call stack is too deep.
CallTooDeep,
/// Trying to pop from an empty stack.
StackUnderflow,
/// Trying to push into a stack over stack limit.
StackOverflow,
/// Jump destination is invalid.
InvalidJump,
/// An opcode accesses memory region, but the region is invalid.
InvalidRange,
/// Encountered the designated invalid opcode.
DesignatedInvalid,
/// Create opcode encountered collision.
CreateCollision,
/// Create init code exceeds limit.
CreateContractLimit,
/// Invalid opcode during execution or starting byte is 0xef. See [EIP-3541](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-3541.md).
InvalidCode(u8),
/// PC underflow (unused).
#[allow(clippy::upper_case_acronyms)]
PCUnderflow,
/// Attempt to create an empty account (unused).
CreateEmpty,
/// Nonce reached maximum value of 2^64-1
MaxNonce,
/// `usize` casting overflow
UsizeOverflow,
/// Other normal errors.
Other(crate::Cow<'static, str>),
}

impl TransactionStatus {
Expand All @@ -201,11 +236,8 @@ impl TransactionStatus {
}

#[must_use]
pub fn is_fail(&self) -> bool {
*self == Self::OutOfGas
|| *self == Self::OutOfFund
|| *self == Self::OutOfOffset
|| *self == Self::CallTooDeep
pub const fn is_fail(&self) -> bool {
!matches!(*self, Self::Succeed(_) | Self::Revert(_))
}
}

Expand All @@ -218,6 +250,19 @@ impl AsRef<[u8]> for TransactionStatus {
Self::OutOfGas => errors::ERR_OUT_OF_GAS,
Self::OutOfOffset => errors::ERR_OUT_OF_OFFSET,
Self::CallTooDeep => errors::ERR_CALL_TOO_DEEP,
Self::StackUnderflow => errors::ERR_STACK_UNDERFLOW,
Self::StackOverflow => errors::ERR_STACK_OVERFLOW,
Self::InvalidJump => errors::ERR_INVALID_JUMP,
Self::InvalidRange => errors::ERR_INVALID_RANGE,
Self::DesignatedInvalid => errors::ERR_DESIGNATED_INVALID,
Self::CreateCollision => errors::ERR_CREATE_COLLISION,
Self::CreateContractLimit => errors::ERR_CREATE_CONTRACT_LIMIT,
Self::InvalidCode(_) => errors::ERR_INVALID_CODE,
Self::PCUnderflow => errors::ERR_PC_UNDERFLOW,
Self::CreateEmpty => errors::ERR_CREATE_EMPTY,
Self::MaxNonce => errors::ERR_MAX_NONCE,
Self::UsizeOverflow => errors::ERR_USIZE_OVERFLOW,
Self::Other(e) => e.as_bytes(),
}
}
}
Expand All @@ -237,7 +282,7 @@ impl SubmitResult {
/// Must be incremented when making breaking changes to the `SubmitResult` ABI.
/// The current value of 7 is chosen because previously a `TransactionStatus` object
/// was first in the serialization, which is an enum with less than 7 variants.
/// Therefore, no previous `SubmitResult` would have began with a leading 7 byte,
/// Therefore, no previous `SubmitResult` would have begun with a leading 7 byte,
/// and this can be used to distinguish the new ABI (with version byte) from the old.
const VERSION: u8 = 7;

Expand Down Expand Up @@ -372,12 +417,24 @@ mod chain_id_deserialize {
pub mod errors {
use crate::{account_id::ParseAccountError, String, ToString};

pub const ERR_REVERT: &[u8; 10] = b"ERR_REVERT";
pub const ERR_NOT_ALLOWED: &[u8; 15] = b"ERR_NOT_ALLOWED";
pub const ERR_OUT_OF_FUNDS: &[u8; 16] = b"ERR_OUT_OF_FUNDS";
pub const ERR_CALL_TOO_DEEP: &[u8; 17] = b"ERR_CALL_TOO_DEEP";
pub const ERR_OUT_OF_OFFSET: &[u8; 17] = b"ERR_OUT_OF_OFFSET";
pub const ERR_OUT_OF_GAS: &[u8; 14] = b"ERR_OUT_OF_GAS";
pub const ERR_REVERT: &[u8] = b"ERR_REVERT";
pub const ERR_NOT_ALLOWED: &[u8] = b"ERR_NOT_ALLOWED";
pub const ERR_OUT_OF_FUNDS: &[u8] = b"ERR_OUT_OF_FUNDS";
pub const ERR_CALL_TOO_DEEP: &[u8] = b"ERR_CALL_TOO_DEEP";
pub const ERR_OUT_OF_OFFSET: &[u8] = b"ERR_OUT_OF_OFFSET";
pub const ERR_OUT_OF_GAS: &[u8] = b"ERR_OUT_OF_GAS";
pub const ERR_STACK_UNDERFLOW: &[u8] = b"STACK_UNDERFLOW";
pub const ERR_STACK_OVERFLOW: &[u8] = b"STACK_OVERFLOW";
pub const ERR_INVALID_JUMP: &[u8] = b"INVALID_JUMP";
pub const ERR_INVALID_RANGE: &[u8] = b"INVALID_RANGE";
pub const ERR_DESIGNATED_INVALID: &[u8] = b"DESIGNATED_INVALID";
pub const ERR_CREATE_COLLISION: &[u8] = b"CREATE_COLLISION";
pub const ERR_CREATE_CONTRACT_LIMIT: &[u8] = b"CREATE_CONTRACT_LIMIT";
pub const ERR_INVALID_CODE: &[u8] = b"INVALID_CODE";
pub const ERR_PC_UNDERFLOW: &[u8] = b"PC_UNDERFLOW";
pub const ERR_CREATE_EMPTY: &[u8] = b"CREATE_EMPTY";
pub const ERR_MAX_NONCE: &[u8] = b"MAX_NONCE";
pub const ERR_USIZE_OVERFLOW: &[u8] = b"USIZE_OVERFLOW";

#[derive(Debug)]
pub enum ParseArgsError {
Expand Down Expand Up @@ -511,4 +568,49 @@ mod tests {
let arguments = NewCallArgs::deserialize(&serde_json::to_vec(&outdated).unwrap());
assert!(arguments.is_err());
}

#[test]
fn test_serialization_transaction_status_regression() {
let bytes =
std::fs::read("../engine-tests/src/tests/res/transaction_status.borsh").unwrap();
let actual = Vec::<TransactionStatus>::try_from_slice(&bytes).unwrap();
let expected = transaction_status_variants();

assert_eq!(actual, expected);
}

#[allow(dead_code)]
fn generate_borsh_bytes() {
let variants = transaction_status_variants();

std::fs::write(
"../engine-tests/src/tests/res/transaction_status.borsh",
borsh::to_vec(&variants).unwrap(),
)
.unwrap();
}

fn transaction_status_variants() -> Vec<TransactionStatus> {
vec![
TransactionStatus::Succeed(Vec::new()),
TransactionStatus::Revert(Vec::new()),
TransactionStatus::OutOfGas,
TransactionStatus::OutOfFund,
TransactionStatus::OutOfOffset,
TransactionStatus::CallTooDeep,
TransactionStatus::StackUnderflow,
TransactionStatus::StackOverflow,
TransactionStatus::InvalidJump,
TransactionStatus::InvalidRange,
TransactionStatus::DesignatedInvalid,
TransactionStatus::CreateCollision,
TransactionStatus::CreateContractLimit,
TransactionStatus::InvalidCode(0),
TransactionStatus::PCUnderflow,
TransactionStatus::CreateEmpty,
TransactionStatus::MaxNonce,
TransactionStatus::UsizeOverflow,
TransactionStatus::Other("error".into()),
]
}
}
Loading

0 comments on commit c505f58

Please sign in to comment.