git.delta.rocks / unique-network / refs/commits / bfab6035a123

difftreelog

Merge pull request #1044 from UniqueNetwork/fix/forbid-relay-as-root

Yaroslav Bolyukin2023-11-27parents: #425f6c1 #5bf7ef4.patch.diff
in: master
Forbid relay as root

5 files changed

addedjs-packages/tests/sub/governance/electsudo.test.tsdiffbeforeafterboth
--- /dev/null
+++ b/js-packages/tests/sub/governance/electsudo.test.ts
@@ -0,0 +1,99 @@
+import type {IKeyringPair} from '@polkadot/types/types';
+import {usingPlaygrounds, itSub, expect, Pallets, requirePalletsOrSkip, describeGov} from '../../util/index.js';
+import {Event} from '@unique/playgrounds/unique.dev.js';
+import {initCouncil, democracyLaunchPeriod, democracyVotingPeriod, democracyEnactmentPeriod, clearCouncil, clearTechComm, initTechComm, ITechComms} from './util.js';
+import type {ICounselors} from './util.js';
+
+describeGov('Governance: Elect Sudo', () => {
+  let sudoer: IKeyringPair;
+  let donor: IKeyringPair;
+  let counselors: ICounselors;
+  let techComm: ITechComms;
+
+  const moreThanHalfCouncilThreshold = 3;
+
+  before(async function() {
+    await usingPlaygrounds(async (helper, privateKey) => {
+      requirePalletsOrSkip(this, helper, [Pallets.Council]);
+
+      sudoer = await privateKey('//Alice');
+      donor = await privateKey({url: import.meta.url});
+      counselors = await initCouncil(donor, sudoer);
+      techComm = await initTechComm(donor, sudoer);
+    });
+  });
+
+  after(async () => {
+    await clearCouncil(sudoer);
+    await clearTechComm(sudoer);
+  });
+
+  itSub('Democracy can elect a sudo account', async ({helper}) => {
+    const [newAccount] = await helper.arrange.createAccounts([1000n], donor);
+    const newSudoKey = newAccount.address;
+
+    // Have to use `afterEach` here instead of `after` to ensure it will be executed before `describe.after`.
+    afterEach(async () => {
+      // For some reason, the outer helper API is not initialized inside `afterEach`.
+      await usingPlaygrounds(async (helper) => {
+        await helper.executeExtrinsic(
+          newAccount,
+          'api.tx.sudo.setKey',
+          [sudoer.address],
+          false,
+        );
+      });
+    });
+
+    const democracyProposal = helper.constructApiCall('api.tx.utility.dispatchAs', [
+      {
+        system: {
+          Signed: sudoer.address,
+        },
+      },
+      helper.constructApiCall('api.tx.sudo.setKey', [newSudoKey]),
+    ]);
+
+    const councilProposal = await helper.democracy.externalProposeDefaultCall(democracyProposal);
+
+    const proposeResult = await helper.council.collective.propose(
+      counselors.filip,
+      councilProposal,
+      moreThanHalfCouncilThreshold,
+    );
+
+    const councilProposedEvent = Event.Council.Proposed.expect(proposeResult);
+    const proposalIndex = councilProposedEvent.proposalIndex;
+    const proposalHash = councilProposedEvent.proposalHash;
+
+    await helper.council.collective.vote(counselors.alex, proposalHash, proposalIndex, true);
+    await helper.council.collective.vote(counselors.charu, proposalHash, proposalIndex, true);
+    await helper.council.collective.vote(counselors.filip, proposalHash, proposalIndex, true);
+
+    await helper.council.collective.close(counselors.filip, proposalHash, proposalIndex);
+
+    const democracyStartedEvent = await helper.wait.expectEvent(democracyLaunchPeriod, Event.Democracy.Started);
+    const democracyReferendumIndex = democracyStartedEvent.referendumIndex;
+    const democracyThreshold = democracyStartedEvent.threshold;
+
+    expect(democracyThreshold).to.be.equal('SuperMajorityAgainst');
+
+    await helper.democracy.vote(newAccount, democracyReferendumIndex, {
+      Standard: {
+        vote: {
+          aye: true,
+          conviction: 1,
+        },
+        balance: 800n,
+      },
+    });
+
+    const passedReferendumEvent = await helper.wait.expectEvent(democracyVotingPeriod, Event.Democracy.Passed);
+    expect(passedReferendumEvent.referendumIndex).to.be.equal(democracyReferendumIndex);
+
+    await helper.wait.expectEvent(democracyEnactmentPeriod, Event.Scheduler.Dispatched);
+    const currentSudoKey = await helper.callRpc('api.query.sudo.key', [])
+      .then(k => k.toString());
+    expect(currentSudoKey).to.be.equal(newSudoKey);
+  });
+});
modifiedjs-packages/tests/xcm/lowLevelXcmQuartz.test.tsdiffbeforeafterboth
--- a/js-packages/tests/xcm/lowLevelXcmQuartz.test.ts
+++ b/js-packages/tests/xcm/lowLevelXcmQuartz.test.ts
@@ -298,67 +298,3 @@
     await testHelper.rejectReserveTransferUNQfrom('shiden', alice);
   });
 });
