-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
It looks like @4meta5 signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
1 similar comment
It looks like @4meta5 signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
Ok just had an idea of how it can be (easily) more user-friendly:
pub trait Trait<I: Instance = DefaultInstance>: system::Trait {
/// The balance of an account.
type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy +
MaybeSerializeDebug + From<Self::BlockNumber>;
+ type WeightToFee: Convert<Weight, Self::Balance>;
....
}
fn make_payment(transactor: &T::AccountId, weight: Weight) -> Result {
+ let transaction_fee = T::WeightToFee::convert(weight);
let imbalance = Self::withdraw(
transactor,
transaction_fee.into(),
WithdrawReason::TransactionPayment,
ExistenceRequirement::KeepAlive
)?;
T::TransactionPayment::on_unbalanced(imbalance);
Ok(())
}
impl balances::Trait for Runtime {
type Balance = Balance;
+ type WeightToFee = SomeStructThatImplementsIt;
type OnFreeBalanceZero = ((Staking, Contract), Session);
type OnNewAccount = Indices;
type Event = Event;
type TransactionPayment = ();
type DustRemoval = ();
type TransferPayment = ();
} cc @bkchr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't see that stuff in srml/executive. That needs to be sorted before this can be merged.
srml/executive/src/mock.rs
Outdated
use system; | ||
pub use balances::Call as balancesCall; | ||
pub use system::Call as systemCall; | ||
use node_runtime::impls::WeightMultiplierUpdateHandler; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid. The point of this is to test executive. The test runtime exists as a minimal runtime designed to facilitate that in a controlled fashion. This should not be testing the fully-featured substrate-node runtime, nor should it be pulling in anything from that set of crates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will take this into account but do note that this PR will probably have to be closed and a portion of it need to be ported to into your PR as a signed extension.
…ate into amar/tx-fee-multiplier
@gavofyork done + brought back the original substrate base/byte fee stuff. Should be okay to be merged and used with your PR. Still contains some historical |
core/sr-primitives/src/weights.rs
Outdated
/// | ||
/// This will perform a saturated `weight + weight * self.0`. | ||
pub fn apply_to(&self, weight: Weight) -> Weight { | ||
let inner = self.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we really shouldn't be mucking around with the inner member of a Fixed64
in here and depending on DIV
being public API (it shouldn't be). operations should really be defined purely in terms of a proper public API of Fixed64
.
/// Apply the inner Fixed64 as a weight multiplier to a weight value. | ||
/// | ||
/// This will perform a saturated `weight + weight * self.0`. | ||
pub fn apply_to(&self, weight: Weight) -> Weight { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should just be called multiply_accumulate
and moved into Fixed64
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit of code shifting needed to clean up FeeMutiplier
but looks good otherwise.
/// | ||
/// Note that this might be lossy. | ||
pub fn from_rational(n: i64, d: u64) -> Self { | ||
Self((n as i128 * DIV as i128 / (d as i128).max(1)).try_into().unwrap_or(Bounded::max_value())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't checked_div be used here to avoid divide by zero issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.max(0)
Implementation of the slow fee mechanism -- required some changes after merging #2799
TODO:
make_payment
insrml/balances
cc @kianenigma