Mainnet: liberated runtime (`drop` sudo pallet) by mnaamani · Pull Request #4478 · Joystream/joystream

mnaamani · 2022-12-01T09:25:54Z

Dropping sudo pallet and updated chain metadata and types.
This of course breaks a few checks, any code that depends on making sudo calls.

…me-drop-sudo-pallet

vercel · 2022-12-01T09:25:58Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
pioneer-testnet	⬜️ Ignored (Inspect)		Dec 6, 2022 at 5:38AM (UTC)

Lezek123

Should we also remove pallet-sudo from the runtime dependencies?
I think we can also remove sudo_upload_data_objects from the storage pallet.
Argus and Colossus have dev commands that rely on sudo, so the builds are now failing. We need to remember to remove them before the release. Some devops scripts and api-scripts will also need an update.

mnaamani · 2022-12-05T07:43:26Z

Should we also remove pallet-sudo from the runtime dependencies?

I think we can also remove sudo_upload_data_objects from the storage pallet.

Argus and Colossus have dev commands that rely on sudo, so the builds are now failing. We need to remember to remove them before the release. Some devops scripts and api-scripts will also need an update.

Makes sense
Done
Fix builds of colossus and argus. Are you suggesting to also remove the dev commands from mainnet branch ?

Only made a few minor changes to some devops and github workflows, but left api-scripts and migration-scripts intact to determine what to do later.

Lezek123

Additional notes:

There's still a mention of sudo in devops/aws/README.md

Besides those minor issues everything looks good.
I ran a following tests to confirm storage/distribution nodes from mainnet branch remain compatible with the no-sudo runtime:

Checked-out mainnet branch
yarn build:packages
RUNTIME_PROFILE=TESTING yarn start
In a separate workspace I checked out this PR's branch and ran: RUNTIME_PROFILE=TESTING ./scripts/cargo-build.sh
Executed a runtime upgrade (via sudo) to this PR's runtime using https://polkadot.js.org/apps
Commented out all tests except channelsAndVideosCliJob, initDistributionBucket and initStorageBucket from tests/integration-tests/scenarios/full.ts
Copied tests/network-tests/output.json file from the workspace where I had mainnet branch checked-out to the workspace where I had this PR checked-out
Executed REUSE_KEYS=true ./tests/network-tests/run-test-scenario.sh full on this PR's branch
Tests executed successfully, see output below:

$ node -r ts-node/register --unhandled-rejections=strict src/scenarios/full.ts
  integration-tests:api-factory Connecting to chain, attempt 1.. +0ms
Runtime Version: 12.1002.0
  integration-tests:scenario Full +0ms
  integration-tests:job:manage channels and videos through CLI Running +0ms
  integration-tests:flow:cliChannelsAndVideos Started +0ms
  integration-tests:flow:cliChannelsAndVideos Creating a channel... +23s
  integration-tests:query-node-api:query getChannelById({"id":"1"}) +0ms
  integration-tests:query-node-api:try:toString()) Unexpected query result (expected false to equal true) +0ms
  integration-tests:query-node-api:try:toString()) Retrying query in 9000ms... +0ms
  integration-tests:query-node-api:query getChannelById({"id":"1"}) +9s
  integration-tests:flow:cliChannelsAndVideos Updating channel... +29s
  integration-tests:query-node-api:query getChannelById({"id":"1"}) +8s
  integration-tests:query-node-api:try:toString()) Unexpected query result (expected 'Test channel' to equal 'Test channel [UPDATED!]') +0ms
  integration-tests:query-node-api:try:toString()) Retrying query in 9000ms... +0ms
  integration-tests:query-node-api:query getChannelById({"id":"1"}) +9s
  integration-tests:flow:cliChannelsAndVideos Creating a video... +17s
  integration-tests:query-node-api:query getVideoById({"videoId":"1"}) +7s
  integration-tests:query-node-api:try:toString()) Unexpected query result (Video not found) +0ms
  integration-tests:query-node-api:try:toString()) Retrying query in 9000ms... +0ms
  integration-tests:query-node-api:query getVideoById({"videoId":"1"}) +9s
  integration-tests:flow:cliChannelsAndVideos Updating video... +17s
  integration-tests:query-node-api:query getVideoById({"videoId":"1"}) +8s
  integration-tests:query-node-api:try:toString()) Unexpected query result (expected 'Test video' to equal 'Test video [UPDATED!]') +0ms
  integration-tests:query-node-api:try:toString()) Retrying query in 9000ms... +0ms
  integration-tests:query-node-api:query getVideoById({"videoId":"1"}) +9s
  integration-tests:flow:cliChannelsAndVideos Done +17s
  integration-tests:job:manage channels and videos through CLI [Succeeded] +2m
  integration-tests:job:init storage and distribution buckets via CLI Running +0ms
  integration-tests:flow:initDistributionBucketViaCLI Started +0ms
  integration-tests:flow:initStorageBucketViaCLI Started +0ms
  integration-tests:flow:initStorageBucketViaCLI Done +26s
  integration-tests:flow:initDistributionBucketViaCLI Done +42s
  integration-tests:job:init storage and distribution buckets via CLI [Succeeded] +42s
