diff --git a/Cargo.lock b/Cargo.lock index dd6e68725a03..de9e924a5796 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5825,6 +5825,7 @@ name = "reth-consensus-common" version = "0.1.0-alpha.10" dependencies = [ "assert_matches", + "cfg-if", "mockall", "reth-interfaces", "reth-primitives", diff --git a/bin/reth/src/init.rs b/bin/reth/src/init.rs index 4eb1b9276bad..3ebc5d407364 100644 --- a/bin/reth/src/init.rs +++ b/bin/reth/src/init.rs @@ -315,8 +315,6 @@ mod tests { genesis_hash: None, paris_block_and_final_difficulty: None, deposit_contract: None, - #[cfg(feature = "optimism")] - optimism: false, ..Default::default() }); diff --git a/bin/reth/src/node/mod.rs b/bin/reth/src/node/mod.rs index 4120379bd4a7..573d03c51729 100644 --- a/bin/reth/src/node/mod.rs +++ b/bin/reth/src/node/mod.rs @@ -556,7 +556,7 @@ impl NodeCommand { // client from performing the derivation pipeline from genesis, and instead // starts syncing from the current tip in the DB. #[cfg(feature = "optimism")] - if self.chain.optimism && !self.rollup.enable_genesis_walkback { + if self.chain.is_optimism() && !self.rollup.enable_genesis_walkback { let client = _rpc_server_handles.auth.http_client(); reth_rpc_api::EngineApiClient::fork_choice_updated_v2( &client, diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 086fa7b7d9c8..34708073f919 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -668,7 +668,7 @@ where // On Optimism, the proposers are allowed to reorg their own chain at will. cfg_if::cfg_if! { if #[cfg(feature = "optimism")] { - if self.chain_spec().optimism { + if self.chain_spec().is_optimism() { debug!( target: "consensus::engine", fcu_head_num=?header.number, @@ -1647,7 +1647,7 @@ where if reached_max_block { // Terminate the sync early if it's reached the maximum user // configured block. - return Some(Ok(())); + return Some(Ok(())) } if let ControlFlow::Unwind { bad_block, .. } = ctrl { @@ -1843,7 +1843,7 @@ where }, )? { this.on_hook_result(result)?; - continue; + continue } // Process one incoming message from the CL. We don't drain the messages right away, @@ -1878,12 +1878,12 @@ where this.listeners.push_listener(tx); } } - continue; + continue } // Both running hook with db write access and engine messages are pending, // proceed to other polls - break; + break } // process sync events if any @@ -1894,7 +1894,7 @@ where } // this could have taken a while, so we start the next cycle to handle any new // engine messages - continue 'main; + continue 'main } Poll::Pending => { // no more sync events to process @@ -1919,7 +1919,7 @@ where // ensure we're polling until pending while also checking for new engine // messages before polling the next hook - continue 'main; + continue 'main } } @@ -2037,7 +2037,7 @@ mod tests { result, Err(BeaconConsensusEngineError::Pipeline(n)) if matches!(*n.as_ref(), PipelineError::Stage(StageError::ChannelClosed)) ); - break; + break } Err(TryRecvError::Empty) => { let _ = env diff --git a/crates/consensus/common/Cargo.toml b/crates/consensus/common/Cargo.toml index a2210039c0e5..0a32714919d3 100644 --- a/crates/consensus/common/Cargo.toml +++ b/crates/consensus/common/Cargo.toml @@ -13,6 +13,9 @@ reth-primitives.workspace = true reth-interfaces.workspace = true reth-provider.workspace = true +# misc +cfg-if = "1.0.0" + [dev-dependencies] reth-interfaces = { workspace = true, features = ["test-utils"] } reth-provider = { workspace = true, features = ["test-utils"] } diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index 6531bd396a88..04b1c1922c80 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -22,41 +22,37 @@ pub fn validate_header_standalone( return Err(ConsensusError::HeaderGasUsedExceedsGasLimit { gas_used: header.gas_used, gas_limit: header.gas_limit, - }); + }) } // Check if base fee is set. if chain_spec.fork(Hardfork::London).active_at_block(header.number) && header.base_fee_per_gas.is_none() { - return Err(ConsensusError::BaseFeeMissing); + return Err(ConsensusError::BaseFeeMissing) } - #[cfg(feature = "optimism")] - let wd_root_missing = header.withdrawals_root.is_none() && !chain_spec.optimism; - - #[cfg(not(feature = "optimism"))] - let wd_root_missing = header.withdrawals_root.is_none(); + let wd_root_missing = header.withdrawals_root.is_none() && !chain_spec.is_optimism(); // EIP-4895: Beacon chain push withdrawals as operations if chain_spec.fork(Hardfork::Shanghai).active_at_timestamp(header.timestamp) && wd_root_missing { - return Err(ConsensusError::WithdrawalsRootMissing); + return Err(ConsensusError::WithdrawalsRootMissing) } else if !chain_spec.fork(Hardfork::Shanghai).active_at_timestamp(header.timestamp) && header.withdrawals_root.is_some() { - return Err(ConsensusError::WithdrawalsRootUnexpected); + return Err(ConsensusError::WithdrawalsRootUnexpected) } // Ensures that EIP-4844 fields are valid once cancun is active. if chain_spec.fork(Hardfork::Cancun).active_at_timestamp(header.timestamp) { validate_4844_header_standalone(header)?; } else if header.blob_gas_used.is_some() { - return Err(ConsensusError::BlobGasUsedUnexpected); + return Err(ConsensusError::BlobGasUsedUnexpected) } else if header.excess_blob_gas.is_some() { - return Err(ConsensusError::ExcessBlobGasUnexpected); + return Err(ConsensusError::ExcessBlobGasUnexpected) } else if header.parent_beacon_block_root.is_some() { - return Err(ConsensusError::ParentBeaconBlockRootUnexpected); + return Err(ConsensusError::ParentBeaconBlockRootUnexpected) } Ok(()) @@ -77,14 +73,14 @@ pub fn validate_transaction_regarding_header( if chain_spec.fork(Hardfork::SpuriousDragon).active_at_block(at_block_number) && chain_id.is_some() { - return Err(InvalidTransactionError::OldLegacyChainId.into()); + return Err(InvalidTransactionError::OldLegacyChainId.into()) } *chain_id } Transaction::Eip2930(TxEip2930 { chain_id, .. }) => { // EIP-2930: Optional access lists: https://eips.ethereum.org/EIPS/eip-2930 (New transaction type) if !chain_spec.fork(Hardfork::Berlin).active_at_block(at_block_number) { - return Err(InvalidTransactionError::Eip2930Disabled.into()); + return Err(InvalidTransactionError::Eip2930Disabled.into()) } Some(*chain_id) } @@ -96,13 +92,13 @@ pub fn validate_transaction_regarding_header( }) => { // EIP-1559: Fee market change for ETH 1.0 chain https://eips.ethereum.org/EIPS/eip-1559 if !chain_spec.fork(Hardfork::Berlin).active_at_block(at_block_number) { - return Err(InvalidTransactionError::Eip1559Disabled.into()); + return Err(InvalidTransactionError::Eip1559Disabled.into()) } // EIP-1559: add more constraints to the tx validation // https://github.com/ethereum/EIPs/pull/3594 if max_priority_fee_per_gas > max_fee_per_gas { - return Err(InvalidTransactionError::TipAboveFeeCap.into()); + return Err(InvalidTransactionError::TipAboveFeeCap.into()) } Some(*chain_id) @@ -116,7 +112,7 @@ pub fn validate_transaction_regarding_header( // EIP-1559: add more constraints to the tx validation // https://github.com/ethereum/EIPs/pull/3594 if max_priority_fee_per_gas > max_fee_per_gas { - return Err(InvalidTransactionError::TipAboveFeeCap.into()); + return Err(InvalidTransactionError::TipAboveFeeCap.into()) } Some(*chain_id) @@ -126,14 +122,14 @@ pub fn validate_transaction_regarding_header( }; if let Some(chain_id) = chain_id { if chain_id != chain_spec.chain().id() { - return Err(InvalidTransactionError::ChainIdMismatch.into()); + return Err(InvalidTransactionError::ChainIdMismatch.into()) } } // Check basefee and few checks that are related to that. // https://github.com/ethereum/EIPs/pull/3594 if let Some(base_fee_per_gas) = base_fee { if transaction.max_fee_per_gas() < base_fee_per_gas as u128 { - return Err(InvalidTransactionError::FeeCapTooLow.into()); + return Err(InvalidTransactionError::FeeCapTooLow.into()) } } @@ -177,7 +173,7 @@ pub fn validate_all_transaction_regarding_block_and_nonces< return Err(ConsensusError::from( InvalidTransactionError::SignerAccountHasBytecode, ) - .into()); + .into()) } let nonce = account.nonce; entry.insert(account.nonce + 1); @@ -187,7 +183,7 @@ pub fn validate_all_transaction_regarding_block_and_nonces< // check nonce if transaction.nonce() != nonce { - return Err(ConsensusError::from(InvalidTransactionError::NonceNotConsistent).into()); + return Err(ConsensusError::from(InvalidTransactionError::NonceNotConsistent).into()) } } @@ -210,7 +206,7 @@ pub fn validate_block_standalone( return Err(ConsensusError::BodyOmmersHashDiff { got: ommers_hash, expected: block.header.ommers_hash, - }); + }) } // Check transaction root @@ -220,7 +216,7 @@ pub fn validate_block_standalone( return Err(ConsensusError::BodyTransactionRootDiff { got: transaction_root, expected: block.header.transactions_root, - }); + }) } // EIP-4895: Beacon chain push withdrawals as operations @@ -234,7 +230,7 @@ pub fn validate_block_standalone( return Err(ConsensusError::BodyWithdrawalsRootDiff { got: withdrawals_root, expected: *header_withdrawals_root, - }); + }) } } @@ -248,7 +244,7 @@ pub fn validate_block_standalone( return Err(ConsensusError::BlobGasUsedDiff { header_blob_gas_used, expected_blob_gas_used: total_blob_gas, - }); + }) } } @@ -257,7 +253,7 @@ pub fn validate_block_standalone( // Check gas limit, max diff between child/parent gas_limit should be max_diff=parent_gas/1024 // On Optimism, the gas limit can adjust instantly, so we skip this check if the optimism -// flag is enabled in the chainspec. +// flag is enabled in the chain spec. #[inline(always)] fn check_gas_limit( parent: &SealedHeader, @@ -277,13 +273,13 @@ fn check_gas_limit( return Err(ConsensusError::GasLimitInvalidIncrease { parent_gas_limit, child_gas_limit: child.gas_limit, - }); + }) } } else if parent_gas_limit - child.gas_limit >= parent_gas_limit / 1024 { return Err(ConsensusError::GasLimitInvalidDecrease { parent_gas_limit, child_gas_limit: child.gas_limit, - }); + }) } Ok(()) @@ -300,14 +296,14 @@ pub fn validate_header_regarding_parent( return Err(ConsensusError::ParentBlockNumberMismatch { parent_block_number: parent.number, block_number: child.number, - }); + }) } if parent.hash != child.parent_hash { return Err(ConsensusError::ParentHashMismatch { expected_parent_hash: parent.hash, got_parent_hash: child.parent_hash, - }); + }) } // timestamp in past check @@ -315,18 +311,22 @@ pub fn validate_header_regarding_parent( return Err(ConsensusError::TimestampIsInPast { parent_timestamp: parent.timestamp, timestamp: child.timestamp, - }); + }) } // TODO Check difficulty increment between parent and child // Ace age did increment it by some formula that we need to follow. - #[cfg(not(feature = "optimism"))] - check_gas_limit(parent, child, chain_spec)?; - - #[cfg(feature = "optimism")] - if !chain_spec.optimism { - check_gas_limit(parent, child, chain_spec)?; + cfg_if::cfg_if! { + if #[cfg(feature = "optimism")] { + // On Optimism, the gas limit can adjust instantly, so we skip this check + // if the optimism feature is enabled in the chain spec. + if !chain_spec.is_optimism() { + check_gas_limit(parent, child, chain_spec)?; + } + } else { + check_gas_limit(parent, child, chain_spec)?; + } } // EIP-1559 check base fee @@ -343,7 +343,7 @@ pub fn validate_header_regarding_parent( .ok_or(ConsensusError::BaseFeeMissing)? }; if expected_base_fee != base_fee { - return Err(ConsensusError::BaseFeeDiff { expected: expected_base_fee, got: base_fee }); + return Err(ConsensusError::BaseFeeDiff { expected: expected_base_fee, got: base_fee }) } } @@ -370,7 +370,7 @@ pub fn validate_block_regarding_chain Result<(), Cons let blob_gas_used = header.blob_gas_used.ok_or(ConsensusError::BlobGasUsedMissing)?; if header.excess_blob_gas.is_none() { - return Err(ConsensusError::ExcessBlobGasMissing); + return Err(ConsensusError::ExcessBlobGasMissing) } if header.parent_beacon_block_root.is_none() { - return Err(ConsensusError::ParentBeaconBlockRootMissing); + return Err(ConsensusError::ParentBeaconBlockRootMissing) } if blob_gas_used > MAX_DATA_GAS_PER_BLOCK { return Err(ConsensusError::BlobGasUsedExceedsMaxBlobGasPerBlock { blob_gas_used, max_blob_gas_per_block: MAX_DATA_GAS_PER_BLOCK, - }); + }) } if blob_gas_used % DATA_GAS_PER_BLOB != 0 { return Err(ConsensusError::BlobGasUsedNotMultipleOfBlobGasPerBlob { blob_gas_used, blob_gas_per_blob: DATA_GAS_PER_BLOB, - }); + }) } Ok(()) diff --git a/crates/net/network/src/manager.rs b/crates/net/network/src/manager.rs index e5d30d2d8332..0874dec61fd5 100644 --- a/crates/net/network/src/manager.rs +++ b/crates/net/network/src/manager.rs @@ -529,7 +529,7 @@ where if self.handle.mode().is_stake() { // See [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#devp2p) warn!(target: "net", "Peer performed block propagation, but it is not supported in proof of stake (EIP-3675)"); - return; + return } let msg = NewBlockMessage { hash, block: Arc::new(block) }; self.swarm.state_mut().announce_new_block(msg); @@ -616,7 +616,7 @@ where // This is only possible if the channel was deliberately closed since we always // have an instance of `NetworkHandle` error!("Network message channel closed."); - return Poll::Ready(()); + return Poll::Ready(()) } Poll::Ready(Some(msg)) => this.on_handle_message(msg), }; @@ -887,7 +887,7 @@ where if budget == 0 { // make sure we're woken up again cx.waker().wake_by_ref(); - break; + break } } diff --git a/crates/net/network/src/network.rs b/crates/net/network/src/network.rs index 086070a56d66..eafe29b603d0 100644 --- a/crates/net/network/src/network.rs +++ b/crates/net/network/src/network.rs @@ -288,7 +288,7 @@ impl SyncStateProvider for NetworkHandle { // used to guard the txpool fn is_initially_syncing(&self) -> bool { if self.inner.initial_sync_done.load(Ordering::Relaxed) { - return false; + return false } self.inner.is_syncing.load(Ordering::Relaxed) } diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index 58a37477f1a2..406c3d6d21aa 100644 --- a/crates/net/network/src/transactions.rs +++ b/crates/net/network/src/transactions.rs @@ -249,7 +249,7 @@ where if let Some(peer) = self.peers.get_mut(&peer_id) { if self.network.tx_gossip_disabled() { let _ = response.send(Ok(PooledTransactions::default())); - return; + return } let transactions = self .pool @@ -278,10 +278,10 @@ where fn on_new_transactions(&mut self, hashes: Vec) { // Nothing to propagate while initially syncing if self.network.is_initially_syncing() { - return; + return } if self.network.tx_gossip_disabled() { - return; + return } trace!(target: "net::tx", num_hashes=?hashes.len(), "Start propagating transactions"); @@ -308,7 +308,7 @@ where ) -> PropagatedTransactions { let mut propagated = PropagatedTransactions::default(); if self.network.tx_gossip_disabled() { - return propagated; + return propagated } // send full transactions to a fraction fo the connected peers (square root of the total @@ -416,7 +416,7 @@ where if full_transactions.transactions.is_empty() { // nothing to propagate - return None; + return None } let new_full_transactions = full_transactions.build(); @@ -443,7 +443,7 @@ where let propagated = { let Some(peer) = self.peers.get_mut(&peer_id) else { // no such peer - return; + return }; let to_propagate: Vec = @@ -464,7 +464,7 @@ where if new_pooled_hashes.is_empty() { // nothing to propagate - return; + return } for hash in new_pooled_hashes.iter_hashes().copied() { @@ -492,10 +492,10 @@ where ) { // If the node is initially syncing, ignore transactions if self.network.is_initially_syncing() { - return; + return } if self.network.tx_gossip_disabled() { - return; + return } let mut num_already_seen = 0; @@ -513,7 +513,7 @@ where if hashes.is_empty() { // nothing to request - return; + return } // enforce recommended soft limit, however the peer may enforce an arbitrary limit on @@ -525,7 +525,7 @@ where self.transaction_fetcher.request_transactions_from_peer(hashes, peer); if !request_sent { self.metrics.egress_peer_channel_full.increment(1); - return; + return } if num_already_seen > 0 { @@ -634,7 +634,7 @@ where // pool if !self.network.is_initially_syncing() { if self.network.tx_gossip_disabled() { - return; + return } let peer = self.peers.get_mut(&peer_id).expect("is present; qed"); @@ -644,7 +644,7 @@ where self.pool.pooled_transactions_max(NEW_POOLED_TRANSACTION_HASHES_SOFT_LIMIT); if pooled_txs.is_empty() { // do not send a message if there are no transactions in the pool - return; + return } for pooled_tx in pooled_txs.into_iter() { @@ -669,10 +669,10 @@ where ) { // If the node is pipeline syncing, ignore transactions if self.network.is_initially_syncing() { - return; + return } if self.network.tx_gossip_disabled() { - return; + return } // tracks the quality of the given transactions @@ -686,7 +686,7 @@ where tx } else { has_bad_transactions = true; - continue; + continue }; // track that the peer knows this transaction, but only if this is a new broadcast. @@ -741,7 +741,7 @@ where RequestError::Timeout => ReputationChangeKind::Timeout, RequestError::ChannelClosed | RequestError::ConnectionDropped => { // peer is already disconnected - return; + return } RequestError::BadResponse => ReputationChangeKind::BadTransactions, }; @@ -830,7 +830,7 @@ where if err.is_bad_transaction() && !this.network.is_syncing() { trace!(target: "net::tx", ?err, "Bad transaction import"); this.on_bad_import(err.hash); - continue; + continue } this.on_good_import(err.hash); } @@ -890,7 +890,7 @@ impl FullTransactionsBuilder { fn push(&mut self, transaction: &PropagateTransaction) { let new_size = self.total_size + transaction.size; if new_size > MAX_FULL_TRANSACTIONS_PACKET_SIZE { - return; + return } self.total_size = new_size; @@ -1105,7 +1105,7 @@ impl TransactionFetcher { error: RequestError::ChannelClosed, }) } - }; + } } Poll::Pending } @@ -1155,7 +1155,7 @@ impl TransactionFetcher { // 2. request all missing from peer if announced_hashes.is_empty() { // nothing to request - return false; + return false } let (response, rx) = oneshot::channel(); @@ -1178,7 +1178,7 @@ impl TransactionFetcher { } } } - return false; + return false } else { //create a new request for it, from that peer self.inflight_requests.push(GetPooledTxRequestFut::new(peer_id, rx)) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 1a4faae83f4c..076f6fb2087d 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -329,7 +329,7 @@ where // check if the deadline is reached if this.deadline.as_mut().poll(cx).is_ready() { trace!(target: "payload_builder", "payload building deadline reached"); - return Poll::Ready(Ok(())); + return Poll::Ready(Ok(())) } // check if the interval is reached @@ -414,7 +414,7 @@ where fn best_payload(&self) -> Result, PayloadBuilderError> { if let Some(ref payload) = self.best_payload { - return Ok(payload.clone()); + return Ok(payload.clone()) } // No payload has been built yet, but we need to return something that the CL then can // deliver, so we need to return an empty payload. @@ -452,7 +452,7 @@ where // upfront with the list of transactions sent in the attributes without caring about // the results of the polling job, if a best payload has not already been built. #[cfg(feature = "optimism")] - if self.config.chain_spec.optimism && + if self.config.chain_spec.is_optimism() && self.config.attributes.optimism_payload_attributes.no_tx_pool { let args = BuildArguments { @@ -476,7 +476,7 @@ where empty_payload, }, KeepPayloadJobAlive::Yes, - ); + ) } } @@ -519,13 +519,13 @@ impl Future for ResolveBestPayload { if let Poll::Ready(res) = fut.poll(cx) { this.maybe_better = None; if let Ok(BuildOutcome::Better { payload, .. }) = res { - return Poll::Ready(Ok(Arc::new(payload))); + return Poll::Ready(Ok(Arc::new(payload))) } } } if let Some(best) = this.best_payload.take() { - return Poll::Ready(Ok(best)); + return Poll::Ready(Ok(best)) } let mut empty_payload = this.empty_payload.take().expect("polled after completion"); @@ -605,8 +605,8 @@ impl PayloadConfig { /// Returns an owned instance of the [PayloadConfig]'s extra_data bytes. pub(crate) fn extra_data(&self) -> reth_primitives::Bytes { #[cfg(feature = "optimism")] - if self.chain_spec.optimism { - return Default::default(); + if self.chain_spec.is_optimism() { + return Default::default() } self.extra_data.clone() } @@ -789,12 +789,12 @@ where // which also removes all dependent transaction from the iterator before we can // continue best_txs.mark_invalid(&pool_tx); - continue; + continue } // check if the job was cancelled, if so we can exit early if cancel.is_cancelled() { - return Ok(BuildOutcome::Cancelled); + return Ok(BuildOutcome::Cancelled) } // convert tx to a signed transaction @@ -810,7 +810,7 @@ where // the gas limit condition for regular transactions above. trace!(target: "payload_builder", tx=?tx.hash, ?sum_blob_gas_used, ?tx_blob_gas, "skipping blob transaction because it would exceed the max data gas per block"); best_txs.mark_invalid(&pool_tx); - continue; + continue } } @@ -839,11 +839,11 @@ where best_txs.mark_invalid(&pool_tx); } - continue; + continue } err => { // this is an error that we should treat as fatal for this attempt - return Err(PayloadBuilderError::EvmExecutionError(err)); + return Err(PayloadBuilderError::EvmExecutionError(err)) } } } @@ -890,7 +890,7 @@ where // check if we have a better block if !is_better_payload(best_payload.as_deref(), total_fees) { // can skip building the block - return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads }); + return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads }) } let WithdrawalsOutcome { withdrawals_root, withdrawals } = @@ -1088,11 +1088,11 @@ fn commit_withdrawals>( withdrawals: Vec, ) -> RethResult { if !chain_spec.is_shanghai_active_at_timestamp(timestamp) { - return Ok(WithdrawalsOutcome::pre_shanghai()); + return Ok(WithdrawalsOutcome::pre_shanghai()) } if withdrawals.is_empty() { - return Ok(WithdrawalsOutcome::empty()); + return Ok(WithdrawalsOutcome::empty()) } let balance_increments = diff --git a/crates/payload/basic/src/optimism.rs b/crates/payload/basic/src/optimism.rs index cd358162b58d..fe59523d6da4 100644 --- a/crates/payload/basic/src/optimism.rs +++ b/crates/payload/basic/src/optimism.rs @@ -58,7 +58,7 @@ where for sequencer_tx in attributes.optimism_payload_attributes.transactions { // Check if the job was cancelled, if so we can exit early. if cancel.is_cancelled() { - return Ok(BuildOutcome::Cancelled); + return Ok(BuildOutcome::Cancelled) } // Convert the transaction to a [TransactionSignedEcRecovered]. This is @@ -102,11 +102,11 @@ where match err { EVMError::Transaction(err) => { trace!(target: "optimism_payload_builder", ?err, ?sequencer_tx, "Error in sequencer transaction, skipping."); - continue; + continue } err => { // this is an error that we should treat as fatal for this attempt - return Err(PayloadBuilderError::EvmExecutionError(err)); + return Err(PayloadBuilderError::EvmExecutionError(err)) } } } @@ -142,12 +142,12 @@ where // which also removes all dependent transaction from the iterator before we can // continue best_txs.mark_invalid(&pool_tx); - continue; + continue } // check if the job was cancelled, if so we can exit early if cancel.is_cancelled() { - return Ok(BuildOutcome::Cancelled); + return Ok(BuildOutcome::Cancelled) } // convert tx to a signed transaction @@ -178,11 +178,11 @@ where best_txs.mark_invalid(&pool_tx); } - continue; + continue } err => { // this is an error that we should treat as fatal for this attempt - return Err(PayloadBuilderError::EvmExecutionError(err)); + return Err(PayloadBuilderError::EvmExecutionError(err)) } } } @@ -220,7 +220,7 @@ where // check if we have a better block if !is_better_payload(best_payload.as_deref(), total_fees) { // can skip building the block - return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads }); + return Ok(BuildOutcome::Aborted { fees: total_fees, cached_reads }) } let WithdrawalsOutcome { withdrawals_root, withdrawals } = diff --git a/crates/payload/builder/src/payload.rs b/crates/payload/builder/src/payload.rs index 4448c6d54700..2063c0341c3b 100644 --- a/crates/payload/builder/src/payload.rs +++ b/crates/payload/builder/src/payload.rs @@ -225,7 +225,7 @@ impl PayloadBuilderAttributes { #[cfg(feature = "optimism")] { - cfg.optimism = chain_spec.optimism; + cfg.optimism = chain_spec.is_optimism(); } // ensure we're not missing any timestamp based hardforks diff --git a/crates/primitives/src/chain/mod.rs b/crates/primitives/src/chain/mod.rs index 216852b3e669..f7178ef6047d 100644 --- a/crates/primitives/src/chain/mod.rs +++ b/crates/primitives/src/chain/mod.rs @@ -123,25 +123,21 @@ impl Chain { } /// Returns the optimism goerli chain. - #[cfg(feature = "optimism")] pub const fn optimism_goerli() -> Self { Chain::Named(NamedChain::OptimismGoerli) } /// Returns the optimism mainnet chain. - #[cfg(feature = "optimism")] pub const fn optimism_mainnet() -> Self { Chain::Named(NamedChain::Optimism) } /// Returns the base goerli chain. - #[cfg(feature = "optimism")] pub const fn base_goerli() -> Self { Chain::Named(NamedChain::BaseGoerli) } /// Returns the base mainnet chain. - #[cfg(feature = "optimism")] pub const fn base_mainnet() -> Self { Chain::Named(NamedChain::Base) } @@ -151,6 +147,18 @@ impl Chain { Chain::Named(NamedChain::Dev) } + /// Returns true if the chain contains Optimism configuration. + pub fn is_optimism(self) -> bool { + matches!( + self, + Chain::Named(NamedChain::Optimism) | + Chain::Named(NamedChain::OptimismGoerli) | + Chain::Named(NamedChain::OptimismKovan) | + Chain::Named(NamedChain::Base) | + Chain::Named(NamedChain::BaseGoerli) + ) + } + /// The id of the chain pub fn id(&self) -> u64 { match self { diff --git a/crates/primitives/src/chain/spec.rs b/crates/primitives/src/chain/spec.rs index 0f5839737c0d..c3d5dfea26f6 100644 --- a/crates/primitives/src/chain/spec.rs +++ b/crates/primitives/src/chain/spec.rs @@ -66,8 +66,6 @@ pub static MAINNET: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::ethereum(), prune_delete_limit: 3500, snapshot_block_interval: 500_000, - #[cfg(feature = "optimism")] - optimism: false, } .into() }); @@ -111,8 +109,6 @@ pub static GOERLI: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::ethereum(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - #[cfg(feature = "optimism")] - optimism: false, } .into() }); @@ -161,8 +157,6 @@ pub static SEPOLIA: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::ethereum(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - #[cfg(feature = "optimism")] - optimism: false, } .into() }); @@ -205,8 +199,6 @@ pub static HOLESKY: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::ethereum(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - #[cfg(feature = "optimism")] - optimism: false, } .into() }); @@ -245,8 +237,6 @@ pub static DEV: Lazy> = Lazy::new(|| { (Hardfork::Shanghai, ForkCondition::Timestamp(0)), ]), deposit_contract: None, // TODO: do we even have? - #[cfg(feature = "optimism")] - optimism: false, ..Default::default() } .into() @@ -319,7 +309,6 @@ pub static OP_GOERLI: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::optimism(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - optimism: true, ..Default::default() } .into() @@ -361,7 +350,6 @@ pub static BASE_GOERLI: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::optimism_goerli(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - optimism: true, ..Default::default() } .into() @@ -403,7 +391,6 @@ pub static BASE_MAINNET: Lazy> = Lazy::new(|| { base_fee_params: BaseFeeParams::optimism(), prune_delete_limit: 1700, snapshot_block_interval: 1_000_000, - optimism: true, ..Default::default() } .into() @@ -459,10 +446,6 @@ pub struct ChainSpec { /// The block interval for creating snapshots. Each snapshot will have that much blocks in it. pub snapshot_block_interval: u64, - - /// Optimism configuration - #[cfg(feature = "optimism")] - pub optimism: bool, } impl Default for ChainSpec { @@ -478,8 +461,6 @@ impl Default for ChainSpec { base_fee_params: BaseFeeParams::ethereum(), prune_delete_limit: MAINNET.prune_delete_limit, snapshot_block_interval: Default::default(), - #[cfg(feature = "optimism")] - optimism: Default::default(), } } } @@ -490,6 +471,11 @@ impl ChainSpec { self.chain } + /// Returns `true` if this chain contains Optimism configuration. + pub fn is_optimism(&self) -> bool { + self.chain.is_optimism() + } + /// Get the genesis block specification. /// /// To get the header for the genesis block, use [`Self::genesis_header`] instead. @@ -819,8 +805,6 @@ impl From for ChainSpec { hardforks, paris_block_and_final_difficulty: None, deposit_contract: None, - #[cfg(feature = "optimism")] - optimism: false, ..Default::default() } } @@ -904,8 +888,6 @@ pub struct ChainSpecBuilder { chain: Option, genesis: Option, hardforks: BTreeMap, - #[cfg(feature = "optimism")] - optimism: bool, } impl ChainSpecBuilder { @@ -915,8 +897,6 @@ impl ChainSpecBuilder { chain: Some(MAINNET.chain), genesis: Some(MAINNET.genesis.clone()), hardforks: MAINNET.hardforks.clone(), - #[cfg(feature = "optimism")] - optimism: false, } } @@ -948,15 +928,6 @@ impl ChainSpecBuilder { ) } - /// Enable Optimism L2 modifications - /// - /// Warning: By itself, this does not enable the earliest Optimism hardfork, Bedrock. - #[cfg(feature = "optimism")] - pub fn optimism_activated(mut self) -> Self { - self.optimism = true; - self - } - /// Enable Frontier at genesis. pub fn frontier_activated(mut self) -> Self { self.hardforks.insert(Hardfork::Frontier, ForkCondition::Block(0)); @@ -1076,8 +1047,6 @@ impl ChainSpecBuilder { hardforks: self.hardforks, paris_block_and_final_difficulty: None, deposit_contract: None, - #[cfg(feature = "optimism")] - optimism: self.optimism, ..Default::default() } } @@ -1089,8 +1058,6 @@ impl From<&Arc> for ChainSpecBuilder { chain: Some(value.chain), genesis: Some(value.genesis.clone()), hardforks: value.hardforks.clone(), - #[cfg(feature = "optimism")] - optimism: value.optimism, } } } diff --git a/crates/primitives/src/hardfork.rs b/crates/primitives/src/hardfork.rs index 614df099327d..e1e29fc3673c 100644 --- a/crates/primitives/src/hardfork.rs +++ b/crates/primitives/src/hardfork.rs @@ -186,8 +186,6 @@ mod tests { hardforks: BTreeMap::from([(Hardfork::Frontier, ForkCondition::Never)]), paris_block_and_final_difficulty: None, deposit_contract: None, - #[cfg(feature = "optimism")] - optimism: false, ..Default::default() }; @@ -203,8 +201,6 @@ mod tests { hardforks: BTreeMap::from([(Hardfork::Shanghai, ForkCondition::Never)]), paris_block_and_final_difficulty: None, deposit_contract: None, - #[cfg(feature = "optimism")] - optimism: false, ..Default::default() }; diff --git a/crates/primitives/src/revm/config.rs b/crates/primitives/src/revm/config.rs index dd74b144c332..33848a844298 100644 --- a/crates/primitives/src/revm/config.rs +++ b/crates/primitives/src/revm/config.rs @@ -9,7 +9,7 @@ pub fn revm_spec_by_timestamp_after_merge( timestamp: u64, ) -> revm_primitives::SpecId { #[cfg(feature = "optimism")] - if chain_spec.optimism { + if chain_spec.is_optimism() { if chain_spec.fork(Hardfork::Regolith).active_at_timestamp(timestamp) { return revm_primitives::REGOLITH } else { @@ -29,7 +29,7 @@ pub fn revm_spec_by_timestamp_after_merge( /// return revm_spec from spec configuration. pub fn revm_spec(chain_spec: &ChainSpec, block: Head) -> revm_primitives::SpecId { #[cfg(feature = "optimism")] - if chain_spec.optimism { + if chain_spec.is_optimism() { if chain_spec.fork(Hardfork::Regolith).active_at_head(&block) { return revm_primitives::REGOLITH } else if chain_spec.fork(Hardfork::Bedrock).active_at_head(&block) { @@ -134,7 +134,7 @@ mod tests { { #[inline(always)] fn op_cs(f: impl FnOnce(ChainSpecBuilder) -> ChainSpecBuilder) -> ChainSpec { - let cs = ChainSpecBuilder::mainnet().optimism_activated(); + let cs = ChainSpecBuilder::mainnet().chain(crate::Chain::Id(10)); f(cs).build() } diff --git a/crates/primitives/src/revm/env.rs b/crates/primitives/src/revm/env.rs index 41d80fce0762..962e08c799ee 100644 --- a/crates/primitives/src/revm/env.rs +++ b/crates/primitives/src/revm/env.rs @@ -47,7 +47,7 @@ pub fn fill_cfg_env( #[cfg(feature = "optimism")] { - cfg_env.optimism = chain_spec.optimism; + cfg_env.optimism = chain_spec.is_optimism(); } } diff --git a/crates/revm/src/optimism/mod.rs b/crates/revm/src/optimism/mod.rs index 9c4562ae5f80..17da8de04740 100644 --- a/crates/revm/src/optimism/mod.rs +++ b/crates/revm/src/optimism/mod.rs @@ -49,7 +49,7 @@ pub fn parse_l1_info_tx(data: impl AsRef<[u8]>) -> Result Result { if is_deposit { - return Ok(U256::ZERO); + return Ok(U256::ZERO) } if chain_spec.is_fork_active_at_timestamp(Hardfork::Regolith, timestamp) { diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index 6416d3a5352d..ba3ac0e66614 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -308,7 +308,7 @@ where ) -> EngineApiResult { let len = hashes.len() as u64; if len > MAX_PAYLOAD_BODIES_LIMIT { - return Err(EngineApiError::PayloadRequestTooLarge { len }); + return Err(EngineApiError::PayloadRequestTooLarge { len }) } let mut result = Vec::with_capacity(hashes.len()); @@ -348,7 +348,7 @@ where return Err(EngineApiError::TerminalTD { execution: merge_terminal_td, consensus: terminal_total_difficulty, - }); + }) } self.inner.beacon_consensus.transition_configuration_exchanged().await; @@ -358,7 +358,7 @@ where return Ok(TransitionConfiguration { terminal_total_difficulty: merge_terminal_td, ..Default::default() - }); + }) } // Attempt to look up terminal block hash @@ -413,7 +413,7 @@ where // 1. Client software **MUST** return `-38005: Unsupported fork` error if the // `timestamp` of payload or payloadAttributes greater or equal to the Cancun // activation timestamp. - return Err(EngineApiError::UnsupportedFork); + return Err(EngineApiError::UnsupportedFork) } if version == EngineApiMessageVersion::V3 && !is_cancun { @@ -423,7 +423,7 @@ where // 1. Client software **MUST** return `-38005: Unsupported fork` error if the // `timestamp` of the built payload does not fall within the time frame of the Cancun // fork. - return Err(EngineApiError::UnsupportedFork); + return Err(EngineApiError::UnsupportedFork) } Ok(()) } @@ -443,18 +443,18 @@ where match version { EngineApiMessageVersion::V1 => { if has_withdrawals { - return Err(EngineApiError::WithdrawalsNotSupportedInV1); + return Err(EngineApiError::WithdrawalsNotSupportedInV1) } if is_shanghai { - return Err(EngineApiError::NoWithdrawalsPostShanghai); + return Err(EngineApiError::NoWithdrawalsPostShanghai) } } EngineApiMessageVersion::V2 | EngineApiMessageVersion::V3 => { if is_shanghai && !has_withdrawals { - return Err(EngineApiError::NoWithdrawalsPostShanghai); + return Err(EngineApiError::NoWithdrawalsPostShanghai) } if !is_shanghai && has_withdrawals { - return Err(EngineApiError::HasWithdrawalsPreShanghai); + return Err(EngineApiError::HasWithdrawalsPreShanghai) } } }; @@ -498,12 +498,12 @@ where match version { EngineApiMessageVersion::V1 | EngineApiMessageVersion::V2 => { if has_parent_beacon_block_root { - return Err(EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3); + return Err(EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3) } } EngineApiMessageVersion::V3 => { if !has_parent_beacon_block_root { - return Err(EngineApiError::NoParentBeaconBlockRootPostCancun); + return Err(EngineApiError::NoParentBeaconBlockRootPostCancun) } } }; @@ -559,9 +559,9 @@ where #[cfg(feature = "optimism")] if attrs.optimism_payload_attributes.gas_limit.is_none() && - self.inner.chain_spec.optimism + self.inner.chain_spec.is_optimism() { - return Err(EngineApiError::MissingGasLimitInPayloadAttributes); + return Err(EngineApiError::MissingGasLimitInPayloadAttributes) } // From the engine API spec: @@ -582,9 +582,9 @@ where // TODO: decide if we want this branch - the FCU INVALID response might be more // useful than the payload attributes INVALID response if fcu_res.is_invalid() { - return Ok(fcu_res); + return Ok(fcu_res) } - return Err(err); + return Err(err) } } diff --git a/crates/rpc/rpc-types/src/eth/block.rs b/crates/rpc/rpc-types/src/eth/block.rs index c9028e401133..c4522c7844b4 100644 --- a/crates/rpc/rpc-types/src/eth/block.rs +++ b/crates/rpc/rpc-types/src/eth/block.rs @@ -334,7 +334,7 @@ impl FromStr for BlockNumberOrTag { let number = u64::from_str_radix(hex_val, 16); BlockNumberOrTag::Number(number?) } else { - return Err(HexStringMissingPrefixError::default().into()); + return Err(HexStringMissingPrefixError::default().into()) } } }; @@ -486,25 +486,25 @@ impl<'de> Deserialize<'de> for BlockId { match key.as_str() { "blockNumber" => { if number.is_some() || block_hash.is_some() { - return Err(serde::de::Error::duplicate_field("blockNumber")); + return Err(serde::de::Error::duplicate_field("blockNumber")) } if require_canonical.is_some() { return Err(serde::de::Error::custom( "Non-valid require_canonical field", - )); + )) } number = Some(map.next_value::()?) } "blockHash" => { if number.is_some() || block_hash.is_some() { - return Err(serde::de::Error::duplicate_field("blockHash")); + return Err(serde::de::Error::duplicate_field("blockHash")) } block_hash = Some(map.next_value::()?); } "requireCanonical" => { if number.is_some() || require_canonical.is_some() { - return Err(serde::de::Error::duplicate_field("requireCanonical")); + return Err(serde::de::Error::duplicate_field("requireCanonical")) } require_canonical = Some(map.next_value::()?) diff --git a/crates/rpc/rpc-types/src/serde_helpers/storage.rs b/crates/rpc/rpc-types/src/serde_helpers/storage.rs index 952f5ebe0ec5..885274f5f07b 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/storage.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/storage.rs @@ -61,7 +61,7 @@ where D: Deserializer<'de>, { if bytes.0.len() > 32 { - return Err(serde::de::Error::custom("input too long to be a B256")); + return Err(serde::de::Error::custom("input too long to be a B256")) } // left pad with zeros to 32 bytes diff --git a/crates/rpc/rpc/src/eth/api/mod.rs b/crates/rpc/rpc/src/eth/api/mod.rs index 503b464b94e7..025cb5449ff6 100644 --- a/crates/rpc/rpc/src/eth/api/mod.rs +++ b/crates/rpc/rpc/src/eth/api/mod.rs @@ -265,7 +265,7 @@ where #[cfg(feature = "optimism")] { - cfg.optimism = self.provider().chain_spec().optimism; + cfg.optimism = self.provider().chain_spec().is_optimism(); } let mut block_env = BlockEnv::default();