People chain integration tests

[rockbmb]

Description

Made as a follow-up of polkadot-fellows/runtimes#499

Integration

N/A

Review Notes

N/A

[acatangiu]

New tests look good, fyi need fixing issues after merging master (adapt for XCMv5) to get CI green.

[rockbmb]

@acatangiu thank you for the the feedback.

Before I put this in the merge queue, there's something I would like to understand.
Consider the governance::*_wrong_origin tests I added in this PR (which I also introduced to polkadot-fellows/runtimes#499).

At present,

• The Polkadot and Kusama runtimes define RegistrarOrigin,ForceOrigin.UsernameAuthorityOrigin as

EitherOfDiverse<
	EnsureRoot<AccountId>,
	EnsureXcm<IsVoiceOfBody<GovernanceLocation, GeneralAdminBodyId>>,
>;

so an incorrect origin check can consist of just checking a SignedOrigin.

• In the Rococo/Westend runtimes, those origins only include EnsureRoot<AccountId>, so the incorrect origin tests try GeneralAdmin and verify that extrinsics fail execution (e.g. see impl pallet_identity::Config for Westend's Runtime here.

The wrong origin tests that I added are: relay_commands_{add_registrar, kill_identity, add_remove_username_authority}_wrong_origin.
As expected, using a signed or general admin origin with those extrinsics fails, but how they fail differs based on:

1. the network (Rococo vs. Westend),
2. the extrinsic, and
3. the origin

For Rococo:

Origin / Extrinsic	add_registrar	kill_identity	{add, remove}_username_authority
Signed origin	xcm.dispatch(origin).is_ok(), no parachain events	xcm.dispatch(origin).is_ok(), no parachain events	xcm.dispatch(origin).is_ok(), no parachain events
General admin origin	xcm.dispatch(origin).is_err(), no parachain events	xcm.dispatch(origin).is_err(), no parachain events	xcm.dispatch(origin).is_err(), no parachain events

For Westend:

Origin / Extrinsic	add_registrar	kill_identity	{add, remove}_username_authority
Signed origin	Event::ProcessingFailed	No parachain events	No parachain events
General admin origin	Event::Processed	No parachain events	No parachain events

Why is this the case? Before these tests, I would have hypothesized that when submitting an extrinsic with an insufficient origin, the mode of failure was to be the same, regardless of extrinsic, network, or origin.

[acatangiu]

I only read diagonally, but first thing you should check is rococo/westend configs. Usually when they are different from prod networks they are simply outdated.

EitherOfDiverse<
EnsureRoot,
EnsureXcm<IsVoiceOfBody<GovernanceLocation, GeneralAdminBodyId>>,

;

Looks right to me, check git blame for the Westend and rococo configs and you can figure out if they are different intentionally (some new feature added not yet reached prod networks), or if they are simply outdated.

Also, Rococo is deprecated/retired. It’s not deployed anywhere anymore. We still keep the code around for bridge emulated tests and zombienet tests, but everything else is deprecated. Aka, no need to add new governance tests on Rococo.

[rockbmb]

@acatangiu

Thank you for the reply!

Even before writing my previous comment, I had checked configs; you're correct that Kusama/Polkadot configs became different from Westend/Rococo.

My wall of text obscured the core of my question:

1. some extrinsics on Westend require a root origin.
2. when executing them from either a general admin origin or a signed origin, they fail, as expected
3. however, the way in which each origin fails is different - unlike expected.

The origins being defined differently between networks is relevant - but a separate issue.
What I'm trying to understand is why each of the unallowed origins fails differently.

[acatangiu]

3. however, the way in which each origin fails is different - unlike expected.

The origins being defined differently between networks is relevant - but a separate issue. What I'm trying to understand is why each of the unallowed origins fails differently.

can you provide more details? how do they fail differently? what are the tracing logs saying?

I am guessing there are different validation functions (per origin type) that are failing, but we'd need to follow the code to figure out exactly - either way, I don't think this is a problem

[rockbmb]

@acatangiu Thanks for the feedback.

This PR fulfills its purpose and the underlying reason for the behavior I described is not problematic, just curious IMO; as you said:

either way, I don't think this is a problem

This can be looked into in a separate issue/PR. I'll merge this (with another approval).