remove concept of expectFailure from Api layer, move to fixtures

tests/network-tests/src/Api.ts

@@ -110,25 +110,19 @@ export class Api {
     }
   }
 
-  public async makeSudoCall(tx: SubmittableExtrinsic<'promise'>, expectFailure = false): Promise<ISubmittableResult> {
+  public async makeSudoCall(tx: SubmittableExtrinsic<'promise'>): Promise<ISubmittableResult> {
     const sudo = await this.api.query.sudo.key()
-    return this.sender.signAndSend(this.api.tx.sudo.sudo(tx), sudo, expectFailure)
+    return this.sender.signAndSend(this.api.tx.sudo.sudo(tx), sudo)
   }
 
   public createPaidTermId(value: BN): PaidTermId {
     return this.api.createType('PaidTermId', value)
   }
 
-  public async buyMembership(
-    account: string,
-    paidTermsId: PaidTermId,
-    name: string,
-    expectFailure = false
-  ): Promise<ISubmittableResult> {
+  public async buyMembership(account: string, paidTermsId: PaidTermId, name: string): Promise<ISubmittableResult> {
     return this.sender.signAndSend(
       this.api.tx.members.buyMembership(paidTermsId, /* Handle: */ name, /* Avatar uri: */ '', /* About: */ ''),
-      account,
-      expectFailure
+      account
     )
   }
 
@@ -552,7 +546,7 @@ export class Api {
   }
 
   private applyForCouncilElection(account: string, amount: BN): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx.councilElection.apply(amount), account, false)
+    return this.sender.signAndSend(this.api.tx.councilElection.apply(amount), account)
   }
 
   public batchApplyForCouncilElection(accounts: string[], amount: BN): Promise<void[]> {
@@ -569,7 +563,7 @@ export class Api {
 
   private voteForCouncilMember(account: string, nominee: string, salt: string, stake: BN): Promise<ISubmittableResult> {
     const hashedVote: string = Utils.hashVote(nominee, salt)
-    return this.sender.signAndSend(this.api.tx.councilElection.vote(hashedVote, stake), account, false)
+    return this.sender.signAndSend(this.api.tx.councilElection.vote(hashedVote, stake), account)
   }
 
   public batchVoteForCouncilMember(accounts: string[], nominees: string[], salt: string[], stake: BN): Promise<void[]> {
@@ -581,7 +575,7 @@ export class Api {
   }
 
   private revealVote(account: string, commitment: string, nominee: string, salt: string): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx.councilElection.reveal(commitment, nominee, salt), account, false)
+    return this.sender.signAndSend(this.api.tx.councilElection.reveal(commitment, nominee, salt), account)
   }
 
   public batchRevealVote(accounts: string[], nominees: string[], salt: string[]): Promise<void[]> {
@@ -594,19 +588,19 @@ export class Api {
   }
 
   public sudoStartAnnouncingPeriod(endsAtBlock: BN): Promise<ISubmittableResult> {
-    return this.makeSudoCall(this.api.tx.councilElection.setStageAnnouncing(endsAtBlock), false)
+    return this.makeSudoCall(this.api.tx.councilElection.setStageAnnouncing(endsAtBlock))
   }
 
   public sudoStartVotingPeriod(endsAtBlock: BN): Promise<ISubmittableResult> {
-    return this.makeSudoCall(this.api.tx.councilElection.setStageVoting(endsAtBlock), false)
+    return this.makeSudoCall(this.api.tx.councilElection.setStageVoting(endsAtBlock))
   }
 
   public sudoStartRevealingPeriod(endsAtBlock: BN): Promise<ISubmittableResult> {
-    return this.makeSudoCall(this.api.tx.councilElection.setStageRevealing(endsAtBlock), false)
+    return this.makeSudoCall(this.api.tx.councilElection.setStageRevealing(endsAtBlock))
   }
 
   public sudoSetCouncilMintCapacity(capacity: BN): Promise<ISubmittableResult> {
-    return this.makeSudoCall(this.api.tx.council.setCouncilMintCapacity(capacity), false)
+    return this.makeSudoCall(this.api.tx.council.setCouncilMintCapacity(capacity))
   }
 
   public getBestBlock(): Promise<BN> {
@@ -640,8 +634,7 @@ export class Api {
     const memberId: MemberId = (await this.getMemberIds(account))[0]
     return this.sender.signAndSend(
       this.api.tx.proposalsCodex.createRuntimeUpgradeProposal(memberId, name, description, stake, runtime),
-      account,
-      false
+      account
     )
   }
 
@@ -655,8 +648,7 @@ export class Api {
     const memberId: MemberId = (await this.getMemberIds(account))[0]
     return this.sender.signAndSend(
       this.api.tx.proposalsCodex.createTextProposal(memberId, name, description, stake, text),
-      account,
-      false
+      account
     )
   }
 
@@ -671,8 +663,7 @@ export class Api {
     const memberId: MemberId = (await this.getMemberIds(account))[0]
     return this.sender.signAndSend(
       this.api.tx.proposalsCodex.createSpendingProposal(memberId, title, description, stake, balance, destination),
-      account,
-      false
+      account
     )
   }
 
@@ -686,8 +677,7 @@ export class Api {
     const memberId: MemberId = (await this.getMemberIds(account))[0]
     return this.sender.signAndSend(
       this.api.tx.proposalsCodex.createSetValidatorCountProposal(memberId, title, description, stake, validatorCount),
-      account,
-      false
+      account
     )
   }
 
@@ -717,8 +707,7 @@ export class Api {
         min_council_stake: minCouncilStake,
         min_voting_stake: minVotingStake,
       }),
-      account,
-      false
+      account
     )
   }
 
