Skip to content
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

Investigate how to reduce feature flags #62

Open
poplexity opened this issue Oct 8, 2024 · 1 comment
Open

Investigate how to reduce feature flags #62

poplexity opened this issue Oct 8, 2024 · 1 comment

Comments

@poplexity
Copy link
Member

Ideally all changes are contained in crates within the crates/telos directory, and not managed via feature flags. This issue is to track investigation into how we could reduce feature flags by moving changes into the telos directory.

If we look at how optimism has added their customizations, we can see there is intentional ways to customize reth which we are not yet using.

Also, where we have implementations in the telos directory which do not involve telos specific changes, we'd like to avoid any code duplication so we do not miss future updates/fixes.

Examples... note that there are likely many more cases we can improve on, which should be added to this issue so we can track their resolution.

  • In the crates/telos/rpc crate we implement the EthApi related traits for TelosEthApi, much of which is done via the self.inner pattern however in some cases we have not been able use this pattern and had to copy the implementation from ethereum, such as here:

    async fn build_transaction_receipt(
    &self,
    tx: TransactionSigned,
    meta: TransactionMeta,
    receipt: Receipt,
    ) -> Result<RpcReceipt<Self::NetworkTypes>, Self::Error> {
    let hash = meta.block_hash;
    // get all receipts for the block
    let all_receipts = self
    .inner
    .cache()
    .get_receipts(hash)
    .await
    .map_err(Self::Error::from_eth_err)?
    .ok_or(EthApiError::HeaderNotFound(hash.into()))?;
    Ok(ReceiptBuilder::new(&tx, meta, &receipt, &all_receipts)?.build())
    }

  • Telos has additional fields we have added to the engine API, we have done this with feature flags but there is possibly a better way if we look at how optimism has done it here:

    type Engine = OptimismEngineTypes;
    which then uses their overrides of V3/V4 here:
    impl EngineTypes for OptimismEngineTypes {
    type ExecutionPayloadV1 = ExecutionPayloadV1;
    type ExecutionPayloadV2 = ExecutionPayloadEnvelopeV2;
    type ExecutionPayloadV3 = OptimismExecutionPayloadEnvelopeV3;
    type ExecutionPayloadV4 = OptimismExecutionPayloadEnvelopeV4;

  • Chainspec related changes for optimism are handled via a custom chainspec implementation here:

    type ChainSpec = OpChainSpec;

@poplexity
Copy link
Member Author

poplexity commented Oct 8, 2024

Adding more... we currently don't plan to support starting from non-zero genesis and there might be a few places we can just remove our changes entirely, need to confirm with @aamirpashaa which places these would be. (example, storage/db-common crate)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant