Merge pull request #11 from confio/properly-sign-tx

Properly sign tx

src/address.spec.ts

@@ -1,7 +1,7 @@
 import { Address, Algorithm, PubkeyBytes } from "@iov/bcp";
 import { Encoding } from "@iov/encoding";
 
-import { decodeCosmosAddress, isValidAddress, pubkeyToAddress } from "./address";
+import { decodeCosmosAddress, decodeCosmosPubkey, isValidAddress, pubkeyToAddress } from "./address";
 
 const { fromBase64, fromHex } = Encoding;
 
@@ -28,6 +28,17 @@ describe("address", () => {
     });
   });
 
+  describe("decodeCosmosPubkey", () => {
+    it("works", () => {
+      expect(
+        decodeCosmosPubkey("cosmospub1addwnpepqd8sgxq7aw348ydctp3n5ajufgxp395hksxjzc6565yfp56scupfqhlgyg5"),
+      ).toEqual({
+        prefix: "cosmospub",
+        data: fromBase64("A08EGB7ro1ORuFhjOnZcSgwYlpe0DSFjVNUIkNNQxwKQ"),
+      });
+    });
+  });
+
   describe("isValidAddress", () => {
     it("accepts valid addresses", () => {
       expect(isValidAddress("cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6")).toEqual(true);

src/address.ts

@@ -1,15 +1,23 @@
 import { Address, Algorithm, PubkeyBundle } from "@iov/bcp";
 import { Ripemd160, Secp256k1, Sha256 } from "@iov/crypto";
-import { Bech32 } from "@iov/encoding";
+import { Bech32, Encoding } from "@iov/encoding";
+import equal from "fast-deep-equal";
 
 export type CosmosAddressBech32Prefix = "cosmos" | "cosmosvalcons" | "cosmosvaloper";
 export type CosmosPubkeyBech32Prefix = "cosmospub" | "cosmosvalconspub" | "cosmosvaloperpub";
 export type CosmosBech32Prefix = CosmosAddressBech32Prefix | CosmosPubkeyBech32Prefix;
 
+// As discussed in https://github.com/binance-chain/javascript-sdk/issues/163
+const pubkeyAminoPrefix = Encoding.fromHex("eb5ae98721");
+
 function isCosmosAddressBech32Prefix(prefix: string): prefix is CosmosAddressBech32Prefix {
   return ["cosmos", "cosmosvalcons", "cosmosvaloper"].includes(prefix);
 }
 
+function isCosmosPubkeyBech32Prefix(prefix: string): prefix is CosmosPubkeyBech32Prefix {
+  return ["cosmospub", "cosmosvalconspub", "cosmosvaloperpub"].includes(prefix);
+}
+
 export function decodeCosmosAddress(
   address: Address,
 ): { readonly prefix: CosmosAddressBech32Prefix; readonly data: Uint8Array } {
@@ -23,6 +31,26 @@ export function decodeCosmosAddress(
   return { prefix: prefix, data: data };
 }
 
+export function decodeCosmosPubkey(
+  encodedPubkey: string,
+): { readonly prefix: CosmosPubkeyBech32Prefix; readonly data: Uint8Array } {
+  const { prefix, data } = Bech32.decode(encodedPubkey);
+  if (!isCosmosPubkeyBech32Prefix(prefix)) {
+    throw new Error(`Invalid bech32 prefix. Must be one of cosmos, cosmosvalcons, or cosmosvaloper.`);
+  }
+
+  if (!equal(data.slice(0, pubkeyAminoPrefix.length), pubkeyAminoPrefix)) {
+    throw new Error("Pubkey does not have the expected amino prefix " + Encoding.toHex(pubkeyAminoPrefix));
+  }
+
+  const rest = data.slice(pubkeyAminoPrefix.length);
+  if (rest.length !== 33) {
+    throw new Error("Invalid rest data length. Expected 33 bytes (compressed secp256k1 pubkey).");
+  }
+
+  return { prefix: prefix, data: rest };
+}
+
 export function isValidAddress(address: string): boolean {
   try {
     decodeCosmosAddress(address as Address);

src/cosmoscodec.ts

@@ -22,7 +22,7 @@ import { CosmosBech32Prefix, isValidAddress, pubkeyToAddress } from "./address";
 import { Caip5 } from "./caip5";
 import { parseTx } from "./decode";
 import { buildSignedTx, buildUnsignedTx } from "./encode";
-import { TokenInfos } from "./types";
+import { nonceToAccountNumber, nonceToSequence, TokenInfos } from "./types";
 
 const { toHex, toUtf8 } = Encoding;
 
@@ -54,17 +54,16 @@ export class CosmosCodec implements TxCodec {
   }
 
   public bytesToSign(unsigned: UnsignedTransaction, nonce: Nonce): SigningJob {
-    const accountNumber = 0;
     const memo = (unsigned as any).memo;
     const built = buildUnsignedTx(unsigned, this.tokens);
 
     const signMsg = sortJson({
-      account_number: accountNumber.toString(),
+      account_number: nonceToAccountNumber(nonce),
       chain_id: Caip5.decode(unsigned.chainId),
       fee: (built.value as any).fee,
       memo: memo,
       msgs: (built.value as any).msg,
-      sequence: nonce.toString(),
+      sequence: nonceToSequence(nonce),
     });
     const signBytes = toUtf8(JSON.stringify(signMsg));
 

src/cosmosconnection.spec.ts

@@ -17,7 +17,7 @@ import { HdPaths, Secp256k1HdWallet, UserProfile } from "@iov/keycontrol";
 import { CosmosBech32Prefix } from "./address";
 import { CosmosCodec, cosmosCodec } from "./cosmoscodec";
 import { CosmosConnection } from "./cosmosconnection";
-import { TokenInfos } from "./types";
+import { nonceToSequence, TokenInfos } from "./types";
 
 const { fromBase64, toHex } = Encoding;
 
@@ -150,8 +150,10 @@ describe("CosmosConnection", () => {
         throw new Error("Expected account not to be undefined");
       }
       expect(account.address).toEqual(defaultAddress);
-      // Undefined until we sign a transaction
+      // Undefined until we sign a transaction (on multiple runs against one server this will be set), allow both
-      expect(account.pubkey).toEqual(undefined);
+      if (account.pubkey !== undefined) {
+        expect(account.pubkey).toEqual(defaultPubkey);
+      }
       // Starts with two tokens
       expect(account.balance.length).toEqual(2);
       connection.disconnect();
@@ -165,8 +167,10 @@ describe("CosmosConnection", () => {
         throw new Error("Expected account not to be undefined");
       }
       expect(account.address).toEqual(defaultAddress);
-      // Undefined until we sign a transaction
+      // Undefined until we sign a transaction (on multiple runs against one server this will be set), allow both
-      expect(account.pubkey).toEqual(undefined);
+      if (account.pubkey !== undefined) {
+        expect(account.pubkey).toEqual(defaultPubkey);
+      }
       // Starts with two tokens
       expect(account.balance.length).toEqual(2);
       connection.disconnect();
@@ -223,7 +227,9 @@ describe("CosmosConnection", () => {
       expect(transaction.chainId).toEqual(unsigned.chainId);
 
       expect(signatures.length).toEqual(1);
-      expect(signatures[0].nonce).toEqual(signed.signatures[0].nonce);
+      // TODO: the nonce we recover in response doesn't have accountNumber, only sequence
+      const signedSequence = parseInt(nonceToSequence(signed.signatures[0].nonce), 10);
+      expect(signatures[0].nonce).toEqual(signedSequence);
       expect(signatures[0].pubkey.algo).toEqual(signed.signatures[0].pubkey.algo);
       expect(toHex(signatures[0].pubkey.data)).toEqual(
         toHex(Secp256k1.compressPubkey(signed.signatures[0].pubkey.data)),

src/cosmosconnection.ts

@@ -28,19 +28,17 @@ import {
   TransactionState,
   UnsignedTransaction,
 } from "@iov/bcp";
-import { Encoding, Uint53 } from "@iov/encoding";
+import { Uint53 } from "@iov/encoding";
 import { DefaultValueProducer, ValueAndUpdates } from "@iov/stream";
 import equal from "fast-deep-equal";
 import { ReadonlyDate } from "readonly-date";
 import { Stream } from "xstream";
 
-import { CosmosBech32Prefix, pubkeyToAddress } from "./address";
+import { CosmosBech32Prefix, decodeCosmosPubkey, pubkeyToAddress } from "./address";
 import { Caip5 } from "./caip5";
 import { decodeAmount, parseTxsResponse } from "./decode";
 import { RestClient, TxsResponse } from "./restclient";
-import { TokenInfos } from "./types";
+import { accountToNonce, TokenInfos } from "./types";
-
-const { fromBase64 } = Encoding;
 
 interface ChainData {
   readonly chainId: ChainId;
@@ -150,11 +148,13 @@ export class CosmosConnection implements BlockchainConnection {
     const supportedCoins = account.coins.filter(({ denom }) =>
       this.tokenInfo.find(token => token.denom === denom),
     );
+
     const pubkey = !account.public_key
       ? undefined
       : {
           algo: Algorithm.Secp256k1,
-          data: fromBase64(account.public_key.value) as PubkeyBytes,
+          // amino-js has wrong (outdated) types
+          data: decodeCosmosPubkey(account.public_key as any).data as PubkeyBytes,
         };
     return {
       address: address,
@@ -171,7 +171,7 @@ export class CosmosConnection implements BlockchainConnection {
     const address = isPubkeyQuery(query) ? pubkeyToAddress(query.pubkey, this.prefix) : query.address;
     const { result } = await this.restClient.authAccounts(address);
     const account = result.value;
-    return parseInt(account.sequence, 10) as Nonce;
+    return accountToNonce(account);
   }
 
   public async getNonces(query: AddressQuery | PubkeyQuery, count: number): Promise<readonly Nonce[]> {
@@ -180,6 +180,7 @@ export class CosmosConnection implements BlockchainConnection {
       return [];
     }
     const firstNonce = await this.getNonce(query);
+    // Note: this still works with the encoded format (see types/accountToNonce) as least-significant digits are sequence
     return [...new Array(checkedCount)].map((_, i) => (firstNonce + i) as Nonce);
   }
 
@@ -214,7 +215,7 @@ export class CosmosConnection implements BlockchainConnection {
 
   public async postTx(tx: PostableBytes): Promise<PostTxResponse> {
     const { code, txhash, raw_log } = await this.restClient.postTx(tx);
-    if (code !== 0) {
+    if (code) {
       throw new Error(raw_log);
     }
     const transactionId = txhash as TransactionId;
@@ -302,7 +303,9 @@ export class CosmosConnection implements BlockchainConnection {
   ): Promise<ConfirmedAndSignedTransaction<UnsignedTransaction> | FailedTransaction> {
     const sender = (response.tx.value as any).msg[0].value.from_address;
     const accountForHeight = await this.restClient.authAccounts(sender, response.height);
-    const nonce = (parseInt(accountForHeight.result.value.sequence, 10) - 1) as Nonce;
+    // this is technically not the proper nonce. maybe this causes issues for sig validation?
-    return parseTxsResponse(chainId, parseInt(response.height, 10), nonce, response, this.tokenInfo);
+    // leaving for now unless it causes issues
+    const sequence = (parseInt(accountForHeight.result.value.sequence, 10) - 1) as Nonce;
+    return parseTxsResponse(chainId, parseInt(response.height, 10), sequence, response, this.tokenInfo);
   }
 }

src/types.spec.ts

@@ -0,0 +1,28 @@
+/* eslint-disable @typescript-eslint/camelcase */
+import { accountToNonce, nonceToAccountNumber, nonceToSequence } from "./types";
+
+describe("nonceEncoding", () => {
+  it("works for input in range", () => {
+    const nonce = accountToNonce({
+      account_number: "1234",
+      sequence: "7890",
+    });
+    expect(nonceToAccountNumber(nonce)).toEqual("1234");
+    expect(nonceToSequence(nonce)).toEqual("7890");
+  });
+
+  it("errors on input too large", () => {
+    expect(() =>
+      accountToNonce({
+        account_number: "1234567890",
+        sequence: "7890",
+      }),
+    ).toThrow();
+    expect(() =>
+      accountToNonce({
+        account_number: "178",
+        sequence: "97320247923",
+      }),
+    ).toThrow();
+  });
+});

src/types.ts

@@ -1,4 +1,4 @@
-import { Amount, Token } from "@iov/bcp";
+import { Amount, Nonce, Token } from "@iov/bcp";
 import amino from "@tendermint/amino-js";
 
 export type AminoTx = amino.Tx & { readonly value: amino.StdTx };
@@ -40,3 +40,50 @@ export function coinToAmount(tokens: TokenInfos, coin: amino.Coin): Amount {
     quantity: coin.amount,
   };
 }
+
+// tslint:disable-next-line:no-bitwise
+const maxAcct = 1 << 23;
+// tslint:disable-next-line:no-bitwise
+const maxSeq = 1 << 20;
+
+// NonceInfo is the data we need from account to create a nonce
+// Use this so no confusion about order of arguments
+export interface NonceInfo {
+  readonly account_number: string;
+  readonly sequence: string;
+}
+
+// this (lossily) encodes the two pieces of info (uint64) needed to sign into
+// one (53-bit) number. Cross your fingers.
+/* eslint-disable-next-line @typescript-eslint/camelcase */
+export function accountToNonce({ account_number, sequence }: NonceInfo): Nonce {
+  const acct = parseInt(account_number, 10);
+  const seq = parseInt(sequence, 10);
+
+  // we allow 23 bits (8 million) for accounts, and 20 bits (1 million) for tx/account
+  // let's fix this soon
+  if (acct > maxAcct) {
+    throw new Error("Account number is greater than 2^23, must update Nonce handler");
+  }
+  if (seq > maxSeq) {
+    throw new Error("Sequence is greater than 2^20, must update Nonce handler");
+  }
+
+  const val = acct * maxSeq + seq;
+  return val as Nonce;
+}
+
+// this extracts info from nonce for signing
+export function nonceToAccountNumber(nonce: Nonce): string {
+  const acct = nonce / maxSeq;
+  if (acct > maxAcct) {
+    throw new Error("Invalid Nonce, account number is higher than can safely be encoded in Nonce");
+  }
+  return Math.round(acct).toString();
+}
+
+// this extracts info from nonce for signing
+export function nonceToSequence(nonce: Nonce): string {
+  const seq = nonce % maxSeq;
+  return Math.round(seq).toString();
+}

types/address.d.ts

@@ -8,5 +8,11 @@ export declare function decodeCosmosAddress(
   readonly prefix: CosmosAddressBech32Prefix;
   readonly data: Uint8Array;
 };
+export declare function decodeCosmosPubkey(
+  encodedPubkey: string,
+): {
+  readonly prefix: CosmosPubkeyBech32Prefix;
+  readonly data: Uint8Array;
+};
 export declare function isValidAddress(address: string): boolean;
 export declare function pubkeyToAddress(pubkey: PubkeyBundle, prefix: CosmosBech32Prefix): Address;

types/types.d.ts

@@ -1,4 +1,4 @@
-import { Amount, Token } from "@iov/bcp";
+import { Amount, Nonce, Token } from "@iov/bcp";
 import amino from "@tendermint/amino-js";
 export declare type AminoTx = amino.Tx & {
   readonly value: amino.StdTx;
@@ -10,3 +10,10 @@ export interface TokenInfo extends Token {
 export declare type TokenInfos = ReadonlyArray<TokenInfo>;
 export declare function amountToCoin(lookup: ReadonlyArray<TokenInfo>, amount: Amount): amino.Coin;
 export declare function coinToAmount(tokens: TokenInfos, coin: amino.Coin): Amount;
+export interface NonceInfo {
+  readonly account_number: string;
+  readonly sequence: string;
+}
+export declare function accountToNonce({ account_number, sequence }: NonceInfo): Nonce;
+export declare function nonceToAccountNumber(nonce: Nonce): string;
+export declare function nonceToSequence(nonce: Nonce): string;