@@ -740,13 +729,12 @@ export class Api {
         openingId,
         this.api.createType('WorkingGroup', workingGroup)
       ),
-      account,
-      false
+      account
     )
   }
 
   public approveProposal(account: string, memberId: MemberId, proposal: ProposalId): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx.proposalsEngine.vote(memberId, proposal, 'Approve'), account, false)
+    return this.sender.signAndSend(this.api.tx.proposalsEngine.vote(memberId, proposal, 'Approve'), account)
   }
 
   public async batchApproveProposal(proposal: ProposalId): Promise<void[]> {
@@ -949,8 +937,7 @@ export class Api {
       text: string
       type: string
     },
-    module: WorkingGroups,
-    expectFailure: boolean
+    module: WorkingGroups
   ): Promise<ISubmittableResult> {
     const activateAt: ActivateOpeningAt = this.api.createType(
       'ActivateOpeningAt',
@@ -1015,8 +1002,7 @@ export class Api {
 
     return this.sender.signAndSend(
       this.createAddOpeningTransaction(activateAt, commitment, openingParameters.text, openingParameters.type, module),
-      lead,
-      expectFailure
+      lead
     )
   }
 
@@ -1107,8 +1093,7 @@ export class Api {
     })
 
     return this.makeSudoCall(
-      this.createAddOpeningTransaction(activateAt, commitment, openingParameters.text, openingParameters.type, module),
-      false
+      this.createAddOpeningTransaction(activateAt, commitment, openingParameters.text, openingParameters.type, module)
     )
   }
 
@@ -1206,8 +1191,7 @@ export class Api {
           working_group: leaderOpening.workingGroup,
         }
       ),
-      leaderOpening.account,
-      false
+      leaderOpening.account
     )
   }
 
@@ -1244,8 +1228,7 @@ export class Api {
         fillOpening.proposalStake,
         fillOpeningParameters
       ),
-      fillOpening.account,
-      false
+      fillOpening.account
     )
   }
 
@@ -1273,8 +1256,7 @@ export class Api {
           'working_group': workingGroup,
         }
       ),
-      account,
-      false
+      account
     )
   }
 
@@ -1298,8 +1280,7 @@ export class Api {
         rewardAmount,
         this.api.createType('WorkingGroup', workingGroup)
       ),
-      account,
-      false
+      account
     )
   }
 
@@ -1323,8 +1304,7 @@ export class Api {
         rewardAmount,
         this.api.createType('WorkingGroup', workingGroup)
       ),
-      account,
-      false
+      account
     )
   }
 
@@ -1348,8 +1328,7 @@ export class Api {
         rewardAmount,
         this.api.createType('WorkingGroup', workingGroup)
       ),
