Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Increase target block fullness to 50% #2692

Merged
merged 3 commits into from
Mar 4, 2024
Merged

Increase target block fullness to 50% #2692

merged 3 commits into from
Mar 4, 2024

Conversation

noandrea
Copy link
Collaborator

@noandrea noandrea commented Mar 4, 2024

What does it do?

Increases the target block fullness to 50%.
It impacts the fee increase mechanism, the base fee will increase only when the target block fullness is greater than or equal to the target block fullness

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

On high network usage, the transaction fees should increase at a slower pace

@noandrea noandrea added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Mar 4, 2024
@librelois librelois mentioned this pull request Mar 4, 2024
Closed
21 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 4, 2024 •
edited
Loading

Coverage Report

@@                     Coverage Diff                     @@
##           master   noandrea-block-fullness      +/-   ##
===========================================================
- Coverage   80.44%                    73.45%   -6.99%     
- Files         299                       228      -71     
- Lines       91688                     70294   -21394     
===========================================================
- Hits        73751                     51631   -22120     
+ Misses      17937                     18663     +726
Files Changed Coverage
/client/rpc/finality/src/lib.rs 0.00% (-100.00%) 🔽
/client/rpc/manual-xcm/src/lib.rs 0.00% (-84.62%) 🔽
/client/rpc/txpool/src/lib.rs 0.00% (-65.38%) 🔽
/client/rpc-core/txpool/src/lib.rs 0.00% (-100.00%) 🔽
/client/rpc-core/txpool/src/types/content.rs 0.00% (-70.37%) 🔽
/client/rpc-core/txpool/src/types/inspect.rs 0.00% (-88.89%) 🔽
/client/rpc-core/txpool/src/types/mod.rs 0.00% (-100.00%) 🔽
/client/vrf/src/lib.rs 0.00% (-96.00%) 🔽
/node/cli-opt/src/account_key.rs 31.15% (-1.64%) 🔽
/node/cli-opt/src/lib.rs 0.00% (-48.00%) 🔽
/pallets/asset-manager/src/lib.rs 76.26% (-7.64%) 🔽
/pallets/asset-manager/src/mock.rs 97.90% (+4.09%) 🔼
/pallets/asset-manager/src/tests.rs 99.79% (-0.21%) 🔽
/pallets/erc20-xcm-bridge/src/erc20_matcher.rs 92.77% (-4.88%) 🔽
/pallets/erc20-xcm-bridge/src/lib.rs 8.90% (-3.26%) 🔽
/pallets/erc20-xcm-bridge/src/mock.rs 0.96% (-3.55%) 🔽
/pallets/erc20-xcm-bridge/src/xcm_holding_ext.rs 81.20% (-13.67%) 🔽
/pallets/ethereum-xcm/src/lib.rs 82.09% (-2.69%) 🔽
/pallets/ethereum-xcm/src/mock.rs 32.29% (-6.88%) 🔽
/pallets/ethereum-xcm/src/tests/v1/eip1559.rs 99.41% (-0.01%) 🔽
/pallets/ethereum-xcm/src/tests/v1/eip2930.rs 99.43% (-0.01%) 🔽
/pallets/moonbeam-lazy-migrations/src/lib.rs 87.50% (-1.69%) 🔽
/pallets/moonbeam-lazy-migrations/src/mock.rs 4.88% (-10.83%) 🔽
/pallets/moonbeam-orbiters/src/lib.rs 68.95% (-5.55%) 🔽
/pallets/moonbeam-orbiters/src/mock.rs 99.19% (+1.98%) 🔼
/pallets/moonbeam-orbiters/src/types.rs 65.26% (+0.61%) 🔼
/pallets/moonbeam-xcm-benchmarks/src/weights/fungible.rs 0.00% (-24.24%) 🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/generic.rs 0.00% (-2.96%) 🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/mod.rs 0.00% (-13.11%) 🔽
/pallets/parachain-staking/src/auto_compound.rs 95.64% (-0.03%) 🔽
/pallets/parachain-staking/src/delegation_requests.rs 91.92% (-0.07%) 🔽
/pallets/parachain-staking/src/inflation.rs 85.98% (-1.59%) 🔽
/pallets/parachain-staking/src/lib.rs 90.24% (-0.53%) 🔽
/pallets/parachain-staking/src/mock.rs 99.28% (-0.14%) 🔽
/pallets/parachain-staking/src/set.rs 40.00% (-0.74%) 🔽
/pallets/parachain-staking/src/tests.rs 91.57% (-0.05%) 🔽
/pallets/parachain-staking/src/types.rs 77.35% (-0.60%) 🔽
/pallets/precompile-benchmarks/src/lib.rs 10.00% (-17.27%) 🔽
/pallets/proxy-genesis-companion/src/lib.rs 57.14% (-24.68%) 🔽
/pallets/proxy-genesis-companion/src/mock.rs 7.98% (-19.70%) 🔽
/pallets/xcm-transactor/src/encode.rs 90.32% (-9.68%) 🔽
/pallets/xcm-transactor/src/lib.rs 85.64% (-2.84%) 🔽
/pallets/xcm-transactor/src/mock.rs 95.04% (+2.96%) 🔼
/precompiles/assets-erc20/src/mock.rs 9.95% (-58.90%) 🔽
/precompiles/author-mapping/src/mock.rs 5.71% (-12.57%) 🔽
/precompiles/balances-erc20/src/mock.rs 99.60% (+2.27%) 🔼
/precompiles/batch/src/mock.rs 99.68% (+0.80%) 🔼
/precompiles/call-permit/src/lib.rs 97.83% (-0.01%) 🔽
/precompiles/call-permit/src/mock.rs 6.00% (-17.27%) 🔽
/precompiles/collective/src/mock.rs 97.19% (+2.36%) 🔼
/precompiles/conviction-voting/src/lib.rs 93.41% (+0.01%) 🔼
/precompiles/conviction-voting/src/mock.rs 97.05% (+3.72%) 🔼
/precompiles/crowdloan-rewards/src/mock.rs 99.17% (+1.10%) 🔼
/precompiles/crowdloan-rewards/src/tests.rs 98.47% (-0.01%) 🔽
/precompiles/gmp/src/mock.rs 5.62% (-2.73%) 🔽
/precompiles/identity/src/mock.rs 99.02% (+1.93%) 🔼
/precompiles/parachain-staking/src/mock.rs 98.85% (+1.03%) 🔼
/precompiles/precompile-registry/src/mock.rs 6.16% (-18.36%) 🔽
/precompiles/preimage/src/mock.rs 99.35% (+1.72%) 🔼
/precompiles/proxy/src/mock.rs 97.86% (+3.72%) 🔼
/precompiles/proxy/src/tests.rs 96.79% (-0.02%) 🔽
/precompiles/randomness/src/mock.rs 98.38% (+2.90%) 🔼
/precompiles/referenda/src/mock.rs 98.52% (+1.00%) 🔼
/precompiles/relay-data-verifier/src/mock.rs 9.68% (-13.46%) 🔽
/precompiles/relay-encoder/src/mock.rs 5.25% (-3.69%) 🔽
/precompiles/utils/src/precompile_set.rs 61.06% (-16.21%) 🔽
/precompiles/utils/src/solidity/codec/xcm.rs 67.95% (-0.14%) 🔽
/precompiles/utils/src/testing/modifier.rs 87.67% (-0.64%) 🔽
/precompiles/utils/tests-external/lib.rs 33.22% (-15.93%) 🔽
/precompiles/xcm-transactor/src/mock.rs 95.51% (+2.50%) 🔼
/precompiles/xcm-utils/src/lib.rs 88.52% (-0.82%) 🔽
/precompiles/xcm-utils/src/mock.rs 96.84% (+1.17%) 🔼
/precompiles/xtokens/src/mock.rs 16.25% (-7.47%) 🔽
/precompiles/xtokens/src/tests.rs 99.52% (-0.01%) 🔽
/primitives/account/src/lib.rs 64.23% (+0.52%) 🔼
/primitives/ext/src/lib.rs 44.83% (-3.45%) 🔽
/primitives/rpc/debug/src/lib.rs 0.00% (-84.85%) 🔽
/primitives/rpc/txpool/src/lib.rs 0.00% (-83.33%) 🔽
/primitives/xcm/src/asset_id_conversions.rs 0.00% (-66.67%) 🔽
/primitives/xcm/src/ethereum_xcm.rs 93.07% (-1.80%) 🔽
/primitives/xcm/src/fee_handlers.rs 95.57% (-2.85%) 🔽
/primitives/xcm/src/origin_conversion.rs 0.00% (-74.07%) 🔽
/runtime/common/src/migrations.rs 0.00% (-90.00%) 🔽
/runtime/common/src/weights/pallet_assets.rs 0.00% (-8.98%) 🔽
/runtime/common/src/weights/pallet_author_mapping.rs 0.00% (-80.00%) 🔽
/runtime/common/src/weights/pallet_balances.rs 0.00% (-12.05%) 🔽
/runtime/common/src/weights/pallet_collective.rs 0.00% (-11.25%) 🔽
/runtime/common/src/weights/pallet_crowdloan_rewards.rs 0.00% (-66.18%) 🔽
/runtime/common/src/weights/pallet_moonbeam_orbiters.rs 0.00% (-9.26%) 🔽
/runtime/common/src/weights/pallet_parachain_staking.rs 0.00% (-19.83%) 🔽
/runtime/common/src/weights/pallet_utility.rs 0.00% (-44.00%) 🔽
/runtime/common/src/weights/pallet_xcm_transactor.rs 0.00% (-11.90%) 🔽
/runtime/evm_tracer/src/lib.rs 0.00% (-65.96%) 🔽

