fix: re-enable storage in CI & fix setupNewChain test by dobertRowneySr · Pull Request #4608 · Joystream/joystream

dobertRowneySr · 2023-02-07T20:06:43Z

┆Issue is synchronized with this Asana task by Unito

vercel · 2023-02-07T20:06:48Z

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
pioneer-testnet	⬜️ Ignored (Inspect)			Feb 16, 2023 at 10:07AM (UTC)

dobertRowneySr · 2023-02-08T09:59:06Z

The playground deployment is failing locally due to council election timing out, I have updated the timeout limits and I am testing

dobertRowneySr · 2023-02-08T16:41:16Z

The playground deployment is failing locally due to council election timing out, I have updated the timeout limits and I am testing

This is was due to the tests/network-tests/src/Api::untilCouncilStage final check for which the IdlePeriod (=1) >= reserve (=4) (in the playground config). So the playground election used to get stuck.
I have set reserve = 1 and then ran the local playground successfully

dobertRowneySr · 2023-02-08T16:43:27Z

I have also set shorter duration parameters for duration election and lead proposal

mnaamani · 2023-02-09T12:49:08Z

Was able to do a deployment, and running a test now also through github action https://github.com/Joystream/joystream/actions/runs/4134320191

One useful addition I can think of is to perhaps give the council member member controller account a custom key, like we do for the working group workers to setup the storage infrastructure with pre-determined accounts. This will make it easy for testers of the playground to add these keys rather than needed to setup a new council with their own keys. As they will need to test specific proposals..

mnaamani · 2023-02-09T21:37:01Z

Was able to do a deployment, and running a test now also through github action https://github.com/Joystream/joystream/actions/runs/4134320191

The ansible job timed out, so prepared a PR to increase the timeout dobertRowneySr#9

Successful deployment https://github.com/Joystream/joystream/actions/runs/4138130693/jobs/7154156247

mnaamani

Great job fixing this. Please update the timeout for ansible job by merging dobertRowneySr#9

If you can add the suggested deterministic account for council memebers it would be great finishing touch.

allow more time for playground to deploy before timeout

Lezek123 · 2023-02-10T10:02:47Z

    targetStage: 'Announcing' | 'Voting' | 'Revealing' | 'Idle',
    announcementPeriodNr: number | null = null,
-    blocksReserve = 4,
+    blocksReserve = 1, // TODO dynamically adjust this as it stuck the election process


What do you mean by "stuck the election process"?
I think there is a risk in having the reserve default to as low as 1, as this may not be enough time for the integration tests to issue all transactions intended for a given stage (like announcing all candidacies during announcing stage) and we may experience some issues w/ the integration tests occasionally failing.

This is was due to the tests/network-tests/src/Api::untilCouncilStage final check for which the IdlePeriod (=1) >= reserve (=4) (in the playground config). So the playground election used to get stuck.
I have set reserve = 1 and then ran the local playground successfully

I will to set Idle period to 5 for the playground

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

mnaamani

Looks good, just left a couple of code cleanup points, and suggestion to increase the timeout of the ansible job slightly, otherwise its good to go.

mnaamani · 2023-02-15T11:06:03Z

    // Announcing stage
    await this.api.untilCouncilStage('Announcing')
+    const x = await this.api.query.council.announcementPeriodNr()
+    this.debug(`announcement period ${x.toNumber()}`)


Perhaps your intention here is to debug/check that we are still in the announcing period expected. Maybe assert the condition instead of logging and continuing?

I used it to display the announcement period, because the first election starts in Announcing stage while subsequent test elections starts in Idle, that way the announcing period between the first and the second election was the same and so the addresses for the candidates

I have removed it now

mnaamani · 2023-02-15T11:09:07Z

