Feature/nicaea runtime upgrade

[gleb-urvanov]

Runtime upgrade test from Constantinople to Nicaea.
Includes #769

[mnaamani]

I'm currently getting some build errors:

MokoBook:joystream mokhtar$ yarn workspace joystream-testing build
yarn workspace v1.22.4
yarn run v1.22.4
$ tsc --build tsconfig.json
src/constantinople/tests/proposals/impl/storageRoleParametersProposal.ts:7:32 - error TS2307: Cannot find module '@constantinople/types/roles'.

7 import { RoleParameters } from '@constantinople/types/roles';
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/nicaea/utils/utils.ts:7:22 - error TS2307: Cannot find module '@nicaea/types/lib/council'.

7 import { Seat } from '@nicaea/types/lib/council';

I think for the first error you we are missing the lib/ path (the older types library)

and in the second case, with the new types library we should remove the lib/ from the path.

[mnaamani]

There are some other build errors probably worth looking into as well, perhaps some indication of incorrect versions of @types/mocha and @types/jest with the version of typescript we are using?

../../node_modules/@types/mocha/index.d.ts:2680:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'beforeEach' must be of type 'Lifecycle', but here has type 'HookFunction'.

2680 declare var beforeEach: Mocha.HookFunction;
                 ~~~~~~~~~~

  ../../node_modules/@types/jest/index.d.ts:34:13
    34 declare var beforeEach: jest.Lifecycle;
                   ~~~~~~~~~~
    'beforeEach' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2698:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'afterEach' must be of type 'Lifecycle', but here has type 'HookFunction'.

2698 declare var afterEach: Mocha.HookFunction;
                 ~~~~~~~~~

  ../../node_modules/@types/jest/index.d.ts:36:13
    36 declare var afterEach: jest.Lifecycle;
                   ~~~~~~~~~
    'afterEach' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2714:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'describe' must be of type 'Describe', but here has type 'SuiteFunction'.

2714 declare var describe: Mocha.SuiteFunction;
                 ~~~~~~~~

  ../../node_modules/@types/jest/index.d.ts:37:13
    37 declare var describe: jest.Describe;
                   ~~~~~~~~
    'describe' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2735:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'xdescribe' must be of type 'Describe', but here has type 'PendingSuiteFunction'.

2735 declare var xdescribe: Mocha.PendingSuiteFunction;
                 ~~~~~~~~~

  ../../node_modules/@types/jest/index.d.ts:39:13
    39 declare var xdescribe: jest.Describe;
                   ~~~~~~~~~
    'xdescribe' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2749:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'it' must be of type 'It', but here has type 'TestFunction'.

2749 declare var it: Mocha.TestFunction;
                 ~~

  ../../node_modules/@types/jest/index.d.ts:40:13
    40 declare var it: jest.It;
                   ~~
    'it' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2763:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'test' must be of type 'It', but here has type 'TestFunction'.

2763 declare var test: Mocha.TestFunction;
                 ~~~~

  ../../node_modules/@types/jest/index.d.ts:43:13
    43 declare var test: jest.It;
                   ~~~~
    'test' was also declared here.

../../node_modules/@types/mocha/index.d.ts:2770:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'xit' must be of type 'It', but here has type 'PendingTestFunction'.

2770 declare var xit: Mocha.PendingTestFunction;
                 ~~~

  ../../node_modules/@types/jest/index.d.ts:42:13
    42 declare var xit: jest.It;
                   ~~~
    'xit' was also declared here.


Found 7 errors.

[mnaamani]

I've only reviewed the ApiWrapper code so far in nicaea. Will continue review of the actual scenarios soon.

I noticed in most places when dealing with Id types, like ApplicationId, WorkerId etc. . you are just using BN instead. Although those types extend u64 which extends BN it is probably better to be type safe and use the real underlying type as defined in the types library. As there a particular reason you did this, and is there any potential problems with this approach?

[mnaamani]

when you have such a long list of arguments, I highly recommend passing them in as an object so you can have named arguments. This would be very hard to debug if arguments were passed in the wrong order.

