Storage node conversion to the runtime version based on Substrate 2.0.0-rc4 #1189

shamil-gadelshin · 2020-08-26T11:28:08Z

Main changes:

updated runtime types: "@joystream/types": "^0.13.0",
updated API "@polkadot/api": "^1.26.1",
transaction status IsFinalized was changed to asCompleted

Lezek123 · 2020-08-28T15:59:39Z

storage-node/packages/runtime-api/balances.js

  async freeBalance(accountId) {
    const decoded = this.base.identities.keyring.decodeAddress(accountId, true)
-    return this.base.api.query.balances.freeBalance(decoded)
+    return (await this.base.api.derive.balances.all(decoded)).freeBalance


In this context I think (await this.base.api.derive.balances.all(decoded)).availableBalance may actually be more suitable, because freeBalance = avaialble + locked and if the balance is locked it seems like it cannot be transferred or used to pay the fee.

@Lezek123 is correct. The way we are using it we were not accounting for locked balance. It is only directly called by hasMinimumBalanceOf() which is the call that is really interested in a minimum transferable balance. We can fix this in a later PR.

Leszek's solution is only a single line. I'm including the fix in the current PR.

mnaamani · 2020-09-01T14:43:43Z

storage-node/packages/cli/src/commands/dev.ts

  const leadOpeningId = await api.workers.devAddStorageLeadOpening()
  const leadApplicationId = await api.workers.devApplyOnOpening(leadOpeningId, aliceMemberId, alice, alice)
-  api.workers.devBeginLeadOpeningReview(leadOpeningId)
+  await api.workers.devBeginLeadOpeningReview(leadOpeningId)


this await was left out so the two transactions can be finalized in the same block.

It works because the second call doesn't need to get any values from event(s) emitted by the first transaction.
Of course the second one will fail if the first fails.

So its just a speed optimization :)

Ok. I'm rolling it back.

mnaamani · 2020-09-01T14:58:11Z

storage-node/packages/runtime-api/balances.js

  async freeBalance(accountId) {
    const decoded = this.base.identities.keyring.decodeAddress(accountId, true)
-    return this.base.api.query.balances.freeBalance(decoded)
+    return (await this.base.api.derive.balances.all(decoded)).freeBalance


@Lezek123 is correct. The way we are using it we were not accounting for locked balance. It is only directly called by hasMinimumBalanceOf() which is the call that is really interested in a minimum transferable balance. We can fix this in a later PR.

mnaamani · 2020-09-01T15:08:12Z

storage-node/packages/runtime-api/workers.js

-        providers[id] = new Worker(value)
-      }
-    }
+    const entries = (await this.base.api.query.storageWorkingGroup.workerById.entries());


love the new api

mnaamani · 2020-09-01T15:18:04Z

The changes look good.

To build I had to fix call to ContentId.generate() to ContentId.generate(this.api.api.registry), in:

MokoBook:storage-node mokhtar$ git diff packages/cli/src/commands/upload.ts 
diff --git a/storage-node/packages/cli/src/commands/upload.ts b/storage-node/packages/cli/src/commands/upload.ts
index 7a1ff8cfe..f0a9f7c34 100644
--- a/storage-node/packages/cli/src/commands/upload.ts
+++ b/storage-node/packages/cli/src/commands/upload.ts
@@ -96,7 +96,7 @@ export class UploadCommand extends BaseCommand {
     return {
       accountId,
       ipfsCid: await this.computeIpfsHash(),
-      contentId: ContentId.generate(),
+      contentId: ContentId.generate(this.api.api.registry),
       fileSize: new BN(this.getFileSize()),
       dataObjectTypeId,
       memberId,

mnaamani · 2020-09-01T15:20:00Z

I'm seeing 9 linter errors that need to be fixed, we can fix in a separate PR?

Running yarn storage-cli dev-init works as expected after the build fix.

shamil-gadelshin · 2020-09-01T15:32:53Z

That is strange - I had successfully uploaded the video file during the testing.

mnaamani · 2020-09-02T05:52:02Z

That is strange - I had successfully uploaded the video file during the testing.

Is it possible that the packages wasn't rebuilt and used some pre-compiled code you had?

I noticed storage-node is the only project we have where a tsconfig.tsbuildinfo is built. And sometimes its presence is preventing some code from being recompiled if the source doesn't change.

mnaamani · 2020-09-02T06:20:26Z

Did some additional testing. LGTM.

One bug I just realized in the async getAllProviders() it returns all the worker ids (including the lead worker id). I opened an issue #1211

shamil-gadelshin added 3 commits August 26, 2020 13:26

storage-node: Fix dev-init command

48ecb1c

storage-node: Fix dev-check command.

d31a385

storage-node: Global yarn.lock update.

0f3f707

shamil-gadelshin added colossus alexandria-release labels Aug 26, 2020

shamil-gadelshin self-assigned this Aug 26, 2020

shamil-gadelshin requested a review from mnaamani August 26, 2020 12:14

Lezek123 reviewed Aug 28, 2020

View reviewed changes

mnaamani reviewed Sep 1, 2020

View reviewed changes

shamil-gadelshin added 2 commits September 1, 2020 19:00

storage-node: Fix linter errors.

6081788

storage-node: Review fixes.

c2115da

shamil-gadelshin requested review from Lezek123 and mnaamani September 1, 2020 16:45

mnaamani merged commit e88b6a2 into Joystream:iznik Sep 2, 2020

mnaamani mentioned this pull request Sep 2, 2020

Pioneer upgrade - final adjustments #1195

Merged

Storage node conversion to the runtime version based on Substrate 2.0.0-rc4 #1189

Storage node conversion to the runtime version based on Substrate 2.0.0-rc4 #1189

Uh oh!

Conversation

shamil-gadelshin commented Aug 26, 2020

Uh oh!

Lezek123 Aug 28, 2020 • 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 Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

shamil-gadelshin Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

mnaamani Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

shamil-gadelshin Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

mnaamani Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

mnaamani Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

mnaamani commented Sep 1, 2020

Uh oh!

mnaamani commented Sep 1, 2020

Uh oh!

shamil-gadelshin commented Sep 1, 2020

Uh oh!

mnaamani commented Sep 2, 2020

Uh oh!

mnaamani commented Sep 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Lezek123 Aug 28, 2020 •

edited

Loading