-
-describeXCM('[XCMLL] Integration test: The relay can do some root ops', () => {
-  let sudoer: IKeyringPair;
-
-  before(async function () {
-    await usingRelayPlaygrounds(relayUrl, async (_, privateKey) => {
-      sudoer = await privateKey('//Alice');
-    });
-  });
-
-  // At the moment there is no reliable way
-  // to establish the correspondence between the `ExecutedDownward` event
-  // and the relay's sent message due to `SetTopic` instruction
-  // containing an unpredictable topic silently added by the relay's messages on the router level.
-  // This changes the message hash on arrival to our chain.
-  //
-  // See:
-  // * The relay's router: https://github.com/paritytech/polkadot-sdk/blob/f60318f68687e601c47de5ad5ca88e2c3f8139a7/polkadot/runtime/westend/src/xcm_config.rs#L83
-  // * The `WithUniqueTopic` helper: https://github.com/paritytech/polkadot-sdk/blob/945ebbbcf66646be13d5b1d1bc26c8b0d3296d9e/polkadot/xcm/xcm-builder/src/routing.rs#L36
-  //
-  // Because of this, we insert time gaps between tests so
-  // different `ExecutedDownward` events won't interfere with each other.
-  afterEach(async () => {
-    await usingPlaygrounds(async (helper) => {
-      await helper.wait.newBlocks(3);
-    });
-  });
-
-  itSub('The relay can set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'plain');
-  });
-
-  itSub('The relay can batch set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'batch');
-  });
-
-  itSub('The relay can batchAll set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'batchAll');
-  });
-
-  itSub('The relay can forceBatch set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'forceBatch');
-  });
-
-  itSub('[negative] The relay cannot set balance', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'plain');
-  });
-
-  itSub('[negative] The relay cannot set balance via batch', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'batch');
-  });
-
-  itSub('[negative] The relay cannot set balance via batchAll', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'batchAll');
-  });
-
-  itSub('[negative] The relay cannot set balance via forceBatch', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'forceBatch');
-  });
-
-  itSub('[negative] The relay cannot set balance via dispatchAs', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'dispatchAs');
-  });
-});
modifiedjs-packages/tests/xcm/lowLevelXcmUnique.test.tsdiffbeforeafterboth
--- a/js-packages/tests/xcm/lowLevelXcmUnique.test.ts
+++ b/js-packages/tests/xcm/lowLevelXcmUnique.test.ts
@@ -364,67 +364,3 @@
     await testHelper.rejectReserveTransferUNQfrom('astar', alice);
   });
 });