Job Results:
manage channels and videos through CLI: Succeeded
init storage and distribution buckets via CLI: Succeeded
Writing generated account to output.json
Done in 156.25s.
Done in 156.46s.

real	2m36,571s
user	2m55,918s
sys	0m6,312s

Lezek123 · 2022-12-05T09:26:08Z

runtime-modules/storage/src/lib.rs

        /// Create a dynamic bag. Development mode.
        /// <weight>
        ///


Just noticed there's a leftover comment

Suggested change

/// Create a dynamic bag. Development mode.

/// <weight>

///

funny this comment is wrong for the function :) will fix.

Lezek123 · 2022-12-05T09:40:16Z

tests/network-tests/src/scenarios/full.ts

  ]).requires(councilFailuresJob)

+  return
+  // Below flows depend on sudo which has been disabled. So skipping for now.


Seems like:

creatingMembers,

creatingFoundingMembers,

invitingMebers and

proposals

flow also depends on sudo

Yes I noticed it failed in CI checks. I think will leave the effort to fix integrations properly in a future PR as it will need quite a bit of refactoring.

Might also make sense to remove the createMembership dispatch from membership pallet, since they require root origin and I doubt we will add a proposal type that create a membership.

Lezek123 · 2022-12-05T09:45:31Z

devops/ansible/roles/rpc/tasks/main.yml

@@ -48,7 +48,6 @@
  environment:
    JOYSTREAM_NODE_WS: 'ws://{{ inventory_hostname }}:9944/'
    # TREASURY_ACCOUNT_URI: '{% if endowed_key is defined %}{{ endowed_key }}{% else %}{{ sudo_key }}{% endif %}'


Suggested change

# TREASURY_ACCOUNT_URI: '{% if endowed_key is defined %}{{ endowed_key }}{% else %}{{ sudo_key }}{% endif %}'

# TREASURY_ACCOUNT_URI: '{{ endowed_key }}'

Will drop all env variables here expect the ws endpoint.
The other variables where there because previously instead of starting only query-node the playbook also attempted to start a faucet, and initialized it with the faucet setup scenario from integration tests.

Lezek123 · 2022-12-05T09:48:40Z

storage-node/src/services/runtime/api.ts

  api: ApiPromise,
  account: KeyringPair,
  tx: SubmittableExtrinsic<'promise'>,
-  sudoCall = false,


Should also be removed from the parameters list in the comment above (@param sudoCall)

mnaamani · 2022-12-06T05:41:29Z

Thank you for verifying the behavior of the apps after runtime upgrade.

Lezek123

Looks good, I'll leave it up to you whether to also fix #4478 (comment) and remove the createMembership dispatch as part of this PR

mnaamani added 8 commits November 30, 2022 12:07

supervised runtime

6eb9ea1

mainnet liberated runtime - remove Call on sudo pallet

ded6065

update chain metadata

2d9159c

bump types version to v0.20.6

64d99f6

remove comments

a42dd6c

drop sudo pallet from runtime

a4de019

Merge branch 'mainnet-liberated-runtime' into mainnet-liberated-runti…

fe39bbb

…me-drop-sudo-pallet

update chain metadata

7c44e18

mnaamani mentioned this pull request Dec 1, 2022

Mainnet: liberated runtime (disable sudo call) #4477

Closed

mnaamani requested review from Lezek123 and dobertRowneySr December 1, 2022 09:38

Lezek123 suggested changes Dec 2, 2022

View reviewed changes

mnaamani added 3 commits December 5, 2022 10:01

drop sudo pallet and storage dev dispatchables

57d1a31

remove sudo calls from storage and distributor nodes

ae88679

remove sudo account setup from devops and github workflows

24c465b

run partial integration tests

6443552

mnaamani requested a review from Lezek123 December 5, 2022 09:07

Lezek123 reviewed Dec 5, 2022

View reviewed changes

drop more reference to sudo, make integration tests pass

635d1ad

mnaamani requested a review from Lezek123 December 6, 2022 05:41

Lezek123 approved these changes Dec 6, 2022

View reviewed changes

mnaamani merged commit 58d2775 into Joystream:mainnet-liberated Dec 6, 2022

	# TREASURY_ACCOUNT_URI: '{% if endowed_key is defined %}{{ endowed_key }}{% else %}{{ sudo_key }}{% endif %}'
	# TREASURY_ACCOUNT_URI: '{{ endowed_key }}'

Conversation

mnaamani commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lezek123 left a comment

Choose a reason for hiding this comment

Uh oh!

mnaamani commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lezek123 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnaamani commented Dec 6, 2022

Uh oh!

Lezek123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mnaamani commented Dec 1, 2022 •

edited

Loading

vercel bot commented Dec 1, 2022 •

edited

Loading

mnaamani commented Dec 5, 2022 •

edited

Loading