-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
chore: rm optimism feature from chainspec #11722
Conversation
@@ -462,8 +462,7 @@ where | |||
) -> bool { | |||
// On Optimism, the proposers are allowed to reorg their own chain at will. | |||
#[cfg(feature = "optimism")] | |||
if reth_chainspec::EthChainSpec::chain(self.blockchain.chain_spec().as_ref()).is_optimism() | |||
{ | |||
if reth_chainspec::EthChainSpec::is_optimism(&self.blockchain.chain_spec()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should open an issue to move away from this type of behaviour, since it requires every rollup to add a new is_my_rollup
trait method to EthChainSpec
, and instead solve it at sdk level for engine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this crate will be deprecated soon, so no need to change anything here
impl<N> OpEthApi<N> | ||
where | ||
N: FullNodeComponents<Types: NodeTypes<ChainSpec = ChainSpec>>, | ||
{ | ||
/// Builds a receipt w.r.t. chain spec. | ||
pub fn build_op_receipt_meta( | ||
&self, | ||
tx: &TransactionSigned, | ||
l1_block_info: revm::L1BlockInfo, | ||
receipt: &Receipt, | ||
) -> Result<OpTransactionReceiptFields, OpEthApiError> { | ||
Ok(OpReceiptFieldsBuilder::default() | ||
.l1_block_info(&self.inner.provider().chain_spec(), tx, l1_block_info)? | ||
.deposit_nonce(receipt.deposit_nonce) | ||
.deposit_version(receipt.deposit_receipt_version) | ||
.build()) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of scope to remove unused code, but ok
closes #8904
we can finally do this, has no impact on any usage in beacon-consensus because we already use the proper OpChainSpec type everywhere