diff --git a/engine-sdk/src/caching.rs b/engine-sdk/src/caching.rs new file mode 100644 index 000000000..310d91a3f --- /dev/null +++ b/engine-sdk/src/caching.rs @@ -0,0 +1,13 @@ +use aurora_engine_types::BTreeMap; + +/// A naive cache storing all key-value pairs it learns about.. +#[derive(Default)] +pub struct FullCache { + inner: BTreeMap, +} + +impl FullCache { + pub fn get_or_insert_with V>(&mut self, k: K, f: F) -> &mut V { + self.inner.entry(k).or_insert_with(f) + } +} diff --git a/engine-sdk/src/dup_cache.rs b/engine-sdk/src/dup_cache.rs deleted file mode 100644 index 67be908e5..000000000 --- a/engine-sdk/src/dup_cache.rs +++ /dev/null @@ -1,41 +0,0 @@ -/// The intention of this struct is to prevent repeating duplicate computations/IO with the -/// same input (key). However, unlike memoization or typical caching, this only remembers the -/// most recent key-value pair. This means it is optimized for consecutive duplicate lookups, -/// as opposed to general duplicated lookups. The benefit is that its memory footprint and -/// internal logic are both minimal, and the drawback is that its use case is very narrow. -#[derive(Default)] -pub struct DupCache { - key: K, - value: V, -} - -impl DupCache { - pub fn get_or_insert_with V>(&mut self, k: &K, f: F) -> &mut V { - if &self.key != k { - let new_value = f(); - self.value = new_value; - self.key = *k; - } - - &mut self.value - } -} - -/// Same as `DupCache` but optimized for the case that `K = (K1, K2)`. -#[derive(Default)] -pub struct PairDupCache { - key: (K1, K2), - value: V, -} - -impl PairDupCache { - pub fn get_or_insert_with V>(&mut self, k: (&K1, &K2), f: F) -> &mut V { - if (&self.key.0 != k.0) || (&self.key.1 != k.1) { - let new_value = f(); - self.value = new_value; - self.key = (*k.0, *k.1); - } - - &mut self.value - } -} diff --git a/engine-sdk/src/lib.rs b/engine-sdk/src/lib.rs index 876bdf9ef..4df77bcb1 100644 --- a/engine-sdk/src/lib.rs +++ b/engine-sdk/src/lib.rs @@ -6,7 +6,7 @@ use crate::prelude::Address; use crate::prelude::{H256, STORAGE_PRICE_PER_BYTE}; pub use types::keccak; -pub mod dup_cache; +pub mod caching; pub mod env; pub mod error; pub mod io; diff --git a/engine-tests/src/test_utils/mod.rs b/engine-tests/src/test_utils/mod.rs index 41ab39c22..94b43e8ee 100644 --- a/engine-tests/src/test_utils/mod.rs +++ b/engine-tests/src/test_utils/mod.rs @@ -827,11 +827,12 @@ pub fn panic_on_fail(status: TransactionStatus) { } pub fn assert_gas_bound(total_gas: u64, tgas_bound: u64) { - let bound = tgas_bound * 1_000_000_000_000; + // Add 1 to round up + let tgas_used = (total_gas / 1_000_000_000_000) + 1; assert!( - total_gas <= bound, - "{} Tgas is not less than {} Tgas", - total_gas / 1_000_000_000_000, + tgas_used == tgas_bound, + "{} Tgas is not equal to {} Tgas", + tgas_used, tgas_bound, ); } diff --git a/engine-tests/src/tests/one_inch.rs b/engine-tests/src/tests/one_inch.rs index 49ef8313b..2b53d5e80 100644 --- a/engine-tests/src/tests/one_inch.rs +++ b/engine-tests/src/tests/one_inch.rs @@ -39,7 +39,7 @@ fn test_1inch_liquidity_protocol() { let (result, profile, pool) = helper.create_pool(&pool_factory, token_a.0.address, token_b.0.address); assert!(result.gas_used >= 4_500_000); // more than 4.5M EVM gas used - assert_gas_bound(profile.all_gas(), 33); // less than 33 NEAR Tgas used + assert_gas_bound(profile.all_gas(), 29); // Approve giving ERC-20 tokens to the pool helper.approve_erc20_tokens(&token_a, pool.address()); @@ -58,7 +58,7 @@ fn test_1inch_liquidity_protocol() { }, ); assert!(result.gas_used >= 302_000); // more than 302k EVM gas used - assert_gas_bound(profile.all_gas(), 44); // less than 44 NEAR Tgas used + assert_gas_bound(profile.all_gas(), 35); // Same here helper.runner.context.block_timestamp += 10_000_000 * 1_000_000_000; @@ -73,7 +73,7 @@ fn test_1inch_liquidity_protocol() { }, ); assert!(result.gas_used >= 210_000); // more than 210k EVM gas used - assert_gas_bound(profile.all_gas(), 46); // less than 46 NEAR Tgas used + assert_gas_bound(profile.all_gas(), 37); let (result, profile) = helper.pool_withdraw( &pool, @@ -84,7 +84,7 @@ fn test_1inch_liquidity_protocol() { }, ); assert!(result.gas_used >= 150_000); // more than 150k EVM gas used - assert_gas_bound(profile.all_gas(), 38); // less than 38 NEAR Tgas used + assert_gas_bound(profile.all_gas(), 32); } #[test] diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs index c6907dc19..7fb58d619 100644 --- a/engine-tests/src/tests/repro.rs +++ b/engine-tests/src/tests/repro.rs @@ -46,7 +46,7 @@ fn repro_GdASJ3KESs() { SubmitResult::try_from_slice(&outcome.return_data.as_value().unwrap()).unwrap(); assert_eq!(submit_result.gas_used, 706713); - assert_eq!(239, profile.all_gas() / 1_000_000_000_000); + assert_eq!(173, profile.all_gas() / 1_000_000_000_000); // Also validate the SubmitResult in the standalone engine let mut standalone = standalone::StandaloneRunner::default(); @@ -99,7 +99,7 @@ fn repro_8ru7VEA() { SubmitResult::try_from_slice(&outcome.return_data.as_value().unwrap()).unwrap(); assert_eq!(submit_result.gas_used, 1732181); - assert_eq!(411, profile.all_gas() / 1_000_000_000_000); + assert_eq!(309, profile.all_gas() / 1_000_000_000_000); // Also validate the SubmitResult in the standalone engine let mut standalone = standalone::StandaloneRunner::default(); diff --git a/engine-tests/src/tests/uniswap.rs b/engine-tests/src/tests/uniswap.rs index d471b02fc..67c2ff048 100644 --- a/engine-tests/src/tests/uniswap.rs +++ b/engine-tests/src/tests/uniswap.rs @@ -28,10 +28,7 @@ fn test_uniswap_input_multihop() { let mut context = UniswapTestContext::new("uniswap"); // evm_gas = 970k - // near total gas = 204 Tgas - // Wish: optimize so that this transaction costs less than 200 Tgas. - // For now we just have to increase the burnt gas limit to make it run to completion. - context.runner.wasm_config.limit_config.max_gas_burnt = 500_000_000_000_000; + // near total gas = 163 Tgas let tokens = context.create_tokens(10, MINT_AMOUNT.into()); for (token_a, token_b) in tokens.iter().zip(tokens.iter().skip(1)) { @@ -41,9 +38,7 @@ fn test_uniswap_input_multihop() { let (_amount_out, _evm_gas, profile) = context.exact_input(&tokens, INPUT_AMOUNT.into()); - println!("{:?}", profile.host_breakdown); - println!("NEAR_GAS_WASM {:?}", profile.wasm_gas()); - println!("NEAR_GAS_TOTAL {:?}", profile.all_gas()); + assert_eq!(163, profile.all_gas() / 1_000_000_000_000); } #[test] @@ -54,7 +49,7 @@ fn test_uniswap_exact_output() { let (_result, profile) = context.add_equal_liquidity(LIQUIDITY_AMOUNT.into(), &token_a, &token_b); - test_utils::assert_gas_bound(profile.all_gas(), 57); + test_utils::assert_gas_bound(profile.all_gas(), 47); let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas(); assert!( 25 <= wasm_fraction && wasm_fraction <= 35, @@ -64,7 +59,7 @@ fn test_uniswap_exact_output() { let (_amount_in, profile) = context.exact_output_single(&token_a, &token_b, OUTPUT_AMOUNT.into()); - test_utils::assert_gas_bound(profile.all_gas(), 31); + test_utils::assert_gas_bound(profile.all_gas(), 26); let wasm_fraction = 100 * profile.wasm_gas() / profile.all_gas(); assert!( 25 <= wasm_fraction && wasm_fraction <= 35, diff --git a/engine/src/engine.rs b/engine/src/engine.rs index 2ba0068a9..203b15a96 100644 --- a/engine/src/engine.rs +++ b/engine/src/engine.rs @@ -6,7 +6,7 @@ use evm::{Config, CreateScheme, ExitError, ExitFatal, ExitReason}; use crate::connector::EthConnectorContract; use crate::map::BijectionMap; -use aurora_engine_sdk::dup_cache::{DupCache, PairDupCache}; +use aurora_engine_sdk::caching::FullCache; use aurora_engine_sdk::env::Env; use aurora_engine_sdk::io::{StorageIntermediate, IO}; use aurora_engine_sdk::promise::{PromiseHandler, PromiseId}; @@ -422,9 +422,9 @@ pub struct Engine<'env, I: IO, E: Env> { io: I, env: &'env E, generation_cache: RefCell>, - account_info_cache: RefCell>, - contract_code_cache: RefCell>>, - contract_storage_cache: RefCell>, + account_info_cache: RefCell>, + contract_code_cache: RefCell>>, + contract_storage_cache: RefCell>, } pub(crate) const CONFIG: &Config = &Config::london(); @@ -457,9 +457,9 @@ impl<'env, I: IO + Copy, E: Env> Engine<'env, I, E> { io, env, generation_cache: RefCell::new(BTreeMap::new()), - account_info_cache: RefCell::new(DupCache::default()), - contract_code_cache: RefCell::new(DupCache::default()), - contract_storage_cache: RefCell::new(PairDupCache::default()), + account_info_cache: RefCell::new(FullCache::default()), + contract_code_cache: RefCell::new(FullCache::default()), + contract_storage_cache: RefCell::new(FullCache::default()), } } @@ -1519,7 +1519,7 @@ impl<'env, I: IO + Copy, E: Env> evm::backend::Backend for Engine<'env, I, E> { fn exists(&self, address: H160) -> bool { let address = Address::new(address); let mut cache = self.account_info_cache.borrow_mut(); - let basic_info = cache.get_or_insert_with(&address, || Basic { + let basic_info = cache.get_or_insert_with(address, || Basic { nonce: get_nonce(&self.io, &address), balance: get_balance(&self.io, &address).raw(), }); @@ -1527,7 +1527,7 @@ impl<'env, I: IO + Copy, E: Env> evm::backend::Backend for Engine<'env, I, E> { return true; } let mut cache = self.contract_code_cache.borrow_mut(); - let code = cache.get_or_insert_with(&address, || get_code(&self.io, &address)); + let code = cache.get_or_insert_with(address, || get_code(&self.io, &address)); !code.is_empty() } @@ -1537,7 +1537,7 @@ impl<'env, I: IO + Copy, E: Env> evm::backend::Backend for Engine<'env, I, E> { let result = self .account_info_cache .borrow_mut() - .get_or_insert_with(&address, || Basic { + .get_or_insert_with(address, || Basic { nonce: get_nonce(&self.io, &address), balance: get_balance(&self.io, &address).raw(), }) @@ -1550,7 +1550,7 @@ impl<'env, I: IO + Copy, E: Env> evm::backend::Backend for Engine<'env, I, E> { let address = Address::new(address); self.contract_code_cache .borrow_mut() - .get_or_insert_with(&address, || get_code(&self.io, &address)) + .get_or_insert_with(address, || get_code(&self.io, &address)) .clone() } @@ -1565,7 +1565,7 @@ impl<'env, I: IO + Copy, E: Env> evm::backend::Backend for Engine<'env, I, E> { let result = *self .contract_storage_cache .borrow_mut() - .get_or_insert_with((&address, &index), || { + .get_or_insert_with((address, index), || { get_storage(&self.io, &address, &index, generation) }); result