FRAME: Meta Transaction (pallet based version) #4122
Conversation
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
…tx-ext Signed-off-by: georgepisaltu <[email protected]>
Signed-off-by: georgepisaltu <[email protected]>
| // TODO: to get the len we have to encode the original `meta_tx`. | ||
| let len = 0; | ||
| let mut ctx = T::TxContext::default(); | ||
|
|
There was a problem hiding this comment.
we can use set the DispatchTransaction bound for Config::TxExtension and use it's impl for everything below. here, for the demonstration purpose, I'm not utilizing it.
There was a problem hiding this comment.
DispatchTransaction is meant to work only for "legacy" extensions which use a () context. I believe the current impl is the right thing 👍 .
| @@ -0,0 +1,53 @@ | |||
| [package] | |||
There was a problem hiding this comment.
This whole crate is great, and should live in frame/examples if it remains in any form.
| /// Signature type for meta transactions. | ||
| type Signature: Parameter + Verify<Signer = Self::PublicKey>; | ||
| /// Public key type used for signature verification. | ||
| type PublicKey: IdentifyAccount<AccountId = Self::AccountId>; |
There was a problem hiding this comment.
unrelated to this PR: Kind of a pitty that we can't fetch PublicKey and Signature from AccountId. The reverse is possible. You can always go from Signature (eg. MultiSignature) to PublicKey and then AccountId via IdentifyAccount.
There was a problem hiding this comment.
in this scope I do not even need a public key type, but have no other way to ensure the signature type implemented over the account id the system operates with.
substrate/frame/meta-tx/src/lib.rs
Outdated
| // TODO: The `MetaTx` type is similar to `sp_runtime::generic::UncheckedExtrinsic`. However, | ||
| // `MetaTx` cannot replace generic::UncheckedExtrinsic because we need to box the call type, | ||
| // given that `MetaTx` is used as an argument type for a call. |
There was a problem hiding this comment.
We could probably have this "destructured" in the extrinsic argument list if it looks funny because of the UncheckedExtrinsic similarity. I think it's fine.
substrate/frame/meta-tx/src/lib.rs
Outdated
| pub enum Proof<Address, Signature> { | ||
| Signed(Address, Signature), | ||
| // TODO `General` as in `sp_runtime::generic::Preamble`. | ||
| } |
There was a problem hiding this comment.
I think we can branch off the UncheckedExtrinsic and Preamble logic a bit here and consider introducing a variant for any type of custom validation logic supported by the pallet. In TransactionExtensions, they would be individual extensions in the pipeline, one for each validation type, each wrapping a General transaction.
Here, we care only about the call and validation type, which is nice.
| // TODO: to get the len we have to encode the original `meta_tx`. | ||
| let len = 0; | ||
| let mut ctx = T::TxContext::default(); | ||
|
|
There was a problem hiding this comment.
DispatchTransaction is meant to work only for "legacy" extensions which use a () context. I believe the current impl is the right thing 👍 .
substrate/frame/meta-tx/src/lib.rs
Outdated
| let post_info = res.unwrap_or_else(|err| err.post_info); | ||
| let pd_res = res.map(|_| ()).map_err(|e| e.error); | ||
|
|
||
| T::TxExtension::post_dispatch(pre, &info, &post_info, len, &pd_res, &ctx) |
There was a problem hiding this comment.
This would allow a graceful and harmless failure in cases like the one described here when a post_dispatch fail is critical.
substrate/frame/meta-tx/src/lib.rs
Outdated
| pub struct MetaTx<Address, Signature, Call, TxExtension> { | ||
| proof: Proof<Address, Signature>, | ||
| call: Box<Call>, | ||
| tx_ext: TxExtension, |
There was a problem hiding this comment.
Is there any reason why we can't get away with just a CheckNonce here? The other stuff will be checked when the agent submits the tx anyway, we just want the replay protection for Alice.
If we want to move off the extension approach completely, we could do the nonce check and inc without the extension even, just run the frame_system::Account mutation inside the extrinsic, but the signed payload would need to include the nonce. Maybe it's too complicated, just a thought.
|
The CI pipeline was cancelled due to failure one of the required jobs. |
|
Clippy CI check fails on the file that is not edited by this PR. I will wait for the #3685 to be updated from master, to update this branch, which should address the issue. Currently version of this branch compiles, tests pass. |
github-actions
bot
requested review from
acatangiu,
cheme and
a team
as code owners
|
Kindly ping. |
|
@jasl I am working on it |
|
in favor of #6428 |
Meta transaction implementation based on a pallet's call and configurable via transaction extensions.
To see an example, refer to the
sign_and_execute_meta_txtest case insubstrate/frame/meta-tx/src/tests.rsfile.This implementation serves to demonstrate the concept and not production ready.
Based on: #3685
RFC: #4123
Alternative solution: #3712
TODOs: