Skip to content

Commit a96d045

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

6 files changed

Lines changed: 197 additions & 61 deletions

File tree

‎crates/evm/src/executor.rs‎

Lines changed: 135 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,17 @@ 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 _, abi::AbiDecoderConfig};
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;
2223
use tempo_zone_contracts::IZoneOutbox;
2324
use zone_chainspec::ZoneChainSpec;
2425
use zone_l1::state::L1StateProvider;
25-
use zone_precompiles::{
26-
ADVANCE_TEMPO_SELECTOR, L1StorageReader, is_finalize_withdrawal_batch_calldata,
27-
};
26+
use zone_precompiles::{ADVANCE_TEMPO_SELECTOR, L1StorageReader};
2827
use zone_primitives::constants::{ZONE_INBOX_ADDRESS, ZONE_OUTBOX_ADDRESS};
2928

3029
use crate::{L1OverlayDB, ZoneEvm};
@@ -41,15 +40,19 @@ enum ZoneBlockPhase {
4140
}
4241

4342
impl ZoneBlockPhase {
44-
fn validate_transaction(self, tx: &TempoTxEnvelope) -> Result<Self, BlockExecutionError> {
43+
fn validate_transaction(
44+
self,
45+
tx: &TempoTxEnvelope,
46+
spec: TempoHardfork,
47+
) -> Result<Self, BlockExecutionError> {
4548
if tx.subblock_proposer().is_some() {
4649
return Err(BlockValidationError::msg(
4750
"subblock transactions are not supported in zone blocks",
4851
)
4952
.into());
5053
}
5154

52-
let tx_kind = ZoneTransactionKind::classify(tx);
55+
let tx_kind = ZoneTransactionKind::classify(tx, spec);
5356

5457
match (self, tx_kind) {
5558
(Self::AwaitingAdvanceTempo, ZoneTransactionKind::AdvanceTempo) => Ok(Self::Executing),
@@ -94,7 +97,7 @@ enum ZoneTransactionKind {
9497
}
9598

9699
impl ZoneTransactionKind {
97-
fn classify(tx: &TempoTxEnvelope) -> Self {
100+
fn classify(tx: &TempoTxEnvelope, spec: TempoHardfork) -> Self {
98101
if !tx.is_system_tx() {
99102
return Self::Regular;
100103
}
@@ -106,7 +109,17 @@ impl ZoneTransactionKind {
106109
}
107110

108111
if tx.calls().any(|(kind, input)| {
109-
kind.to() == Some(&ZONE_OUTBOX_ADDRESS) && is_finalize_withdrawal_batch_calldata(input)
112+
kind.to() == Some(&ZONE_OUTBOX_ADDRESS)
113+
&& if spec.is_t11() {
114+
input.get(..4)
115+
== Some(IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR.as_slice())
116+
} else {
117+
IZoneOutbox::finalizeWithdrawalBatchCall::abi_decode_with_config(
118+
input,
119+
AbiDecoderConfig::new().strict(true),
120+
)
121+
.is_ok()
122+
}
110123
}) {
111124
return Self::FinalizeWithdrawalBatch;
112125
}
@@ -208,7 +221,8 @@ where
208221
tempo_tx_env.expiring_nonce_idx = None;
209222
}
210223

211-
let next_phase = self.phase.validate_transaction(recovered.tx())?;
224+
let spec = self.evm().ctx().cfg.spec;
225+
let next_phase = self.phase.validate_transaction(recovered.tx(), spec)?;
212226

213227
let result = self
214228
.inner
@@ -285,7 +299,7 @@ mod tests {
285299
use reth_chainspec::EthChainSpec as _;
286300
use reth_primitives_traits::Recovered;
287301
use revm::database::{CacheDB, EmptyDB};
288-
use tempo_chainspec::spec::DEV;
302+
use tempo_chainspec::{hardfork::TempoHardfork, spec::DEV};
289303
use tempo_evm::TempoBlockExecutionCtx;
290304
use tempo_precompiles::{
291305
DEFAULT_FEE_TOKEN, TIP_FEE_MANAGER_ADDRESS,
@@ -399,7 +413,9 @@ mod tests {
399413
let advance_tempo = advance_tempo_tx();
400414
let mut phase = ZoneBlockPhase::AwaitingAdvanceTempo;
401415

402-
let next_phase = phase.validate_transaction(&advance_tempo).unwrap();
416+
let next_phase = phase
417+
.validate_transaction(&advance_tempo, TempoHardfork::T11)
418+
.unwrap();
403419
assert_eq!(phase, ZoneBlockPhase::AwaitingAdvanceTempo);
404420
assert_eq!(next_phase, ZoneBlockPhase::Executing);
405421

@@ -414,15 +430,15 @@ mod tests {
414430
let ordinary = system_tx(Address::ZERO, Bytes::new());
415431

416432
let missing_first = ZoneBlockPhase::AwaitingAdvanceTempo
417-
.validate_transaction(&ordinary)
433+
.validate_transaction(&ordinary, TempoHardfork::T11)
418434
.unwrap_err();
419435
assert_eq!(
420436
missing_first.to_string(),
421437
"advanceTempo must be the first transaction in a zone block"
422438
);
423439

424440
let duplicate = ZoneBlockPhase::Executing
425-
.validate_transaction(&advance_tempo)
441+
.validate_transaction(&advance_tempo, TempoHardfork::T11)
426442
.unwrap_err();
427443
assert_eq!(
428444
duplicate.to_string(),
@@ -435,7 +451,7 @@ mod tests {
435451
let finalize = finalize_withdrawal_batch_tx();
436452
assert_eq!(
437453
ZoneBlockPhase::Executing
438-
.validate_transaction(&finalize)
454+
.validate_transaction(&finalize, TempoHardfork::T11)
439455
.unwrap(),
440456
ZoneBlockPhase::WithdrawalsFinalized
441457
);
@@ -449,7 +465,7 @@ mod tests {
449465
.into(),
450466
);
451467
let error = ZoneBlockPhase::Executing
452-
.validate_transaction(&setter)
468+
.validate_transaction(&setter, TempoHardfork::T11)
453469
.unwrap_err();
454470
assert_eq!(
455471
error.to_string(),
@@ -462,21 +478,97 @@ mod tests {
462478
Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR),
463479
);
464480
let error = ZoneBlockPhase::Executing
465-
.validate_transaction(&malformed_finalize)
481+
.validate_transaction(&malformed_finalize, TempoHardfork::T10)
466482
.unwrap_err();
467483
assert_eq!(
468484
error.to_string(),
469485
"system transactions after advanceTempo must call \
470486
ZoneOutbox.finalizeWithdrawalBatch"
471487
);
488+
assert_eq!(
489+
ZoneBlockPhase::Executing
490+
.validate_transaction(&malformed_finalize, TempoHardfork::T11)
491+
.unwrap(),
492+
ZoneBlockPhase::WithdrawalsFinalized
493+
);
494+
495+
let mut trailing_calldata = IZoneOutbox::finalizeWithdrawalBatchCall {
496+
count: U256::ZERO,
497+
blockNumber: 1,
498+
encryptedSenders: vec![],
499+
}
500+
.abi_encode();
501+
trailing_calldata.extend_from_slice(&[0; 32]);
502+
let trailing_finalize = system_tx(ZONE_OUTBOX_ADDRESS, trailing_calldata.into());
503+
let error = ZoneBlockPhase::Executing
504+
.validate_transaction(&trailing_finalize, TempoHardfork::T10)
505+
.unwrap_err();
506+
assert_eq!(
507+
error.to_string(),
508+
"system transactions after advanceTempo must call \
509+
ZoneOutbox.finalizeWithdrawalBatch"
510+
);
511+
assert_eq!(
512+
ZoneBlockPhase::Executing
513+
.validate_transaction(&trailing_finalize, TempoHardfork::T11)
514+
.unwrap(),
515+
ZoneBlockPhase::WithdrawalsFinalized
516+
);
517+
}
518+
519+
#[test]
520+
fn malformed_t11_finalization_does_not_advance_block_phase() {
521+
let mut zone_genesis = DEV.genesis().clone();
522+
zone_genesis.config.chain_id = zone_chain_id(DEV.chain().id(), 2).unwrap();
523+
let chain_spec = std::sync::Arc::new(ZoneChainSpec::from_genesis(zone_genesis).unwrap());
524+
let factory =
525+
ZoneEvmFactory::new(chain_spec.clone(), MockL1Reader::default(), Address::ZERO);
526+
let mut env = EvmEnv::default();
527+
env.cfg_env.spec = TempoHardfork::T11;
528+
let evm = factory.create_evm(CacheDB::new(EmptyDB::default()), env);
529+
let ctx = TempoBlockExecutionCtx {
530+
inner: EthBlockExecutionCtx {
531+
parent_hash: B256::ZERO,
532+
parent_beacon_block_root: None,
533+
ommers: &[],
534+
withdrawals: None,
535+
extra_data: Bytes::new(),
536+
tx_count_hint: Some(1),
537+
slot_number: None,
538+
},
539+
general_gas_limit: 0,
540+
shared_gas_limit: 0,
541+
validator_set: None,
542+
consensus_context: None,
543+
subblock_fee_recipients: Default::default(),
544+
};
545+
let mut executor = ZoneBlockExecutor::new(evm, ctx, &chain_spec);
546+
executor.phase = ZoneBlockPhase::Executing;
547+
548+
let tx = Recovered::new_unchecked(
549+
system_tx(
550+
ZONE_OUTBOX_ADDRESS,
551+
Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR),
552+
),
553+
TEMPO_SYSTEM_TX_SENDER,
554+
);
555+
let error = executor.execute_transaction_without_commit(tx).unwrap_err();
556+
557+
assert!(
558+
error
559+
.to_string()
560+
.contains("system transaction execution failed"),
561+
"unexpected error: {error}"
562+
);
563+
assert_eq!(executor.phase, ZoneBlockPhase::Executing);
472564
}
473565

474566
#[test]
475567
fn ordinary_transactions_are_allowed_after_advance_tempo() {
476568
let ordinary = ordinary_tx(Address::ZERO, Bytes::new());
477569
assert_eq!(
478570
ZoneBlockPhase::Executing
479-
.validate_transaction(&ordinary)
571+
.validate_transaction(&ordinary, TempoHardfork::T11)
480572
.unwrap(),
481573
ZoneBlockPhase::Executing
482574
);
@@ -534,7 +626,9 @@ mod tests {
534626
ZoneBlockPhase::Executing,
535627
ZoneBlockPhase::WithdrawalsFinalized,
536628
] {
537-
let error = phase.validate_transaction(&subblock).unwrap_err();
629+
let error = phase
630+
.validate_transaction(&subblock, TempoHardfork::T11)
631+
.unwrap_err();
538632
assert_eq!(
539633
error.to_string(),
540634
"subblock transactions are not supported in zone blocks"
@@ -549,12 +643,16 @@ mod tests {
549643
let advance_tempo = advance_tempo_tx();
550644
let mut phase = ZoneBlockPhase::Executing;
551645

552-
let next_phase = phase.validate_transaction(&finalize).unwrap();
646+
let next_phase = phase
647+
.validate_transaction(&finalize, TempoHardfork::T11)
648+
.unwrap();
553649
assert_eq!(phase, ZoneBlockPhase::Executing);
554650

555651
phase.advance_to(next_phase);
556652
for tx in [&ordinary, &finalize, &advance_tempo] {
557-
let error = phase.validate_transaction(tx).unwrap_err();
653+
let error = phase
654+
.validate_transaction(tx, TempoHardfork::T11)
655+
.unwrap_err();
558656
assert_eq!(
559657
error.to_string(),
560658
"finalizeWithdrawalBatch must be the last transaction in a zone block"
@@ -571,14 +669,14 @@ mod tests {
571669

572670
assert_eq!(
573671
ZoneBlockPhase::AwaitingAdvanceTempo
574-
.validate_transaction(&advance)
672+
.validate_transaction(&advance, TempoHardfork::T11)
575673
.unwrap(),
576674
ZoneBlockPhase::Executing
577675
);
578676
for tx in [&regular, &finalize, &unexpected_system] {
579677
assert_eq!(
580678
ZoneBlockPhase::AwaitingAdvanceTempo
581-
.validate_transaction(tx)
679+
.validate_transaction(tx, TempoHardfork::T11)
582680
.unwrap_err()
583681
.to_string(),
584682
"advanceTempo must be the first transaction in a zone block"
@@ -587,26 +685,26 @@ mod tests {
587685

588686
assert_eq!(
589687
ZoneBlockPhase::Executing
590-
.validate_transaction(&regular)
688+
.validate_transaction(&regular, TempoHardfork::T11)
591689
.unwrap(),
592690
ZoneBlockPhase::Executing
593691
);
594692
assert_eq!(
595693
ZoneBlockPhase::Executing
596-
.validate_transaction(&finalize)
694+
.validate_transaction(&finalize, TempoHardfork::T11)
597695
.unwrap(),
598696
ZoneBlockPhase::WithdrawalsFinalized
599697
);
600698
assert_eq!(
601699
ZoneBlockPhase::Executing
602-
.validate_transaction(&advance)
700+
.validate_transaction(&advance, TempoHardfork::T11)
603701
.unwrap_err()
604702
.to_string(),
605703
"advanceTempo must only execute once per zone block"
606704
);
607705
assert_eq!(
608706
ZoneBlockPhase::Executing
609-
.validate_transaction(&unexpected_system)
707+
.validate_transaction(&unexpected_system, TempoHardfork::T11)
610708
.unwrap_err()
611709
.to_string(),
612710
"system transactions after advanceTempo must call \
@@ -616,7 +714,7 @@ mod tests {
616714
for tx in [&advance, &regular, &finalize, &unexpected_system] {
617715
assert_eq!(
618716
ZoneBlockPhase::WithdrawalsFinalized
619-
.validate_transaction(tx)
717+
.validate_transaction(tx, TempoHardfork::T11)
620718
.unwrap_err()
621719
.to_string(),
622720
"finalizeWithdrawalBatch must be the last transaction in a zone block"
@@ -630,8 +728,12 @@ mod tests {
630728
let ordinary = ordinary_tx(Address::ZERO, Bytes::new());
631729
let mut phase = ZoneBlockPhase::Executing;
632730

633-
let ordinary_phase = phase.validate_transaction(&ordinary).unwrap();
634-
let finalized_phase = phase.validate_transaction(&finalize).unwrap();
731+
let ordinary_phase = phase
732+
.validate_transaction(&ordinary, TempoHardfork::T11)
733+
.unwrap();
734+
let finalized_phase = phase
735+
.validate_transaction(&finalize, TempoHardfork::T11)
736+
.unwrap();
635737
assert_eq!(phase, ZoneBlockPhase::Executing);
636738

637739
phase.advance_to(finalized_phase);
@@ -657,22 +759,22 @@ mod tests {
657759
);
658760

659761
assert_eq!(
660-
ZoneTransactionKind::classify(&advance_lookalike),
762+
ZoneTransactionKind::classify(&advance_lookalike, TempoHardfork::T11),
661763
ZoneTransactionKind::Regular
662764
);
663765
assert_eq!(
664-
ZoneTransactionKind::classify(&finalize_lookalike),
766+
ZoneTransactionKind::classify(&finalize_lookalike, TempoHardfork::T11),
665767
ZoneTransactionKind::Regular
666768
);
667769
assert_eq!(
668770
ZoneBlockPhase::Executing
669-
.validate_transaction(&advance_lookalike)
771+
.validate_transaction(&advance_lookalike, TempoHardfork::T11)
670772
.unwrap(),
671773
ZoneBlockPhase::Executing
672774
);
673775
assert_eq!(
674776
ZoneBlockPhase::Executing
675-
.validate_transaction(&finalize_lookalike)
777+
.validate_transaction(&finalize_lookalike, TempoHardfork::T11)
676778
.unwrap(),
677779
ZoneBlockPhase::Executing
678780
);

‎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)