-      account,
-      false
+      account
     )
   }
 
@@ -1371,8 +1350,7 @@ export class Api {
         mintCapacity,
         this.api.createType('WorkingGroup', workingGroup)
       ),
-      account,
-      false
+      account
     )
   }
 
@@ -1391,7 +1369,7 @@ export class Api {
     openingId: OpeningId,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].acceptApplications(openingId), leader, false)
+    return this.sender.signAndSend(this.api.tx[module].acceptApplications(openingId), leader)
   }
 
   public async beginApplicantReview(
@@ -1399,11 +1377,11 @@ export class Api {
     openingId: OpeningId,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].beginApplicantReview(openingId), leader, false)
+    return this.sender.signAndSend(this.api.tx[module].beginApplicantReview(openingId), leader)
   }
 
   public async sudoBeginApplicantReview(openingId: OpeningId, module: WorkingGroups): Promise<ISubmittableResult> {
-    return this.makeSudoCall(this.api.tx[module].beginApplicantReview(openingId), false)
+    return this.makeSudoCall(this.api.tx[module].beginApplicantReview(openingId))
   }
 
   public async applyOnOpening(
@@ -1413,14 +1391,12 @@ export class Api {
     roleStake: BN,
     applicantStake: BN,
     text: string,
-    expectFailure: boolean,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
     const memberId: MemberId = (await this.getMemberIds(account))[0]
     return this.sender.signAndSend(
       this.api.tx[module].applyOnOpening(memberId, openingId, roleAccountAddress, roleStake, applicantStake, text),
-      account,
-      expectFailure
+      account
     )
   }
 
@@ -1430,12 +1406,11 @@ export class Api {
     roleStake: BN,
     applicantStake: BN,
     text: string,
-    module: WorkingGroups,
-    expectFailure: boolean
+    module: WorkingGroups
   ): Promise<ISubmittableResult[]> {
     return Promise.all(
       accounts.map(async (account) =>
-        this.applyOnOpening(account, account, openingId, roleStake, applicantStake, text, expectFailure, module)
+        this.applyOnOpening(account, account, openingId, roleStake, applicantStake, text, module)
       )
     )
   }
@@ -1455,8 +1430,7 @@ export class Api {
         next_payment_at_block: nextPaymentBlock,
         payout_interval: payoutInterval,
       }),
-      leader,
-      false
+      leader
     )
   }
 
@@ -1473,8 +1447,7 @@ export class Api {
         'amount_per_payout': amountPerPayout,
         'next_payment_at_block': nextPaymentBlock,
         'payout_interval': payoutInterval,
-      }),
-      false
+      })
     )
   }
 
@@ -1484,27 +1457,25 @@ export class Api {
     stake: BN,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].increaseStake(workerId, stake), worker, false)
+    return this.sender.signAndSend(this.api.tx[module].increaseStake(workerId, stake), worker)
   }
 
   public async decreaseStake(
     leader: string,
     workerId: WorkerId,
     stake: BN,
-    module: WorkingGroups,
-    expectFailure: boolean
+    module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].decreaseStake(workerId, stake), leader, expectFailure)
+    return this.sender.signAndSend(this.api.tx[module].decreaseStake(workerId, stake), leader)
   }
 
   public async slashStake(
     leader: string,
     workerId: WorkerId,
     stake: BN,
-    module: WorkingGroups,
-    expectFailure: boolean
+    module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].slashStake(workerId, stake), leader, expectFailure)
+    return this.sender.signAndSend(this.api.tx[module].slashStake(workerId, stake), leader)
   }
 
   public async updateRoleAccount(
@@ -1513,7 +1484,7 @@ export class Api {
     newRoleAccount: string,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].updateRoleAccount(workerId, newRoleAccount), worker, false)
+    return this.sender.signAndSend(this.api.tx[module].updateRoleAccount(workerId, newRoleAccount), worker)
   }
 
   public async updateRewardAccount(
@@ -1522,7 +1493,7 @@ export class Api {
     newRewardAccount: string,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].updateRewardAccount(workerId, newRewardAccount), worker, false)
