review feedback applied

.dockerignore

@@ -1 +1,2 @@
-**target*
+**target*
+**node_modules*

README.md

@@ -82,9 +82,12 @@ cargo test
 ### API integration tests
 
 ```bash
+./scripts/run-dev-chain.sh
 yarn test
 ```
 
+To run the integration tests with a different chain, you can omit step running the local development chain and set the node URL using `NODE_URL` environment variable.
+
 ## Joystream Runtime
 
 ![Joystream Runtime](./runtime/runtime-banner.svg)

runtime/src/lib.rs

@@ -246,7 +246,7 @@ parameter_types! {
     pub const TransferFee: u128 = 0;
     pub const CreationFee: u128 = 0;
     pub const TransactionBaseFee: u128 = 1;
-    pub const TransactionByteFee: u128 = 0;
+    pub const TransactionByteFee: u128 = 1;
     pub const InitialMembersBalance: u32 = 2000;
 }
 

tests/.env

@@ -1,4 +1,8 @@
+# Address of the Joystream node.
 NODE_URL = ws://127.0.0.1:9944
-SUDO_ACCOUNT_URL = //Alice
+# Account which is expected to provide sufficient funds to test accounts.
+SUDO_ACCOUNT_URI = //Alice
+# Amount of members able to buy membership in membership creation test.
 MEMBERSHIP_CREATION_N = 1
+# ID of the membership paid terms used in membership creation test.
 MEMBERSHIP_PAID_TERMS = 0

tests/src/tests/membershipCreationTest.ts