-
-describeXCM('[XCMLL] Integration test: The relay can do some root ops', () => {
-  let sudoer: IKeyringPair;
-
-  before(async function () {
-    await usingRelayPlaygrounds(relayUrl, async (_, privateKey) => {
-      sudoer = await privateKey('//Alice');
-    });
-  });
-
-  // At the moment there is no reliable way
-  // to establish the correspondence between the `ExecutedDownward` event
-  // and the relay's sent message due to `SetTopic` instruction
-  // containing an unpredictable topic silently added by the relay's messages on the router level.
-  // This changes the message hash on arrival to our chain.
-  //
-  // See:
-  // * The relay's router: https://github.com/paritytech/polkadot-sdk/blob/f60318f68687e601c47de5ad5ca88e2c3f8139a7/polkadot/runtime/westend/src/xcm_config.rs#L83
-  // * The `WithUniqueTopic` helper: https://github.com/paritytech/polkadot-sdk/blob/945ebbbcf66646be13d5b1d1bc26c8b0d3296d9e/polkadot/xcm/xcm-builder/src/routing.rs#L36
-  //
-  // Because of this, we insert time gaps between tests so
-  // different `ExecutedDownward` events won't interfere with each other.
-  afterEach(async () => {
-    await usingPlaygrounds(async (helper) => {
-      await helper.wait.newBlocks(3);
-    });
-  });
-
-  itSub('The relay can set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'plain');
-  });
-
-  itSub('The relay can batch set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'batch');
-  });
-
-  itSub('The relay can batchAll set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'batchAll');
-  });
-
-  itSub('The relay can forceBatch set storage', async () => {
-    await testHelper.relayIsPermittedToSetStorage(sudoer, 'forceBatch');
-  });
-
-  itSub('[negative] The relay cannot set balance', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'plain');
-  });
-
-  itSub('[negative] The relay cannot set balance via batch', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'batch');
-  });
-
-  itSub('[negative] The relay cannot set balance via batchAll', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'batchAll');
-  });
-
-  itSub('[negative] The relay cannot set balance via forceBatch', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'forceBatch');
-  });
-
-  itSub('[negative] The relay cannot set balance via dispatchAs', async () => {
-    await testHelper.relayIsNotPermittedToSetBalance(sudoer, 'dispatchAs');
-  });
-});
modifiedjs-packages/tests/xcm/xcm.types.tsdiffbeforeafterboth
506    });506    });
507  }507  }
508
509  private async _relayXcmTransactSetStorage(variant: 'plain' | 'batch' | 'batchAll' | 'forceBatch') {
510    // eslint-disable-next-line require-await
511    return await usingPlaygrounds(async (helper) => {
512      const relayForceKV = () => {
513        const random = Math.random();
514        const key = `relay-forced-key (instance: ${random})`;
515        const val = `relay-forced-value (instance: ${random})`;
516        const call = helper.constructApiCall('api.tx.system.setStorage', [[[key, val]]]).method.toHex();
517
518        return {
519          call,
520          key,
521          val,
522        };
523      };
524
525      if(variant == 'plain') {
526        const kv = relayForceKV();
527        return {
528          program: helper.arrange.makeUnpaidSudoTransactProgram({
529            weightMultiplier: 1,
530            call: kv.call,
531          }),
532          kvs: [kv],
533        };
534      } else {
535        const kv0 = relayForceKV();
536        const kv1 = relayForceKV();
537
538        const batchCall = helper.constructApiCall(`api.tx.utility.${variant}`, [[kv0.call, kv1.call]]).method.toHex();
539        return {
540          program: helper.arrange.makeUnpaidSudoTransactProgram({
541            weightMultiplier: 2,
542            call: batchCall,
543          }),
544          kvs: [kv0, kv1],
545        };
546      }
547    });
548  }
549
550  async relayIsPermittedToSetStorage(relaySudoer: IKeyringPair, variant: 'plain' | 'batch' | 'batchAll' | 'forceBatch') {
551    const {program, kvs} = await this._relayXcmTransactSetStorage(variant);
552
553    await usingRelayPlaygrounds(relayUrl, async (helper) => {
554      await helper.getSudo().executeExtrinsic(relaySudoer, 'api.tx.xcmPallet.send', [
555        this._uniqueChainMultilocationForRelay(),
556        program,
557      ]);
558    });
559
560    await usingPlaygrounds(async (helper) => {
561      await expectDownwardXcmComplete(helper);
562
563      for(const kv of kvs) {
564        const forcedValue = await helper.callRpc('api.rpc.state.getStorage', [kv.key]);
565        expect(hexToString(forcedValue.toHex())).to.be.equal(kv.val);
566      }
567    });
568  }
569
570  private async _relayXcmTransactSetBalance(variant: 'plain' | 'batch' | 'batchAll' | 'forceBatch' | 'dispatchAs') {
571    // eslint-disable-next-line require-await
572    return await usingPlaygrounds(async (helper) => {
573      const emptyAccount = helper.arrange.createEmptyAccount().address;
574
575      const forceSetBalanceCall = helper.constructApiCall('api.tx.balances.forceSetBalance', [emptyAccount, 10_000n]).method.toHex();
576
577      let call;
578
579      if(variant == 'plain') {
580        call = forceSetBalanceCall;
581
582      } else if(variant == 'dispatchAs') {
583        call = helper.constructApiCall('api.tx.utility.dispatchAs', [
584          {
585            system: 'Root',
586          },
587          forceSetBalanceCall,
588        ]).method.toHex();
589      } else {
590        call = helper.constructApiCall(`api.tx.utility.${variant}`, [[forceSetBalanceCall]]).method.toHex();
591      }
592
593      return {
594        program: helper.arrange.makeUnpaidSudoTransactProgram({
595          weightMultiplier: 1,
596          call,
597        }),
598        emptyAccount,
599      };
600    });
601  }
602
603  async relayIsNotPermittedToSetBalance(
604    relaySudoer: IKeyringPair,
605    variant: 'plain' | 'batch' | 'batchAll' | 'forceBatch' | 'dispatchAs',
606  ) {
607    const {program, emptyAccount} = await this._relayXcmTransactSetBalance(variant);
608
609    await usingRelayPlaygrounds(relayUrl, async (helper) => {
610      await helper.getSudo().executeExtrinsic(relaySudoer, 'api.tx.xcmPallet.send', [
611        this._uniqueChainMultilocationForRelay(),
612        program,
613      ]);
614    });
615
616    await usingPlaygrounds(async (helper) => {
617      await expectDownwardXcmNoPermission(helper);
618      expect(await helper.balance.getSubstrate(emptyAccount)).to.be.equal(0n);
619    });
620  }
621}508}
622509
modifiedruntime/common/config/xcm/mod.rsdiffbeforeafterboth
--- a/runtime/common/config/xcm/mod.rs
+++ b/runtime/common/config/xcm/mod.rs
@@ -17,7 +17,7 @@
 use cumulus_primitives_core::ParaId;
 use frame_support::{
 	parameter_types,
-	traits::{ConstU32, Contains, Everything, Get, Nothing, ProcessMessageError},
+	traits::{ConstU32, Everything, Get, Nothing, ProcessMessageError},
 };
 use frame_system::EnsureRoot;
 use pallet_xcm::XcmPassthrough;
