Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Better Parameterisation for Fee system #3823

Merged
merged 9 commits into from
Oct 24, 2019
Merged

Better Parameterisation for Fee system #3823

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0a5e4db
Better fee parameters
kianenigma Oct 15, 2019
fd1867a
Fix build
kianenigma Oct 15, 2019
bd284fa
Master.into()
kianenigma Oct 17, 2019
734bfd8
Better runtime tests
kianenigma Oct 18, 2019
9fef4e5
Price to Weight ratio as type parameter (#3856)
JoshOrndorff Oct 22, 2019
d1d57d1
Fix tests.
kianenigma Oct 22, 2019
65fee2d
Master.into()
kianenigma Oct 22, 2019
1709e84
Remove todo
kianenigma Oct 22, 2019
3448f85
Fix build
kianenigma Oct 22, 2019
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
View file
Open in desktop

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

6 changes: 6 additions & 0 deletions core/sr-primitives/src/sr_arithmetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,12 @@ impl Fixed64 {
Self(int.saturating_mul(DIV))
}

/// Return the inner value. Only for testing.
#[cfg(any(feature = "std", test))]
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is only for testing, then why not ?

Suggested change
#[cfg(any(feature = "std", test))]
#[cfg(test)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point and I (naively) used the gating feature that I used here https://github.com/paritytech/substrate/pull/3823/files#diff-24bd3cac4cae1c620f82f7aa79ef28ceR707

Declaring the function with just #[cfg(test)] was not enough and it cannot be found in tests though 🤔 and I had to make it test or std. Do you know the reason of the difference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that logically it should be just cfg[test]. Something is wrong maybe in cargo files

Copy link
Contributor

Choose a reason for hiding this comment

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

oh actually this is because the test configure is only set for the tested crate.
if you test the runtime then sr-arithmetic isn't compiled with test.

I don't know what is the prefered way to solve this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(won't work as discussed on off-band. will leave the comment open)

pub fn into_inner(self) -> i64 {
self.0
}

/// Return the accuracy of the type. Given that this function returns the value `X`, it means
/// that an instance composed of `X` parts (`Fixed64::from_parts(X)`) is equal to `1`.
pub fn accuracy() -> i64 {
Expand Down
4 changes: 1 addition & 3 deletions node/executor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ mod tests {
use node_runtime::{
Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances, BuildStorage,
System, TransactionPayment, Event, TransferFee, TransactionBaseFee, TransactionByteFee,
constants::currency::*, impls::WeightToFee,
WeightToFee, constants::currency::*,
};
use node_primitives::{Balance, Hash, BlockNumber};
use node_testing::keyring::*;
Expand Down Expand Up @@ -480,8 +480,6 @@ mod tests {
).0.unwrap();

t.execute_with(|| {
// NOTE: fees differ slightly in tests that execute more than one block due to the
// weight update. Hence, using `assert_eq_error_rate`.
assert_eq!(
Balances::total_balance(&alice()),
alice_last_known_balance - 10 * DOLLARS - transfer_fee(&xt(), fm),
Expand Down
3 changes: 3 additions & 0 deletions node/runtime/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ transaction-payment = { package = "srml-transaction-payment", path = "../../srml
[build-dependencies]
wasm-builder-runner = { package = "substrate-wasm-builder-runner", version = "1.0.2", path = "../../core/utils/wasm-builder-runner" }

[dev-dependencies]
runtime_io = { package = "sr-io", path = "../../core/sr-io" }

[features]
default = ["std"]
std = [
Expand Down
16 changes: 0 additions & 16 deletions node/runtime/src/constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,3 @@ pub mod time {
pub const HOURS: BlockNumber = MINUTES * 60;
pub const DAYS: BlockNumber = HOURS * 24;
}

// CRITICAL NOTE: The system module maintains two constants: a _maximum_ block weight and a _ratio_
// of it yielding the portion which is accessible to normal transactions (reserving the rest for
// operational ones). `TARGET_BLOCK_FULLNESS` is entirely independent and the system module is not
// aware of if, nor should it care about it. This constant simply denotes on which ratio of the
// _maximum_ block weight we tweak the fees. It does NOT care about the type of the dispatch.
//
// For the system to be configured in a sane way, `TARGET_BLOCK_FULLNESS` should always be less than
// the ratio that `system` module uses to find normal transaction quota.
/// Fee-related.
pub mod fee {
pub use sr_primitives::Perbill;

/// The block saturation level. Fees will be updates based on this value.
pub const TARGET_BLOCK_FULLNESS: Perbill = Perbill::from_percent(25);
}
Loading