Add more integrations tests to People chains #499

rockbmb · 2024-11-04T14:09:14Z

https://github.com/open-web3-stack/polkadot-ecosystem-tests relies on chopsticks to create E2E tests using periodically (roughly daily) obtained runtimes from production relay chains/system parachains.

In particular, open-web3-stack/polkadot-ecosystem-tests#63 does this for the People chains.
In writing those tests, I was unable to use XCM to test pallet_identity::{kill_identity/add_username_authority/remove_username_authority}.
I wrote this PR to replicate those tests at this layer, whose passing status falsify the hypothesis that there may have been an issue with the runtimes, and confirm that it was a problem with the way I wrote those tests.

Does not require a CHANGELOG entry

acatangiu

Tests look good! 👍

Please add the same to https://github.com/paritytech/polkadot-sdk/tree/master/cumulus/parachains/integration-tests/emulated/tests/people to keep the covered functionality in sync.

rockbmb · 2024-11-05T10:43:53Z

@acatangiu certainly.

Thanks for the review!

bkchr · 2024-11-05T22:59:14Z

In writing those tests, I was unable to use XCM to test pallet_identity::{kill_identity/add_username_authority/remove_username_authority}.

Why?

github-actions · 2024-11-07T14:30:03Z

Review required! Latest push from author must always be reviewed

github-actions · 2024-11-07T14:30:41Z

Review required! Latest push from author must always be reviewed

rockbmb · 2024-11-12T02:05:11Z

In writing those tests, I was unable to use XCM to test pallet_identity::{kill_identity/add_username_authority/remove_username_authority}.

Why?

To test the following example, you'll need to:

have https://github.com/AcalaNetwork/chopsticks available locally
create a local network with a relay chain and people parachain:

cd chopsticks
npx @acala-network/chopsticks@latest xcm -r configs/polkadot.yml -p configs/polkadot-people.yml

Open each of them in PJS

Consider the following encoded call: 0x630004000100b10f040406020700aea68f0282841e008c320a0078a302c5370300ab2bd8a65469a5061220cd07b3b3a17e5e3233d283e2ad46f0.

Decoding it from the relay chain, you'll see it consists of a V4 XCM with a Transact instruction, to be sent to the people chain with a SuperUser origin, containing the encoded call: 0x320a0078a302c5370300ab2bd8a65469a5061220cd07b3b3a17e5e3233d283e2ad46f0.
If you were to subsequently decode this in the people chain's tab, you would see a pallet_identity::kill_identity call on 13jBAtYJar4xujPaEx41FxjSt9PqU7LqJRbySJiVdMtuWN42, which is my DOT address. There should be an associated identity.

Now either

sign and send it using the Alice development account, or
Paste and run this in the relay chain's JavaScript tab:

const call = '0x630004000100b10f040406020700aea68f0282841e008c320a0078a302c5370300ab2bd8a65469a5061220cd07b3b3a17e5e3233d283e2ad46f0'

const number = (await api.rpc.chain.getHeader()).number.toNumber()

await api.rpc('dev_setStorage', {
  scheduler: {
    agenda: [
      [
        [number + 1], [
          {
            call: {
              Inline: call
            },
            origin: {
              system: 'Root'
            }
          }
        ]
      ]
    ]
  }
})
// Make a block to include the extrinsic
await api.rpc('dev_newBlock', { count: 1 })

In either instance, you will find, in the people chain's "recent events" list, messageQueue.ProcessingFailed.

@bkchr in conclusion, and to answer your question: I don't know why this happens, but I suspect it shouldn't.
The above process is exactly how I tested add_registrar in open-web3-stack/polkadot-ecosystem-tests#63, and it worked then.

I've been working to understand why, and this PR, as well as paritytech/polkadot-sdk#6377 (comment) are how.

rockbmb · 2024-11-12T02:07:02Z

@seadanda also, apologies for the delay: the above comment contains the snippet I mentioned in DMs.

My identity should exist when you fork the chain.

# Description Made as a follow-up of polkadot-fellows/runtimes#499 ## Integration N/A ## Review Notes N/A --------- Co-authored-by: Dónal Murray <[email protected]>

rockbmb · 2024-11-29T17:36:01Z

Like I said in paritytech/polkadot-sdk#6377 (comment), knowing the reason for the differential failures in the wrong origin tests does not impact their immediate usefulness, so this can be merged, and that matter considered in another issue/PR.

# Description Made as a follow-up of polkadot-fellows/runtimes#499 ## Integration N/A ## Review Notes N/A --------- Co-authored-by: Dónal Murray <[email protected]>

fellowship-merge-bot · 2024-11-29T19:20:31Z

There was a problem running the action.

❌😵❌

Please find more information in the logs.

# Description Made as a follow-up of polkadot-fellows/runtimes#499 ## Integration N/A ## Review Notes N/A --------- Co-authored-by: Dónal Murray <[email protected]>

seadanda · 2024-12-11T15:05:25Z

integration-tests/emulated/tests/people/people-kusama/src/tests/governance.rs

+	// I mistakenly assumed that to test the removal of an authority would need one to exist.
+	// However, since the origin check is the very first extrinsic in `remove_username_authority`,
+	// an authority need not exist to test the safety of the origin check.


Suggested change

// I mistakenly assumed that to test the removal of an authority would need one to exist.

// However, since the origin check is the very first extrinsic in `remove_username_authority`,

// an authority need not exist to test the safety of the origin check.

// Since the origin check is the very first extrinsic in `remove_username_authority`, an

// authority need not exist to test the safety of the origin check.

The dreaded first-person slip. Thanks for noticing.

seadanda · 2024-12-11T15:11:09Z

integration-tests/emulated/tests/people/people-kusama/src/tests/governance.rs

