Skip to content

Commit 304cff9

Browse files
0xalpharush0xalpharush
committed
fix: bound ABI decoding in Zone transaction paths
1 parent 961e664 commit 304cff9

5 files changed

Lines changed: 197 additions & 53 deletions

File tree

‎crates/evm/src/executor.rs‎

Lines changed: 130 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ use alloy_evm::{
1313
},
1414
eth::{EthBlockExecutor, EthTxResult},
1515
};
16-
use alloy_sol_types::SolEvent as _;
16+
use alloy_sol_types::{SolCall as _, SolEvent as _};
1717
use reth_evm::block::StateDB;
1818
use reth_revm::{Inspector, context::result::ResultAndState};
19+
use tempo_chainspec::hardfork::TempoHardfork;
1920
use tempo_evm::{TempoBlockExecutionCtx, TempoReceiptBuilder};
2021
use tempo_primitives::{TempoReceipt, TempoTxEnvelope, TempoTxType};
2122
use tempo_revm::evm::TempoContext;
@@ -41,15 +42,19 @@ enum ZoneBlockPhase {
4142
}
4243

4344
impl ZoneBlockPhase {
44-
fn validate_transaction(self, tx: &TempoTxEnvelope) -> Result<Self, BlockExecutionError> {
45+
fn validate_transaction(
46+
self,
47+
tx: &TempoTxEnvelope,
48+
spec: TempoHardfork,
49+
) -> Result<Self, BlockExecutionError> {
4550
if tx.subblock_proposer().is_some() {
4651
return Err(BlockValidationError::msg(
4752
"subblock transactions are not supported in zone blocks",
4853
)
4954
.into());
5055
}
5156

52-
let tx_kind = ZoneTransactionKind::classify(tx);
57+
let tx_kind = ZoneTransactionKind::classify(tx, spec);
5358

5459
match (self, tx_kind) {
5560
(Self::AwaitingAdvanceTempo, ZoneTransactionKind::AdvanceTempo) => Ok(Self::Executing),
@@ -94,7 +99,7 @@ enum ZoneTransactionKind {
9499
}
95100

96101
impl ZoneTransactionKind {
97-
fn classify(tx: &TempoTxEnvelope) -> Self {
102+
fn classify(tx: &TempoTxEnvelope, spec: TempoHardfork) -> Self {
98103
if !tx.is_system_tx() {
99104
return Self::Regular;
100105
}
@@ -106,7 +111,13 @@ impl ZoneTransactionKind {
106111
}
107112

108113
if tx.calls().any(|(kind, input)| {
109-
kind.to() == Some(&ZONE_OUTBOX_ADDRESS) && is_finalize_withdrawal_batch_calldata(input)
114+
kind.to() == Some(&ZONE_OUTBOX_ADDRESS)
115+
&& if spec.is_t11() {
116+
input.get(..4)
117+
== Some(IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR.as_slice())
118+
} else {
119+
is_finalize_withdrawal_batch_calldata(input)
120+
}
110121
}) {
111122
return Self::FinalizeWithdrawalBatch;
112123
}
@@ -208,7 +219,8 @@ where
208219
tempo_tx_env.expiring_nonce_idx = None;
209220
}
210221

211-
let next_phase = self.phase.validate_transaction(recovered.tx())?;
222+
let spec = self.evm().ctx().cfg.spec;
223+
let next_phase = self.phase.validate_transaction(recovered.tx(), spec)?;
212224

213225
let result = self
214226
.inner
@@ -285,7 +297,7 @@ mod tests {
285297
use reth_chainspec::EthChainSpec as _;
286298
use reth_primitives_traits::Recovered;
287299
use revm::database::{CacheDB, EmptyDB};
288-
use tempo_chainspec::spec::DEV;
300+
use tempo_chainspec::{hardfork::TempoHardfork, spec::DEV};
289301
use tempo_evm::TempoBlockExecutionCtx;
290302
use tempo_precompiles::{
291303
DEFAULT_FEE_TOKEN, TIP_FEE_MANAGER_ADDRESS,
@@ -399,7 +411,9 @@ mod tests {
399411
let advance_tempo = advance_tempo_tx();
400412
let mut phase = ZoneBlockPhase::AwaitingAdvanceTempo;
401413

402-
let next_phase = phase.validate_transaction(&advance_tempo).unwrap();
414+
let next_phase = phase
415+
.validate_transaction(&advance_tempo, TempoHardfork::T11)
416+
.unwrap();
403417
assert_eq!(phase, ZoneBlockPhase::AwaitingAdvanceTempo);
404418
assert_eq!(next_phase, ZoneBlockPhase::Executing);
405419

@@ -414,15 +428,15 @@ mod tests {
414428
let ordinary = system_tx(Address::ZERO, Bytes::new());
415429

416430
let missing_first = ZoneBlockPhase::AwaitingAdvanceTempo
417-
.validate_transaction(&ordinary)
431+
.validate_transaction(&ordinary, TempoHardfork::T11)
418432
.unwrap_err();
419433
assert_eq!(
420434
missing_first.to_string(),
421435
"advanceTempo must be the first transaction in a zone block"
422436
);
423437

424438
let duplicate = ZoneBlockPhase::Executing
425-
.validate_transaction(&advance_tempo)
439+
.validate_transaction(&advance_tempo, TempoHardfork::T11)
426440
.unwrap_err();
427441
assert_eq!(
428442
duplicate.to_string(),
@@ -435,7 +449,7 @@ mod tests {
435449
let finalize = finalize_withdrawal_batch_tx();
436450
assert_eq!(
437451
ZoneBlockPhase::Executing
438-
.validate_transaction(&finalize)
452+
.validate_transaction(&finalize, TempoHardfork::T11)
439453
.unwrap(),
440454
ZoneBlockPhase::WithdrawalsFinalized
441455
);
@@ -449,7 +463,7 @@ mod tests {
449463
.into(),
450464
);
451465
let error = ZoneBlockPhase::Executing
452-
.validate_transaction(&setter)
466+
.validate_transaction(&setter, TempoHardfork::T11)
453467
.unwrap_err();
454468
assert_eq!(
455469
error.to_string(),
@@ -462,21 +476,97 @@ mod tests {
462476
Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR),
463477
);
464478
let error = ZoneBlockPhase::Executing
465-
.validate_transaction(&malformed_finalize)
479+
.validate_transaction(&malformed_finalize, TempoHardfork::T10)
480+
.unwrap_err();
481+
assert_eq!(
482+
error.to_string(),
483+
"system transactions after advanceTempo must call \
484+
ZoneOutbox.finalizeWithdrawalBatch"
485+
);
486+
assert_eq!(
487+
ZoneBlockPhase::Executing
488+
.validate_transaction(&malformed_finalize, TempoHardfork::T11)
489+
.unwrap(),
490+
ZoneBlockPhase::WithdrawalsFinalized
491+
);
492+
493+
let mut trailing_calldata = IZoneOutbox::finalizeWithdrawalBatchCall {
494+
count: U256::ZERO,
495+
blockNumber: 1,
496+
encryptedSenders: vec![],
497+
}
498+
.abi_encode();
499+
trailing_calldata.extend_from_slice(&[0; 32]);
500+
let trailing_finalize = system_tx(ZONE_OUTBOX_ADDRESS, trailing_calldata.into());
501+
let error = ZoneBlockPhase::Executing
502+
.validate_transaction(&trailing_finalize, TempoHardfork::T10)
466503
.unwrap_err();
467504
assert_eq!(
468505
error.to_string(),
469506
"system transactions after advanceTempo must call \
470507
ZoneOutbox.finalizeWithdrawalBatch"
471508
);
509+
assert_eq!(
510+
ZoneBlockPhase::Executing
511+
.validate_transaction(&trailing_finalize, TempoHardfork::T11)
512+
.unwrap(),
513+
ZoneBlockPhase::WithdrawalsFinalized
514+
);
515+
}
516+
517+
#[test]
518+
fn malformed_t11_finalization_does_not_advance_block_phase() {
519+
let mut zone_genesis = DEV.genesis().clone();
520+
zone_genesis.config.chain_id = zone_chain_id(DEV.chain().id(), 2).unwrap();
521+
let chain_spec = std::sync::Arc::new(ZoneChainSpec::from_genesis(zone_genesis).unwrap());
522+
let factory =
523+
ZoneEvmFactory::new(chain_spec.clone(), MockL1Reader::default(), Address::ZERO);
524+
let mut env = EvmEnv::default();
525+
env.cfg_env.spec = TempoHardfork::T11;
526+
let evm = factory.create_evm(CacheDB::new(EmptyDB::default()), env);
527+
let ctx = TempoBlockExecutionCtx {
528+
inner: EthBlockExecutionCtx {
529+
parent_hash: B256::ZERO,
530+
parent_beacon_block_root: None,
531+
ommers: &[],
532+
withdrawals: None,
533+
extra_data: Bytes::new(),
534+
tx_count_hint: Some(1),
535+
slot_number: None,
536+
},
537+
general_gas_limit: 0,
538+
shared_gas_limit: 0,
539+
validator_set: None,
540+
consensus_context: None,
541+
subblock_fee_recipients: Default::default(),
542+
};
543+
let mut executor = ZoneBlockExecutor::new(evm, ctx, &chain_spec);
544+
executor.phase = ZoneBlockPhase::Executing;
545+
546+
let tx = Recovered::new_unchecked(
547+
system_tx(
548+
ZONE_OUTBOX_ADDRESS,
549+
Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR),
550+
),
551+
TEMPO_SYSTEM_TX_SENDER,
552+
);
553+
let error = executor.execute_transaction_without_commit(tx).unwrap_err();
554+
555+
assert!(
556+
error
557+
.to_string()
558+
.contains("system transaction execution failed"),
559+
"unexpected error: {error}"
560+
);
561+
assert_eq!(executor.phase, ZoneBlockPhase::Executing);
472562
}
473563

474564
#[test]
475565
fn ordinary_transactions_are_allowed_after_advance_tempo() {
476566
let ordinary = ordinary_tx(Address::ZERO, Bytes::new());
477567
assert_eq!(
478568
ZoneBlockPhase::Executing
479-
.validate_transaction(&ordinary)
569+
.validate_transaction(&ordinary, TempoHardfork::T11)
480570
.unwrap(),
481571
ZoneBlockPhase::Executing
482572
);
@@ -534,7 +624,9 @@ mod tests {
534624
ZoneBlockPhase::Executing,
535625
ZoneBlockPhase::WithdrawalsFinalized,
536626
] {
537-
let error = phase.validate_transaction(&subblock).unwrap_err();
627+
let error = phase
628+
.validate_transaction(&subblock, TempoHardfork::T11)
629+
.unwrap_err();
538630
assert_eq!(
539631
error.to_string(),
540632
"subblock transactions are not supported in zone blocks"
@@ -549,12 +641,16 @@ mod tests {
549641
let advance_tempo = advance_tempo_tx();
550642
let mut phase = ZoneBlockPhase::Executing;
551643

552-
let next_phase = phase.validate_transaction(&finalize).unwrap();
644+
let next_phase = phase
645+
.validate_transaction(&finalize, TempoHardfork::T11)
646+
.unwrap();
553647
assert_eq!(phase, ZoneBlockPhase::Executing);
554648

555649
phase.advance_to(next_phase);
556650
for tx in [&ordinary, &finalize, &advance_tempo] {
557-
let error = phase.validate_transaction(tx).unwrap_err();
651+
let error = phase
652+
.validate_transaction(tx, TempoHardfork::T11)
653+
.unwrap_err();
558654
assert_eq!(
559655
error.to_string(),
560656
"finalizeWithdrawalBatch must be the last transaction in a zone block"
@@ -571,14 +667,14 @@ mod tests {
571667

572668
assert_eq!(
573669
ZoneBlockPhase::AwaitingAdvanceTempo
574-
.validate_transaction(&advance)
670+
.validate_transaction(&advance, TempoHardfork::T11)
575671
.unwrap(),
576672
ZoneBlockPhase::Executing
577673
);
578674
for tx in [&regular, &finalize, &unexpected_system] {
579675
assert_eq!(
580676
ZoneBlockPhase::AwaitingAdvanceTempo
581-
.validate_transaction(tx)
677+
.validate_transaction(tx, TempoHardfork::T11)
582678
.unwrap_err()
583679
.to_string(),
584680
"advanceTempo must be the first transaction in a zone block"
@@ -587,26 +683,26 @@ mod tests {
587683

588684
assert_eq!(
589685
ZoneBlockPhase::Executing
590-
.validate_transaction(&regular)
686+
.validate_transaction(&regular, TempoHardfork::T11)
591687
.unwrap(),
592688
ZoneBlockPhase::Executing
593689
);
594690
assert_eq!(
595691
ZoneBlockPhase::Executing
596-
.validate_transaction(&finalize)
692+
.validate_transaction(&finalize, TempoHardfork::T11)
597693
.unwrap(),
598694
ZoneBlockPhase::WithdrawalsFinalized
599695
);
600696
assert_eq!(
601697
ZoneBlockPhase::Executing
602-
.validate_transaction(&advance)
698+
.validate_transaction(&advance, TempoHardfork::T11)
603699
.unwrap_err()
604700
.to_string(),
605701
"advanceTempo must only execute once per zone block"
606702
);
607703
assert_eq!(
608704
ZoneBlockPhase::Executing
609-
.validate_transaction(&unexpected_system)
705+
.validate_transaction(&unexpected_system, TempoHardfork::T11)
610706
.unwrap_err()
611707
.to_string(),
612708
"system transactions after advanceTempo must call \
@@ -616,7 +712,7 @@ mod tests {
616712
for tx in [&advance, &regular, &finalize, &unexpected_system] {
617713
assert_eq!(
618714
ZoneBlockPhase::WithdrawalsFinalized
619-
.validate_transaction(tx)
715+
.validate_transaction(tx, TempoHardfork::T11)
620716
.unwrap_err()
621717
.to_string(),
622718
"finalizeWithdrawalBatch must be the last transaction in a zone block"
@@ -630,8 +726,12 @@ mod tests {
630726
let ordinary = ordinary_tx(Address::ZERO, Bytes::new());
631727
let mut phase = ZoneBlockPhase::Executing;
632728

633-
let ordinary_phase = phase.validate_transaction(&ordinary).unwrap();
634-
let finalized_phase = phase.validate_transaction(&finalize).unwrap();
729+
let ordinary_phase = phase
730+
.validate_transaction(&ordinary, TempoHardfork::T11)
731+
.unwrap();
732+
let finalized_phase = phase
733+
.validate_transaction(&finalize, TempoHardfork::T11)
734+
.unwrap();
635735
assert_eq!(phase, ZoneBlockPhase::Executing);
636736

637737
phase.advance_to(finalized_phase);
@@ -657,22 +757,22 @@ mod tests {
657757
);
658758

659759
assert_eq!(
660-
ZoneTransactionKind::classify(&advance_lookalike),
760+
ZoneTransactionKind::classify(&advance_lookalike, TempoHardfork::T11),
661761
ZoneTransactionKind::Regular
662762
);
663763
assert_eq!(
664-
ZoneTransactionKind::classify(&finalize_lookalike),
764+
ZoneTransactionKind::classify(&finalize_lookalike, TempoHardfork::T11),
665765
ZoneTransactionKind::Regular
666766
);
667767
assert_eq!(
668768
ZoneBlockPhase::Executing
669-
.validate_transaction(&advance_lookalike)
769+
.validate_transaction(&advance_lookalike, TempoHardfork::T11)
670770
.unwrap(),
671771
ZoneBlockPhase::Executing
672772
);
673773
assert_eq!(
674774
ZoneBlockPhase::Executing
675-
.validate_transaction(&finalize_lookalike)
775+
.validate_transaction(&finalize_lookalike, TempoHardfork::T11)
676776
.unwrap(),
677777
ZoneBlockPhase::Executing
678778
);

‎crates/node/src/replication.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use alloy_primitives::B256;
66
use alloy_provider::DynProvider;
77
use alloy_rlp::Decodable as _;
88
use alloy_rpc_types_engine::ForkchoiceState;
9-
use alloy_sol_types::SolCall as _;
9+
use alloy_sol_types::{SolCall as _, abi::AbiDecoderConfig};
1010
use futures::{StreamExt as _, stream::BoxStream};
1111
use reth_chain_state::PersistedBlockSubscriptions;
1212
use reth_node_api::{ConsensusEngineHandle, PayloadTypes as _};
@@ -18,6 +18,7 @@ use std::{
1818
time::Duration,
1919
};
2020
use tempo_alloy::TempoNetwork;
21+
use tempo_precompiles::dispatch::ABI_DECODER_MEMORY_LIMIT;
2122
use tempo_primitives::{Block, TempoHeader, TempoTxEnvelope};
2223
use tokio::sync::{mpsc, oneshot};
2324
use tokio_util::sync;
@@ -1375,8 +1376,13 @@ fn decode_advance_tempo(
13751376
if signed.tx().to != ZONE_INBOX_ADDRESS.into() {
13761377
eyre::bail!("first Tempo system transaction is not sent to IZoneInbox")
13771378
}
1378-
let call = IZoneInbox::advanceTempoCall::abi_decode(signed.tx().input.as_ref())
1379-
.map_err(|err| eyre::eyre!("first transaction does not decode as advanceTempo: {err}"))?;
1379+
let call = IZoneInbox::advanceTempoCall::abi_decode_with_config(
1380+
signed.tx().input.as_ref(),
1381+
AbiDecoderConfig::new()
1382+
.memory_limit(ABI_DECODER_MEMORY_LIMIT)
1383+
.strict(true),
1384+
)
1385+
.map_err(|err| eyre::eyre!("first transaction does not decode as advanceTempo: {err}"))?;
13801386

13811387
// 3. the system tx is valid.
13821388
let mut header_rlp = call.header.as_ref();

0 commit comments

Comments
 (0)