Skip to content

fix(evm): remove system tx presence check from executor finish() #2736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gakonst wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(evm): remove system tx presence check from executor finish() #2736

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 1 addition & 29 deletions crates/evm/src/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,15 +466,6 @@ where
fn finish(
self,
) -> Result<(Self::Evm, BlockExecutionResult<Self::Receipt>), BlockExecutionError> {
if self.section
Copy link
Contributor

@mattsse mattsse Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we remove this, is this field then even useful?

this isnt documented so i dont really know how this is used

Copy link
Member

@klkvr klkvr Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this field is used in validate_tx to track section transitions:

tempo/crates/evm/src/block.rs

Lines 289 to 349 in 174ae32

pub(crate) fn validate_tx(
&self,
tx: &TempoTxEnvelope,
gas_used: u64,
) -> Result<BlockSection, BlockValidationError> {
// Start with processing of transaction kinds that require specific sections.
if tx.is_system_tx() {
self.validate_system_tx(tx)
} else if let Some(tx_proposer) = tx.subblock_proposer() {
match self.section {
BlockSection::GasIncentive | BlockSection::System { .. } => {
Err(BlockValidationError::msg("subblock section already passed"))
}
BlockSection::StartOfBlock | BlockSection::NonShared => {
Ok(BlockSection::SubBlock {
proposer: tx_proposer,
})
}
BlockSection::SubBlock { proposer } => {
if proposer == tx_proposer
|| !self.seen_subblocks.iter().any(|(p, _)| *p == tx_proposer)
{
Ok(BlockSection::SubBlock {
proposer: tx_proposer,
})
} else {
Err(BlockValidationError::msg(
"proposer's subblock already processed",
))
}
}
}
} else {
match self.section {
BlockSection::StartOfBlock | BlockSection::NonShared => {
if gas_used > self.non_shared_gas_left
|| (!tx.is_payment() && gas_used > self.non_payment_gas_left)
{
// Assume that this transaction wants to make use of gas incentive section
//
// This would only be possible if no non-empty subblocks were included.
Ok(BlockSection::GasIncentive)
} else {
Ok(BlockSection::NonShared)
}
}
BlockSection::SubBlock { .. } => {
// If we were just processing a subblock, assume that this transaction wants to make
// use of gas incentive section, thus concluding subblocks execution.
Ok(BlockSection::GasIncentive)
}
BlockSection::GasIncentive => Ok(BlockSection::GasIncentive),
BlockSection::System { .. } => {
trace!(target: "tempo::block", tx_hash = ?*tx.tx_hash(), "Rejecting: regular transaction after system transaction");
Err(BlockValidationError::msg(
"regular transaction can't follow system transaction",
))
}
}
}
}

so the idea for this PR is that we already check on TempoConsensus side that system tx is present:

tempo/crates/consensus/src/lib.rs

Lines 167 to 171 in 174ae32

if end_of_block_system_txs.len() != SYSTEM_TX_COUNT {
return Err(ConsensusError::Other(
"Block must contain end-of-block system txs".to_string(),
));
}

and thus executor only needs to enforce that this system tx contents are correct. given that consenus validation is skipped in simulateV1, this should unblock it

!= (BlockSection::System {
seen_subblocks_signatures: true,
})
{
return Err(
BlockValidationError::msg("end-of-block system transactions not seen").into(),
);
}
self.inner.finish()
}

Expand Down Expand Up @@ -1103,28 +1094,9 @@ mod tests {
fn test_finish() {
let chainspec = test_chainspec();
let mut db = State::builder().with_bundle_update().build();

// Set section to System with seen_subblocks_signatures
let executor = TestExecutorBuilder::default()
.with_section(BlockSection::System {
seen_subblocks_signatures: true,
})
.build(&mut db, &chainspec);

let result = executor.finish();
assert!(result.is_ok());
}

#[test]
fn test_finish_system_tx_not_seen() {
let chainspec = test_chainspec();
let mut db = State::builder().with_bundle_update().build();
let executor = TestExecutorBuilder::default().build(&mut db, &chainspec);

// Section is StartOfBlock, so system tx not seen
let result = executor.finish();
assert!(result.is_err());
let err = result.err().unwrap();
assert_eq!(err.to_string(), "end-of-block system transactions not seen");
assert!(result.is_ok());
}
}
Loading