From 370f548f7a6b66477154ffbf46b5ec93fe9ffcf7 Mon Sep 17 00:00:00 2001 From: Benjamin Gallois <business@gallois.cc> Date: Wed, 2 Oct 2024 15:20:02 +0200 Subject: [PATCH] Fix #235 allow remark in prod with a limit on extrinsic size for free transaction (nodes/rust/duniter-v2s!278) * add fees consts * allow remark with event * fix filter * fix fees for small length * update documentation * adjust length fees * refactore fee tests * add extrinsic length limit * authorize remarks --- runtime/common/src/fees.rs | 44 ++++++++++---- runtime/g1/src/lib.rs | 11 ---- runtime/gdev/src/lib.rs | 9 +-- runtime/gdev/tests/fee_tests.rs | 102 +++++++++++++------------------- runtime/gtest/src/lib.rs | 3 - 5 files changed, 77 insertions(+), 92 deletions(-) diff --git a/runtime/common/src/fees.rs b/runtime/common/src/fees.rs index 9d7a166bb..818601166 100644 --- a/runtime/common/src/fees.rs +++ b/runtime/common/src/fees.rs @@ -14,12 +14,26 @@ // You should have received a copy of the GNU Affero General Public License // along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>. -// In the deployed fees model, a mapping of 5 (5cG) corresponds to a base extrinsic weight, -// achieved through a 1-dimensional polynomial. Additionally, 1 (1cG) corresponds to an extrinsic length of 100 bytes. -// -// For testing purposes, we adopt a human-predictable weight system that remains invariant to the chosen fees model for release. -// This involves setting a constant weight_to_fee equal to 1 and a constant length_to_fee set to 0, resulting in each extrinsic costing 2 (2cG). - +/// In our deployed fee model, users will not pay any fees if blockchain usage remains below a +/// specified threshold, and fees are applied based on transaction weight and length once this +/// threshold is exceeded, helping to prevent spamming attacks. +/// +/// When the current block's weight and length are below the targeted thresholds, no fees are charged, +/// as the weight-to-fee conversion results in zero. Once the block's weight and length exceed these +/// targets, the weight-to-fee conversion maps BASE_EXTRINSIC_WEIGHT_COST to a base extrinsic weight. +/// Additionally, a fee is applied based on the length of the extrinsic and is mapped affinely: +/// 2_000 (20G) corresponds to an extrinsic length of BYTES_PER_UNIT*10 plus the BASE_EXTRINSIC_LENGTH_COST and will be applied only if the extrinsic +/// exceeds MAX_EXTRINSIC_LENGTH bytes or if the block target in weight or length is surpassed. +/// +/// To further deter abuse, if the previous block's weight or length the target thresholds, +/// the chain increases the fees by multiplying the transaction weight with a `FeeMultiplier`. For each +/// consecutive block that exceeds the targets, this multiplier increases by one. If the targets are +/// not reached, the multiplier decreases by one. The `FeeMultiplier` ranges from 1 (normal usage) to +/// `MaxMultiplier`, where heavy usage causes a number `MaxMultiplier` of consecutive blocks to exceed targets. +/// +/// For testing purposes, a simplified, human-predictable weight system is used. This test model sets +/// a constant `weight_to_fee` of 1 and a `length_to_fee` of 0, making each extrinsic cost 2 (2cG), +/// and can be activated with the #[cfg(feature = "constant-fees")] feature. pub use frame_support::weights::{Weight, WeightToFee}; use pallet_transaction_payment::{Multiplier, MultiplierUpdate}; use sp_arithmetic::traits::{BaseArithmetic, Unsigned}; @@ -61,10 +75,16 @@ where /// Function to convert weight to fee when "constant-fees" feature is not enabled. /// /// This function calculates the fee based on the length of the transaction in bytes. - /// If the current block weight and length are less than a fraction of the max block weight and length and the fee multiplier is one, - /// it returns a zero fee. Otherwise, it calculates the fee based on the length in bytes. + /// If the current block weight and length are less than a fraction of the max block weight and length, the fee multiplier is one, + /// and the extrinsic length is less than MAX_EXTRINSIC_LENGTH bytes, no fees are applied. Otherwise, it calculates the fee based on the length in bytes. #[cfg(not(feature = "constant-fees"))] fn weight_to_fee(length_in_bytes: &Weight) -> Self::Balance { + // The extrinsic overhead for a remark is approximately 110 bytes. + // This leaves 146 bytes available for the actual remark content. + const MAX_EXTRINSIC_LENGTH: u64 = 256; + const BYTES_PER_UNIT: u64 = 350; + const BASE_EXTRINSIC_LENGTH_COST: u64 = 5; + let weights = Runtime::BlockWeights::get(); let fee_multiplier = pallet_transaction_payment::Pallet::<Runtime>::next_fee_multiplier(); let normal_max_weight = weights @@ -82,10 +102,13 @@ where .all_lt(Target::get() * normal_max_weight) && current_block_length < (Target::get() * normal_max_length) && fee_multiplier.is_one() + && length_in_bytes.ref_time() < MAX_EXTRINSIC_LENGTH { 0u32.into() } else { - Self::Balance::saturated_from(length_in_bytes.ref_time() / 100u64) + Self::Balance::saturated_from( + length_in_bytes.ref_time() / BYTES_PER_UNIT + BASE_EXTRINSIC_LENGTH_COST, + ) } } @@ -152,7 +175,8 @@ where }] } else { // The extrinsic base weight (smallest non-zero weight) is mapped to 5 cents - let p: Self::Balance = 5u64.into(); + const BASE_EXTRINSIC_WEIGHT_COST: u64 = 5; + let p: Self::Balance = BASE_EXTRINSIC_WEIGHT_COST.into(); let q: Self::Balance = Self::Balance::from(weights.get(DispatchClass::Normal).base_extrinsic.ref_time()); smallvec![WeightToFeeCoefficient { diff --git a/runtime/g1/src/lib.rs b/runtime/g1/src/lib.rs index 865c524eb..ce21fbb33 100644 --- a/runtime/g1/src/lib.rs +++ b/runtime/g1/src/lib.rs @@ -173,17 +173,6 @@ mod benches { pub struct BaseCallFilter; impl Contains<RuntimeCall> for BaseCallFilter { - #[cfg(not(feature = "runtime-benchmarks"))] - fn contains(call: &RuntimeCall) -> bool { - !matches!( - call, - RuntimeCall::System( - frame_system::Call::remark { .. } | frame_system::Call::remark_with_event { .. } - ) | RuntimeCall::Session(_) - ) - } - - #[cfg(feature = "runtime-benchmarks")] fn contains(call: &RuntimeCall) -> bool { !matches!(call, RuntimeCall::Session(_)) } diff --git a/runtime/gdev/src/lib.rs b/runtime/gdev/src/lib.rs index bfd9a714a..fd34a3880 100644 --- a/runtime/gdev/src/lib.rs +++ b/runtime/gdev/src/lib.rs @@ -174,16 +174,9 @@ mod benches { } pub struct BaseCallFilter; - -// implement filter impl Contains<RuntimeCall> for BaseCallFilter { fn contains(call: &RuntimeCall) -> bool { - !matches!( - call, - // session calls can not be called directly - // it should be done through authority-members pallet - RuntimeCall::Session(_) - ) + !matches!(call, RuntimeCall::Session(_)) } } diff --git a/runtime/gdev/tests/fee_tests.rs b/runtime/gdev/tests/fee_tests.rs index 45983d7e9..14c7f15e3 100644 --- a/runtime/gdev/tests/fee_tests.rs +++ b/runtime/gdev/tests/fee_tests.rs @@ -22,7 +22,6 @@ use common::*; use frame_support::{assert_ok, pallet_prelude::DispatchClass}; use gdev_runtime::*; use sp_keyring::AccountKeyring; -use sp_runtime::Perquintill; /// This test checks that an almost empty block incurs no fees for an extrinsic. #[test] @@ -39,8 +38,14 @@ fn test_fees_empty() { value: 500, }); - let xt = - common::get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Alice, 0u64, 0); + let xt = common::get_unchecked_extrinsic( + call, + 4u64, + 8u64, + AccountKeyring::Alice, + 0u64, + 0u32, + ); assert_ok!(Executive::apply_extrinsic(xt)); // The block is almost empty, so the extrinsic should incur no fee assert_eq!( @@ -62,35 +67,25 @@ fn test_fees_weight() { ]) .build() .execute_with(|| { - let mut transactions = 0u64; let weights = BlockWeights::get(); let normal_max_weight = weights .get(DispatchClass::Normal) .max_total .unwrap_or(weights.max_block); - // Stopping just below the limit - while System::block_weight() - .get(DispatchClass::Normal) - .all_lt(Target::get() * normal_max_weight * Perbill::from_percent(99)) - { - let call = RuntimeCall::System(SystemCall::remark { - remark: vec![255u8; 1], - }); - let xt = get_unchecked_extrinsic( - call, - 4u64, - 8u64, - AccountKeyring::Alice, - 0u64, - transactions as u32, - ); - assert_ok!(Executive::apply_extrinsic(xt)); - transactions += 1; - } + + // Ensure that the next extrinsic is below the limit. + System::set_block_consumed_resources(Weight::zero(), 0_usize); + + let call = RuntimeCall::System(SystemCall::remark { + remark: vec![255u8; 1], + }); + let xt = get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Alice, 0u64, 0); + assert_ok!(Executive::apply_extrinsic(xt)); assert_eq!( Balances::free_balance(AccountKeyring::Alice.to_account_id()), 10_000 ); + // Ensure that the next extrinsic exceeds the limit. System::set_block_consumed_resources(Target::get() * normal_max_weight, 0_usize); // The block will reach the fee limit, so the next extrinsic should start incurring fees. @@ -98,14 +93,7 @@ fn test_fees_weight() { remark: vec![255u8; 1], }); - let xt = get_unchecked_extrinsic( - call, - 4u64, - 8u64, - AccountKeyring::Alice, - 0u64, - transactions as u32, - ); + let xt = get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Alice, 0u64, 1u32); assert_ok!(Executive::apply_extrinsic(xt)); assert_ne!( Balances::free_balance(AccountKeyring::Alice.to_account_id()), @@ -126,33 +114,34 @@ fn test_fees_length() { ]) .build() .execute_with(|| { - let mut transactions = 0u64; let length = BlockLength::get(); let normal_max_length = *length.max.get(DispatchClass::Normal) as u64; - // Stopping just below the limit - while u64::from(System::all_extrinsics_len()) - < (Target::get() * Perquintill::from_percent(99) * normal_max_length) - { - let call = RuntimeCall::System(SystemCall::remark { - remark: vec![255u8; 1_000], - }); - let xt = get_unchecked_extrinsic( - call, - 4u64, - 8u64, - AccountKeyring::Alice, - 0u64, - transactions as u32, - ); - assert_ok!(Executive::apply_extrinsic(xt)); - transactions += 1; - } + // Ensure that the next extrinsic is below the limit. + System::set_block_consumed_resources(Weight::zero(), 0usize); + let call = RuntimeCall::System(SystemCall::remark { + remark: vec![255u8; 100], + }); + let xt = get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Alice, 0u64, 0u32); + assert_ok!(Executive::apply_extrinsic(xt)); assert_eq!( Balances::free_balance(AccountKeyring::Alice.to_account_id()), 10_000 ); - // Ensure that the next extrinsic exceeds the limit. + + // Ensure that the next extrinsic exceeds the extrinsic limit. + System::set_block_consumed_resources(Weight::zero(), 0usize); + let call = RuntimeCall::System(SystemCall::remark { + remark: vec![0u8; 147], + }); + let xt = get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Alice, 0u64, 1u32); + assert_ok!(Executive::apply_extrinsic(xt)); + assert_ne!( + Balances::free_balance(AccountKeyring::Alice.to_account_id()), + 10_000 + ); + + // Ensure that the next extrinsic exceeds the block limit. System::set_block_consumed_resources( Weight::zero(), (Target::get() * normal_max_length).try_into().unwrap(), @@ -162,17 +151,10 @@ fn test_fees_length() { remark: vec![255u8; 1], }); - let xt = get_unchecked_extrinsic( - call, - 4u64, - 8u64, - AccountKeyring::Alice, - 0u64, - transactions as u32, - ); + let xt = get_unchecked_extrinsic(call, 4u64, 8u64, AccountKeyring::Eve, 0u64, 0u32); assert_ok!(Executive::apply_extrinsic(xt)); assert_ne!( - Balances::free_balance(AccountKeyring::Alice.to_account_id()), + Balances::free_balance(AccountKeyring::Eve.to_account_id()), 10_000 ); }) diff --git a/runtime/gtest/src/lib.rs b/runtime/gtest/src/lib.rs index 356b288ac..8d1972f55 100644 --- a/runtime/gtest/src/lib.rs +++ b/runtime/gtest/src/lib.rs @@ -172,9 +172,6 @@ mod benches { } pub struct BaseCallFilter; - -// implement filter -// session pallet calls are filtered in order to use authority member instead impl Contains<RuntimeCall> for BaseCallFilter { fn contains(call: &RuntimeCall) -> bool { !matches!(call, RuntimeCall::Session(_)) -- GitLab