@@ -29,9 +29,9 @@
 	v3::Instruction,
 };
 use staging_xcm_builder::{
-	AccountId32Aliases, EnsureXcmOrigin, FixedWeightBounds, ParentAsSuperuser, ParentIsPreset,
-	RelayChainAsNative, SiblingParachainAsNative, SiblingParachainConvertsVia,
-	SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation,
+	AccountId32Aliases, EnsureXcmOrigin, FixedWeightBounds, ParentIsPreset, RelayChainAsNative,
+	SiblingParachainAsNative, SiblingParachainConvertsVia, SignedAccountId32AsNative,
+	SignedToAccountId32, SovereignSignedViaLocation,
 };
 use staging_xcm_executor::{
 	traits::{Properties, ShouldExecute},
@@ -111,9 +111,6 @@
 	// Native converter for sibling Parachains; will convert to a `SiblingPara` origin when
 	// recognised.
 	SiblingParachainAsNative<cumulus_pallet_xcm::Origin, RuntimeOrigin>,
-	// Superuser converter for the Relay-chain (Parent) location. This will allow it to issue a
-	// transaction from the Root origin.
-	ParentAsSuperuser<RuntimeOrigin>,
 	// Native signed account converter; this just converts an `AccountId32` origin into a normal
 	// `Origin::Signed` origin of the same 32-byte value.
 	SignedAccountId32AsNative<RelayNetwork, RuntimeOrigin>,
@@ -166,55 +163,7 @@
 }
 
 pub type Weigher = FixedWeightBounds<UnitWeightCost, RuntimeCall, MaxInstructions>;
-
-pub struct XcmCallFilter;
-impl XcmCallFilter {
-	fn allow_gov_and_sys_call(call: &RuntimeCall) -> bool {
-		match call {
-			RuntimeCall::System(..) => true,
-
-			#[cfg(feature = "governance")]
-			RuntimeCall::Identity(..)
-			| RuntimeCall::Preimage(..)
-			| RuntimeCall::Democracy(..)
-			| RuntimeCall::Council(..)
-			| RuntimeCall::TechnicalCommittee(..)
-			| RuntimeCall::CouncilMembership(..)
-			| RuntimeCall::TechnicalCommitteeMembership(..)
-			| RuntimeCall::FellowshipCollective(..)
-			| RuntimeCall::FellowshipReferenda(..) => true,
-			_ => false,
-		}
-	}
 
-	fn allow_utility_call(call: &RuntimeCall) -> bool {
-		match call {
-			RuntimeCall::Utility(pallet_utility::Call::batch { calls, .. }) => {
-				calls.iter().all(Self::allow_gov_and_sys_call)
-			}
-			RuntimeCall::Utility(pallet_utility::Call::batch_all { calls, .. }) => {
-				calls.iter().all(Self::allow_gov_and_sys_call)
-			}
-			RuntimeCall::Utility(pallet_utility::Call::as_derivative { call, .. }) => {
-				Self::allow_gov_and_sys_call(call)
-			}
-			RuntimeCall::Utility(pallet_utility::Call::dispatch_as { call, .. }) => {
-				Self::allow_gov_and_sys_call(call)
-			}
-			RuntimeCall::Utility(pallet_utility::Call::force_batch { calls, .. }) => {
-				calls.iter().all(Self::allow_gov_and_sys_call)
-			}
-			_ => false,
-		}
-	}
-}
-
-impl Contains<RuntimeCall> for XcmCallFilter {
-	fn contains(call: &RuntimeCall) -> bool {
-		Self::allow_gov_and_sys_call(call) || Self::allow_utility_call(call)
-	}
-}
-
 pub struct XcmExecutorConfig<T>(PhantomData<T>);
 impl<T> staging_xcm_executor::Config for XcmExecutorConfig<T>
 where
@@ -244,7 +193,7 @@
 	type MessageExporter = ();
 	type UniversalAliases = Nothing;
 	type CallDispatcher = RuntimeCall;
-	type SafeCallFilter = XcmCallFilter;
+	type SafeCallFilter = Nothing;
 	type Aliasers = Nothing;
 }