Coverage generated Mon Mar 4 16:16:05 UTC 2024

@noandrea noandrea marked this pull request as ready for review March 4, 2024 15:43
elfedy
elfedy reviewed Mar 4, 2024
View reviewed changes
Copy link
Contributor

@elfedy elfedy left a comment

Choose a reason for hiding this comment

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

Added some comments/questions about hardcoding values in test. Not sure we want to do this or the safer version of having a storage value just in case

test/suites/dev/moonbase/test-fees/test-fee-multiplier-max.ts Show resolved Hide resolved
test/suites/dev/moonbase/test-fees/test-fee-multiplier-max.ts
@@ -171,7 +171,7 @@ describeSuite({
expect(withdrawEvents?.length).to.equal(1);
const withdrawEvent = withdrawEvents![0];
const amount = withdrawEvent.event.data.amount.toBigInt();
expect(amount).to.equal(11_880_382_026_331_795_584n);
expect(amount).to.equal(11_868_507_595_257_710_784n);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does this hardcoded value come from?

runtime/moonbase/tests/integration_test.rs
@@ -2932,37 +2932,37 @@ mod fee_tests {

assert_eq!(
sim(1_000_000_000, Perbill::from_percent(0), 1),
U256::from(999_000_500),
U256::from(998_002_000),
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do these hardcoded values come from? If we use a formula maybe compute expected value with the formula?

RomarQ
RomarQ approved these changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

LGTM

@noandrea noandrea merged commit 09aa6d8 into master Mar 4, 2024
27 checks passed
@noandrea noandrea deleted the noandrea-block-fullness branch March 4, 2024 21:50
noandrea added a commit that referenced this pull request Mar 4, 2024
@noandrea
Increase target block fullness to 50% (#2692)
1176bce 
* increase target block fullness to 50%
* fix tests
* change the target block fullness for moonbeam and moonriver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@librelois librelois librelois approved these changes

@ahmadkaouk ahmadkaouk ahmadkaouk approved these changes

@RomarQ RomarQ RomarQ approved these changes

@elfedy elfedy elfedy approved these changes

Assignees
No one assigned
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@noandrea @elfedy @RomarQ @librelois @ahmadkaouk