Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions prdoc/pr_9780.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
title: '[pallet_transaction_payment]: Share withdrawn tx fee credit with other pallets'
doc:
- audience: Runtime Dev
description: |-
Replaces https://github.com/paritytech/polkadot-sdk/pull/9590.

The audit of #9590 showed that holding the txfee as held balance and especially playing around with `providers` causes a lot of troubles.

This PR is a much lighter change. It keeps the original withdraw/deposit pattern. It simply stores the withdrawn `Credit` and allows other pallets to withdraw from it.

It is also better in terms of performance since all tx signers share a single storage item (instead of a named hold per account).
crates:
- name: pallet-origin-restriction
bump: major
- name: frame-support
bump: major
- name: pallet-transaction-payment
bump: major
- name: pallet-asset-tx-payment
bump: major
5 changes: 5 additions & 0 deletions substrate/frame/origin-restriction/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ frame_support::parameter_types! {
}

pub struct OnChargeTransaction;

impl pallet_transaction_payment::OnChargeTransaction<Test> for OnChargeTransaction {
type Balance = u64;
type LiquidityInfo = ();
Expand Down Expand Up @@ -224,6 +225,10 @@ impl pallet_transaction_payment::OnChargeTransaction<Test> for OnChargeTransacti
}
}

impl pallet_transaction_payment::TxCreditHold<Test> for OnChargeTransaction {
type Credit = ();
}

impl pallet_transaction_payment::Config for Test {
type WeightInfo = ();
type RuntimeEvent = RuntimeEvent;
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ mod storage;
pub use storage::MaybeConsideration;
pub use storage::{
Consideration, ConstantStoragePrice, Disabled, Footprint, Incrementable, Instance,
LinearStoragePrice, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance,
TrackedStorageKey, WhitelistedStorageKeys,
LinearStoragePrice, NoDrop, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait,
StorageInstance, SuppressedDrop, TrackedStorageKey, WhitelistedStorageKeys,
};

mod dispatch;
Expand Down
78 changes: 76 additions & 2 deletions substrate/frame/support/src/traits/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
//! Traits for encoding data related to pallet's storage items.

use alloc::{collections::btree_set::BTreeSet, vec, vec::Vec};
use codec::{Decode, Encode, FullCodec, MaxEncodedLen};
use core::marker::PhantomData;
use codec::{Decode, DecodeWithMemTracking, Encode, FullCodec, MaxEncodedLen};
use core::{marker::PhantomData, mem, ops::Drop};
use frame_support::CloneNoBound;
use impl_trait_for_tuples::impl_for_tuples;
use scale_info::TypeInfo;
Expand Down Expand Up @@ -342,6 +342,80 @@ where

impl_incrementable!(u8, u16, u32, u64, u128, i8, i16, i32, i64, i128);

/// Wrap a type so that is `Drop` impl is never called.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Wrap a type so that is `Drop` impl is never called.
/// Wrap a type so that its `Drop` impl is never called.

///
/// Useful when storing types like `Imbalance` which would trigger their `Drop`
/// implementation whenever they are written to storage as they are dropped after
/// being serialized.
#[derive(Default, Encode, Decode, DecodeWithMemTracking, MaxEncodedLen, TypeInfo)]
pub struct NoDrop<T: Default>(T);

impl<T: Default> Drop for NoDrop<T> {
fn drop(&mut self) {
mem::forget(mem::take(&mut self.0))
}
}

/// Sealed trait that marks a type with a suppressed Drop implementation.
///
/// Useful for constraining your storage items types by this bound to make
/// sure they won't runD rop when stored.
pub trait SuppressedDrop: sealed::Sealed {
/// The wrapped whose drop function is ignored.
type Inner;

fn new(inner: Self::Inner) -> Self;
fn as_ref(&self) -> &Self::Inner;
fn as_mut(&mut self) -> &mut Self::Inner;
fn into_inner(self) -> Self::Inner;
}

impl SuppressedDrop for () {
type Inner = ();

fn new(inner: Self::Inner) -> Self {
inner
}

fn as_ref(&self) -> &Self::Inner {
self
}

fn as_mut(&mut self) -> &mut Self::Inner {
self
}

fn into_inner(self) -> Self::Inner {
self
}
}

impl<T: Default> SuppressedDrop for NoDrop<T> {
type Inner = T;

fn as_ref(&self) -> &Self::Inner {
&self.0
}

fn as_mut(&mut self) -> &mut Self::Inner {
&mut self.0
}

fn into_inner(mut self) -> Self::Inner {
mem::take(&mut self.0)
}

fn new(inner: Self::Inner) -> Self {
Self(inner)
}
}

mod sealed {
pub trait Sealed {}
impl Sealed for () {}
impl<T: Default> Sealed for super::NoDrop<T> {}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
23 changes: 18 additions & 5 deletions substrate/frame/support/src/traits/tokens/fungible/imbalance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
//! See the [`crate::traits::fungible`] doc for more information about fungible traits.

use super::{super::Imbalance as ImbalanceT, Balanced, *};
use crate::traits::{
fungibles,
misc::{SameOrOther, TryDrop},
tokens::{imbalance::TryMerge, AssetId, Balance},
use crate::{
pallet_prelude::{Decode, DecodeWithMemTracking, Encode, MaxEncodedLen, TypeInfo},
traits::{
fungibles,
misc::{SameOrOther, TryDrop},
tokens::{imbalance::TryMerge, AssetId, Balance},
},
};
use core::marker::PhantomData;
use frame_support_procedural::{EqNoBound, PartialEqNoBound, RuntimeDebugNoBound};
Expand All @@ -47,7 +50,17 @@ impl<Balance> HandleImbalanceDrop<Balance> for () {
///
/// Importantly, it has a special `Drop` impl, and cannot be created outside of this module.
#[must_use]
#[derive(EqNoBound, PartialEqNoBound, RuntimeDebugNoBound)]
#[derive(
EqNoBound,
PartialEqNoBound,
RuntimeDebugNoBound,
Encode,
Decode,
DecodeWithMemTracking,
MaxEncodedLen,
TypeInfo,
)]
#[scale_info(skip_type_params(OnDrop, OppositeOnDrop))]
pub struct Imbalance<
B: Balance,
OnDrop: HandleImbalanceDrop<B>,
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/transaction-payment/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ codec = { features = ["derive"], workspace = true }
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
log = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
serde = { optional = true, workspace = true, default-features = true }
sp-io = { workspace = true }
Expand All @@ -36,6 +37,7 @@ std = [
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
"log/std",
"scale-info/std",
"serde",
"sp-io/std",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,19 +384,15 @@ where
};

match initial_payment {
InitialPayment::Native(already_withdrawn) => {
InitialPayment::Native(liquidity_info) => {
// Take into account the weight used by this extension before calculating the
// refund.
let actual_ext_weight = <T as Config>::WeightInfo::charge_asset_tx_payment_native();
let unspent_weight = extension_weight.saturating_sub(actual_ext_weight);
let mut actual_post_info = *post_info;
actual_post_info.refund(unspent_weight);
pallet_transaction_payment::ChargeTransactionPayment::<T>::post_dispatch_details(
pallet_transaction_payment::Pre::Charge {
tip,
who,
imbalance: already_withdrawn,
},
pallet_transaction_payment::Pre::Charge { tip, who, liquidity_info },
info,
&actual_post_info,
len,
Expand Down
Loading
Loading