Skip to content

Commit fd5f929

Browse files
gavofyorkgupnikgeorgepisaltuchevdorbkchr
authored
FRAME: Create TransactionExtension as a replacement for SignedExtension (#2280)
Closes #2160 First part of [Extrinsic Horizon](#2415) Introduces a new trait `TransactionExtension` to replace `SignedExtension`. Introduce the idea of transactions which obey the runtime's extensions and have according Extension data (né Extra data) yet do not have hard-coded signatures. Deprecate the terminology of "Unsigned" when used for transactions/extrinsics owing to there now being "proper" unsigned transactions which obey the extension framework and "old-style" unsigned which do not. Instead we have __*General*__ for the former and __*Bare*__ for the latter. (Ultimately, the latter will be phased out as a type of transaction, and Bare will only be used for Inherents.) Types of extrinsic are now therefore: - Bare (no hardcoded signature, no Extra data; used to be known as "Unsigned") - Bare transactions (deprecated): Gossiped, validated with `ValidateUnsigned` (deprecated) and the `_bare_compat` bits of `TransactionExtension` (deprecated). - Inherents: Not gossiped, validated with `ProvideInherent`. - Extended (Extra data): Gossiped, validated via `TransactionExtension`. - Signed transactions (with a hardcoded signature). - General transactions (without a hardcoded signature). `TransactionExtension` differs from `SignedExtension` because: - A signature on the underlying transaction may validly not be present. - It may alter the origin during validation. - `pre_dispatch` is renamed to `prepare` and need not contain the checks present in `validate`. - `validate` and `prepare` is passed an `Origin` rather than a `AccountId`. - `validate` may pass arbitrary information into `prepare` via a new user-specifiable type `Val`. - `AdditionalSigned`/`additional_signed` is renamed to `Implicit`/`implicit`. It is encoded *for the entire transaction* and passed in to each extension as a new argument to `validate`. This facilitates the ability of extensions to acts as underlying crypto. There is a new `DispatchTransaction` trait which contains only default function impls and is impl'ed for any `TransactionExtension` impler. It provides several utility functions which reduce some of the tedium from using `TransactionExtension` (indeed, none of its regular functions should now need to be called directly). Three transaction version discriminator ("versions") are now permissible: - 0b000000100: Bare (used to be called "Unsigned"): contains Signature or Extra (extension data). After bare transactions are no longer supported, this will strictly identify an Inherents only. - 0b100000100: Old-school "Signed" Transaction: contains Signature and Extra (extension data). - 0b010000100: New-school "General" Transaction: contains Extra (extension data), but no Signature. For the New-school General Transaction, it becomes trivial for authors to publish extensions to the mechanism for authorizing an Origin, e.g. through new kinds of key-signing schemes, ZK proofs, pallet state, mutations over pre-authenticated origins or any combination of the above. ## Code Migration ### NOW: Getting it to build Wrap your `SignedExtension`s in `AsTransactionExtension`. This should be accompanied by renaming your aggregate type in line with the new terminology. E.g. Before: ```rust /// The SignedExtension to the basic transaction logic. pub type SignedExtra = ( /* snip */ MySpecialSignedExtension, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>; ``` After: ```rust /// The extension to the basic transaction logic. pub type TxExtension = ( /* snip */ AsTransactionExtension<MySpecialSignedExtension>, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, TxExtension>; ``` You'll also need to alter any transaction building logic to add a `.into()` to make the conversion happen. E.g. Before: ```rust fn construct_extrinsic( /* snip */ ) -> UncheckedExtrinsic { let extra: SignedExtra = ( /* snip */ MySpecialSignedExtension::new(/* snip */), ); let payload = SignedPayload::new(call.clone(), extra.clone()).unwrap(); let signature = payload.using_encoded(|e| sender.sign(e)); UncheckedExtrinsic::new_signed( /* snip */ Signature::Sr25519(signature), extra, ) } ``` After: ```rust fn construct_extrinsic( /* snip */ ) -> UncheckedExtrinsic { let tx_ext: TxExtension = ( /* snip */ MySpecialSignedExtension::new(/* snip */).into(), ); let payload = SignedPayload::new(call.clone(), tx_ext.clone()).unwrap(); let signature = payload.using_encoded(|e| sender.sign(e)); UncheckedExtrinsic::new_signed( /* snip */ Signature::Sr25519(signature), tx_ext, ) } ``` ### SOON: Migrating to `TransactionExtension` Most `SignedExtension`s can be trivially converted to become a `TransactionExtension`. There are a few things to know. - Instead of a single trait like `SignedExtension`, you should now implement two traits individually: `TransactionExtensionBase` and `TransactionExtension`. - Weights are now a thing and must be provided via the new function `fn weight`. #### `TransactionExtensionBase` This trait takes care of anything which is not dependent on types specific to your runtime, most notably `Call`. - `AdditionalSigned`/`additional_signed` is renamed to `Implicit`/`implicit`. - Weight must be returned by implementing the `weight` function. If your extension is associated with a pallet, you'll probably want to do this via the pallet's existing benchmarking infrastructure. #### `TransactionExtension` Generally: - `pre_dispatch` is now `prepare` and you *should not reexecute the `validate` functionality in there*! - You don't get an account ID any more; you get an origin instead. If you need to presume an account ID, then you can use the trait function `AsSystemOriginSigner::as_system_origin_signer`. - You get an additional ticket, similar to `Pre`, called `Val`. This defines data which is passed from `validate` into `prepare`. This is important since you should not be duplicating logic from `validate` to `prepare`, you need a way of passing your working from the former into the latter. This is it. - This trait takes two type parameters: `Call` and `Context`. `Call` is the runtime call type which used to be an associated type; you can just move it to become a type parameter for your trait impl. `Context` is not currently used and you can safely implement over it as an unbounded type. - There's no `AccountId` associated type any more. Just remove it. Regarding `validate`: - You get three new parameters in `validate`; all can be ignored when migrating from `SignedExtension`. - `validate` returns a tuple on success; the second item in the tuple is the new ticket type `Self::Val` which gets passed in to `prepare`. If you use any information extracted during `validate` (off-chain and on-chain, non-mutating) in `prepare` (on-chain, mutating) then you can pass it through with this. For the tuple's last item, just return the `origin` argument. Regarding `prepare`: - This is renamed from `pre_dispatch`, but there is one change: - FUNCTIONALITY TO VALIDATE THE TRANSACTION NEED NOT BE DUPLICATED FROM `validate`!! - (This is different to `SignedExtension` which was required to run the same checks in `pre_dispatch` as in `validate`.) Regarding `post_dispatch`: - Since there are no unsigned transactions handled by `TransactionExtension`, `Pre` is always defined, so the first parameter is `Self::Pre` rather than `Option<Self::Pre>`. If you make use of `SignedExtension::validate_unsigned` or `SignedExtension::pre_dispatch_unsigned`, then: - Just use the regular versions of these functions instead. - Have your logic execute in the case that the `origin` is `None`. - Ensure your transaction creation logic creates a General Transaction rather than a Bare Transaction; this means having to include all `TransactionExtension`s' data. - `ValidateUnsigned` can still be used (for now) if you need to be able to construct transactions which contain none of the extension data, however these will be phased out in stage 2 of the Transactions Horizon, so you should consider moving to an extension-centric design. ## TODO - [x] Introduce `CheckSignature` impl of `TransactionExtension` to ensure it's possible to have crypto be done wholly in a `TransactionExtension`. - [x] Deprecate `SignedExtension` and move all uses in codebase to `TransactionExtension`. - [x] `ChargeTransactionPayment` - [x] `DummyExtension` - [x] `ChargeAssetTxPayment` (asset-tx-payment) - [x] `ChargeAssetTxPayment` (asset-conversion-tx-payment) - [x] `CheckWeight` - [x] `CheckTxVersion` - [x] `CheckSpecVersion` - [x] `CheckNonce` - [x] `CheckNonZeroSender` - [x] `CheckMortality` - [x] `CheckGenesis` - [x] `CheckOnlySudoAccount` - [x] `WatchDummy` - [x] `PrevalidateAttests` - [x] `GenericSignedExtension` - [x] `SignedExtension` (chain-polkadot-bulletin) - [x] `RefundSignedExtensionAdapter` - [x] Implement `fn weight` across the board. - [ ] Go through all pre-existing extensions which assume an account signer and explicitly handle the possibility of another kind of origin. - [x] `CheckNonce` should probably succeed in the case of a non-account origin. - [x] `CheckNonZeroSender` should succeed in the case of a non-account origin. - [x] `ChargeTransactionPayment` and family should fail in the case of a non-account origin. - [ ] - [x] Fix any broken tests. --------- Signed-off-by: georgepisaltu <[email protected]> Signed-off-by: Alexandru Vasile <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Oliver Tale-Yazdi <[email protected]> Signed-off-by: Alexandru Gheorghe <[email protected]> Signed-off-by: Andrei Sandu <[email protected]> Co-authored-by: Nikhil Gupta <[email protected]> Co-authored-by: georgepisaltu <[email protected]> Co-authored-by: Chevdor <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Maciej <[email protected]> Co-authored-by: Javier Viola <[email protected]> Co-authored-by: Marcin S. <[email protected]> Co-authored-by: Tsvetomir Dimitrov <[email protected]> Co-authored-by: Javier Bullrich <[email protected]> Co-authored-by: Koute <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]> Co-authored-by: Vladimir Istyufeev <[email protected]> Co-authored-by: Ross Bulat <[email protected]> Co-authored-by: Gonçalo Pestana <[email protected]> Co-authored-by: Liam Aharon <[email protected]> Co-authored-by: Svyatoslav Nikolsky <[email protected]> Co-authored-by: André Silva <[email protected]> Co-authored-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: s0me0ne-unkn0wn <[email protected]> Co-authored-by: ordian <[email protected]> Co-authored-by: Sebastian Kunert <[email protected]> Co-authored-by: Aaro Altonen <[email protected]> Co-authored-by: Dmitry Markin <[email protected]> Co-authored-by: Alexandru Vasile <[email protected]> Co-authored-by: Alexander Samusev <[email protected]> Co-authored-by: Julian Eager <[email protected]> Co-authored-by: Michal Kucharczyk <[email protected]> Co-authored-by: Davide Galassi <[email protected]> Co-authored-by: Dónal Murray <[email protected]> Co-authored-by: yjh <[email protected]> Co-authored-by: Tom Mi <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Will | Paradox | ParaNodes.io <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Joshy Orndorff <[email protected]> Co-authored-by: Joshy Orndorff <[email protected]> Co-authored-by: PG Herveou <[email protected]> Co-authored-by: Alexander Theißen <[email protected]> Co-authored-by: Kian Paimani <[email protected]> Co-authored-by: Juan Girini <[email protected]> Co-authored-by: bader y <[email protected]> Co-authored-by: James Wilson <[email protected]> Co-authored-by: joe petrowski <[email protected]> Co-authored-by: asynchronous rob <[email protected]> Co-authored-by: Parth <[email protected]> Co-authored-by: Andrew Jones <[email protected]> Co-authored-by: Jonathan Udd <[email protected]> Co-authored-by: Serban Iorga <[email protected]> Co-authored-by: Egor_P <[email protected]> Co-authored-by: Branislav Kontur <[email protected]> Co-authored-by: Evgeny Snitko <[email protected]> Co-authored-by: Just van Stam <[email protected]> Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: gupnik <[email protected]> Co-authored-by: dzmitry-lahoda <[email protected]> Co-authored-by: zhiqiangxu <[email protected]> Co-authored-by: Nazar Mokrynskyi <[email protected]> Co-authored-by: Anwesh <[email protected]> Co-authored-by: cheme <[email protected]> Co-authored-by: Sam Johnson <[email protected]> Co-authored-by: kianenigma <[email protected]> Co-authored-by: Jegor Sidorenko <[email protected]> Co-authored-by: Muharem <[email protected]> Co-authored-by: joepetrowski <[email protected]> Co-authored-by: Alexandru Gheorghe <[email protected]> Co-authored-by: Gabriel Facco de Arruda <[email protected]> Co-authored-by: Squirrel <[email protected]> Co-authored-by: Andrei Sandu <[email protected]> Co-authored-by: georgepisaltu <[email protected]> Co-authored-by: command-bot <>
1 parent b0741d4 commit fd5f929

File tree

349 files changed

+25345
-16846
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

349 files changed

+25345
-16846
lines changed

Cargo.lock

Lines changed: 23 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bridges/bin/runtime-common/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ runtime-benchmarks = [
9393
"pallet-bridge-messages/runtime-benchmarks",
9494
"pallet-bridge-parachains/runtime-benchmarks",
9595
"pallet-bridge-relayers/runtime-benchmarks",
96+
"pallet-transaction-payment/runtime-benchmarks",
9697
"pallet-utility/runtime-benchmarks",
9798
"sp-runtime/runtime-benchmarks",
9899
"xcm-builder/runtime-benchmarks",

bridges/bin/runtime-common/src/lib.rs

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -105,43 +105,48 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages {
105105
($call:ty, $account_id:ty, $($filter_call:ty),*) => {
106106
#[derive(Clone, codec::Decode, Default, codec::Encode, Eq, PartialEq, sp_runtime::RuntimeDebug, scale_info::TypeInfo)]
107107
pub struct BridgeRejectObsoleteHeadersAndMessages;
108-
impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteHeadersAndMessages {
108+
impl sp_runtime::traits::TransactionExtensionBase for BridgeRejectObsoleteHeadersAndMessages {
109109
const IDENTIFIER: &'static str = "BridgeRejectObsoleteHeadersAndMessages";
110-
type AccountId = $account_id;
111-
type Call = $call;
112-
type AdditionalSigned = ();
110+
type Implicit = ();
111+
}
112+
impl<Context> sp_runtime::traits::TransactionExtension<$call, Context> for BridgeRejectObsoleteHeadersAndMessages {
113113
type Pre = ();
114-
115-
fn additional_signed(&self) -> sp_std::result::Result<
116-
(),
117-
sp_runtime::transaction_validity::TransactionValidityError,
118-
> {
119-
Ok(())
120-
}
114+
type Val = ();
121115

122116
fn validate(
123117
&self,
124-
_who: &Self::AccountId,
125-
call: &Self::Call,
126-
_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
118+
origin: <$call as sp_runtime::traits::Dispatchable>::RuntimeOrigin,
119+
call: &$call,
120+
_info: &sp_runtime::traits::DispatchInfoOf<$call>,
127121
_len: usize,
128-
) -> sp_runtime::transaction_validity::TransactionValidity {
129-
let valid = sp_runtime::transaction_validity::ValidTransaction::default();
122+
_context: &mut Context,
123+
_self_implicit: Self::Implicit,
124+
_inherited_implication: &impl codec::Encode,
125+
) -> Result<
126+
(
127+
sp_runtime::transaction_validity::ValidTransaction,
128+
Self::Val,
129+
<$call as sp_runtime::traits::Dispatchable>::RuntimeOrigin,
130+
), sp_runtime::transaction_validity::TransactionValidityError
131+
> {
132+
let tx_validity = sp_runtime::transaction_validity::ValidTransaction::default();
130133
$(
131-
let valid = valid
132-
.combine_with(<$filter_call as $crate::BridgeRuntimeFilterCall<$call>>::validate(call)?);
134+
let call_filter_validity = <$filter_call as $crate::BridgeRuntimeFilterCall<$call>>::validate(call)?;
135+
let tx_validity = tx_validity.combine_with(call_filter_validity);
133136
)*
134-
Ok(valid)
137+
Ok((tx_validity, (), origin))
135138
}
136139

137-
fn pre_dispatch(
140+
fn prepare(
138141
self,
139-
who: &Self::AccountId,
140-
call: &Self::Call,
141-
info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
142-
len: usize,
142+
_val: Self::Val,
143+
_origin: &<$call as sp_runtime::traits::Dispatchable>::RuntimeOrigin,
144+
_call: &$call,
145+
_info: &sp_runtime::traits::DispatchInfoOf<$call>,
146+
_len: usize,
147+
_context: &Context,
143148
) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> {
144-
self.validate(who, call, info, len).map(drop)
149+
Ok(())
145150
}
146151
}
147152
};
@@ -150,12 +155,14 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages {
150155
#[cfg(test)]
151156
mod tests {
152157
use crate::BridgeRuntimeFilterCall;
153-
use frame_support::{assert_err, assert_ok};
158+
use codec::Encode;
159+
use frame_support::assert_err;
154160
use sp_runtime::{
155-
traits::SignedExtension,
161+
traits::DispatchTransaction,
156162
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
157163
};
158164

165+
#[derive(Encode)]
159166
pub struct MockCall {
160167
data: u32,
161168
}
@@ -206,17 +213,20 @@ mod tests {
206213
);
207214

208215
assert_err!(
209-
BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 1 }, &(), 0),
216+
BridgeRejectObsoleteHeadersAndMessages.validate_only((), &MockCall { data: 1 }, &(), 0),
210217
InvalidTransaction::Custom(1)
211218
);
212219

213220
assert_err!(
214-
BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 2 }, &(), 0),
221+
BridgeRejectObsoleteHeadersAndMessages.validate_only((), &MockCall { data: 2 }, &(), 0),
215222
InvalidTransaction::Custom(2)
216223
);
217224

218-
assert_ok!(
219-
BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 3 }, &(), 0),
225+
assert_eq!(
226+
BridgeRejectObsoleteHeadersAndMessages
227+
.validate_only((), &MockCall { data: 3 }, &(), 0)
228+
.unwrap()
229+
.0,
220230
ValidTransaction { priority: 3, ..Default::default() }
221231
)
222232
}

bridges/bin/runtime-common/src/mock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ impl pallet_balances::Config for TestRuntime {
164164
type AccountStore = System;
165165
}
166166

167+
#[derive_impl(pallet_transaction_payment::config_preludes::TestDefaultConfig as pallet_transaction_payment::DefaultConfig)]
167168
impl pallet_transaction_payment::Config for TestRuntime {
168169
type OnChargeTransaction = pallet_transaction_payment::CurrencyAdapter<Balances, ()>;
169170
type OperationalFeeMultiplier = ConstU8<5>;
@@ -176,7 +177,6 @@ impl pallet_transaction_payment::Config for TestRuntime {
176177
MinimumMultiplier,
177178
MaximumMultiplier,
178179
>;
179-
type RuntimeEvent = RuntimeEvent;
180180
}
181181

182182
impl pallet_bridge_grandpa::Config for TestRuntime {

bridges/bin/runtime-common/src/priority_calculator.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,20 +169,23 @@ mod integrity_tests {
169169
// nodes to the proof (x0.5 because we expect some nodes to be reused)
170170
let estimated_message_size = 512;
171171
// let's say all our messages have the same dispatch weight
172-
let estimated_message_dispatch_weight =
173-
Runtime::WeightInfo::message_dispatch_weight(estimated_message_size);
172+
let estimated_message_dispatch_weight = <Runtime as pallet_bridge_messages::Config<
173+
MessagesInstance,
174+
>>::WeightInfo::message_dispatch_weight(
175+
estimated_message_size
176+
);
174177
// messages proof argument size is (for every message) messages size + some additional
175178
// trie nodes. Some of them are reused by different messages, so let's take 2/3 of default
176179
// "overhead" constant
177-
let messages_proof_size = Runtime::WeightInfo::expected_extra_storage_proof_size()
180+
let messages_proof_size = <Runtime as pallet_bridge_messages::Config<MessagesInstance>>::WeightInfo::expected_extra_storage_proof_size()
178181
.saturating_mul(2)
179182
.saturating_div(3)
180183
.saturating_add(estimated_message_size)
181184
.saturating_mul(messages as _);
182185

183186
// finally we are able to estimate transaction size and weight
184187
let transaction_size = base_tx_size.saturating_add(messages_proof_size);
185-
let transaction_weight = Runtime::WeightInfo::receive_messages_proof_weight(
188+
let transaction_weight = <Runtime as pallet_bridge_messages::Config<MessagesInstance>>::WeightInfo::receive_messages_proof_weight(
186189
&PreComputedSize(transaction_size as _),
187190
messages as _,
188191
estimated_message_dispatch_weight.saturating_mul(messages),

0 commit comments

Comments
 (0)