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.
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
Extrinsic v5 definition and specification #124
base: main
Are you sure you want to change the base?
Extrinsic v5 definition and specification #124
Changes from 5 commits
380bdbb
01bc758
f2f4f19
dd9e353
e13f615
0ac87df
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the component is called
extensions
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension itself is usually a tuple of multiple extensions, generally referred to as the extension pipeline. Technically it's only one extension, the
TxExtension
commonly defined in runtimes, but that is always a tuple of extensions likeCheckNonce
,CheckWeight
,ChargeTransactionPayment
etc., so it would be only one extension version, as it is the version of the tuple, but there are multiple extensions in the pipeline.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dumb Q: would it not be possible to use a set of extensions that ultimately behave differently than classic "Signed Origins Extension", but still have one extension responsible for authorizing
Signed
origins within this set?Respectively, I'm not sure this "alternative" sentence is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To your question, yes it is entirely possible and, in fact, what I expect to be implemented in most runtimes. The "alternative" wasn't helpful so I removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a drawback IMO. Metadata v15 should show v4 and metadata v16 and ahead have a vector of extrinsic versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is something extra to support in the metadata for both the runtime and users, is this not a drawback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is both a drawback and an improvement - the new metadata support is an improvement, but having to do this enhancement to the metadata is a drawback to this RFC 😛
maybe add another line explicitly calling out that adding this metadata enhancement is ultimately a good thing that should be useful for potential future scenarios too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not true. Signature schemes and addresses were configurable by runtime devs through rust generics. For example, Moonbeam uses only ECDSA signatures with EVM-like addresses.
Besides that, I wouldn't mention
VerifySignature
since it should be RFC-ed anyways.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your statement about configurable signature schemes is true. However, I still consider my statement to be true because:
Signed
origin variant, with arbitrary logic in anyTransactionExtension
being able to authorize that origin; before, a user HAD to provide a transaction signed by a specific account.All of this static logic is now moved to extensions. The extensions receive the inherited implication, the generation of which is still hardcoded and handled in this RFC, but is not in any way mandatory to be used in any signing scheme.
I'd agree though that the phrasing isn't clear, but I'm not sure how to improve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the phrasing is fine as it is. The point is to highlight the increase in configurability, which it does.