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.

Sign up for GitHub

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

Jump to bottom

Migrate to frontier Extrinsics #871

Merged
merged 19 commits into from
Oct 18, 2021
Merged

Migrate to frontier Extrinsics #871

merged 19 commits into from
Oct 18, 2021

Conversation

librelois
Copy link
Collaborator

@librelois librelois commented Sep 30, 2021 •
edited
Loading

What does it do?

Replace substrate CheckedExtrinsic and UncheckedExtrinsic by fp_self_contained::CheckedExtrinsic and fp_self_contained::UncheckedExtrinsic

Based on the purestake/moonbeam-polkadot-v0.9.10-frontier-extrinsics frontier branch that incorporates this PR: polkadot-evm/frontier#482

What important points reviewers should know?

Evm transactions are no longer wrapped in an unsigned extrinsic but in a new dedicated extrinsic type named "Selfcontained".
Frontier defines a SelfContainedCall trait that the Call type of each runtime must implement.
In our case, the implementation is common and located in runtime/common/src/impl_self_contained_call.rs.

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@librelois librelois added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes A3-inprogress Pull request is in progress. No review needed at this stage. labels Sep 30, 2021
@librelois librelois added the D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. label Oct 4, 2021
@librelois librelois added A0-pleasereview Pull request needs code review. and removed A3-inprogress Pull request is in progress. No review needed at this stage. labels Oct 4, 2021
@librelois librelois marked this pull request as ready for review October 4, 2021 19:43
@librelois librelois mentioned this pull request Oct 5, 2021
Merged
tgmichel
tgmichel reviewed Oct 5, 2021
View reviewed changes
Copy link
Contributor

@tgmichel tgmichel left a comment

Choose a reason for hiding this comment

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

LGTM, awesome work. Just left a couple of comments.

runtime/moonbeam/tests/integration_test.rs Outdated Show resolved Hide resolved
runtime/common/src/impl_self_contained_call.rs Show resolved Hide resolved
@tgmichel
Copy link
Contributor

tgmichel commented Oct 5, 2021

I would add D9 - needsaudit intead of D5 - nicetohaveaudit.

@librelois librelois added D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Oct 12, 2021
@librelois
Copy link
Collaborator Author

Blocked by polkadot-evm/frontier#495

@librelois librelois marked this pull request as draft October 12, 2021 14:36
@librelois librelois marked this pull request as ready for review October 13, 2021 13:35
@librelois librelois merged commit 26e63db into master Oct 18, 2021
@librelois librelois deleted the elois-frontier-signed branch October 18, 2021 16:11
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Feb 9, 2022
notlesh added a commit that referenced this pull request Oct 6, 2022
@notlesh
Bump frontier to include #857 and #871
380a8f3
crystalin pushed a commit that referenced this pull request Oct 11, 2022
@notlesh
Cherry-pick Frontier #857, #871, and #875 (#1854)
8bb76bb 
* Bump frontier to include #857 and #871

* Update test fee expectations

* Revert "Update test fee expectations"

This reverts commit 947ab82.

* Bump frontier to include #875
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgmichel tgmichel tgmichel approved these changes

@notlesh notlesh Awaiting requested review from notlesh

@nanocryk nanocryk Awaiting requested review from nanocryk

Assignees
No one assigned
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@librelois @tgmichel @notlesh