+				UnpaidExecution { weight_limit: Unlimited, check_origin: None },
+				Transact {
+					origin_kind,
+					// Making the weight's ref time any lower will prevent the XCM from triggering


Can be removed when we update to XCM v5 in this repo

seadanda · 2024-12-11T15:11:52Z

integration-tests/emulated/tests/people/people-polkadot/src/tests/governance.rs

+			);
+		});
+
+		// Now, remove the username authority with another priviledged XCM call.


Suggested change

// Now, remove the username authority with another priviledged XCM call.

// Now, remove the username authority with another privileged XCM call.

seadanda · 2024-12-11T15:13:15Z

integration-tests/emulated/tests/people/people-kusama/src/tests/governance.rs

+			);
+		});
+
+		// Now, remove the username authority with another priviledged XCM call.


Suggested change

// Now, remove the username authority with another priviledged XCM call.

// Now, remove the username authority with another privileged XCM call.

rockbmb · 2024-12-12T21:03:43Z

/merge

fellowship-merge-bot · 2024-12-12T21:05:35Z

Enabled auto-merge in Pull Request

Available commands

/merge: Enables auto-merge for Pull Request
/merge cancel: Cancels auto-merge for Pull Request
/merge help: Shows this menu

For more information see the documentation

# Description Made as a follow-up of polkadot-fellows/runtimes#499 ## Integration N/A ## Review Notes N/A --------- Co-authored-by: Dónal Murray <[email protected]>

rockbmb added 5 commits November 1, 2024 00:53

Test kill_identity via root origin XCM from relay

39a797b

Test root XCM to add a username authority

79a5016

Complete username authority test for Polkadot

a74c88f

Replicate XCM integration tests in Kusama

1325e59

Apply cargo fmt

4752d1c

rockbmb changed the title ~~Add~~ Add more integrations tests to People chains Nov 4, 2024

rockbmb added 2 commits November 4, 2024 16:30

Merge branch 'main' into pallet-identity-root-extrinsics

5d5a710

Implement clippy fixes

90ba7d8

acatangiu approved these changes Nov 5, 2024

View reviewed changes

rockbmb mentioned this pull request Nov 6, 2024

People chain integration tests paritytech/polkadot-sdk#6377

Merged

rockbmb added 3 commits November 7, 2024 13:52

Test safety of origin checks in People Kusama

2f38434

Remove unneeded operation in username authority test

96d46f0

Add origin check tests to People Polkadot as well

a780f51

rockbmb force-pushed the pallet-identity-root-extrinsics branch from fec09e0 to a780f51 Compare November 7, 2024 14:28

github-actions bot requested a review from acatangiu November 7, 2024 14:30

Apply formatter

e4b993b

acatangiu approved these changes Nov 11, 2024

View reviewed changes

Merge branch 'main' into pallet-identity-root-extrinsics

8547326

github-actions bot requested a review from acatangiu November 19, 2024 13:47

acatangiu approved these changes Nov 20, 2024

View reviewed changes

Merge branch 'main' into pallet-identity-root-extrinsics

d28c776

github-actions bot requested a review from acatangiu November 29, 2024 17:44

seadanda approved these changes Dec 11, 2024

View reviewed changes

Address PR review

40cfec2

fellowship-merge-bot bot enabled auto-merge (squash) December 12, 2024 21:05

Merge branch 'main' into pallet-identity-root-extrinsics

aa5122e

fellowship-merge-bot bot merged commit 46dcafc into polkadot-fellows:main Dec 13, 2024
47 of 48 checks passed

rockbmb deleted the pallet-identity-root-extrinsics branch December 13, 2024 12:34

rockbmb mentioned this pull request Dec 19, 2024

Reduce gas/storage limits in nested calls paritytech/polkadot-sdk#6890

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more integrations tests to People chains #499

Add more integrations tests to People chains #499

rockbmb commented Nov 4, 2024

acatangiu left a comment •

edited

Loading

rockbmb commented Nov 5, 2024

bkchr commented Nov 5, 2024

github-actions bot commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

rockbmb commented Nov 12, 2024 •

edited

Loading

rockbmb commented Nov 12, 2024

rockbmb commented Nov 29, 2024

fellowship-merge-bot bot commented Nov 29, 2024

seadanda Dec 11, 2024

rockbmb Dec 12, 2024

seadanda Dec 11, 2024

seadanda Dec 11, 2024

seadanda Dec 11, 2024

rockbmb commented Dec 12, 2024

fellowship-merge-bot bot commented Dec 12, 2024

	// Now, remove the username authority with another priviledged XCM call.
	// Now, remove the username authority with another privileged XCM call.

Add more integrations tests to People chains #499

Add more integrations tests to People chains #499

Conversation

rockbmb commented Nov 4, 2024

acatangiu left a comment • edited Loading

Choose a reason for hiding this comment

rockbmb commented Nov 5, 2024

bkchr commented Nov 5, 2024

github-actions bot commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

rockbmb commented Nov 12, 2024 • edited Loading

rockbmb commented Nov 12, 2024

rockbmb commented Nov 29, 2024

fellowship-merge-bot bot commented Nov 29, 2024

There was a problem running the action.

seadanda Dec 11, 2024

Choose a reason for hiding this comment

rockbmb Dec 12, 2024

Choose a reason for hiding this comment

seadanda Dec 11, 2024

Choose a reason for hiding this comment

seadanda Dec 11, 2024

Choose a reason for hiding this comment

seadanda Dec 11, 2024

Choose a reason for hiding this comment

rockbmb commented Dec 12, 2024

fellowship-merge-bot bot commented Dec 12, 2024

acatangiu left a comment •

edited

Loading

rockbmb commented Nov 12, 2024 •

edited

Loading