From 534b43d38961f6491e5c06be90d559b2483837b9 Mon Sep 17 00:00:00 2001 From: gabriele-0201 Date: Thu, 25 Jan 2024 10:24:55 +0100 Subject: [PATCH] fees: add MaximumSkippedBlocks constant Limit the number of skipped blocks in the fee update mechanism to the expected maximum. This is done to avoid the possibility of incorrectly updating the fee multiplier. For example, if the chain halts and skips more than 10000 blocks, the approximation given by the first n terms of e^x could diverge significantly from the actual value. Therefore, we set a boundary on the number of skipped blocks to ensure the accuracy of the approximation --- Cargo.lock | 1 + .../pallets/length-fee-adjustment/Cargo.toml | 3 +- .../pallets/length-fee-adjustment/src/lib.rs | 18 ++++++--- .../pallets/length-fee-adjustment/src/mock.rs | 2 + .../length-fee-adjustment/src/tests.rs | 40 ++++++++++++++++--- .../chain/runtimes/sugondat-kusama/src/lib.rs | 4 ++ sugondat/chain/runtimes/test/src/lib.rs | 4 ++ 7 files changed, 60 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 93005496..8628c7da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7309,6 +7309,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-weights", + "sugondat-primitives", ] [[package]] diff --git a/sugondat/chain/pallets/length-fee-adjustment/Cargo.toml b/sugondat/chain/pallets/length-fee-adjustment/Cargo.toml index acb0c20d..0047ebd5 100644 --- a/sugondat/chain/pallets/length-fee-adjustment/Cargo.toml +++ b/sugondat/chain/pallets/length-fee-adjustment/Cargo.toml @@ -29,6 +29,7 @@ polkadot-primitives = { workspace = true } [dev-dependencies] sp-io = { workspace = true } sp-core = { workspace = true } +sugondat-primitives = { workspace = true, features = ["std"] } [features] default = [ "std" ] @@ -53,4 +54,4 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", "dep:sp-io" ] -try-runtime = [ "frame-support/try-runtime" ] \ No newline at end of file +try-runtime = [ "frame-support/try-runtime" ] diff --git a/sugondat/chain/pallets/length-fee-adjustment/src/lib.rs b/sugondat/chain/pallets/length-fee-adjustment/src/lib.rs index 0a1f86a5..afada350 100644 --- a/sugondat/chain/pallets/length-fee-adjustment/src/lib.rs +++ b/sugondat/chain/pallets/length-fee-adjustment/src/lib.rs @@ -99,13 +99,18 @@ pub mod pallet { /// /// Here, m represents the number of terms needed to be below the error. /// The smallest value of m that satisfies the inequality should be used as the value for this parameter. - /// - /// The maximum number of skipped blocks is used to ensure that only the right amount of terms is used, - /// and no terms are wasted. If n is exceeded, the approximation could diverge significantly from the actual value of e^x. - /// This means that producing a block at most every n skipped blocks should be enforced to avoid falling into this error. #[pallet::constant] type SkippedBlocksNumberTerms: Get; + /// The maximum number of skipped blocks is used to ensure that only the right amount of terms is used + /// and no terms are wasted. If n is exceeded, the approximation could diverge significantly from the actual value of e^x. + /// If the number of skipped blocks exceeds this value, it will be bounded to this value. + /// However, this limitation comes at the cost of losing the real fee adjustment update. + /// Therefore, producing a block at most every n skipped blocks should be enforced to avoid falling into this circumstance. + #[pallet::constant] + type MaximumSkippedBlocks: Get; + + /// A source to provide the relay-parent number of the previous block. type LastRelayBlockNumberProvider: LastRelayBlockNumberProvider; } @@ -243,10 +248,11 @@ pub mod pallet { // However, saturating it to zero will prevent the multiplier from changing let relay_parent_distance = relay_block_number.saturating_sub(prev_relay_block_number); - // TODO: update with `n_skipped_blocks = relay_parent_distance.saturating_sub(1)` + // TODO: update with `relay_parent_distance.saturating_sub(1)` // when updating to asynchronous backing // https://github.com/thrumdev/blobs/issues/166 - let n_skipped_blocks = relay_parent_distance.saturating_sub(2) / 2; + let n_skipped_blocks = + (relay_parent_distance.saturating_sub(2) / 2).min(T::MaximumSkippedBlocks::get()); let n_skipped_blocks = Multiplier::saturating_from_integer(n_skipped_blocks); let target_block_size = Multiplier::from(TargetBlockSize::::get()); diff --git a/sugondat/chain/pallets/length-fee-adjustment/src/mock.rs b/sugondat/chain/pallets/length-fee-adjustment/src/mock.rs index 7ac37263..d6229c75 100644 --- a/sugondat/chain/pallets/length-fee-adjustment/src/mock.rs +++ b/sugondat/chain/pallets/length-fee-adjustment/src/mock.rs @@ -87,6 +87,7 @@ parameter_types! { // https://github.com/thrumdev/blobs/issues/166 // Accepted error is less than 10^(-2) pub SkippedBlocksNumberTerms: u32 = 3; + pub MaximumSkippedBlocks: u32 = sugondat_primitives::MAX_SKIPPED_BLOCKS; pub static WeightToFee: u64 = 1; pub static OperationalFeeMultiplier: u8 = 5; @@ -130,5 +131,6 @@ impl pallet_sugondat_length_fee_adjustment::Config for Test { type MinimumMultiplierBlockSize = MinimumMultiplierBlockSize; type MaximumMultiplierBlockSize = MaximumMultiplierBlockSize; type SkippedBlocksNumberTerms = SkippedBlocksNumberTerms; + type MaximumSkippedBlocks = MaximumSkippedBlocks; type LastRelayBlockNumberProvider = MockLastRelayBlockNumberProvider; } diff --git a/sugondat/chain/pallets/length-fee-adjustment/src/tests.rs b/sugondat/chain/pallets/length-fee-adjustment/src/tests.rs index ec362704..034fc5e5 100644 --- a/sugondat/chain/pallets/length-fee-adjustment/src/tests.rs +++ b/sugondat/chain/pallets/length-fee-adjustment/src/tests.rs @@ -89,12 +89,9 @@ fn test_no_update_when_prev_is_zero() { #[test] fn test_skipped_block_multiplier_update() { - // TODO: change max_skipped_blocks to 7200 - // when updating to asynchronous backing - // https://github.com/thrumdev/blobs/issues/166 new_test_ext().execute_with(|| { - let max_skipped_blocks = 3600; - for d in (0..max_skipped_blocks).step_by(max_skipped_blocks as usize / 1000) { + let max_skipped_blocks = ::MaximumSkippedBlocks::get(); + for d in (0..max_skipped_blocks).step_by(max_skipped_blocks as usize / 100) { // using Multiplier::one() only e^(-vnt) is tested NextLengthMultiplier::::put(Multiplier::one()); mock::set_last_relay_block_number(1); @@ -124,6 +121,39 @@ fn test_skipped_block_multiplier_update() { }); } +#[test] +fn test_max_skipped_block_exceeded() { + new_test_ext().execute_with(|| { + NextLengthMultiplier::::put(Multiplier::one()); + mock::set_last_relay_block_number(1); + + let max_skipped_blocks = ::MaximumSkippedBlocks::get(); + let relay_data = PersistedValidationData { + parent_head: HeadData(vec![]), + // The previous relay parent was 10 times greater than the expected MaximumSkippedBlocks. + // If the multiplier is updated with that number of skipped blocks + // there's should be a significant divergence in the final result. + // However, we expect it to be bounded by max_skipped_blocks. + relay_parent_number: 1 + 2 + ((max_skipped_blocks * 10) * 2), + relay_parent_storage_root: sp_core::H256::zero(), + max_pov_size: 0, + }; + + LengthFeeAdjustment::on_validation_data(&relay_data); + + let mul = NextLengthMultiplier::::get(); + + // calculate expected result using f64::exp and assert on the error rate + let target = Multiplier::from(TargetBlockSize::::get()).to_float(); + let v = ::AdjustmentVariableBlockSize::get().to_float(); + let expected_mul = + Multiplier::from_float((-1.0 * target * v * max_skipped_blocks as f64).exp()); + + //Accepted error is less than 10^(-2) + assert_eq_error_rate!(mul, expected_mul, Multiplier::from_inner(10000000000000000)); + }); +} + #[test] fn test_skipped_block_no_prev_data() { new_test_ext().execute_with(|| { diff --git a/sugondat/chain/runtimes/sugondat-kusama/src/lib.rs b/sugondat/chain/runtimes/sugondat-kusama/src/lib.rs index 681565ee..936a2e1d 100644 --- a/sugondat/chain/runtimes/sugondat-kusama/src/lib.rs +++ b/sugondat/chain/runtimes/sugondat-kusama/src/lib.rs @@ -303,6 +303,9 @@ parameter_types! { // The accepted error is less than 10^(-2) for an expected // maximum of 3600 skipped blocks (half a day) pub SkippedBlocksNumberTerms: u32 = 3; + + // Maximum acceptable number of skipped parachain blocks. + pub MaximumSkippedBlocks: u32 = sugondat_primitives::MAX_SKIPPED_BLOCKS; } impl pallet_sugondat_length_fee_adjustment::Config for Runtime { @@ -312,6 +315,7 @@ impl pallet_sugondat_length_fee_adjustment::Config for Runtime { type MaximumMultiplierBlockSize = MaximumMultiplierBlockSize; type MinimumMultiplierBlockSize = MinimumMultiplierBlockSize; type SkippedBlocksNumberTerms = SkippedBlocksNumberTerms; + type MaximumSkippedBlocks = MaximumSkippedBlocks; type LastRelayBlockNumberProvider = Runtime; } diff --git a/sugondat/chain/runtimes/test/src/lib.rs b/sugondat/chain/runtimes/test/src/lib.rs index d088cba4..a282a5a1 100644 --- a/sugondat/chain/runtimes/test/src/lib.rs +++ b/sugondat/chain/runtimes/test/src/lib.rs @@ -370,6 +370,9 @@ parameter_types! { // The accepted error is less than 10^(-2) for an expected // maximum of 3600 skipped blocks (half a day) pub SkippedBlocksNumberTerms: u32 = 3; + + // Maximum acceptable number of skipped parachain blocks. + pub MaximumSkippedBlocks: u32 = sugondat_primitives::MAX_SKIPPED_BLOCKS; } impl pallet_sugondat_length_fee_adjustment::Config for Runtime { @@ -379,6 +382,7 @@ impl pallet_sugondat_length_fee_adjustment::Config for Runtime { type MaximumMultiplierBlockSize = MaximumMultiplierBlockSize; type MinimumMultiplierBlockSize = MinimumMultiplierBlockSize; type SkippedBlocksNumberTerms = SkippedBlocksNumberTerms; + type MaximumSkippedBlocks = MaximumSkippedBlocks; type LastRelayBlockNumberProvider = Runtime; }