Signed Frontier-specific extrinsic #482
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
Comment on lines
+17
to
+22
| /// A extrinsic right from the external world. This is unchecked and so | ||
| /// can contain a signature. | ||
| #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] | ||
| pub struct UncheckedExtrinsic<Address, Call, Signature, Extra: SignedExtension>( | ||
| pub sp_runtime::generic::UncheckedExtrinsic<Address, Call, Signature, Extra>, | ||
| ); |
There was a problem hiding this comment.
This wrapping technique makes a lot of sense. I could imagine using this same technique in a repo that brings bitcoin compatibility to substrate. Same for any other existing-chain-compatibility.
If I want a chain that uses frontier and also the bitcoin-compatibility stuff, I would want to compose the unchecked extrinsic wrappers from both repos. Therefore, I think it would be cool to be generic over the thing you're wrapping.
pub struct FrontierUncheckedExtrinsic<I: Extrinsic + Checkable + ...>{
pub inner: I,
}There was a problem hiding this comment.
We can't yet do that now cause the struct relies on the specific information available in UncheckedExtrinsic, but I agree this is the direction to go.
abhijeetbhagat
pushed a commit
to web3labs/frontier
that referenced
this pull request
Jan 11, 2023
* Signed custom extrinsic implementation * Fix some compile issues * Fully verify the transaction in UncheckedExtrinsic * Propogate hash information to runtime * Generalize the ethereum signature verification to SelfContainedCall * Rename fp-ethereum to fp-self-contained * Add new origin to pallet-ethereum * Define the actual new origin type * Add EnsureEthereumTransaction origin check * Remove validate_unsigned and add self-contained check * Finish template build * Run rustfmt * Fix tests * Fix lints * Use where OriginFor<T> clause instead of Config::Origin
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is based on @xlc's work.
Use a Frontier-specific extrinsic struct, so that we don't have to treat Ethereum transactions as "unsigned". This is generalized to suit Frontier library's need, namely that we must support multiple different
AccountIdimplementations.Transaction struct is backward-compatible by marking certain call functions as "self-contained". A custom
CheckedExtrinsicstruct is then used to dispatch it under a Frontier-specificEthereumTransactionorigin, which is later read by the runtime.The code now should be able to verify an Ethereum transaction as safe as a Substrate extrinsic. This also includes any gas limit or nonce related data, as now we use the same code path and do the verifications in exact same places as it happens on Substrate extrinsics.
FAQ
Why do we have to define our own
CheckedExtrinsicgiven that Ethereum transaction senders already have mappings toAccountId?We must not use any existing
SignedExtensions. They added things that should have been signed by the signature, but we don't have the means for additional signatures. In addition, if included, they will lead to double charge of transaction fees and other unwanted side effects.The
SignedExtensionsinvocation logic is handled inCheckedExtrinsic.Upgrade notice
To use the new code, you need to change in your runtime checked and unchecked extrinsics to point to
fp_self_contained::CheckedExtrinsicandfp_self_contained::UncheckedExtrinsic. This will also require you to implementSelfContainedCalltrait for runtimeCall, which the node template contains sample code.