Extract Storage Read costs for dev tests involved in #2786 #2915

pLabarta · 2024-08-26T13:58:02Z

What does it do?

This PR extracts two moonbase constants used in several dev-tests involved in PR #2786 in order to ease future changes. Constants extracted are now included in helpers/constants.ts and exposed via the ConstantStore fn:

MOONBASE: {
    ...
    // Storage read/write costs
    STORAGE_READ_COST: 41_742_000n,
    STORAGE_READ_GAS_COST: 1669n,
}

Only tests changed in #2786 were updated to use the new constants.

What important points reviewers should know?

Constants were extracted by identifying which tests had changes in #2786 to expected values related to either 41_742_000n or 1669n, which reference Storage Read weight and gas cost. Expected values had the Storage costs deducted and then added using the new constants.

Tests which already used named variables for the Storage read costs only had the local variable removed in favor of importing it through the ConstantStore.

One of the more obscure extractions is in test/suites/dev/moonbase/test-precompile/test-precompile-batch.ts. The used gas changes in #2786 match the changes from 1000 to 1669 reflected in rust-tests. This led to the extraction of the Storage Read gas cost into a new constant.

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?

github-actions · 2024-08-26T14:39:43Z

Coverage Report

@@                       Coverage Diff                       @@
##           master   pablo/extract-test-constants     +/-   ##
===============================================================
  Coverage   80.93%                         80.93%   0.00%     
  Files         284                            284             
  Lines       81724                          81724             
===============================================================
  Hits        66136                          66136             
  Misses      15588                          15588

Files Changed	Coverage

Coverage generated Wed Aug 28 21:06:29 UTC 2024

RomarQ · 2024-08-26T20:35:48Z

test/helpers/constants.ts

