From 1dc97de2e220154f38f49ad8d89d3a44fcb8e783 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 6 Dec 2021 20:41:38 +1300 Subject: [PATCH 1/5] allow tip value of 1 --- frame/transaction-payment/src/lib.rs | 42 ++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index ed96f26cc0079..3672017ca841d 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -53,7 +53,7 @@ use scale_info::TypeInfo; use sp_runtime::{ traits::{ Convert, DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SaturatedConversion, Saturating, - SignedExtension, Zero, + SignedExtension, Zero, One, }, transaction_validity::{ TransactionPriority, TransactionValidity, TransactionValidityError, ValidTransaction, @@ -651,7 +651,7 @@ where // To distribute no-tip transactions a little bit, we set the minimal tip as `1`. // This means that given two transactions without a tip, smaller one will be preferred. - let tip = tip.max(1.saturated_into()); + let tip = tip.saturating_add(One::one()); let scaled_tip = max_reward(tip); match info.class { @@ -1480,14 +1480,14 @@ mod tests { .unwrap() .priority; - assert_eq!(priority, 50); + assert_eq!(priority, 60); let priority = ChargeTransactionPayment::(2 * tip) .validate(&2, CALL, &normal, len) .unwrap() .priority; - assert_eq!(priority, 100); + assert_eq!(priority, 110); }); ExtBuilder::default().balance_factor(100).build().execute_with(|| { @@ -1500,13 +1500,13 @@ mod tests { .validate(&2, CALL, &op, len) .unwrap() .priority; - assert_eq!(priority, 5800); + assert_eq!(priority, 5810); let priority = ChargeTransactionPayment::(2 * tip) .validate(&2, CALL, &op, len) .unwrap() .priority; - assert_eq!(priority, 6100); + assert_eq!(priority, 6110); }); } @@ -1540,6 +1540,36 @@ mod tests { }); } + #[test] + fn one_tip_has_more_priority() { + let tip = 1; + let len = 10; + + ExtBuilder::default().balance_factor(100).build().execute_with(|| { + let normal = + DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes }; + let priority = ChargeTransactionPayment::(tip) + .validate(&2, CALL, &normal, len) + .unwrap() + .priority; + + assert_eq!(priority, 20); + }); + + ExtBuilder::default().balance_factor(100).build().execute_with(|| { + let op = DispatchInfo { + weight: 100, + class: DispatchClass::Operational, + pays_fee: Pays::Yes, + }; + let priority = ChargeTransactionPayment::(tip) + .validate(&2, CALL, &op, len) + .unwrap() + .priority; + assert_eq!(priority, 5570); + }); + } + #[test] fn post_info_can_change_pays_fee() { ExtBuilder::default() From f7ef63122f548ad52393458abbab32ab5982ee22 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 6 Dec 2021 20:43:00 +1300 Subject: [PATCH 2/5] update comment --- frame/transaction-payment/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 3672017ca841d..a147e21ebc8e0 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -649,7 +649,7 @@ where .saturated_into::>(); let max_reward = |val: BalanceOf| val.saturating_mul(max_tx_per_block); - // To distribute no-tip transactions a little bit, we set the minimal tip as `1`. + // To distribute no-tip transactions a little bit, we increase the tip value by one. // This means that given two transactions without a tip, smaller one will be preferred. let tip = tip.saturating_add(One::one()); let scaled_tip = max_reward(tip); From be3809c61a76d3d4910457155101ac6a316bbda8 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 6 Dec 2021 20:46:40 +1300 Subject: [PATCH 3/5] fmt --- frame/transaction-payment/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index a147e21ebc8e0..38331f84147e3 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -52,8 +52,8 @@ use scale_info::TypeInfo; use sp_runtime::{ traits::{ - Convert, DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SaturatedConversion, Saturating, - SignedExtension, Zero, One, + Convert, DispatchInfoOf, Dispatchable, One, PostDispatchInfoOf, SaturatedConversion, + Saturating, SignedExtension, Zero, }, transaction_validity::{ TransactionPriority, TransactionValidity, TransactionValidityError, ValidTransaction, From d973bbf6d0881418cabe9782f329e5f9eb9d5de0 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 6 Dec 2021 21:38:25 +1300 Subject: [PATCH 4/5] update test --- frame/transaction-payment/src/lib.rs | 59 +++++++++++++++++----------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 38331f84147e3..ee42c74e14dea 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -1541,33 +1541,44 @@ mod tests { } #[test] - fn one_tip_has_more_priority() { - let tip = 1; - let len = 10; + fn higher_tip_have_higher_priority() { + let get_priorities = |tip: u64| { + let mut priority1 = 0; + let mut priority2 = 0; + let len = 10; + ExtBuilder::default().balance_factor(100).build().execute_with(|| { + let normal = + DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes }; + priority1 = ChargeTransactionPayment::(tip) + .validate(&2, CALL, &normal, len) + .unwrap() + .priority; + }); - ExtBuilder::default().balance_factor(100).build().execute_with(|| { - let normal = - DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes }; - let priority = ChargeTransactionPayment::(tip) - .validate(&2, CALL, &normal, len) - .unwrap() - .priority; + ExtBuilder::default().balance_factor(100).build().execute_with(|| { + let op = DispatchInfo { + weight: 100, + class: DispatchClass::Operational, + pays_fee: Pays::Yes, + }; + priority2 = ChargeTransactionPayment::(tip) + .validate(&2, CALL, &op, len) + .unwrap() + .priority; + }); - assert_eq!(priority, 20); - }); + (priority1, priority2) + }; - ExtBuilder::default().balance_factor(100).build().execute_with(|| { - let op = DispatchInfo { - weight: 100, - class: DispatchClass::Operational, - pays_fee: Pays::Yes, - }; - let priority = ChargeTransactionPayment::(tip) - .validate(&2, CALL, &op, len) - .unwrap() - .priority; - assert_eq!(priority, 5570); - }); + + let mut prev_priorities = get_priorities(0); + + for tip in 1..3 { + let priorities = get_priorities(tip); + assert!(prev_priorities.0 < priorities.0); + assert!(prev_priorities.1 < priorities.1); + prev_priorities = priorities; + } } #[test] From 01525c10db867a93ce0f340ab9cb2b7c948ea1ec Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 6 Dec 2021 21:39:18 +1300 Subject: [PATCH 5/5] fmt --- frame/transaction-payment/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index ee42c74e14dea..02ba9621c175d 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -1570,7 +1570,6 @@ mod tests { (priority1, priority2) }; - let mut prev_priorities = get_priorities(0); for tip in 1..3 {