From 0fd86c86d1bc1ff4a46b4e573e392d68db804a5f Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Thu, 28 Nov 2024 16:00:21 -0300 Subject: [PATCH] enha: remove mutex that keeps transactions from executing while a block is being commited --- .github/workflows/e2e-test.yml | 10 +++++ e2e/contracts/TestEvmInput.sol | 42 +++++++++++++++++++++ e2e/test/helpers/rpc.ts | 7 ++++ e2e/test/interval/evm-input.test.ts | 30 +++++++++++++++ justfile | 7 +++- src/eth/executor/evm_input.rs | 2 +- src/eth/executor/executor.rs | 39 +++++++++---------- src/eth/primitives/point_in_time.rs | 2 +- src/eth/primitives/stratus_error.rs | 5 +++ src/eth/primitives/transaction_execution.rs | 6 ++- src/eth/storage/temporary/inmemory.rs | 13 +++++++ 11 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 e2e/contracts/TestEvmInput.sol create mode 100644 e2e/test/interval/evm-input.test.ts diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 76f8cecf5..4d741d662 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -67,6 +67,16 @@ jobs: group: ${{ github.workflow }}-external-rocks-${{ github.ref || github.run_id }} cancel-in-progress: true + e2e-interval-stratus: + name: E2E Inverval Stratus + uses: ./.github/workflows/_setup-e2e.yml + with: + justfile_recipe: "e2e-stratus 10ms" + + concurrency: + group: ${{ github.workflow }}-interval-${{ github.ref || github.run_id }} + cancel-in-progress: true + e2e-clock-stratus: name: E2E Clock Stratus in-memory uses: ./.github/workflows/_setup-e2e.yml diff --git a/e2e/contracts/TestEvmInput.sol b/e2e/contracts/TestEvmInput.sol new file mode 100644 index 000000000..e58f1fa1a --- /dev/null +++ b/e2e/contracts/TestEvmInput.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +contract TestEvmInput { + uint256 public executions; + uint256 public result; + + // Define the event for logging + event ComputationStarted(uint256 blockNumber, bool isSlow); + + function heavyComputation() public { + // Emit event with current block number + // Check if this is the first execution + if (block.number < 1000) { + emit ComputationStarted(block.number, true); + + // Perform a time-consuming loop + uint256 temp = 0; + for(uint i = 0; i < 10000; i++) { + for(uint j = 0; j < 100; j++) { + temp += i * j; + temp = temp % 1000000; // Prevent overflow + } + } + result = temp; + } else { + emit ComputationStarted(block.number, false); + result = 42; + } + executions += 1; + } + + // View function to get current block number (for testing) + function getCurrentBlock() public view returns (uint256) { + return block.number; + } + + // View function to get result + function getExecutions() public view returns (uint256) { + return executions; + } +} diff --git a/e2e/test/helpers/rpc.ts b/e2e/test/helpers/rpc.ts index d5ba747a2..a3cacafb7 100644 --- a/e2e/test/helpers/rpc.ts +++ b/e2e/test/helpers/rpc.ts @@ -20,6 +20,7 @@ import { TestContractBlockTimestamp, TestContractCounter, TestContractDenseStorage, + TestEvmInput, } from "../../typechain-types"; import { Account, CHARLIE } from "./account"; import { currentMiningIntervalInMs, currentNetwork, isStratus } from "./network"; @@ -162,6 +163,12 @@ export async function deployTestContractBalances(): Promise { + const testContractFactory = await ethers.getContractFactory("TestEvmInput"); + return await testContractFactory.connect(CHARLIE.signer()).deploy(); +} + // Deploys the "TestBlockTimestamp" contract. export async function deployTestContractBlockTimestamp(): Promise { const testContractFactory = await ethers.getContractFactory("TestContractBlockTimestamp"); diff --git a/e2e/test/interval/evm-input.test.ts b/e2e/test/interval/evm-input.test.ts new file mode 100644 index 000000000..548faf662 --- /dev/null +++ b/e2e/test/interval/evm-input.test.ts @@ -0,0 +1,30 @@ +import { expect } from "chai"; + +import { + deployTestEvmInput, + sendReset, +} from "../helpers/rpc"; +import { ContractTransactionReceipt, Result } from "ethers"; + +// This test needs to be ran with stratus on a very fast block production rate (around 10ms is fast enough) +describe("Evm Input", () => { + describe("Transaction Execution", () => { + it("should not be executed in one block but added to another", async () => { + await sendReset(); + const contract = await deployTestEvmInput(); + await contract.waitForDeployment(); + let tx = await contract.heavyComputation() + let receipt = await tx.wait() as ContractTransactionReceipt; + const event = receipt.logs[0]; + const logs = contract.interface.parseLog({ + topics: event.topics, + data: event.data, + })?.args as Result; + + expect(receipt.blockNumber).to.eq(logs.blockNumber); + expect(logs.blockNumber).gt(1000); + expect(logs.isSlow).to.be.false; + expect(await contract.getExecutions()).to.eq(1); + }); + }); +}); diff --git a/justfile b/justfile index 05eea6c46..08559f5df 100644 --- a/justfile +++ b/justfile @@ -192,7 +192,11 @@ e2e-stratus block-mode="automine" test="": just _wait_for_stratus just _log "Running E2E tests" - just e2e stratus {{block-mode}} "{{test}}" + if [[ {{block-mode}} =~ ^[0-9]+(ms|s)$ ]]; then + just e2e stratus interval "{{test}}" + else + just e2e stratus {{block-mode}} "{{test}}" + fi result_code=$? just _log "Killing Stratus" @@ -213,7 +217,6 @@ e2e-stratus-rocks block-mode="automine" test="": just _wait_for_stratus just _log "Running E2E tests" - just e2e stratus {{block-mode}} "{{test}}" result_code=$? just _log "Killing Stratus" diff --git a/src/eth/executor/evm_input.rs b/src/eth/executor/evm_input.rs index 82def1f2d..193d2783a 100644 --- a/src/eth/executor/evm_input.rs +++ b/src/eth/executor/evm_input.rs @@ -21,7 +21,7 @@ use crate::if_else; use crate::log_and_err; /// EVM input data. Usually derived from a transaction or call. -#[derive(DebugAsJson, Clone, Default, serde::Serialize)] +#[derive(DebugAsJson, Clone, Default, serde::Serialize, serde::Deserialize, fake::Dummy, PartialEq)] pub struct EvmInput { /// Operation party address. /// diff --git a/src/eth/executor/executor.rs b/src/eth/executor/executor.rs index 8984c884d..225267f84 100644 --- a/src/eth/executor/executor.rs +++ b/src/eth/executor/executor.rs @@ -397,17 +397,6 @@ impl Executor { // acquire serial execution lock let _serial_lock = self.locks.serial.lock_or_clear("executor serial lock was poisoned"); - // WORKAROUND: prevents interval miner mining blocks while a transaction is being executed. - // this can be removed when we implement conflict detection for block number - let _miner_lock = { - if self.miner.mode().is_interval() { - let miner_lock = Some(self.miner.locks.mine_and_commit.lock_or_clear("miner mine_and_commit lock was poisoned")); - miner_lock - } else { - None - } - }; - // execute transaction self.execute_local_transaction_attempts(tx.clone(), EvmRoute::Serial, INFINITE_ATTEMPTS) } @@ -418,12 +407,13 @@ impl Executor { let parallel_attempt = self.execute_local_transaction_attempts(tx.clone(), EvmRoute::Parallel, 1); match parallel_attempt { Ok(tx_execution) => Ok(tx_execution), - Err(e) => + Err(e) => { if let StratusError::TransactionConflict(_) = e { self.execute_local_transaction_attempts(tx.clone(), EvmRoute::Serial, INFINITE_ATTEMPTS) } else { Err(e) - }, + } + } } } }; @@ -499,28 +489,35 @@ impl Executor { "executing local transaction attempt" ); - let evm_result = match self.evms.execute(evm_input, evm_route) { + let evm_result = match self.evms.execute(evm_input.clone(), evm_route) { Ok(evm_result) => evm_result, Err(e) => return Err(e), }; // save execution to temporary storage // in case of failure, retry if conflict or abandon if unexpected error - let tx_execution = TransactionExecution::new_local(tx_input.clone(), evm_result.clone()); - match self.miner.save_execution(tx_execution.clone(), true) { + let tx_execution = TransactionExecution::new_local(tx_input.clone(), evm_input, evm_result.clone()); + match self.miner.save_execution(tx_execution.clone(), matches!(evm_route, EvmRoute::Parallel)) { Ok(_) => { return Ok(tx_execution); } - Err(e) => - if let StratusError::TransactionConflict(ref conflicts) = e { + Err(e) => match e { + StratusError::TransactionConflict(ref conflicts) => { tracing::warn!(%attempt, ?conflicts, "temporary storage conflict detected when saving execution"); if attempt >= max_attempts { return Err(e); } continue; - } else { - return Err(e); - }, + } + StratusError::TransactionEvmInputMismatch {ref expected, ref actual} => { + tracing::warn!(?expected, ?actual, "evm input and block header mismatch"); + if attempt >= max_attempts { + return Err(e); + } + continue; + } + _ => return Err(e), + }, } } } diff --git a/src/eth/primitives/point_in_time.rs b/src/eth/primitives/point_in_time.rs index fad13cd1a..13920c838 100644 --- a/src/eth/primitives/point_in_time.rs +++ b/src/eth/primitives/point_in_time.rs @@ -2,7 +2,7 @@ use crate::eth::primitives::BlockNumber; use crate::infra::metrics::MetricLabelValue; /// EVM storage point-in-time indicator. -#[derive(Debug, strum::Display, Clone, Copy, Default, strum::EnumIs, serde::Serialize)] +#[derive(Debug, strum::Display, Clone, Copy, Default, strum::EnumIs, serde::Serialize, serde::Deserialize, fake::Dummy, PartialEq)] pub enum PointInTime { /// State of `Account` or `Slot` at the pending block being mined. /// diff --git a/src/eth/primitives/stratus_error.rs b/src/eth/primitives/stratus_error.rs index 003d48152..2e5d9c0e3 100644 --- a/src/eth/primitives/stratus_error.rs +++ b/src/eth/primitives/stratus_error.rs @@ -7,6 +7,7 @@ use jsonrpsee::types::ErrorObjectOwned; use strum::EnumProperty; use crate::alias::JsonValue; +use crate::eth::executor::EvmInput; use crate::eth::primitives::Address; use crate::eth::primitives::BlockFilter; use crate::eth::primitives::BlockNumber; @@ -101,6 +102,10 @@ pub enum StratusError { #[strum(props(kind = "execution"))] TransactionFromZeroAddress, + #[error("Transaction input does not match block header")] + #[strum(props(kind = "execution"))] + TransactionEvmInputMismatch { expected: EvmInput, actual: EvmInput }, + // ------------------------------------------------------------------------- // Storage // ------------------------------------------------------------------------- diff --git a/src/eth/primitives/transaction_execution.rs b/src/eth/primitives/transaction_execution.rs index 352696224..692ca22b3 100644 --- a/src/eth/primitives/transaction_execution.rs +++ b/src/eth/primitives/transaction_execution.rs @@ -1,6 +1,7 @@ use display_json::DebugAsJson; use crate::eth::executor::EvmExecutionResult; +use crate::eth::executor::EvmInput; use crate::eth::primitives::EvmExecution; use crate::eth::primitives::EvmExecutionMetrics; use crate::eth::primitives::ExternalReceipt; @@ -20,8 +21,8 @@ pub enum TransactionExecution { impl TransactionExecution { /// Creates a new local transaction execution. - pub fn new_local(tx: TransactionInput, result: EvmExecutionResult) -> Self { - Self::Local(LocalTransactionExecution { input: tx, result }) + pub fn new_local(tx: TransactionInput, evm_input: EvmInput, result: EvmExecutionResult) -> Self { + Self::Local(LocalTransactionExecution { input: tx, evm_input, result }) } /// Extracts the inner [`LocalTransactionExecution`] if the execution is a local execution. @@ -85,6 +86,7 @@ impl TransactionExecution { #[cfg_attr(test, derive(serde::Deserialize, fake::Dummy, PartialEq))] pub struct LocalTransactionExecution { pub input: TransactionInput, + pub evm_input: EvmInput, pub result: EvmExecutionResult, } diff --git a/src/eth/storage/temporary/inmemory.rs b/src/eth/storage/temporary/inmemory.rs index 12b66e3e9..23ca6bccd 100644 --- a/src/eth/storage/temporary/inmemory.rs +++ b/src/eth/storage/temporary/inmemory.rs @@ -7,6 +7,7 @@ use std::sync::RwLockWriteGuard; use nonempty::NonEmpty; +use crate::eth::executor::EvmInput; use crate::eth::primitives::Account; use crate::eth::primitives::Address; use crate::eth::primitives::BlockNumber; @@ -117,6 +118,18 @@ impl TemporaryStorage for InMemoryTemporaryStorage { fn save_pending_execution(&self, tx: TransactionExecution, check_conflicts: bool) -> Result<(), StratusError> { // check conflicts let mut states = self.lock_write(); + if let TransactionExecution::Local(tx) = &tx { + tracing::info!("hereeeeee"); + + let expected_input = EvmInput::from_eth_transaction(tx.input.clone(), states.head.block.header.clone()); + + if expected_input != tx.evm_input { + return Err(StratusError::TransactionEvmInputMismatch { + expected: expected_input, + actual: tx.evm_input.clone(), + }); + } + } if check_conflicts { if let Some(conflicts) = do_check_conflicts(&states, tx.execution()) {