Update PET to integrate Polkadot AHM changes #446
Conversation
Vesting periods now elapse according to a non-local block provider, the relay's, as opposed to a local one; vesting E2E tests and snapshots required an update that was not done for KAHM as vesting was mistakenly filtered there (even after the migration).
rockbmb
added
e2e tests
|
Will also close #451 . |
rockbmb
changed the title
Treasury, System, the relay's staking client pallet, `paras`.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found a typo, copy-pasted comments, and a significant logic issue where proxy types on the Polkadot relay chain are configured to disallow their own primary functions after the AHM changes. For instance, the Staking proxy is set to disallow staking actions.
Suggestions that couldn't be attached to a specific line
packages/polkadot/src/polkadot.accounts.e2e.test.ts:25-31
The comments on these lines appear to be copy-pasted from a Kusama-related file, as they refer to "Kusama relay" within a Polkadot test file. Please update the comments to be specific to Polkadot.
packages/polkadot/src/polkadot.proxy.e2e.test.ts:94, 109-110, 121-122
There seems to be a logic error in the configuration for several proxy types. The Governance, Staking, and NominationPools proxy types are configured to disallow their own core actions (e.g., buildGovernanceAction is disallowed for the Governance proxy). This defeats the purpose of these proxy types. Please verify if this is intended; otherwise, these actions should be in buildAllowedActions or removed from buildDisallowedActions.
* Multisig+proxy, multisig * Also, disable some failing PAH XCM tests
The issue in #8108 has been corrected in all Sys. Parachains in the Polkadot network, so liquidity restriction tests now pass. Also, a `system` test for Polkadot relay has been updated.
In particular, runtime upgrade tests via Collectives have been updated to reflect the new centrality of the AH in this process, where it was formerly the relay through which the process occurred.
May be re-added in thefuture; it was used only for local testing.
Multisig addresses are deterministic, the when they involve proxy addresses, either as signatories or approvers, they are not. Thus, they must be redacted to avoid spurious failures.
It did not take into account the possibility that a test task would be scheduled in the same block as a real task.
| .toMatchSnapshot('bounty canceled event') | ||
|
|
||
| // verify the curator transfer of balance event to the treasury | ||
| await checkSystemEvents(client, { section: 'balances', method: 'Transfer' }) |
There was a problem hiding this comment.
@dhirajs0 this was capturing some fee events, which caused spurious test failures. Might help to know this for treasury tests!
| expect(treasuryTransferEvent).toBeDefined() | ||
| assert(client.api.events.balances.Transfer.is(treasuryTransferEvent!.event)) | ||
| const treasuryTransferEventData = treasuryTransferEvent!.event.data | ||
| // TODO: @dhirajs0, shouldn't this be unnecessary? |
There was a problem hiding this comment.
@dhirajs0 Can you take a look at this? I feel like this multiplication should not be necessary.
There was a problem hiding this comment.
Ah, you are correct. Instead of using the multiplication, we can directly use the bountyValue. I will updated it. 👍
| .redact({ | ||
| redactKeys: /height/, | ||
| // The approving address is the proxy, which is not deterministic. | ||
| redactKeys: /approving|multisig/, |
There was a problem hiding this comment.
@andreitrand regarding our conversation; while multisig addresses are indeed deterministic,
- a multisig account that has a proxy as signatory, or
- a proxy account created from a multisig account
are not; this was causing some false positive test failures due to thismultisigaddress field differing in snapshots.
I overlooked this in my review of #429 , apologies.
Closes #432 , closes #451, closes #449.
To be merged only after the Polkadot Asset Hub Migration finishes on both Polkadot relay and AH.
It is scheduled for November 4th.