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

Blocking issues in upgrading Polkadot SDK dependencies to stable2412 #1584

Open
conr2d opened this issue Jan 3, 2025 · 5 comments
Open

Blocking issues in upgrading Polkadot SDK dependencies to stable2412 #1584

conr2d opened this issue Jan 3, 2025 · 5 comments
Labels

Comments

@conr2d
Copy link
Contributor

conr2d commented Jan 3, 2025

Version conflicts with primitive-types and Dependencies on ethereum/evm

Polkadot SDK now depends on [email protected], but the latest published versions of ethereum and evm crates still depend on [email protected]. This results in incompatibilities for types like H256, U256, and others between Polkadot SDK and Frontier.

Currently, the only person with publishing rights for the ethereum crate is @sorpaas, but it seems that he is not actively engaged in its maintenance. IMHO, if @sorpaas consents, one of the Frontier maintainers should be added as a co-maintainer for the ethereum/evm crates. Alternatively, forking those crates could be considered as a viable option.

Given the long-term maintenance requirements of tracking changes to Ethereum hard forks, another approach to consider might be supporting revm together.

Extrinsic V5 and the future of SelfContainedCall

In Extrinsic V5, verifying signatures has be separated into a TransactionExtension (superseding SignedExtension), allowing the functionality previously handled by SelfContainedCall to be implemented as a TransactionExtension.

I investigated supporting backward compatibility regardless of removing wrapping types of (Checked|Unchecked)Extrinsic, but I couldn't find a straightforward way to invoke call.apply_self_contained() instead of call.dispatch(). As a result, separate extrinsic types would still be required.

However, for chains launching from scratch without the need to support existing extrinsics, adopting the V5 format from the beginning could allow TransactionExtension to handle functionality previously implemented via SelfContainedCall. While supporting SelfContainedCall may still be necessary, it should be considered for deprecation.

@conr2d conr2d added the question label Jan 3, 2025
@boundless-forest
Copy link
Collaborator

For the first block issue, I think rust-ethereum/evm#305 resolves it.

@0xbillw
Copy link
Contributor

0xbillw commented Jan 18, 2025

For the first block issue, I think rust-ethereum/evm#305 resolves it.

But it's blocked on PR now :(

@vedhavyas
Copy link
Contributor

@conr2d

but I couldn't find a straightforward way to invoke call.apply_self_contained() instead of call.dispatch()

I was looking at this. TransactionExtension provided by the pallet-ethereum would just need to return the EthereumOrigin and call can be dispatched as usual

@conr2d
Copy link
Contributor Author

conr2d commented Mar 18, 2025

I was looking at this. TransactionExtension provided by the pallet-ethereum would just need to return the EthereumOrigin and call can be dispatched as usual

Interesting. However, what I was trying to do was to see if self-contained extrinsics, already included in an existing chain, could be processed using a new extrinsic format with TransactionExtension. This is because a self-contained extrinsic is inherently an unsigned (or bare, in the current version) extrinsic, and in this case, the origin isn’t included in bare_validate or bare_validate_and_prepare.

@vedhavyas
Copy link
Contributor

We have recently migrated bunch of our Unsigned calls from Bare to general. All that was required was runtime upgrade and no further client changes.

Once the new format is implemented and runtime is upgrades, rpc ConvertTransactionRuntimeApi would return a General extrinsic format instead of bare format and it should run as usual through extension validation that was included in the new runtime.

This approach would completely elimate the sel-contained crate and fallback to underlying generic Unchecked extrinsic itself.

already included in an existing chain, could be processed using a new extrinsic format with TransactionExtension

If I understand you correctly, you are talking about the transactions that are already in the mempool before runtime was upgraded. These transactions would be included in the block with previous runtime itself. If they were not included, the mempool would revalidate them and discard them due to failed check.
Note: this only happens once for all the transactions that are in mempool but not included yet. If they are discarded, users can resubmit the transactions but this time with new format and they would be included as usual.

I'm happy to provide a draft change if it helps to get some feedback on this

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

No branches or pull requests

4 participants