[gleb-urvanov]

Done, created in dto/workingGroupOpening.ts

[mnaamani]

I was able to run the tests successfully so so far so good, obviously there will be alot of updates to the scenarios when you update from nicaea branch so will re test when ready.

 tap --files ts-node/register src/nicaea/tests/proposals/*Test.ts --files ts-node/register src/nicaea/tests/workingGroup/*Test.ts -T
 PASS  src/nicaea/tests/proposals/electionParametersProposalTest.ts 11 OK 3m
src/nicaea/tests/proposals/setLeadProposalTest.ts 2> ({"role_account":"AccountId","reward_relationship":"Option<u64>","inducted":"u32","stage":"{\"_enum\":{\"Active\":\"Null\",\"Exited\":\"{\\\"initiated_at_block_number\\\":\\\"u32\\\"}\"}}"},Linkage<u64>):: Input doesn't match output, received 0x00000000000038004863373011ac63d086c7f9d4b780f4ee4bc5e5a7651cd1a565ccf45ec69a3db6, created 0xb63d9ac65ef4cc65a5d11c65a7e5c54beef480b7d4f9c786d063ac1130376348003800000000
 PASS  src/nicaea/tests/proposals/setLeadProposalTest.ts 11 OK 3m
 PASS  src/nicaea/tests/proposals/spendingProposalTest.ts 11 OK 3m
 PASS  src/nicaea/tests/proposals/textProposalTest.ts 11 OK 3m
 PASS  src/nicaea/tests/proposals/updateRuntimeTest.ts 15 OK 3m
 PASS  src/nicaea/tests/proposals/validatorCountProposalTest.ts 11 OK 3m
 PASS  src/nicaea/tests/proposals/workingGroupMintCapacityProposalTest.ts 11 OK 3m
src/nicaea/tests/workingGroup/manageWorkerAsLeadTest.ts 2> ({"role_account_id":"AccountId","opening_id":"u64","member_id":"u64","application_id":"u64"},Linkage<u64>):: Input doesn't match output, received 0x000000000000000000000000000000000031000000000000000040c027a632f34aa9456572bf254b3eda2db9fd8ef40658c1687831efce3812a6, created 0xa61238ceef317868c15806f48efdb92dda3e4b25bf726545a94af332a627c040000000000000000031000000000000000000000000000000
src/nicaea/tests/workingGroup/manageWorkerAsLeadTest.ts 2> ({"opening_id":"u64","application_index_in_opening":"u32","add_to_opening_in_block":"u32","active_role_staking_id":"Option<u64>","active_application_staking_id":"Option<u64>","stage":"{\"_enum\":{\"Active\":\"Null\",\"Unstaking\":\"{\\\"deactivation_initiated\\\":\\\"u32\\\",\\\"cause\\\":\\\"{\\\\\\\"_enum\\\\\\\":[\\\\\\\"External\\\\\\\",\\\\\\\"Hired\\\\\\\",\\\\\\\"NotHired\\\\\\\",\\\\\\\"CrowdedOut\\\\\\\",\\\\\\\"OpeningCancelled\\\\\\\",\\\\\\\"ReviewPeriodExpired\\\\\\\",\\\\\\\"OpeningFilled\\\\\\\"]}\\\"}\",\"Inactive\":\"{\\\"deactivation_initiated\\\":\\\"u32\\\",\\\"deactivated\\\":\\\"u32\\\",\\\"cause\\\":\\\"{\\\\\\\"_enum\\\\\\\":[\\\\\\\"External\\\\\\\",\\\\\\\"Hired\\\\\\\",\\\\\\\"NotHired\\\\\\\",\\\\\\\"CrowdedOut\\\\\\\",\\\\\\\"OpeningCancelled\\\\\\\",\\\\\\\"ReviewPeriodExpired\\\\\\\",\\\\\\\"OpeningFilled\\\\\\\"]}\\\"}\"}}","human_readable_text":"Text"},Linkage<u64>):: Input doesn't match output, received 0x000064383435386637612000000000000000000801000000000000000701000000d8000000000000000000000000, created 0x000000000000000000000000d800000001070000000000000001080000000000000000206137663835343864
src/nicaea/tests/workingGroup/manageWorkerAsLeadTest.ts 2> ({"member_id":"u64","role_account_id":"AccountId","reward_relationship":"Option<u64>","role_stake_profile":"Option<{\"stake_id\":\"u64\",\"termination_unstaking_period\":\"Option<u32>\",\"exit_unstaking_period\":\"Option<u32>\"}>"},Linkage<u64>):: Input doesn't match output, received 0x00000000000101000000010100000000000000070100000000000000080140c027a632f34aa9456572bf254b3eda2db9fd8ef40658c1687831efce3812a60000000000000031, created 0x3100000000000000a61238ceef317868c15806f48efdb92dda3e4b25bf726545a94af332a627c04001080000000000000001070000000000000001010000000101000000
src/nicaea/tests/workingGroup/manageWorkerAsLeadTest.ts 2> ({"role_account_id":"AccountId","opening_id":"u64","member_id":"u64","application_id":"u64"},Linkage<u64>):: Input doesn't match output, received 0x000000000000000000000000000000000031000000000000000040c027a632f34aa9456572bf254b3eda2db9fd8ef40658c1687831efce3812a6, created 0xa61238ceef317868c15806f48efdb92dda3e4b25bf726545a94af332a627c040000000000000000031000000000000000000000000000000
 PASS  src/nicaea/tests/workingGroup/manageWorkerAsLeadTest.ts 27 OK 4m
 PASS  src/nicaea/tests/workingGroup/manageWorkerAsWorkerTest.ts 21 OK 3m
 PASS  src/nicaea/tests/workingGroup/workerApplicationHappyCaseTest.ts 20 OK 3m
 PASS  src/nicaea/tests/workingGroup/workerApplicationRejectionCaseTest.ts 20 OK 3m

[gleb-urvanov]

@mnaamani Does the approach to use BN instead of exact ID types breaks type safety? If so, I will use the exact ID types in ApiWrapper.

[gleb-urvanov]

I have to further investigate errors connected with @types/mocha and @types/mocha. These does not affect tests functionality.

[mnaamani]

@mnaamani Does the approach to use BN instead of exact ID types breaks type safety? If so, I will use the exact ID types in ApiWrapper.

It does to some extent yes, although the u32, u64, u128 all extend BN, I think when you loose the exact type information when constructing transaction arguments, it can lead to incorrect encoding of the transaction. So I'm more concerned with that aspect. using primitive javascript number would be safer than just plain BN. But we might as well use the "exact" type.

We did have a brief discussion on this with @Lezek123 in #742 (comment)

Perhaps i'm being too paranoid, and if you pass a BN as an argument in a api.tx.module.method(bn_a, bn_b) the constructor for each argument type will then effectively do, new u64/u128(bn_a), but then again you could be trying to use a number that is outside the range. :)

[mnaamani]

From the linting failure in the CI checks, you will most likely need to checkin the yarn.lock file:

error Your lockfile needs to be updated, but yarn was run with '--frozen-lockfile'.

[mnaamani]

yarn debug command is failing:

$ tap --files src/nicaea/tests/workingGroup/atLeastValueBugTest.ts -T
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2> 
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2> /Users/mokhtar/joystream/joystream/node_modules/tap/node_modules/ts-node/src/index.ts:434
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2>     return new TSError(diagnosticText, diagnosticCodes)
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2>            ^
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2> TSError: ⨯ Unable to compile TypeScript:
src/nicaea/tests/workingGroup/atLeastValueBugTest.ts 2> src/nicaea/utils/apiWrapper.ts(11,22): error TS2307: Cannot find module '@nicaea/types/lib/council'.

The lib/ in '@nicaea/types/lib/council' is no longer required in latest types library.

[mnaamani]

All tests passing 👍