+    return this.sender.signAndSend(this.api.tx[module].updateRewardAccount(workerId, newRewardAccount), worker)
   }
 
   public async withdrawApplication(
@@ -1530,7 +1501,7 @@ export class Api {
     applicationId: ApplicationId,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].withdrawApplication(applicationId), account, false)
+    return this.sender.signAndSend(this.api.tx[module].withdrawApplication(applicationId), account)
   }
 
   public async batchWithdrawActiveApplications(
@@ -1557,7 +1528,7 @@ export class Api {
     applicationId: ApplicationId,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].terminateApplication(applicationId), leader, false)
+    return this.sender.signAndSend(this.api.tx[module].terminateApplication(applicationId), leader)
   }
 
   public async batchTerminateApplication(
@@ -1572,33 +1543,26 @@ export class Api {
     leader: string,
     workerId: WorkerId,
     text: string,
-    module: WorkingGroups,
-    expectFailure: boolean
+    module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].terminateRole(workerId, text, false), leader, expectFailure)
+    return this.sender.signAndSend(this.api.tx[module].terminateRole(workerId, text, false), leader)
   }
 
   public async leaveRole(
     account: string,
     workerId: WorkerId,
     text: string,
-    expectFailure: boolean,
     module: WorkingGroups
   ): Promise<ISubmittableResult> {
-    return this.sender.signAndSend(this.api.tx[module].leaveRole(workerId, text), account, expectFailure)
+    return this.sender.signAndSend(this.api.tx[module].leaveRole(workerId, text), account)
   }
 
