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.

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

add unlock democracy funds TS test #2673

Merged
merged 13 commits into from
Feb 27, 2024

Conversation

links234
Copy link
Contributor

What does it do?

Adds E2E TS test for unlock democracy funds lazy migration added in PR 2651.

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?

Copy link
Contributor

github-actions bot commented Feb 22, 2024

Coverage Report

@@                             Coverage Diff                             @@
##           master   am-lazy-migration-democracy-unlock-test      +/-   ##
===========================================================================
- Coverage   80.33%                                    73.68%   -6.65%     
- Files         301                                       232      -69     
- Lines       93843                                     72750   -21093     
===========================================================================
- Hits        75386                                     53603   -21783     
+ Misses      18457                                     19147     +690     
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 69.23% (-0.98%) 🔽
/pallets/moonbeam-lazy-migrations/src/mock.rs 4.88% (-8.57%) 🔽
/pallets/moonbeam-orbiters/src/lib.rs 69.80% (-4.39%) 🔽
/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.02%) 🔽
/pallets/parachain-staking/src/inflation.rs 85.98% (-1.52%) 🔽
/pallets/parachain-staking/src/lib.rs 90.24% (-0.38%) 🔽
/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.02%) 🔽
/pallets/parachain-staking/src/types.rs 77.35% (-0.56%) 🔽
/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/pallet-democracy/src/mock.rs 99.02% (+1.06%) 🔼
/precompiles/pallet-democracy/src/tests.rs 89.29% (-0.04%) 🔽
/precompiles/parachain-staking/src/mock.rs 98.85% (+1.04%) 🔼
/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% (-66.18%) 🔽
/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 Tue Feb 27 11:38:48 UTC 2024

@links234 links234 added B0-silent Changes should not be mentioned in any release notes A0-pleasereview Pull request needs code review. D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes labels Feb 22, 2024
@links234 links234 self-assigned this Feb 23, 2024
"0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
"00000000000000000000000000",
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move theses raw storage definitions to a json file or generate them via a ts script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is data scrapped from Moonbeam and I'd like to keep this one. I'll move it in a json file.
I suppose the right place is next to .ts.

Copy link
Collaborator

@librelois librelois Feb 23, 2024

Choose a reason for hiding this comment

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

@timbrinded do you have a strong opinion on where to put this json file?

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 might have to do it a .ts file in order to please the max line length requirement.
Unless there is another doing it?

Copy link
Contributor

@timbrinded timbrinded Feb 26, 2024

Choose a reason for hiding this comment

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

@librelois not with the tests but with the other test data files in /helpers

Another example of this would be /helpers/modexp.ts which gives the expected results for modexp precompile testing

@links234 feel free to use annotation // editorconfig-checker-disable-file on this expected test results file, since raw data is always going to be verbose

}
}

const tx2 = await context.createBlock(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Each check should be on a separate it block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Hopefully I understood correctly.

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've asked @timbrinded and since this is a "complex" test where we need to do multiple sequential things, it should be in 1 single test.
However, I did extract the one checking the upper limit.

@links234 links234 requested a review from librelois February 23, 2024 15:55
@links234 links234 requested a review from RomarQ February 26, 2024 09:33
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider changing this to a TS file like this:
https://github.com/moonbeam-foundation/moonbeam/blob/master/test/helpers/modexp.ts

As it'll be easier to maintain and reason about than JSON. Namely you'll be able to use comments, format it, get type inference, import it nicely.

@links234 links234 requested a review from timbrinded February 27, 2024 11:09
@links234 links234 merged commit 90b340d into master Feb 27, 2024
27 checks passed
@links234 links234 deleted the am-lazy-migration-democracy-unlock-test branch February 27, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants