From 9b15a373d3003066610e09fddd013d7e8235e873 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Tue, 16 Jul 2019 12:12:17 +0200 Subject: [PATCH] Signing cleanups (w/ additional tests) (#1134) --- packages/api/src/SubmittableExtrinsic.ts | 43 ++--- packages/api/test/e2e/promise-tx.spec.ts | 147 ++++++++++-------- packages/api/test/util/SingleAccountSigner.ts | 13 +- 3 files changed, 112 insertions(+), 91 deletions(-) diff --git a/packages/api/src/SubmittableExtrinsic.ts b/packages/api/src/SubmittableExtrinsic.ts index e89680ac52..2618c1d1ea 100644 --- a/packages/api/src/SubmittableExtrinsic.ts +++ b/packages/api/src/SubmittableExtrinsic.ts @@ -50,6 +50,10 @@ interface SignerOptions { const ONE_MINUTE = 15; const DEFAULT_MORTAL_LENGTH = 5 * ONE_MINUTE; +function isKeyringPair (account: string | IKeyringPair | AccountId | Address): account is IKeyringPair { + return isFunction((account as IKeyringPair).sign); +} + export class SubmittableResult implements ISubmittableResult { public readonly events: EventRecord[]; @@ -163,15 +167,16 @@ export default function createSubmittableExtrinsic ( ); } - function expandOptions (options: Partial): SignatureOptions { + function expandOptions (options: Partial, extras: { blockHash?: Hash; era?: ExtrinsicEra; nonce?: Index }): SignatureOptions { return { blockHash: api.genesisHash, version: api.runtimeVersion, - ...options + ...options, + ...extras } as unknown as SignatureOptions; } - function setupEraOptions (header: Header | null, options: Partial): Partial { + function expandEraOptions (options: Partial, { header, nonce }: { header: Header | null; nonce: Index }): SignatureOptions { if (!header) { if (isNumber(options.era)) { // since we have no header, it is immortal, remove any option overrides @@ -180,18 +185,19 @@ export default function createSubmittableExtrinsic ( delete options.blockHash; } - return {}; + return expandOptions(options, { nonce }); } const { blockNumber, hash } = header; - return { + return expandOptions(options, { blockHash: hash, era: new ExtrinsicEra({ current: blockNumber, period: options.era || DEFAULT_MORTAL_LENGTH - }) - }; + }), + nonce + }); } const signOrigin = _extrinsic.sign; @@ -207,14 +213,14 @@ export default function createSubmittableExtrinsic ( } }, sign: { - value: function (account: IKeyringPair, _options: Partial): SubmittableExtrinsic { + value: function (account: IKeyringPair, optionOrNonce: Partial): SubmittableExtrinsic { // HACK here we actually override nonce if it was specified (backwards compat for // the previous signature - don't let userspace break, but allow then time to upgrade) - const options: Partial = isBn(_options) || isNumber(_options) - ? { nonce: _options as any as number } - : _options; + const options: Partial = isBn(optionOrNonce) || isNumber(optionOrNonce) + ? { nonce: optionOrNonce } + : optionOrNonce; - signOrigin.apply(_extrinsic, [account, expandOptions(options)]); + signOrigin.apply(_extrinsic, [account, expandOptions(options, {})]); return this; } @@ -230,8 +236,7 @@ export default function createSubmittableExtrinsic ( } const isSubscription = _noStatusCb || !!statusCb; - const isKeyringPair = isFunction((account as IKeyringPair).sign); - const address = isKeyringPair ? (account as IKeyringPair).address : account.toString(); + const address = isKeyringPair(account) ? account.address : account.toString(); let updateId: number | undefined; return decorateMethod( @@ -250,13 +255,13 @@ export default function createSubmittableExtrinsic ( ).pipe( first(), mergeMap(async ([nonce, header]): Promise => { - const eraOptions = setupEraOptions(header, options); + const eraOptions = expandEraOptions(options, { header, nonce }); - if (isKeyringPair) { - this.sign(account as IKeyringPair, { ...options, ...eraOptions, nonce }); + if (isKeyringPair(account)) { + this.sign(account, eraOptions); } else if (api.signer) { updateId = await api.signer.sign(_extrinsic, address, { - ...expandOptions({ ...options, ...eraOptions, nonce }), + ...eraOptions, blockNumber: header ? header.blockNumber : new BN(0), genesisHash: api.genesisHash }); @@ -267,7 +272,7 @@ export default function createSubmittableExtrinsic ( switchMap((): Observable | Observable => { return isSubscription ? subscribeObservable(updateId) - : sendObservable(updateId) as any; // ??? + : sendObservable(updateId); }) ) as Observable) )(statusCb); diff --git a/packages/api/test/e2e/promise-tx.spec.ts b/packages/api/test/e2e/promise-tx.spec.ts index afafe9ac3f..e9a4c48385 100644 --- a/packages/api/test/e2e/promise-tx.spec.ts +++ b/packages/api/test/e2e/promise-tx.spec.ts @@ -6,7 +6,7 @@ import Keyring from '@polkadot/keyring'; import testingPairs from '@polkadot/keyring/testingPairs'; import WsProvider from '@polkadot/rpc-provider/ws'; import { randomAsHex } from '@polkadot/util-crypto'; -import { Balance, EventRecord, ExtrinsicEra, Hash, Header, Index, SignedBlock } from '@polkadot/types'; +import { EventRecord, ExtrinsicEra, Hash, Header, Index, SignedBlock } from '@polkadot/types'; import { SubmittableResult } from './../../src'; import ApiPromise from '../../src/promise'; @@ -79,45 +79,6 @@ describeE2E({ .signAndSend(keyring.charlie, { era: 0 }, logEvents(done)); }); - it('makes a transfer (signAndSend via Signer)', async (done): Promise<() => void> => { - const signer = new SingleAccountSigner(keyring.charlie) as Signer; - - api.setSigner(signer); - - return api.tx.balances - .transfer(keyring.eve.address, 12345) - .signAndSend(keyring.charlie.address, logEvents(done)); - }); - - it('makes a transfer (signAndSend via Signer) with undefined Signer', async (): Promise => { - const signer: any = undefined; - // no signer - api.setSigner(signer); - - await expect(api.tx.balances - .transfer(keyring.eve.address, 12345) - .signAndSend(keyring.alice.address)).rejects.toThrow('no signer exists'); - }); - - it('makes a transfer (signAndSend via Signer) with the wrong keyring pair', async (): Promise => { - const signer: Signer = new SingleAccountSigner(keyring.dave); - - api.setSigner(signer); - - // no callback - await expect(api.tx.balances - .transfer(keyring.eve.address, 12345) - .signAndSend(keyring.alice.address)).rejects.toThrow('does not have the keyringPair'); - }); - - it('makes a transfer (signAndSend via Signer) with the wrong keyring pair with a callback', async (): Promise => { - // with callback - await expect(api.tx.balances - .transfer(keyring.eve.address, 12345) - // eslint-disable-next-line @typescript-eslint/no-unused-vars - .signAndSend(keyring.alice.address, (cb: any): void => { /* do nothing */ })).rejects.toThrow('does not have the keyringPair'); - }); - it('makes a transfer (no callback)', async (): Promise => { const hash = await api.tx.balances .transfer(keyring.eve.address, 12345) @@ -158,22 +119,33 @@ describeE2E({ }); }); + // TODO split this into a separate file describe('eras', (): void => { it('makes a transfer (specified era)', async (done): Promise => { const signedBlock = await api.rpc.chain.getBlock() as SignedBlock; const currentHeight = signedBlock.block.header.number; const exERA = new ExtrinsicEra({ current: currentHeight, period: 10 }); const ex = api.tx.balances.transfer(keyring.eve.address, 12345); - const hash = await ex.signAndSend(keyring.charlie, { + + await ex.signAndSend(keyring.charlie, { blockHash: signedBlock.block.header.hash, era: exERA - }); - - expect(hash.toHex()).toHaveLength(66); - done(); + }, logEvents(done)); }); - it('makes a transfer with invalid time', async (done): Promise<() => void> => { + it('makes a transfer (specified era, previous block)', async (done): Promise => { + const signedBlock = await api.rpc.chain.getBlock() as SignedBlock; + const currentHeight = signedBlock.block.header.number.subn(1); + const exERA = new ExtrinsicEra({ current: currentHeight, period: 10 }); + const ex = api.tx.balances.transfer(keyring.eve.address, 12345); + + await ex.signAndSend(keyring.charlie, { + blockHash: signedBlock.block.header.parentHash, + era: exERA + }, logEvents(done)); + }); + + it('fails on a transfer with invalid time', async (done): Promise => { const nonce = await api.query.system.accountNonce(keyring.alice.address) as Index; const signedBlock = await api.rpc.chain.getBlock() as SignedBlock; const currentHeight = signedBlock.block.header.number; @@ -182,38 +154,75 @@ describeE2E({ const blockHash = signedBlock.block.header.hash; const ex = api.tx.balances.transfer(keyring.eve.address, 12345); - return ( - api.rpc.chain.subscribeNewHead(async (header: Header): Promise => { - if (header.blockNumber.toNumber() === eraDeath - 1) { - try { - await ex.signAndSend(keyring.alice, { blockHash, era: exERA, nonce } as any); - } catch (error) { - expect(error.message).toMatch(/1010: Invalid Transaction \(0\)/); - done(); - } + await api.rpc.chain.subscribeNewHead(async (header: Header): Promise => { + if (header.blockNumber.toNumber() === eraDeath - 1) { + try { + await ex.signAndSend(keyring.alice, { blockHash, era: exERA, nonce } as any); + } catch (error) { + expect(error.message).toMatch(/1010: Invalid Transaction \(0\)/); + done(); } - }) - ); + } + }); }); it('makes a transfer with custom numeric era', async (done): Promise => { - const hash = await api.tx.balances + await api.tx.balances .transfer(keyring.eve.address, 12345) - .signAndSend(keyring.charlie, { era: 2 }); + .signAndSend(keyring.charlie, { era: 2 }, logEvents(done)); + }); + }); - expect(hash.toHex()).toHaveLength(66); - done(); + // TODO split this into a seperate file + describe('Signer injection', (): void => { + it('makes a transfer (signAndSend via Signer)', async (done): Promise => { + const signer = new SingleAccountSigner(keyring.charlie); + + api.setSigner(signer); + + await api.tx.balances + .transfer(keyring.eve.address, 12345) + .signAndSend(keyring.charlie.address, logEvents(done)); }); - it('should fire an error with invalid transaction', async (done): Promise => { - const aliceBalance = await api.query.balances.freeBalance(keyring.alice.address) as Balance; + it('succeeds when waiting some blocks before submission', async (done): Promise => { + // 10 second delay + const signer = new SingleAccountSigner(keyring.charlie, 10000); - return api.tx.balances - .transfer(keyring.bob.address, aliceBalance.muln(2)) - .signAndSend(keyring.alice).catch((error): void => { - expect(error.message).toMatch(/1010: Invalid Transaction \(0\)/); - done(); - }); + api.setSigner(signer); + + await api.tx.balances + .transfer(keyring.eve.address, 12345) + .signAndSend(keyring.charlie.address, logEvents(done)); + }); + + it('fails (signAndSend via Signer) with undefined Signer', async (): Promise => { + const signer: any = undefined; + // no signer + api.setSigner(signer); + + await expect(api.tx.balances + .transfer(keyring.eve.address, 12345) + .signAndSend(keyring.alice.address)).rejects.toThrow('no signer exists'); + }); + + it('fails (signAndSend via Signer) with the wrong keyring pair', async (): Promise => { + const signer: Signer = new SingleAccountSigner(keyring.dave); + + api.setSigner(signer); + + // no callback + await expect(api.tx.balances + .transfer(keyring.eve.address, 12345) + .signAndSend(keyring.alice.address)).rejects.toThrow('does not have the keyringPair'); + }); + + it('fails (signAndSend via Signer) with the wrong keyring pair with a callback', async (): Promise => { + // with callback + await expect(api.tx.balances + .transfer(keyring.eve.address, 12345) + // eslint-disable-next-line @typescript-eslint/no-unused-vars + .signAndSend(keyring.alice.address, (cb: any): void => { /* do nothing */ })).rejects.toThrow('no signer exists'); }); }); }); diff --git a/packages/api/test/util/SingleAccountSigner.ts b/packages/api/test/util/SingleAccountSigner.ts index bc529a49aa..a2bc2e6dc3 100644 --- a/packages/api/test/util/SingleAccountSigner.ts +++ b/packages/api/test/util/SingleAccountSigner.ts @@ -12,8 +12,11 @@ let id = 0; export default class SingleAccountSigner { private keyringPair: KeyringPair; - public constructor (keyringPair: KeyringPair) { + private signDelay: number; + + public constructor (keyringPair: KeyringPair, signDelay: number = 0) { this.keyringPair = keyringPair; + this.signDelay = signDelay; } public async sign (extrinsic: Extrinsic, address: string, options: SignatureOptions): Promise { @@ -21,8 +24,12 @@ export default class SingleAccountSigner { throw new Error('does not have the keyringPair'); } - extrinsic.sign(this.keyringPair, options); + return new Promise((resolve): void => { + setTimeout((): void => { + extrinsic.sign(this.keyringPair, options); - return ++id; + resolve(++id); + }, this.signDelay); + }); } }