Refactor E2E suites to allow AHM integration#384
Conversation
Also, create new `testAccounts` object.
To run these tests in post-migration Asset Hubs, the scheduling technique used to coerce execution of extrinsics with origins other than signed must change to allow use of non-local block providers. Also, when running these tests in testnets, some of the default dev accounts have dirty states, so this commit begins the move to fresh accounts in E2E. Lastly, in some locations `scheduleInlineCallWithOrigin` was used without an `await` before it, which led to occasionally wrong behavior.
There was a problem hiding this comment.
The changes are a significant and valuable refactoring to enable E2E tests on Asset Hub runtimes, primarily by abstracting away relay vs. para chain differences and using fresh accounts to prevent state conflicts. The addition of await to async calls fixes potential race conditions. However, there are several opportunities to improve clarity and robustness in the staking tests.
|
can you merge master |
Some tests still need a second pass, but the idea is that they can now run on relay chains, parachains with or without AB, with local or non-local block providers.
|
CI is failing due to the introduction of a I will remove this before completing this PR. |
* they now use the new `testAccounts` (snapshots (partially) updated, * pass new `TestConfig` parameter to E2E test trees * update proxy E2E tests to use new block provider (80% done)
|
Kusama scheduler E2E tests are failing. The behavior of |
|
@xlc I still need to update scheduler tests for Kusama, but this is ready to be looked at. In order to update proxy E2E tests for use with post-AHM parachains, I had to update them in the same manner you did with #399. |
|
The changes introduce a new test configuration to support different block providers, which is a great improvement for testing across different chain types. However, I found one potential issue in the staking tests where a performance optimization is now only applied to local block providers, which might affect tests on other chains. Review Suggestionspackages/shared/src/staking.ts:1948-1963 The logic to remove storage prefixes for |
|
The two failing tests belong to Kusama's scheduler E2E test suite. They are:
The first one is failing because on Kusama, I will push a commit to allow different behavior on Kusama for now; investigating this is outside the scope of this PR, which has grown large enough. |
Without it, the unapplied slashing test times out, because building the era-change block takes some time to complete will all of the era's stakers.
PR #384 required some additional fixes that auto-merge could not perform.
* Init balances e2e tests (wip) * Rework initial tests for clarity Most of them were written with recourse to Cursor, and were just not very useful. * Refactor `transfer_allow_death` test It scrutinizes the extrinsic's events and their inner data. * Recreate tests for all Polkadot/Kusama relay/SPs * Rework networks that run `transferAllowDeath` test Now, at the calling site of the test tree creation function, chains can signal whether their ED is too low to run the basic `transferAllowDeath` test. * Scrutinize more events in `transferAllowDeath` test * Test non-destructive `transfer_allow_death` * Test `force_transfer` with account reaping * Test to `force_transfer` reaping * Test `transfer_allow_death` below ED * Test `force_transfer` below ED * Test overtransferring account * Update comments and snapshots * Test `force_transfer` with insufficient funds * Test self `transfer_allow_death` of near-entire balance * Update snapshots * Update accounts tests after merge from `master` PR #384 required some additional fixes that auto-merge could not perform. * Update snapshots * Fund new test account on Bridge Hub * Revert change to Polkadot AH multisig suite name * Create test to liquidity restrictions bug See paritytech/polkadot-sdk#9560 and paritytech/polkadot-sdk#8108. TL;DR proxy and multisig pallets are using the old `Currency` trait and not the new fungible traits, it is possible for an operation that is permissible to an account at a given point in time and state to fail. See #401 * Test liquidity restrictions when creating proxy * Parametrize deposit-requiring action liquidity restriction test The `balances.LiquidityRestrictions` error can occur is different contexts, as multiple pallets still use the old `Currency::reserve` operation. This commit parametrizes the test to allow for different kinds of actions to be tested, not just proxy addition. * Extend liquidity restriction tests to cover more cases The test now combinatorially covers several cases: 1. Reserve action can vary between staking bond and nomination pool creation 2. Lock action is always vested transfer (for now) 3. Deposit action can vary between proxy creation, referendum submission and staking bond * Test manual reserve/lock in liquidity restriction tests This adds the possibility of manually set reserve/locks to the reserve/ lock actions used to combinatorially generate tests to liquidity restrictions. This is needed because some chains have no vesting or staking, but the behavior should still be tested. * Fix manual reserve/lock actions in liq. restriction tests Fees were not being accounted for correctly when performing checks on free/frozen balances. * Update snapshots and accounts E2E test tree call sites * Filter vesting as locking action on Asset Hubs ... while the AHM is pending - vesting operations are filtered, and thus cannot be used. * Add more comments to liquidity restriction test Especially the action interfaces. * Add more comments to liquidity restriction tests * Remove leftover debug code from self-transfer test * Fix Asset Hub test suites' use of relay chain An invalid argument was being passed to the `scheduleInlineCallWithOrigin` function: the base's chains block provider, instead of the relay chain's, which is always `Local`. * Snapshot skipped liquidity restriction tests ... instead of logging a message. Logs add some noise to CI test output. * Apply lint fixes * Update block numbers * Change lock identifier used * Remove unstable `Transfer` event from snapshots * Update snapshots * Simplify liquidity restriction tests The `Balance.locks` storage is no longer modified - Polkadot Relay is failing for an undetermined reason. * Use nonces when setting storage * Test `transfer_allow_death` with reserve * Add test to `transfer_all` with reserve * Add `force_transfer` with reserve test * Add origin check tests for gated extrinsics * Add tests to transfer functions with insufficient funds * Test `transfer_all` with `keepAlive = true` * Correct previous transfer all test snapshot * Test `transfer_all` with `keep_alive = false` * Correct snapshots of accounts tests * Test `transfer_keep_alive` with source left below ED * Split `transfer_keep_alive` tests ... into two separate tests: 1. one for chains with low ED (below a typical transfer fee) 2. one for chains with normal ED * Test `force_adjust_total_issuance` with zero delta * Test total issuance changes * Test `force_unreserve` no-op cases * Update accounts E2E test snapshots * Update liquidity restriction test snapshots for AHs * Test forceful unreservation of funds * Add more self-transfer tests * Add self-transfer test to `force_transfer` * Add tests to `force_set_balance` * Refactor `burn` tests into low/normal ED tests * Update snapshots with new `burn` tests * Test burning from account with consumer reference * Add test to burning of entire balance (or more than it) * Update snapshots to remove obsolete data * Improve documentation and `README`
Closes #353, closes #382, closes #258.
Regarding #353
In order to keep a single test suite that can, with minimal changes, be run both on
zombie-biteforks of post-migration Asset Hubs (used for testing)some changes are necessary, most importantly to the scheduler E2E test suite.
This pallet, in a post-migration Asset Hub runtime setting, will use a non-local block provider, which changes some of the expectations that PET E2E tests have; see this PR for an example change.
Regarding #382
Furthermore, this PR also changes some
devaccounts used in testing to fresh accounts with no state - Westend and Paseo runs of PET had false positives due to unclean states forAlice, Bob, etc.Regarding #258
With the changes required by #353, the scheduler E2E test suite now has tests that are expected to run on relay chains, parachains with local block providers, and parachains with non-local block providers.
This entailed several
.match()pattern matches to determine the correct offset at which to schedule tasks.Documentation on the module and each test has been improved to clarify the new branching logic of these tests.