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

Upgrade Substrate and Frontier #1954

Merged
merged 13 commits into from
Sep 13, 2023
Merged

Upgrade Substrate and Frontier #1954

merged 13 commits into from
Sep 13, 2023

Conversation

nazar-pc
Copy link
Member

Frontier updates are done in polkadot-evm/frontier#1193, Substrate updates are done in subspace-v6 branch in our fork (rebased due to various conflicts, primarily with block relay).

This upgrades us to latest commit in the old Substrate repo, I'll upgrade us to latest polkadot-sdk in a separate PR.

Interesting changes:

Other important upstream changes that resulted in downstream modifications:

paritytech/substrate#14709 was promising, but not sufficient, so I opened paritytech/polkadot-sdk#1533

Code contributor checklist:

vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
Copy link
Member

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

construct_runtime seems much more readable now.

LGTM

@nazar-pc
Copy link
Member Author

construct_runtime seems much more readable now.

Recall what it looked like 2 years ago. I hope they will get to attribute macro there at some point too 🙂

@nazar-pc nazar-pc enabled auto-merge September 13, 2023 09:35
@nazar-pc nazar-pc disabled auto-merge September 13, 2023 09:51
@nazar-pc
Copy link
Member Author

Okay, looks like something else related to transaction pool needs to be changed to fix tests 😕

@nazar-pc
Copy link
Member Author

@vedhavyas @NingLin-P I'll need your help fixing domains tests here

@nazar-pc
Copy link
Member Author

Interesting, looks like to be able to submit transactions this is now required:

let mut runtime_api = self.client.runtime_api();
runtime_api.register_extension(
    offchain_tx_pool_factory.offchain_transaction_pool(best_hash),
);

Let me fix this and see if tests pass.

vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
@NingLin-P
Copy link
Member

Is it possible to register the extension just once like main branch does instead of registering the extension whenever we need to submit tx?

@nazar-pc
Copy link
Member Author

I believe it should be possible, but it is also recommended to not do that.

rahulksnv
rahulksnv previously approved these changes Sep 13, 2023
We had to introduce Inherent data providers since upstream introduced `pending` tags for Eth rpc calls.
When an rpc call with pending tag is received, we construct pending block by building a new block with extrinsics from tx pool.
So they use inherent data providers to provide inherent data. For now, we just provide timestamp since that is always required.
I have added an TODO to handle any new inherents that needs to be present. Ex runtime upgrade can be added in later PR
@nazar-pc nazar-pc dismissed stale reviews from rahulksnv and vedhavyas via 7a478d9 September 13, 2023 19:22
vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
@nazar-pc
Copy link
Member Author

Hm, merge conflicts already 😒

@nazar-pc nazar-pc merged commit 498c4a2 into main Sep 13, 2023
9 checks passed
@nazar-pc nazar-pc deleted the upgrade-substrate branch September 13, 2023 21:27
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

Successfully merging this pull request may close these issues.

4 participants