@@ -14,13 +14,13 @@ describe('Membership integration tests', () => {
   const N: number = +process.env.MEMBERSHIP_CREATION_N!;
   const paidTerms: number = +process.env.MEMBERSHIP_PAID_TERMS!;
   const nodeUrl: string = process.env.NODE_URL!;
-  const sudoUri: string = process.env.SUDO_ACCOUNT_URL!;
+  const sudoUri: string = process.env.SUDO_ACCOUNT_URI!;
   const defaultTimeout: number = 30000;
   let apiMethods: ApiMethods;
   let sudo: KeyringPair;
   let aKeyPair: KeyringPair;
-  let membershipFee: number;
-  let membershipTransactionFee: number;
+  let membershipFee: BN;
+  let membershipTransactionFee: BN;
 
   before(async function () {
     this.timeout(defaultTimeout);
@@ -40,46 +40,48 @@ describe('Membership integration tests', () => {
     );
     let nonce = await apiMethods.getNonce(sudo);
     nonce = nonce.sub(new BN(1));
-    await apiMethods.transferBalanceToAccounts(sudo, nKeyPairs, membershipFee + membershipTransactionFee, nonce);
-    await apiMethods.transferBalance(sudo, aKeyPair.address, membershipTransactionFee * 2);
+    await apiMethods.transferBalanceToAccounts(
+      sudo,
+      nKeyPairs,
+      membershipTransactionFee.add(new BN(membershipFee)),
+      nonce
+    );
+    await apiMethods.transferBalance(sudo, aKeyPair.address, membershipTransactionFee);
   });
 
   it('Buy membeship is accepted with sufficient funds', async () => {
     await Promise.all(
-      nKeyPairs.map(async keyPair => {
-        await apiMethods.buyMembership(keyPair, paidTerms, 'new_member');
+      nKeyPairs.map(async (keyPair, index) => {
+        await apiMethods.buyMembership(keyPair, paidTerms, `new_member_${index}`);
       })
     );
-    nKeyPairs.map(keyPair =>
+    nKeyPairs.forEach((keyPair, index) =>
       apiMethods
         .getMembership(keyPair.address)
-        .then(membership => assert(!membership.isEmpty, 'Account m is not a member'))
+        .then(membership => assert(!membership.isEmpty, `Account ${index} is not a member`))
     );
   }).timeout(defaultTimeout);
 
-  it('Accont A has insufficient funds to buy membership', async () => {
+  it('Account A can not buy the membership with insufficient funds', async () => {
     apiMethods
       .getBalance(aKeyPair.address)
       .then(balance =>
-        assert(balance.toNumber() < membershipFee, 'Account A already have sufficient balance to purchase membership')
+        assert(
+          balance.toBn() < membershipFee.add(membershipTransactionFee),
+          'Account A already have sufficient balance to purchase membership'
+        )
       );
-  }).timeout(defaultTimeout);
-
-  it('Account A can not buy the membership with insufficient funds', async () => {
     await apiMethods.buyMembership(aKeyPair, paidTerms, 'late_member', true);
     apiMethods.getMembership(aKeyPair.address).then(membership => assert(membership.isEmpty, 'Account A is a member'));
   }).timeout(defaultTimeout);
 
-  it('Account A has been provided with funds to buy the membership', async () => {
-    await apiMethods.transferBalance(sudo, aKeyPair.address, membershipFee);
+  it('Account A was able to buy the membership with insufficient funds', async () => {
+    await apiMethods.transferBalance(sudo, aKeyPair.address, membershipFee.add(membershipTransactionFee));
     apiMethods
       .getBalance(aKeyPair.address)
       .then(balance =>
-        assert(balance.toNumber() >= membershipFee, 'The account balance is insufficient to purchase membership')
+        assert(balance.toBn() >= membershipFee, 'The account balance is insufficient to purchase membership')
       );
-  }).timeout(defaultTimeout);
-
-  it('Account A was able to buy the membership', async () => {
     await apiMethods.buyMembership(aKeyPair, paidTerms, 'late_member');
     apiMethods
       .getMembership(aKeyPair.address)

tests/src/utils/apiMethods.ts

@@ -1,21 +1,25 @@
 import { ApiPromise, WsProvider } from '@polkadot/api';
 import { Option } from '@polkadot/types';
 import { KeyringPair } from '@polkadot/keyring/types';
-import { Utils } from './utils';
 import { UserInfo, PaidMembershipTerms } from '@joystream/types/lib/members';
-import { Balance } from '@polkadot/types/interfaces';
+import { Balance, Index } from '@polkadot/types/interfaces';
 import BN = require('bn.js');
 import { SubmittableExtrinsic } from '@polkadot/api/types';
+import { Sender } from './sender';
+import { Utils } from './utils';
 
 export class ApiMethods {
+  private readonly api: ApiPromise;
+  private readonly sender: Sender;
+
   public static async create(provider: WsProvider): Promise<ApiMethods> {
     const api = await ApiPromise.create({ provider });
     return new ApiMethods(api);
   }
 
-  private readonly api: ApiPromise;
   constructor(api: ApiPromise) {
     this.api = api;
+    this.sender = new Sender(api);
   }
 
   public close() {
@@ -44,7 +48,7 @@ export class ApiMethods {
     return this.api.query.balances.freeBalance<Balance>(address);
   }
 
-  public async transferBalance(from: KeyringPair, to: string, amount: number, nonce: BN = new BN(-1)): Promise<void> {
+  public async transferBalance(from: KeyringPair, to: string, amount: BN, nonce: BN = new BN(-1)): Promise<void> {
     const _nonce = nonce.isNeg() ? await this.getNonce(from) : nonce;
     return Utils.signAndSend(this.api.tx.balances.transfer(to, amount), from, _nonce);
   }
@@ -53,16 +57,11 @@ export class ApiMethods {
     return this.api.query.members.paidMembershipTermsById<Option<PaidMembershipTerms>>(paidTermsId);
   }
 
-  public getMembershipFee(paidTermsId: number): Promise<number> {
-    return this.getPaidMembershipTerms(paidTermsId).then(terms => terms.unwrap().fee.toNumber());
+  public getMembershipFee(paidTermsId: number): Promise<BN> {
+    return this.getPaidMembershipTerms(paidTermsId).then(terms => terms.unwrap().fee.toBn());
   }
 
-  public async transferBalanceToAccounts(
-    from: KeyringPair,
-    to: KeyringPair[],
-    amount: number,
-    nonce: BN
-  ): Promise<void[]> {
+  public async transferBalanceToAccounts(from: KeyringPair, to: KeyringPair[], amount: BN, nonce: BN): Promise<void[]> {
     return Promise.all(
       to.map(async keyPair => {
         nonce = nonce.add(new BN(1));
@@ -72,26 +71,23 @@ export class ApiMethods {
   }
 
   public getNonce(account: KeyringPair): Promise<BN> {
-    return this.api.query.system.accountNonce(account.address).then(nonce => new BN(nonce.toString()));
+    return this.api.query.system.accountNonce<Index>(account.address);
   }
 
-  private getBaseTxFee(): number {
-    return this.api.createType('BalanceOf', this.api.consts.transactionPayment.transactionBaseFee).toNumber();
+  private getBaseTxFee(): BN {
+    return this.api.createType('BalanceOf', this.api.consts.transactionPayment.transactionBaseFee).toBn();
   }
 
-  private estimateTxFee(tx: SubmittableExtrinsic<'promise'>): number {
-    const baseFee: number = this.getBaseTxFee();
-    const byteFee: number = this.api
-      .createType('BalanceOf', this.api.consts.transactionPayment.transactionByteFee)
-      .toNumber();
-    return tx.toHex().length * byteFee + baseFee;
+  private estimateTxFee(tx: SubmittableExtrinsic<'promise'>): BN {
+    const baseFee: BN = this.getBaseTxFee();
+    const byteFee: BN = this.api.createType('BalanceOf', this.api.consts.transactionPayment.transactionByteFee).toBn();
+    return Utils.calcTxLength(tx).mul(byteFee).add(baseFee);
   }
 
-  public estimateBuyMembershipFee(account: KeyringPair, paidTermsId: number, name: string): number {
+  public estimateBuyMembershipFee(account: KeyringPair, paidTermsId: number, name: string): BN {
     const nonce: BN = new BN(0);
-    const signedTx = this.api.tx.members
-      .buyMembership(paidTermsId, new UserInfo({ handle: name, avatar_uri: '', about: '' }))
-      .sign(account, { nonce });
-    return this.estimateTxFee(signedTx);
+    return this.estimateTxFee(
+      this.api.tx.members.buyMembership(paidTermsId, new UserInfo({ handle: name, avatar_uri: '', about: '' }))
+    );
   }
 }

tests/src/utils/sender.ts

@@ -0,0 +1,28 @@
+import BN = require('bn.js');
+import { ApiPromise } from '@polkadot/api';
+import { Index } from '@polkadot/types/interfaces';
+import { SubmittableExtrinsic } from '@polkadot/api/types';
+import { KeyringPair } from '@polkadot/keyring/types';
+
+export class Sender {
+  private readonly api: ApiPromise;
+  private nonceMap: Map<string, BN> = new Map();
+
+  constructor(api: ApiPromise) {
+    this.api = api;
+  }
+
+  private async getNonce(address: string): Promise<BN> {
+    let nonce: BN | undefined = this.nonceMap.get(address);
+    if (!nonce) {
+      nonce = await this.api.query.system.accountNonce<Index>(address);
+    }
+    let nextNonce: BN = nonce.addn(1);
+    this.nonceMap.set(address, nextNonce);
+    return nonce;
+  }
+
+  private clearNonce(address: string): void {
+    this.nonceMap.delete(address);
+  }
+}

tests/src/utils/utils.ts

@@ -1,8 +1,26 @@
 import { KeyringPair } from '@polkadot/keyring/types';
 import { SubmittableExtrinsic } from '@polkadot/api/types';
+import { IExtrinsic } from '@polkadot/types/types';
+import { compactToU8a } from '@polkadot/util';
 import BN = require('bn.js');
 
 export class Utils {
+  private static LENGTH_ADDRESS = 32 + 1; // publicKey + prefix
+  private static LENGTH_ERA = 2; // assuming mortals
+  private static LENGTH_SIGNATURE = 64; // assuming ed25519 or sr25519
+  private static LENGTH_VERSION = 1; // 0x80 & version
+
+  public static calcTxLength = (extrinsic?: IExtrinsic | null, nonce?: BN): BN => {
+    return new BN(
+      Utils.LENGTH_VERSION +
+        Utils.LENGTH_ADDRESS +
+        Utils.LENGTH_SIGNATURE +
+        Utils.LENGTH_ERA +
+        compactToU8a(nonce || 0).length +
+        (extrinsic ? extrinsic.encodedLength : 0)
+    );
+  };
+
   public static async signAndSend(
     tx: SubmittableExtrinsic<'promise'>,
     account: KeyringPair,
@@ -10,6 +28,7 @@ export class Utils {
     expectFailure = false
   ): Promise<void> {
     return new Promise(async (resolve, reject) => {
+      // let nonce: BN = await this.getNonce(account.address);
       const signedTx = tx.sign(account, { nonce });
 
       await signedTx
@@ -26,6 +45,10 @@ export class Utils {
             });
             resolve();
           }
+          if (result.status.isFuture) {
+            this.clearNonce(account.address);
+            reject(new Error('Extrinsic nonce is in future'));
+          }
         })
         .catch(error => {
           reject(error);