-export default async function electCouncil({ api, query }: FlowProps): Promise<void> {
+export default async function electCouncil(props: FlowProps): Promise<void> {
  const debug = extendDebug('flow:elect-council')
+  const { api, query } = props


all changes in this file seem no longer necessary

mnaamani · 2023-02-15T11:09:59Z

 # Mini-secret or mnemonic used in SURI for deterministic key derivation
 SURI_MINI_SECRET=""
+# Single councilor account SURI used for testing
+COUNCILLOR_SURI=//Councillor


this can be removed now.

Co-authored-by: Mokhtar Naamani <mokhtar.naamani@gmail.com>

Lezek123 · 2023-02-15T12:28:32Z

+    if (stage.isAnnouncing) {
+      return announcementPeriodId.toNumber()
+    } else {
+      return announcementPeriodId.toNumber() + 1
+    }


This will not always work, because:

It doesn't take into account the "block reserve" (ie. whether the announcing period has less than blockReserve blocks left)

There are a few more actions ran after this check, for example, the BuyMembershipHappyCaseFixture. After those actions are finished, the stage may already be different (if the announcing period was close to an end).

One way to solve this would be to update the controller accounts of council members after they are already elected. I don't think you even need to include the election cycle number in this case.

That might be best., that's how we do it for the working group workers ->

joystream/tests/network-tests/src/Api.ts

Line 846 in 997c5ae

async assignWorkerWellknownAccount(

There's no way to change a controller account for a member or councilor once set

You can do it via update_accounts extrinsic in the membership module

😃 good catch I have added the suggested fix + assertion

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

Lezek123 · 2023-02-15T18:42:04Z

    )
+
+    // change accounts to known accounts
+    const oldCouncilMemberAccounts = await this.getCouncilMembersControllerAccounts()


Instead of having to fetch those accounts here, you can query for the member's rootAccount in updateMemberControllerAccount, I think that will be simpler. (update_accounts extrinsic should be called using member's rootAccount, not controllerAccount, although in the integration tests they are usually the same)

Lezek123 · 2023-02-15T18:44:09Z

+    const newCouncilMemberAccounts = await this.api.updateCouncillorsAccounts(
+      oldCouncilMemberAccounts,
+      this.councilMembersIds
+    )


updateCouncillorsAccounts can query api.query.council.councilMembers, so that they don't need to be passed as an argument.

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

Lezek123 · 2023-02-16T09:54:45Z

+    const debug = extendDebug('api-factory')
+    debug(`assigning Well Known Councillors Account`)
+    const newAccounts = memberIds.map((id) => {
+      const uri = `//Councillor//` + id.toString()


I noticed there is a small issue here. Because the uri starts with // and isFinalPath = false, the final uri becomes:

`${miniSecret}//testing////Councillor//${memberId}`

(notice 4 slashes before Councillor)

Lezek123

LGTM

fix: re-enable storage in CI & fix setupNewChain test

a56e839

ignazio-bovo added 2 commits February 7, 2023 21:24

fix: newchain multi storage

26e7b7d

fix: CI execution matrix

4b8bba8

dobertRowneySr requested a review from mnaamani February 8, 2023 08:00

ignazio-bovo added 4 commits February 8, 2023 11:48

fix: avoid playground timing out

fc9ae2a

fix: update timeout period

0c35625

fix: timeout period

c7b2e46

fix: params for deploying playground

8c1e23d

mnaamani requested a review from Lezek123 February 9, 2023 08:36

mnaamani added the ephesus label Feb 9, 2023

dobertRowneySr mentioned this pull request Feb 9, 2023

Ephesus runtime upgrade ci #4569

Merged

allow more time for playground to deploy before timeout

1a07f37

mnaamani requested changes Feb 9, 2023

View reviewed changes

Merge pull request #9 from mnaamani/ephesus-playground

43a6eb4

allow more time for playground to deploy before timeout

Lezek123 suggested changes Feb 10, 2023

View reviewed changes

Ignazio Bovo and others added 7 commits February 10, 2023 11:21

Update runtime/src/proposals_configuration/playground.rs

7fcb7e6

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

Update runtime/src/proposals_configuration/playground.rs

5b1b245

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

fix: revert blockReserve and set Idleperiod

5fdd6ed

fix: revert blockReserve and set Idleperiod

bb3bf6b

fix: configure option suri for single councillor

1b16c5f

fix: error in number of parameters

3e28352

fix: linter

677116f

Joystream deleted a comment from vercel Bot Feb 10, 2023

fix: refactor

353deb7

dobertRowneySr closed this Feb 14, 2023

dobertRowneySr reopened this Feb 14, 2023

dobertRowneySr changed the base branch from ephesus to master February 14, 2023 18:32

dobertRowneySr changed the base branch from master to ephesus February 14, 2023 18:32

ignazio-bovo added 3 commits February 14, 2023 19:50

fix: attempt to fix linter

39ad6ba

fix: attempt to fix linter

50dc5e2

Merge branch 'ephesus' into ephesus-CICD-fix

f1df704

mnaamani requested changes Feb 15, 2023

View reviewed changes

Ignazio Bovo and others added 3 commits February 15, 2023 13:04

Update devops/ansible/deploy-playground-playbook.yml

5f8fc1d

Co-authored-by: Mokhtar Naamani <mokhtar.naamani@gmail.com>

Update devops/ansible/deploy-playground-playbook.yml

1fd73b4

Co-authored-by: Mokhtar Naamani <mokhtar.naamani@gmail.com>

fix: address changes

fbc9b6d

dobertRowneySr requested a review from mnaamani February 15, 2023 12:08

Lezek123 reviewed Feb 15, 2023

View reviewed changes

Comment thread tests/network-tests/src/fixtures/council/ElectCouncilFixture.ts Outdated

Lezek123 suggested changes Feb 15, 2023

View reviewed changes

Ignazio Bovo and others added 4 commits February 15, 2023 14:06

Update tests/network-tests/src/fixtures/council/ElectCouncilFixture.ts

b142707

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

fix: period polling

1f3075f

fix: merge ephesus into this

f80c5f9

fix: add well known councillor accounts

7d683a4

dobertRowneySr requested a review from Lezek123 February 15, 2023 16:55

fix: prettier

59b8257

Lezek123 suggested changes Feb 15, 2023

View reviewed changes

Ignazio Bovo and others added 2 commits February 16, 2023 08:53

Update tests/network-tests/src/fixtures/council/ElectCouncilFixture.ts

a25ee06

Co-authored-by: Leszek Wiesner <leszek@jsgenesis.com>

fix: address changes

e8ad3bb

dobertRowneySr requested a review from Lezek123 February 16, 2023 08:27

Lezek123 suggested changes Feb 16, 2023

View reviewed changes

fix: suri path generation

9e369bd

dobertRowneySr requested a review from Lezek123 February 16, 2023 10:07

Lezek123 approved these changes Feb 16, 2023

View reviewed changes

mnaamani approved these changes Feb 20, 2023

View reviewed changes

mnaamani merged commit c3bd50e into Joystream:ephesus Feb 20, 2023

Conversation

dobertRowneySr commented Feb 7, 2023 • edited by sync-by-unito Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dobertRowneySr commented Feb 8, 2023

Uh oh!

dobertRowneySr commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dobertRowneySr commented Feb 8, 2023

Uh oh!

mnaamani commented Feb 9, 2023

Uh oh!

mnaamani commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mnaamani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dobertRowneySr Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnaamani 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!

Uh oh!

Uh oh!

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!

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!

Lezek123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dobertRowneySr commented Feb 7, 2023 •

edited by sync-by-unito Bot

Loading

vercel Bot commented Feb 7, 2023 •

edited

Loading

dobertRowneySr commented Feb 8, 2023 •

edited

Loading

mnaamani commented Feb 9, 2023 •

edited

Loading

dobertRowneySr Feb 10, 2023 •

edited

Loading