-  public async batchLeaveRole(
-    workerIds: WorkerId[],
-    text: string,
-    expectFailure: boolean,
-    module: WorkingGroups
-  ): Promise<void[]> {
+  public async batchLeaveRole(workerIds: WorkerId[], text: string, module: WorkingGroups): Promise<void[]> {
     return Promise.all(
       workerIds.map(async (workerId) => {
         // get role_account of worker
         const worker = await this.getWorkerById(workerId, module)
-        await this.leaveRole(worker.role_account_id.toString(), workerId, text, expectFailure, module)
+        await this.leaveRole(worker.role_account_id.toString(), workerId, text, module)
       })
     )
   }
@@ -1758,7 +1722,7 @@ export class Api {
       operations // We provide parsed operations as second argument
     )
     const lead = (await this.getGroupLead(WorkingGroups.ContentDirectoryWorkingGroup)) as Worker
-    await this.sender.signAndSend(transaction, lead.role_account_id, false)
+    await this.sender.signAndSend(transaction, lead.role_account_id)
   }
 
   public async createChannelEntity(channel: ChannelEntity): Promise<void> {

tests/network-tests/src/Fixture.ts

@@ -1,30 +1,72 @@
 import { Api } from './Api'
 
 export interface Fixture {
-  runner(expectFailure: boolean): Promise<void>
+  run(): Promise<void>
+  executionError(): Error | undefined
 }
 
-// Fixture that measures start and end blocks
-// ensures fixture only runs once
 export class BaseFixture implements Fixture {
   protected api: Api
-  private ran = false
+  private failed = false
+  private executed = false
+  // The reason of the "Unexpected" failure of running the fixture
+  private err: Error | undefined
 
   constructor(api: Api) {
     this.api = api
-    // record starting block
   }
 
-  public async runner(expectFailure: boolean): Promise<void> {
+  public async run(): Promise<void> {
+    this.executed = true
+    await this.execute()
+  }
+
+  protected async execute(): Promise<void> {
+    return
+  }
+
+  // Used by execution implementation to signal failure
+  protected error(err: Error): void {
+    this.failed = true
+    this.err = err
+  }
+
+  public didExecute(): boolean {
+    return this.executed
+  }
+
+  public didFail(): boolean {
+    return this.failed
+  }
+
+  public executionError(): Error | undefined {
+    if (!this.didExecute()) {
+      throw new Error('Trying to check execution result before running fixture')
+    }
+    return this.err
+  }
+}
+
+// Runs a fixture and measures how long it took to run
+// Ensures fixture only runs once
+export class FixtureRunner {
+  private fixture: Fixture
+  private ran = false
+
+  constructor(fixture: Fixture) {
+    this.fixture = fixture
+  }
+
+  public async run(): Promise<Error | undefined> {
     if (this.ran) {
-      return
+      throw new Error('Fixture already ran')
     }
+
     this.ran = true
-    return this.execute(expectFailure)
-    // record end blocks
-  }
+    // record starting block
+    await this.fixture.run()
+    // record ending block
 
-  protected async execute(expectFailure: boolean): Promise<void> {
-    return
+    return this.fixture.executionError()
   }
 }

tests/network-tests/src/fixtures/contentDirectoryModule.ts

@@ -1,9 +1,4 @@
 import { QueryNodeApi } from '../Api'
-import BN from 'bn.js'
-import { assert } from 'chai'
-import { Seat } from '@joystream/types/council'
-import { v4 as uuid } from 'uuid'
-import { Utils } from '../utils'
 import { Fixture } from '../Fixture'
 import { ChannelEntity } from '@joystream/cd-schemas/types/entities/ChannelEntity'
 import { VideoEntity } from '@joystream/cd-schemas/types/entities/VideoEntity'

tests/network-tests/src/fixtures/membershipModule.ts

@@ -1,9 +1,10 @@
 import { Api } from '../Api'
 import BN from 'bn.js'
 import { assert } from 'chai'
-import { Fixture, BaseFixture } from '../Fixture'
+import { BaseFixture } from '../Fixture'
 import { PaidTermId, MemberId } from '@joystream/types/members'
 import Debugger from 'debug'
+// import { DispatchError } from '@polkadot/types/interfaces/system'
 
 export class BuyMembershipHappyCaseFixture extends BaseFixture {
   private accounts: string[]
@@ -22,7 +23,7 @@ export class BuyMembershipHappyCaseFixture extends BaseFixture {
     return this.memberIds.slice()
   }
 
-  public async execute(expectFailure: boolean): Promise<void> {
+  async execute(): Promise<void> {
     this.debug(`Registering ${this.accounts.length} new members`)
     // Fee estimation and transfer
     const membershipFee: BN = await this.api.getMembershipFee(this.paidTerms)
@@ -42,26 +43,24 @@ export class BuyMembershipHappyCaseFixture extends BaseFixture {
     ).map(({ events }) => this.api.expectMemberRegisteredEvent(events))
 
     this.debug(`New member ids: ${this.memberIds}`)
-    if (expectFailure) {
-      throw new Error('Successful fixture run while expecting failure')
-    }
   }
 }
 
-export class BuyMembershipWithInsufficienFundsFixture implements Fixture {
-  private api: Api
+export class BuyMembershipWithInsufficienFundsFixture extends BaseFixture {
   private account: string
   private paidTerms: PaidTermId
 
   public constructor(api: Api, account: string, paidTerms: PaidTermId) {
-    this.api = api
+    super(api)
     this.account = account
     this.paidTerms = paidTerms
   }
 
-  public async runner(expectFailure: boolean) {
+  async execute(): Promise<void> {
     // Assertions
-    this.api.getMemberIds(this.account).then((membership) => assert(membership.length === 0, 'Account A is a member'))
+    const membership = await this.api.getMemberIds(this.account)
+
+    assert(membership.length === 0, 'Account must not be associated with a member')
 
     // Fee estimation and transfer
     const membershipFee: BN = await this.api.getMembershipFee(this.paidTerms)
@@ -70,25 +69,25 @@ export class BuyMembershipWithInsufficienFundsFixture implements Fixture {
       this.paidTerms,
       'member_name_which_is_longer_than_expected'
     )
-    this.api.treasuryTransferBalance(this.account, membershipTransactionFee)
 
-    // Balance assertion
-    await this.api
-      .getBalance(this.account)
-      .then((balance) =>
-        assert(
-          balance.toBn() < membershipFee.add(membershipTransactionFee),
-          'Account A already have sufficient balance to purchase membership'
-        )
-      )
+    // Only provide enough funds for transaction fee but not enough to cover the membership fee
+    await this.api.treasuryTransferBalance(this.account, membershipTransactionFee)
 
-    // Buying memebership
-    await this.api.buyMembership(this.account, this.paidTerms, `late_member_${this.account.substring(0, 8)}`, true)
+    const balance = await this.api.getBalance(this.account)
 
-    // Assertions
-    this.api.getMemberIds(this.account).then((membership) => assert(membership.length === 0, 'Account A is a member'))
-    if (expectFailure) {
-      throw new Error('Successful fixture run while expecting failure')
+    assert(
+      balance.toBn() < membershipFee.add(membershipTransactionFee),
+      'Account already has sufficient balance to purchase membership'
+    )
+
+    try {
+      await this.api.buyMembership(this.account, this.paidTerms, `late_member_${this.account.substring(0, 8)}`)
+      this.error(new Error('Buying membership with insufficient funds should have failed'))
+    } catch (dispatchError) {
+      // We expect buying membership with insuffiencet funds to fail
+      // const err = dispatchError as DispatchError
+      // Assert its the exact error we expect?
+      // assert.eq(err, ... )
     }
   }
 }

tests/network-tests/src/flows/membership/creatingMemberships.ts

@@ -6,31 +6,32 @@ import {
 import { PaidTermId } from '@joystream/types/members'
 import BN from 'bn.js'
 import Debugger from 'debug'
+import { FixtureRunner } from '../../Fixture'
+import { assert } from 'chai'
 
-export default async function membershipCreation(api: Api, env: NodeJS.ProcessEnv) {
+export default async function membershipCreation(api: Api, env: NodeJS.ProcessEnv): Promise<void> {
   const debug = Debugger('flow:memberships')
   debug('started')
 
   const N: number = +env.MEMBERSHIP_CREATION_N!
+  assert(N > 0)
   const nAccounts = api.createKeyPairs(N).map((key) => key.address)
   const aAccount = api.createKeyPairs(1)[0].address
   const paidTerms: PaidTermId = api.createPaidTermId(new BN(+env.MEMBERSHIP_PAID_TERMS!))
 
+  // Assert membership can be bought if sufficient funds are available
   const happyCaseFixture = new BuyMembershipHappyCaseFixture(api, nAccounts, paidTerms)
-  // Buy membeship is accepted with sufficient funds
-  await happyCaseFixture.runner(false)
+  assert.equal(await new FixtureRunner(happyCaseFixture).run(), undefined)
 
+  // Assert account can not buy the membership with insufficient funds
   const insufficientFundsFixture: BuyMembershipWithInsufficienFundsFixture = new BuyMembershipWithInsufficienFundsFixture(
     api,
     aAccount,
     paidTerms
   )
-  // Account A can not buy the membership with insufficient funds
-  await insufficientFundsFixture.runner(false)
+  assert.equal(await new FixtureRunner(insufficientFundsFixture).run(), undefined)
 
+  // Assert account was able to buy the membership with sufficient funds
   const buyMembershipAfterAccountTopUp = new BuyMembershipHappyCaseFixture(api, [aAccount], paidTerms)
-
-  // Account A was able to buy the membership with sufficient funds
-  await buyMembershipAfterAccountTopUp.runner(false)
-  debug('finished')
+  assert.equal(await new FixtureRunner(buyMembershipAfterAccountTopUp).run(), undefined)
 }

tests/network-tests/src/sender.ts

@@ -1,10 +1,13 @@
 import { ApiPromise, Keyring } from '@polkadot/api'
 import { SubmittableExtrinsic } from '@polkadot/api/types'
-import { ISubmittableResult } from '@polkadot/types/types/'
-import { AccountId } from '@polkadot/types/interfaces'
+import { ISubmittableResult, AnyJson } from '@polkadot/types/types/'
+import { AccountId, EventRecord } from '@polkadot/types/interfaces'
+import { DispatchError, DispatchResult } from '@polkadot/types/interfaces/system'
+import { TypeRegistry } from '@polkadot/types'
 import { KeyringPair } from '@polkadot/keyring/types'
 import Debugger from 'debug'
 import AsyncLock from 'async-lock'
+import { assert } from 'chai'
 
 const debug = Debugger('sender')
 
@@ -21,49 +24,89 @@ export class Sender {
 
   // Synchronize all sending of transactions into mempool, so we can always safely read
   // the next account nonce taking mempool into account. This is safe as long as all sending of transactions
-  // from same account occurs in the same process.
-  // Returns a promise that resolves or rejects only after the extrinsic is finalized into a block.
+  // from same account occurs in the same process. Returns a promise.
+  // The promise resolves on successful dispatch (normal and sudo dispatches).
+  // The promise is rejected undef following conditions:
+  // - transaction fails to submit to node. (Encoding issues, bad signature or nonce etc.)
+  // - dispatch error after the extrinsic is finalized into a block.
+  // - successful sudo call (correct sudo key used) but dispatched call fails.
   public async signAndSend(
     tx: SubmittableExtrinsic<'promise'>,
-    account: AccountId | string,
-    shouldFail = false
+    account: AccountId | string
   ): Promise<ISubmittableResult> {
     const addr = this.keyring.encodeAddress(account)
     const senderKeyPair: KeyringPair = this.keyring.getPair(addr)
 
     let finalizedResolve: { (result: ISubmittableResult): void }
-    let finalizedReject: { (err: Error): void }
+    let finalizedReject: { (err: DispatchError): void }
     const finalized: Promise<ISubmittableResult> = new Promise(async (resolve, reject) => {
       finalizedResolve = resolve
       finalizedReject = reject
     })
 
-    const handleEvents = (result: ISubmittableResult) => {
-      if (result.status.isInBlock && result.events !== undefined) {
-        result.events.forEach((event) => {
-          if (event.event.method === 'ExtrinsicFailed') {
-            if (shouldFail) {
-              finalizedResolve(result)
-            } else {
-              finalizedReject(new Error('Extrinsic failed unexpectedly'))
-            }
-          }
-        })
-        finalizedResolve(result)
-      }
+    // saved human representation of the signed tx, will be set before it is submitted.
+    // On error it is logged to help in debugging.
+    let sentTx: AnyJson
 
+    const handleEvents = (result: ISubmittableResult) => {
       if (result.status.isFuture) {
         // Its virtually impossible for use to continue with tests
         // when this occurs and we don't expect the tests to handle this correctly
         // so just abort!
         process.exit(-1)
       }
+
+      if (!result.status.isInBlock) {
+        return
+      }
+
+      const success = result.findRecord('system', 'ExtrinsicSuccess')
+      const failed = result.findRecord('system', 'ExtrinsicFailed')
+
+      if (success) {
+        const sudid = result.findRecord('sudo', 'Sudid')
+        if (sudid) {
+          const dispatchResult = sudid.event.data[0] as DispatchResult
+          if (dispatchResult.isOk) {
+            debug(`Successful Sudo Tx: ${sentTx}`)
+            finalizedResolve(result)
+          } else if (dispatchResult.isError) {
+            const err = dispatchResult.asError
+            debug(`Sudo Error: FailedTx: ${sentTx} dispatch error: ${err.toHuman()}`)
+            if (err.isModule) {
+              const { name, documentation } = (this.api.registry as TypeRegistry).findMetaError(err.asModule)
+              debug(`${name}\n${documentation}`)
+            }
+            finalizedReject(err)
+          } else {
+            // What other result type can it be??
+            assert(false)
+          }
+        } else {
+          debug(`Successful Tx: ${sentTx}`)
+          finalizedResolve(result)
+        }
+      } else {
+        assert(failed)
+        const record = failed as EventRecord
+        const {
+          event: { data },
+        } = record
+        const err = (data[0] as DispatchResult).asError // data[0] as DispatchError
+        debug(`FailedTx: ${sentTx} dispatch error: ${err.toHuman()}`)
+        if (err.isModule) {
+          const { name, documentation } = (this.api.registry as TypeRegistry).findMetaError(err.asModule)
+          debug(`${name}\n${documentation}`)
+        }
+        finalizedReject(err)
+      }
     }
 
     await this.asyncLock.acquire(`${senderKeyPair.address}`, async () => {
       const nonce = await this.api.rpc.system.accountNextIndex(senderKeyPair.address)
       const signedTx = tx.sign(senderKeyPair, { nonce })
-      await signedTx.send(handleEvents)
+      sentTx = signedTx.toHuman()
+      return signedTx.send(handleEvents)
     })
 
     return finalized