Add SignerPayload and signPayload for Signer (#1152)

* Add SignerPayload and signPayload for Signer

* Submittables use signPayload

* Don't pass version on deprecated interface

* Update after all e2e tx tests passes

* Comment around payload generation

* Adjust blocktime to default (nice round 50 block eras)

* Fix calc

* Re-add test comment

packages/api/src/SignerPayload.spec.ts

@@ -0,0 +1,77 @@
+// Copyright 2017-2019 @polkadot/api authors & contributors
+// This software may be modified and distributed under the terms
+// of the Apache-2.0 license. See the LICENSE file for details.
+
+import extrinsics from '@polkadot/api-metadata/extrinsics/static';
+import { Method, ExtrinsicEra, SignaturePayload } from '@polkadot/types';
+
+import SignerPayload from './SignerPayload';
+
+describe('SignerPayload', (): void => {
+  const TEST = {
+    address: '5DTestUPts3kjeXSTMyerHihn1uwMfLj8vU8sqF7qYrFabHE',
+    blockHash: '0xde8f69eeb5e065e18c6950ff708d7e551f68dc9bf59a07c52367c0280f805ec7',
+    blockNumber: '0x0000000000231d30',
+    era: '0x0703',
+    genesisHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
+    method: '0x0500ffd7568e5f0a7eda67a82691ff379ac4bba4f9c9b859fe779b5d46363b61ad2db9e56c',
+    nonce: '0x0000000000001234',
+    tip: '0x00000000000000000000000000005678',
+    version: 2
+  };
+
+  beforeEach((): void => {
+    Method.injectMethods(extrinsics);
+  });
+
+  it('creates a valid JSON output', (): void => {
+    expect(
+      new SignerPayload({
+        address: '5DTestUPts3kjeXSTMyerHihn1uwMfLj8vU8sqF7qYrFabHE',
+        blockHash: '0xde8f69eeb5e065e18c6950ff708d7e551f68dc9bf59a07c52367c0280f805ec7',
+        blockNumber: '0x231d30',
+        era: new ExtrinsicEra({ current: 2301232, period: 200 }),
+        genesisHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
+        method: new Method('0x0500ffd7568e5f0a7eda67a82691ff379ac4bba4f9c9b859fe779b5d46363b61ad2db9e56c'),
+        nonce: 0x1234,
+        tip: 0x5678,
+        version: 2
+      }).toPayload()
+    ).toEqual({
+      address: '5DTestUPts3kjeXSTMyerHihn1uwMfLj8vU8sqF7qYrFabHE',
+      blockHash: '0xde8f69eeb5e065e18c6950ff708d7e551f68dc9bf59a07c52367c0280f805ec7',
+      blockNumber: '0x0000000000231d30',
+      era: '0x0703',
+      genesisHash: '0xdcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b',
+      method: '0x0500ffd7568e5f0a7eda67a82691ff379ac4bba4f9c9b859fe779b5d46363b61ad2db9e56c',
+      nonce: '0x0000000000001234',
+      tip: '0x00000000000000000000000000005678',
+      version: 2
+    });
+  });
+
+  it('re-constructs from JSON', (): void => {
+    expect(
+      new SignerPayload(TEST).toPayload()
+    ).toEqual(TEST);
+  });
+
+  it('re-constructs from itself', (): void => {
+    expect(
+      new SignerPayload(
+        new SignerPayload(TEST)
+      ).toPayload()
+    ).toEqual(TEST);
+  });
+
+  it('can be used as a feed to SignaturePayload', (): void => {
+    const signer = new SignerPayload(TEST).toPayload();
+    const payload = new SignaturePayload(signer, signer.version);
+
+    expect(payload.era.toHex()).toEqual(TEST.era);
+    expect(payload.method.toHex()).toEqual(TEST.method);
+    expect(payload.blockHash.toHex()).toEqual(TEST.blockHash);
+    expect(payload.nonce.eq(TEST.nonce)).toBe(true);
+    expect(payload.tip.eq(TEST.tip)).toBe(true);
+  });
+});

packages/api/src/SignerPayload.ts

@@ -0,0 +1,57 @@
+// Copyright 2017-2019 @polkadot/api authors & contributors
+// This software may be modified and distributed under the terms
+// of the Apache-2.0 license. See the LICENSE file for details.
+
+import { SignerPayload as ISignerPayload } from './types';
+
+import { Address, Balance, BlockNumber, Compact, ExtrinsicEra, Hash, Nonce, Struct, U8, Method } from '@polkadot/types';
+
+export interface SignerPayloadType {
+  address: Address;
+  blockHash: Hash;
+  blockNumber: BlockNumber;
+  era: ExtrinsicEra;
+  genesisHash: Hash;
+  method: Method;
+  nonce: Compact;
+  tip: Compact;
+  version: U8;
+}
+
+export default class SignerPayload extends Struct.with({
+  address: Address,
+  blockHash: Hash,
+  blockNumber: BlockNumber,
+  era: ExtrinsicEra,
+  genesisHash: Hash,
+  method: Method,
+  nonce: Compact.with(Nonce),
+  tip: Compact.with(Balance),
+  version: U8
+}) {
+  /**
+   * @description Returns this as a SignerPayloadType. this works since the Struct.with injects all the getters automaticall
+   */
+  public get self (): SignerPayloadType {
+    return this as any as SignerPayloadType;
+  }
+
+  /**
+   * @description Creates an representation of the structure as an ISignerPayload JSON
+   */
+  public toPayload (): ISignerPayload {
+    const { address, blockHash, blockNumber, era, genesisHash, method, nonce, tip, version } = this.self;
+
+    return {
+      address: address.toString(),
+      blockHash: blockHash.toHex(),
+      blockNumber: blockNumber.toHex(),
+      era: era.toHex(),
+      genesisHash: genesisHash.toHex(),
+      method: method.toHex(),
+      nonce: nonce.toHex(),
+      tip: tip.toHex(),
+      version: version.toNumber()
+    };
+  }
+}

packages/api/src/SubmittableExtrinsic.ts

@@ -11,8 +11,9 @@ import { Observable, combineLatest, of } from 'rxjs';
 import { first, map, mergeMap, switchMap, tap } from 'rxjs/operators';
 import { isBn, isFunction, isNumber, isUndefined } from '@polkadot/util';
 
-import ApiBase from './Base';
 import filterEvents from './util/filterEvents';
+import ApiBase from './Base';
+import SignerPayload from './SignerPayload';
 
 // eslint-disable-next-line @typescript-eslint/interface-name-prefix
 export interface ISubmittableResult {
@@ -44,10 +45,14 @@ interface SignerOptions {
   blockHash: AnyU8a;
   era?: IExtrinsicEra | number;
   nonce: AnyNumber;
+  tip?: AnyNumber;
 }
 
-// pick a default - in the case of 4s blocktimes, this translates to 60 seconds
-const ONE_MINUTE = 15;
+// The default for 6s allowing for 5min eras. When translating this to faster blocks -
+//   - 4s = (10 / 15) * 5 = 3.33m
+//   - 2s = (10 / 30) * 5 = 1.66m
+const BLOCKTIME = 6;
+const ONE_MINUTE = 60 / BLOCKTIME;
 const DEFAULT_MORTAL_LENGTH = 5 * ONE_MINUTE;
 
 function isKeyringPair (account: string | IKeyringPair | AccountId | Address): account is IKeyringPair {
@@ -257,15 +262,39 @@ export default function createSubmittableExtrinsic<ApiType> (
               mergeMap(async ([nonce, header]): Promise<void> => {
                 const eraOptions = expandEraOptions(options, { header, nonce });
 
+                // FIXME This is becoming real messy with all the options - way past
+                // "a method should fit on a single screen" stage. (Probably want to
+                // clean this when we remove `api.signer.sign` in the next beta cycle)
                 if (isKeyringPair(account)) {
                   this.sign(account, eraOptions);
                 } else if (api.signer) {
-                  updateId = await api.signer.sign(_extrinsic, address, {
-                    ...eraOptions,
-                    blockNumber: header ? header.blockNumber : new BN(0),
-                    extrinsicVersion: this.api.extrinsicVersion,
-                    genesisHash: api.genesisHash
-                  });
+                  if (api.signer.signPayload) {
+                    const signPayload = new SignerPayload({
+                      ...eraOptions,
+                      address,
+                      method: _extrinsic.method,
+                      blockNumber: header ? header.blockNumber : 0,
+                      genesisHash: api.genesisHash,
+                      version: api.extrinsicVersion
+                    });
+                    const result = await api.signer.signPayload(signPayload.toPayload());
+
+                    // Here we explicitly call `toPayload()` again instead of working with an object
+                    // (reference) as passed to the signer. This means that we are sure that the
+                    // payload data is not modified from our inputs, but the signer
+                    _extrinsic.addSignature(address, result.signature, signPayload.toPayload());
+                    updateId = result.id;
+                  } else if (api.signer.sign) {
+                    console.warn('The Signer.sign interface is deprecated and will be removed in a future version, Swap to using the Signer.signPayload interface instead.');
+
+                    updateId = await api.signer.sign(_extrinsic, address, {
+                      ...eraOptions,
+                      blockNumber: header ? header.blockNumber : new BN(0),
+                      genesisHash: api.genesisHash
+                    });
+                  } else {
+                    throw new Error('Invalid signer interface');
+                  }
                 } else {
                   throw new Error('no signer exists');
                 }

packages/api/src/types.ts

@@ -226,15 +226,79 @@ export type ApiTypes = 'promise' | 'rxjs';
 
 export interface SignerOptions extends SignatureOptions {
   blockNumber: BN;
-  extrinsicVersion: number;
   genesisHash: Hash;
 }
 
+export interface SignerPayload {
+  /**
+   * @description The ss-58 encoded address
+   */
+  address: string;
+
+  /**
+   * @description The checkpoint hash of the block, in hex
+   */
+  blockHash: string;
+
+  /**
+   * @description The checkpoint block number, in hex
+   */
+  blockNumber: string;
+
+  /**
+   * @description The era for this transaction, in hex
+   */
+  era: string;
+
+  /**
+   * @description The benesis hash of the chain, in hex
+   */
+  genesisHash: string;
+
+  /**
+   * @description The encoded method (with arguments) in hex
+   */
+  method: string;
+
+  /**
+   * @description The nonce for this transaction, in hex
+   */
+  nonce: string;
+
+  /**
+   * @description The tip for this transaction, in hex
+   */
+  tip: string;
+
+  /**
+   * @description The version of the extrinsic we are dealing with
+   */
+  version: number;
+}
+
+export interface SignerResult {
+  /**
+   * @description The id for this request
+   */
+  id: number;
+
+  /**
+   * @description The resulting signature in hex
+   */
+  signature: string;
+}
+
 export interface Signer {
   /**
+   * @deprecated Implement and use signPayload instead
    * @description Signs an extrinsic, returning an id (>0) that can be used to retrieve updates
    */
-  sign (extrinsic: IExtrinsic, address: string, options: SignerOptions): Promise<number>;
+  sign?: (extrinsic: IExtrinsic, address: string, options: SignerOptions) => Promise<number>;
+
+  /**
+   * @description signs an extrinsic payload from a serialized form
+   */
+  signPayload (payload: SignerPayload): Promise<SignerResult>;
 
   /**
    * @description Receives an update for the extrinsic signed by a `signer.sign`

packages/api/test/e2e/api/promise-tx.spec.ts

@@ -41,6 +41,7 @@ describeE2E({
   let api: ApiPromise;
 
   beforeEach(async (done): Promise<void> => {
+    jest.setTimeout(30000);
     api = await ApiPromise.create(new WsProvider(wsUrl));
 
     done();
@@ -182,7 +183,10 @@ describeE2E({
           try {
             await ex.signAndSend(keyring.alice, { blockHash, era: exERA, nonce } as any);
           } catch (error) {
-            expect(error.message).toMatch(/1010: Invalid Transaction \(0\)/);
+            // NOTE This will fail on any version with v1 Extrinsics, the code returned there
+            // is simply (0), so it doesn't have an "invalid-era" specific message. (the -127
+            // error code is introduced along with the transaction version 2
+            expect(error.message).toMatch(/1010: Invalid Transaction \(-127\)/);
             done();
           }
         }
@@ -207,7 +211,8 @@ describeE2E({
           try {
             await ex.signAndSend(keyring.alice.address, { blockHash, era: exERA, nonce } as any);
           } catch (error) {
-            expect(error.message).toMatch(/1010: Invalid Transaction \(0\)/);
+            // NOTE As per above 0 vs -127 note
+            expect(error.message).toMatch(/1010: Invalid Transaction \(-127\)/);
             done();
           }
         }

packages/api/test/util/SingleAccountSigner.ts

@@ -2,14 +2,14 @@
 // This software may be modified and distributed under the terms
 // of the Apache-2.0 license. See the LICENSE file for details.
 
+import { Signer, SignerPayload, SignerResult } from '@polkadot/api/types';
 import { KeyringPair } from '@polkadot/keyring/types';
-import { SignatureOptions } from '@polkadot/types/types';
 
-import { Extrinsic } from '@polkadot/types';
+import { SignaturePayload } from '@polkadot/types';
 
 let id = 0;
 
-export default class SingleAccountSigner {
+export default class SingleAccountSigner implements Signer {
   private keyringPair: KeyringPair;
 
   private signDelay: number;
@@ -19,16 +19,32 @@ export default class SingleAccountSigner {
     this.signDelay = signDelay;
   }
 
-  public async sign (extrinsic: Extrinsic, address: string, options: SignatureOptions): Promise<number> {
-    if (!this.keyringPair || String(address) !== this.keyringPair.address) {
+  // @deprecated Kept here until we have it removed completely
+  // public async sign (extrinsic: IExtrinsic, address: string, options: SignatureOptions): Promise<number> {
+  //   if (!this.keyringPair || String(address) !== this.keyringPair.address) {
+  //     throw new Error('does not have the keyringPair');
+  //   }
+
+  //   return new Promise((resolve): void => {
+  //     setTimeout((): void => {
+  //       extrinsic.sign(this.keyringPair, options);
+
+  //       resolve(++id);
+  //     }, this.signDelay);
+  //   });
+  // }
+
+  public async signPayload (payload: SignerPayload): Promise<SignerResult> {
+    if (!this.keyringPair || payload.address !== this.keyringPair.address) {
       throw new Error('does not have the keyringPair');
     }
 
     return new Promise((resolve): void => {
       setTimeout((): void => {
-        extrinsic.sign(this.keyringPair, options);
+        const signed = new SignaturePayload(payload, payload.version).sign(this.keyringPair);
+        const result: SignerResult = { id: ++id, ...signed };
 
-        resolve(++id);
+        resolve(result);
       }, this.signDelay);
     });
   }

packages/types/src/primitive/Extrinsic/Extrinsic.ts

@@ -2,7 +2,7 @@
 // This software may be modified and distributed under the terms
 // of the Apache-2.0 license. See the LICENSE file for details.
 
-import { AnyU8a, ArgsDef, Codec, IExtrinsic, IHash, IKeyringPair, SignatureOptions } from '../../types';
+import { AnyU8a, ArgsDef, Codec, ExtrinsicPayloadValue, IExtrinsic, IHash, IKeyringPair, SignatureOptions } from '../../types';
 
 import { assert, isHex, isU8a, u8aConcat, u8aToHex, u8aToU8a } from '@polkadot/util';
 import { blake2AsU8a } from '@polkadot/util-crypto';
@@ -224,7 +224,7 @@ export default class Extrinsic extends Base<ExtrinsicV1 | ExtrinsicV2> implement
   /**
    * @description Add an [[ExtrinsicSignature]] to the extrinsic (already generated)
    */
-  public addSignature (signer: Address | Uint8Array | string, signature: Uint8Array | string, payload: Uint8Array | string): Extrinsic {
+  public addSignature (signer: Address | Uint8Array | string, signature: Uint8Array | string, payload: ExtrinsicPayloadValue | Uint8Array | string): Extrinsic {
     this.raw.addSignature(signer, signature, payload);
 
     return this;