@@ -77,6 +77,9 @@ export const RUNTIME_CONSTANTS = {
    EXTRINSIC_GAS_LIMIT: 52_000_000n,
    // Maximum Gas to PoV ratio used in the gasometer
    GAS_PER_POV_BYTES: 16n,
+    // Storage read/write costs
+    STORAGE_READ_COST: 41_742_000n,
+    STORAGE_READ_GAS_COST: 1669n,


I am not sure about this value. From where did you extract it?

It is mainly based on the changes to the rust tests on #2786, which I might be misreading. There, several precompiles tests changed their estimated gas cost from 1000 to 1669 and other worked with doubled values.

Will do some more research on them to see if we can keep the extracted constant (even if the value needs to change), but we could remove it for the purpose of this PR and only have 1 test affected; not that impactful.

I think we should only keep STORAGE_READ_COST for now since it is a known constant.

Giving it a another look, the 1669 updates present in #2786 refer exactly to STORAGE_READ_COST / WEIGHT_TO_GAS_RATIO (47_000_000 / 25_000). This confirms it is the DB read cost. I'll update so we can keep STORAGE_READ_GAS_COST and use it in a way that it is relative to the STORAGE_READ_COST.

github-actions · 2024-08-28T12:59:58Z

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2168 KB (no changes) ✅

Moonbeam runtime: 2116 KB (no changes) ✅

Moonriver runtime: 2116 KB (no changes) ✅

Compared to latest release (runtime-3100)

Moonbase runtime: 2168 KB (+152 KB compared to latest release) ⚠️

Moonbeam runtime: 2116 KB (+140 KB compared to latest release) ⚠️

Moonriver runtime: 2116 KB (+148 KB compared to latest release) ⚠️

* add: storage read cost constant * update test-pov * update xcm-v3 tests * update xcm-v4 tests * add storage read gas cost to constants * update test-precompile * load constant in beforeAll * replace gas cost with weight to gas ratio

* point dependencies to moonbeam-polkadot-sdk-stable2407 * bump packages versions to match polkadot-sdk * apply TransactionPov changes * update runtime configs (wip) * update imports * mute hrmp channel moonriver - Litmus (#2914) * Add step to Build workflow to check for WASM runtime sizes (#2888) * add simple wasm size check * only run on pull requests * add Github token to env * fix download dir name * try comparing previous and current runtimes * change master branch with dynamic target branch ref * add comparison to latest release * test with older release * download from release instead of workflow artifact * debug release tag not saved to env * debug some more * use gh cli to get releases instead of git tag * use gh api instead of releases * remove get build id & fix awk parsing * build runtimes with _, releases with - * add comment to PR and format msg * use markdown report & replace comment * cat report & fix comment formatting * add twiggy diff reports as artifacts * use upload v4 & create reports dir * rm: deprecated ::set-output cmd * add log event (#2918) * Extract Storage Read costs for dev tests involved in #2786 (#2915) * add: storage read cost constant * update test-pov * update xcm-v3 tests * update xcm-v4 tests * add storage read gas cost to constants * update test-precompile * load constant in beforeAll * replace gas cost with weight to gas ratio * Fix auto-pause xcm: incoming XCMP messages where dropped when auto-pause (#2913) * chore: rename crate manual-xcm-rpc -> moonbeam-dev-rpc * add RPC method test_skipRelayBlocks * allow Root to resume XCM execution * update moonkit pin * add dev-test that trigger auto-pause of xcm * prettier * dev test auto-pause xcm: remove unused imports & improve test scenario * add pallet-emergency-para-xcm to moonriver * add pallet-emergency-para-xcm to moonbeam * apply review suggestions * Set the block size to 60mln gas for moonbeam (#2921) * set the block size to 60mln gas for moonbeam * update mbip-5 value in test * Add pallet_parameters (moonbase only) (#2923) * add pallet parameters * XcmFeesAccount * add copyrights * add FeesTreasuryProportion --------- Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * make moonbeam compile * remove cumulus_pallet_dmp_queue * remove unused import * add proper para_id to MockValidationDataInherentDataProvider * update config for tests * add MockTimestampInherentDataProvider to simulate consistent and sequential relay slots * Revert "apply TransactionPov changes" This reverts commit 4382107. * update cargo lock pins * remove import of deleted TransacionPoV * Make cargo test compile * Add XcmRecorder to XcmConfig * Add VersionWrapper to hrmp::COnfig * Remove code from deprecated pallet treasury * Remove code from deprecated DmpQueue * Remove code from deprecated XCMV2 * remove deprecated tresury pallet tests, add some for the new functionality * fix tests * fix remaining dev tests * fix licenses * fix lazy loading backend for stable * remove unused patch * fix linters * fix cargo test with benchmarking and tracing enabled * fix lazy loading * fix command for lazy loading * update frontier pin * support new 'stableYYMM' versioning for polkadot * update polkadot sdk pin * fix download-polkadot script * simplify script * Replace thread_local with AtomicU64 (as per review request) * remove deprecated test code * use latest version of nextest * Update precompiles/collective/src/mock.rs Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * apply review suggestions * Revert "apply review suggestions" This reverts commit 6daef57. * apply review suggestions * fix lazy-loading * re add reset_issuance * add missing weights * remove unused var * fix download correct binaries * cargo fmt * make polkadot worker binearies executable * configure XcmRecorder for mocks --------- Co-authored-by: Éloïs <c@elo.tf> Co-authored-by: pablito テ <pablitolabarta@gmail.com> Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> Co-authored-by: Andrea Giacobino <no.andrea@gmail.com> Co-authored-by: Tarek Mohamed Abdalla <tarekkma@gmail.com> Co-authored-by: Rodrigo Quelhas <rodrigo_quelhas@outlook.pt> Co-authored-by: Agusrodri <agusrodriguez2456@gmail.com>

…#2980) * point dependencies to moonbeam-polkadot-sdk-stable2407 * bump packages versions to match polkadot-sdk * apply TransactionPov changes * update runtime configs (wip) * update imports * mute hrmp channel moonriver - Litmus (#2914) * Add step to Build workflow to check for WASM runtime sizes (#2888) * add simple wasm size check * only run on pull requests * add Github token to env * fix download dir name * try comparing previous and current runtimes * change master branch with dynamic target branch ref * add comparison to latest release * test with older release * download from release instead of workflow artifact * debug release tag not saved to env * debug some more * use gh cli to get releases instead of git tag * use gh api instead of releases * remove get build id & fix awk parsing * build runtimes with _, releases with - * add comment to PR and format msg * use markdown report & replace comment * cat report & fix comment formatting * add twiggy diff reports as artifacts * use upload v4 & create reports dir * rm: deprecated ::set-output cmd * add log event (#2918) * Extract Storage Read costs for dev tests involved in #2786 (#2915) * add: storage read cost constant * update test-pov * update xcm-v3 tests * update xcm-v4 tests * add storage read gas cost to constants * update test-precompile * load constant in beforeAll * replace gas cost with weight to gas ratio * Fix auto-pause xcm: incoming XCMP messages where dropped when auto-pause (#2913) * chore: rename crate manual-xcm-rpc -> moonbeam-dev-rpc * add RPC method test_skipRelayBlocks * allow Root to resume XCM execution * update moonkit pin * add dev-test that trigger auto-pause of xcm * prettier * dev test auto-pause xcm: remove unused imports & improve test scenario * add pallet-emergency-para-xcm to moonriver * add pallet-emergency-para-xcm to moonbeam * apply review suggestions * Set the block size to 60mln gas for moonbeam (#2921) * set the block size to 60mln gas for moonbeam * update mbip-5 value in test * Add pallet_parameters (moonbase only) (#2923) * add pallet parameters * XcmFeesAccount * add copyrights * add FeesTreasuryProportion --------- Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * make moonbeam compile * remove cumulus_pallet_dmp_queue * remove unused import * add proper para_id to MockValidationDataInherentDataProvider * update config for tests * add MockTimestampInherentDataProvider to simulate consistent and sequential relay slots * Revert "apply TransactionPov changes" This reverts commit 4382107. * update cargo lock pins * remove import of deleted TransacionPoV * Make cargo test compile * Add XcmRecorder to XcmConfig * Add VersionWrapper to hrmp::COnfig * Remove code from deprecated pallet treasury * Remove code from deprecated DmpQueue * Remove code from deprecated XCMV2 * remove deprecated tresury pallet tests, add some for the new functionality * fix tests * fix remaining dev tests * fix licenses * fix lazy loading backend for stable * remove unused patch * fix linters * fix cargo test with benchmarking and tracing enabled * fix lazy loading * fix command for lazy loading * update frontier pin * support new 'stableYYMM' versioning for polkadot * update polkadot sdk pin * fix download-polkadot script * simplify script * Replace thread_local with AtomicU64 (as per review request) * remove deprecated test code * use latest version of nextest * add DryRunApi and LocationToAccountApi * Update precompiles/collective/src/mock.rs Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> * apply review suggestions * Revert "apply review suggestions" This reverts commit 6daef57. * apply review suggestions * fix lazy-loading * re add reset_issuance * add missing weights * remove unused var * add TS tests * cleanup * fmt * fix download correct binaries * cargo fmt * make polkadot worker binearies executable * configure XcmRecorder for mocks --------- Co-authored-by: Gonza Montiel <gon.montiel@gmail.com> Co-authored-by: Éloïs <c@elo.tf> Co-authored-by: pablito テ <pablitolabarta@gmail.com> Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> Co-authored-by: Andrea Giacobino <no.andrea@gmail.com> Co-authored-by: Tarek Mohamed Abdalla <tarekkma@gmail.com> Co-authored-by: Rodrigo Quelhas <rodrigo_quelhas@outlook.pt> Co-authored-by: Gonza Montiel <gonzamontiel@users.noreply.github.com>

pLabarta added 6 commits August 26, 2024 09:14

add: storage read cost constant

a531e3d

update test-pov

aaee562

update xcm-v3 tests

593e034

update xcm-v4 tests

72eae92

add storage read gas cost to constants

d27eb4e

update test-precompile

78c2afd

pLabarta added D2-notlive PR doesn't change runtime code (so can't be audited) B0-silent Changes should not be mentioned in any release notes labels Aug 26, 2024

pLabarta marked this pull request as draft August 26, 2024 13:58

pLabarta added 2 commits August 26, 2024 12:48

load constant in beforeAll

50f642b

Merge branch 'master' into pablo/extract-test-constants

4f08627

pLabarta marked this pull request as ready for review August 26, 2024 16:30

pLabarta requested review from a team, gonzamontiel, TarekkMA and RomarQ and removed request for a team August 26, 2024 16:31

RomarQ reviewed Aug 26, 2024

View reviewed changes

replace gas cost with weight to gas ratio

4eb91d0

pLabarta requested a review from RomarQ August 28, 2024 12:49

RomarQ approved these changes Aug 28, 2024

View reviewed changes

pLabarta added 2 commits August 28, 2024 15:04

Merge branch 'master' into pablo/extract-test-constants

79ba724

Merge branch 'master' into pablo/extract-test-constants

4a979de

pLabarta merged commit 80d6f17 into master Aug 29, 2024
38 checks passed

pLabarta deleted the pablo/extract-test-constants branch August 29, 2024 12:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract Storage Read costs for dev tests involved in #2786 #2915

Extract Storage Read costs for dev tests involved in #2786 #2915

pLabarta commented Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 26, 2024 •

edited

Loading

RomarQ Aug 26, 2024 •

edited

Loading

pLabarta Aug 26, 2024

RomarQ Aug 27, 2024

pLabarta Aug 28, 2024

github-actions bot commented Aug 28, 2024 •

edited

Loading

Extract Storage Read costs for dev tests involved in #2786 #2915

Extract Storage Read costs for dev tests involved in #2786 #2915

Conversation

pLabarta commented Aug 26, 2024 • edited Loading

What does it do?

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?

github-actions bot commented Aug 26, 2024 • edited Loading

Coverage Report

RomarQ Aug 26, 2024 • edited Loading

Choose a reason for hiding this comment

pLabarta Aug 26, 2024

Choose a reason for hiding this comment

RomarQ Aug 27, 2024

Choose a reason for hiding this comment

pLabarta Aug 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 28, 2024 • edited Loading

WASM runtime size check:

Compared to target branch

Compared to latest release (runtime-3100)

pLabarta commented Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 26, 2024 •

edited

Loading

RomarQ Aug 26, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading