Skip to content
This repository has been archived by the owner on May 28, 2021. It is now read-only.

Replace signDigestWithEthers with signDigest from crypto module + other improvements #910

Merged
merged 43 commits into from
Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
3f4f89b
replace signDigest from ethers with crypto module
pedrouid Mar 22, 2020
3e8ddd8
remove it.only
pedrouid Mar 22, 2020
05575af
Merge branch 'refactor-signing' of https://github.com/ConnextProject/…
pedrouid Mar 22, 2020
2bf5967
Merge branch 'refactor-signing' of https://github.com/ConnextProject/…
pedrouid Mar 22, 2020
600d9a9
quick fix
pedrouid Mar 23, 2020
4de9eb4
make recoverAddress async
pedrouid Mar 23, 2020
d2ff2aa
move crypto.spec.ts to crypto tests
pedrouid Mar 23, 2020
c2c03a8
update crypto tests
pedrouid Mar 23, 2020
1b4b82d
tweak
pedrouid Mar 23, 2020
149ee9a
remove console.log
pedrouid Mar 23, 2020
506a200
replace recoverAddress from ethers with wrapper
pedrouid Mar 24, 2020
2c791ec
update crypto tests
pedrouid Mar 24, 2020
ae4b62d
add random bytes utils
pedrouid Mar 24, 2020
b03eff3
cleanup
pedrouid Mar 24, 2020
5e271d0
await assetSignatures
pedrouid Mar 24, 2020
3320a85
revert making assertSignatures async
pedrouid Mar 24, 2020
da03d72
resolve merge conflcits
pedrouid Mar 25, 2020
49681e5
resolve merge conflicts
pedrouid Mar 25, 2020
57285da
change nonceLen to 32
pedrouid Mar 25, 2020
d0327ce
Merge branch 'refactor-signing' of https://github.com/ConnextProject/…
pedrouid Mar 25, 2020
205d9e5
Merge branch 'refactor-signing-new' of https://github.com/ConnextProj…
pedrouid Mar 25, 2020
caab302
fix hashLockTransfer.test.ts
pedrouid Mar 25, 2020
c734aaf
Merge branch 'refactor-signing' of https://github.com/ConnextProject/…
pedrouid Mar 25, 2020
df96ccf
Merge branch 'refactor-signing-new' of https://github.com/ConnextProj…
pedrouid Mar 25, 2020
dd86258
Merge branch 'refactor-signing' into refactor-signing-new
LayneHaber Mar 25, 2020
9ae48e5
Merge branch 'refactor-signing' of https://github.com/ConnextProject/…
pedrouid Mar 25, 2020
0f2a545
Merge branch 'refactor-signing-new' of https://github.com/ConnextProj…
pedrouid Mar 25, 2020
0f8d8e8
Merge branch 'refactor-signing-new' of https://github.com/ConnextProj…
LayneHaber Mar 25, 2020
5906993
remove store
LayneHaber Mar 25, 2020
a23d0ed
resolve merge conflcit
pedrouid Mar 25, 2020
cc346d3
simplifying Makefile builds
pedrouid Mar 25, 2020
e620e8c
Merge branch 'staging' into refactor-signing-new
pedrouid Mar 25, 2020
38c7359
Merge pull request #917 from ConnextProject/refactor-signing-async-re…
pedrouid Mar 25, 2020
4894359
clean up error handling
LayneHaber Mar 25, 2020
9edb8b9
use real xpub
LayneHaber Mar 25, 2020
3c21854
use sign digest with ethers
LayneHaber Mar 25, 2020
4179ee2
remove unused deps
LayneHaber Mar 25, 2020
0d54012
fix build
LayneHaber Mar 25, 2020
b419737
complete replacing sigs
LayneHaber Mar 26, 2020
55a6ae5
increase delay
LayneHaber Mar 26, 2020
6caf5a3
fix in crypto package
LayneHaber Mar 26, 2020
6c82959
isolate function call changes
LayneHaber Mar 26, 2020
03c8d64
update import
LayneHaber Mar 26, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion .github/workflows/cd-master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ jobs:
- run: make contracts
- run: make test-contracts

test-crypto:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Cache node modules
uses: actions/cache@v1
with:
path: .npm
key: npm-${{ hashFiles('package-lock.json') }}
restore-keys: npm-
- run: make crypto
- run: make test-crypto

test-node:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -138,7 +151,7 @@ jobs:
INDRA_NATS_JWT_SIGNER_PUBLIC_KEY: ${{ secrets.INDRA_NATS_JWT_SIGNER_PUBLIC_KEY_RINKEBY }}
RINKEBY_DOMAINNAME: rinkeby.indra.connext.network
RINKEBY_ETH_PROVIDER: ${{ secrets.RINKEBY_ETH_PROVIDER }}
needs: [test-backwards-compatibility, test-cf, test-client, test-contracts, test-daicard, test-integration, test-node, test-ssh]
needs: [test-backwards-compatibility, test-cf, test-client, test-contracts, test-crypto, test-daicard, test-integration, test-node, test-ssh]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
Expand Down
15 changes: 14 additions & 1 deletion .github/workflows/cd-staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ jobs:
- run: make contracts
- run: make test-contracts

test-crypto:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Cache node modules
uses: actions/cache@v1
with:
path: .npm
key: npm-${{ hashFiles('package-lock.json') }}
restore-keys: npm-
- run: make crypto
- run: make test-crypto

test-node:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -126,7 +139,7 @@ jobs:
INDRA_NATS_JWT_SIGNER_PUBLIC_KEY: ${{ secrets.INDRA_NATS_JWT_SIGNER_PUBLIC_KEY_RINKEBY }}
RINKEBY_ETH_PROVIDER: ${{ secrets.RINKEBY_ETH_PROVIDER }}
STAGING_DOMAINNAME: staging.indra.connext.network
needs: [test-cf, test-client, test-contracts, test-daicard, test-integration, test-node, test-ssh]
needs: [test-cf, test-client, test-contracts, test-crypto, test-daicard, test-integration, test-node, test-ssh]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
Expand Down
15 changes: 14 additions & 1 deletion .github/workflows/cd-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ jobs:
- run: make contracts
- run: make test-contracts

test-crypto:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Cache node modules
uses: actions/cache@v1
with:
path: .npm
key: npm-${{ hashFiles('package-lock.json') }}
restore-keys: npm-
- run: make crypto
- run: make test-crypto

test-node:
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -98,7 +111,7 @@ jobs:
- uses: actions/checkout@v1
- run: make pull-commit
- run: make start-test-staging
- run: sleep 15 && make dls
- run: sleep 20 && make dls
- run: make test-backwards-compatibility
- name: Print node logs
if: failure()
Expand Down
13 changes: 13 additions & 0 deletions .github/workflows/ci-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ jobs:
- run: make contracts
- run: make test-contracts

test-crypto:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Cache node modules
uses: actions/cache@v1
with:
path: .npm
key: npm-${{ hashFiles('package-lock.json') }}
restore-keys: npm-
- run: make crypto
- run: make test-crypto

test-integration:
env:
INDRA_ADMIN_TOKEN: ${{ secrets.INDRA_ADMIN_TOKEN }}
Expand Down
25 changes: 14 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,12 @@ test-cf: cf-core
test-client: client
bash ops/test/client.sh

test-contracts: contracts types
test-contracts: contracts crypto
bash ops/test/contracts.sh

test-crypto: crypto
bash ops/test/crypto.sh

test-daicard:
bash ops/test/ui.sh daicard

Expand Down Expand Up @@ -285,16 +288,21 @@ test-runner-staging: node-modules client $(shell find $(tests)/src $(tests)/ops
########################################
# JS & bundles

client: cf-core types contracts apps messaging channel-provider $(shell find $(client)/src $(client)/tsconfig.json $(find_options))
apps: node-modules crypto cf-core $(shell find $(apps)/src $(find_options))
$(log_start)
$(docker_run) "cd modules/client && npm run build"
$(docker_run) "cd modules/apps && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

cf-core: node-modules types contracts crypto store $(shell find $(cf-core)/src $(cf-core)/test $(cf-core)/tsconfig.json $(find_options))
cf-core: node-modules crypto contracts store $(shell find $(cf-core)/src $(cf-core)/test $(cf-core)/tsconfig.json $(find_options))
$(log_start)
$(docker_run) "cd modules/cf-core && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

client: apps messaging channel-provider $(shell find $(client)/src $(client)/tsconfig.json $(find_options))
$(log_start)
$(docker_run) "cd modules/client && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

channel-provider: node-modules types $(shell find $(channel-provider)/src $(find_options))
$(log_start)
$(docker_run) "cd modules/channel-provider && npm run build"
Expand All @@ -320,7 +328,7 @@ messaging: node-modules types $(shell find $(messaging)/src $(find_options))
$(docker_run) "cd modules/messaging && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

node: cf-core types contracts crypto apps messaging $(shell find $(node)/src $(node)/migrations $(find_options))
node: apps messaging $(shell find $(node)/src $(node)/migrations $(find_options))
$(log_start)
$(docker_run) "cd modules/node && npm run build && touch src/main.ts"
$(log_finish) && mv -f $(totalTime) $(flags)/$@
Expand All @@ -340,15 +348,10 @@ types: node-modules $(shell find $(types)/src $(find_options))
$(docker_run) "cd modules/types && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

apps: node-modules types crypto cf-core $(shell find $(apps)/src $(find_options))
$(log_start)
$(docker_run) "cd modules/apps && npm run build"
$(log_finish) && mv -f $(totalTime) $(flags)/$@

########################################
# Common Prerequisites

contracts: node-modules types contract-artifacts $(shell find $(contracts)/index.ts $(contracts)/test $(contracts)/tsconfig.json $(find_options))
contracts: node-modules crypto contract-artifacts $(shell find $(contracts)/index.ts $(contracts)/test $(contracts)/tsconfig.json $(find_options))
$(log_start)
$(docker_run) "cd modules/contracts && npm run transpile"
$(log_finish) && mv -f $(totalTime) $(flags)/$@
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/src/HashLockTransferApp/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export function convertHashLockTransferParameters<To extends NumericTypeName>(
to: To,
obj: HashLockTransferParameters<any>,
): HashLockTransferParameters<NumericTypes[To]> {
return convertFields(getType(obj.amount), to, ["timelock", "amount"], { ...obj })
return convertFields(getType(obj.amount), to, ["timelock", "amount"], { ...obj });
}

export function convertHashLockTransferAppState<To extends NumericTypeName>(
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/src/SimpleTwoPartySwapApp/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
SimpleSwapAppState,
SwapParameters,
makeChecksumOrEthAddress,
convertCoinTransfersToObjIfNeeded
convertCoinTransfersToObjIfNeeded,
} from "@connext/types";

export function convertSimpleSwapAppState<To extends NumericTypeName>(
Expand Down
23 changes: 10 additions & 13 deletions modules/apps/src/WithdrawApp/convert.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
NumericTypes,
convertAmountField,
NumericTypeName,
WithdrawAppState,
WithdrawParameters,
makeChecksumOrEthAddress,
convertCoinTransfersToObjIfNeeded
} from "@connext/types";
NumericTypes,
convertAmountField,
NumericTypeName,
WithdrawAppState,
WithdrawParameters,
makeChecksumOrEthAddress,
convertCoinTransfersToObjIfNeeded,
} from "@connext/types";

export function convertWithrawAppState<To extends NumericTypeName>(
to: To,
Expand All @@ -15,10 +15,7 @@ export function convertWithrawAppState<To extends NumericTypeName>(
obj.transfers = convertCoinTransfersToObjIfNeeded(obj.transfers);
return {
...obj,
transfers: [
convertAmountField(to, obj.transfers[0]),
convertAmountField(to, obj.transfers[1]),
],
transfers: [convertAmountField(to, obj.transfers[0]), convertAmountField(to, obj.transfers[1])],
};
}

Expand All @@ -29,7 +26,7 @@ export function convertWithdrawParameters<To extends NumericTypeName>(
const asset: any = {
...obj,
assetId: makeChecksumOrEthAddress(obj.assetId),
recipient: makeChecksumOrEthAddress(obj.recipient)
recipient: makeChecksumOrEthAddress(obj.recipient),
};
return convertAmountField(to, asset);
}
7 changes: 6 additions & 1 deletion modules/apps/src/WithdrawApp/registry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { OutcomeType, WithdrawAppStateEncoding, WithdrawAppActionEncoding, WithdrawApp } from "@connext/types";
import {
OutcomeType,
WithdrawAppStateEncoding,
WithdrawAppActionEncoding,
WithdrawApp,
} from "@connext/types";

import { AppRegistryInfo } from "../shared";

Expand Down
7 changes: 4 additions & 3 deletions modules/apps/src/WithdrawApp/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import {
CoinTransferBigNumber,
bigNumberifyObj,
WithdrawAppState,
recoverAddressWithEthers,
} from "@connext/types";

import { unidirectionalCoinTransferValidation } from "../shared";
import { convertWithrawAppState } from "./convert";
import { BigNumber, recoverAddress } from "ethers/utils";
import { BigNumber } from "ethers/utils";
import { HashZero, Zero } from "ethers/constants";

export const validateWithdrawApp = (
export const validateWithdrawApp = async (
params: CFCoreTypes.ProposeInstallParams,
initiatorPublicIdentifier: string,
responderPublicIdentifier: string,
Expand Down Expand Up @@ -67,7 +68,7 @@ export const validateWithdrawApp = (
);
}

let recovered = recoverAddress(initialState.data, initialState.signatures[0]);
let recovered = await recoverAddressWithEthers(initialState.data, initialState.signatures[0]);

if (recovered !== initialState.signers[0]) {
throw new Error(
Expand Down
8 changes: 7 additions & 1 deletion modules/apps/src/shared/validation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { CFCoreTypes, stringify, bigNumberifyObj, CoinTransferBigNumber, CoinBalanceRefundApp } from "@connext/types";
import {
CFCoreTypes,
stringify,
bigNumberifyObj,
CoinTransferBigNumber,
CoinBalanceRefundApp,
} from "@connext/types";

import { AppRegistryInfo } from "./registry";
import { BigNumber } from "ethers/utils";
Expand Down
2 changes: 1 addition & 1 deletion modules/cf-core/src/auto-nonce-wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export default class AutoNonceWallet extends Wallet {

async sendTransaction(tx: TransactionRequest): Promise<TransactionResponse> {
if (!tx.nonce) {
if (this.noncePromise === undefined) {
if (typeof this.noncePromise === "undefined") {
this.noncePromise = this.provider.getTransactionCount(this.address);
}

Expand Down
6 changes: 3 additions & 3 deletions modules/cf-core/src/ethereum/multisig-commitment.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Interface, keccak256, solidityPack } from "ethers/utils";
import { sortSignaturesBySignerAddress } from "@connext/types";

import { MinimumViableMultisig } from "../contracts";
import { CFCoreTypes, EthereumCommitment, MultisigTransaction } from "../types";
import { sortSignaturesBySignerAddress } from "../utils";

/// A commitment to make MinimumViableMultisig perform a message call
export abstract class MultisigCommitment implements EthereumCommitment {
Expand All @@ -27,11 +27,11 @@ export abstract class MultisigCommitment implements EthereumCommitment {
this.participantSignatures = sigs;
}

public getSignedTransaction(): CFCoreTypes.MinimalTransaction {
public async getSignedTransaction(): Promise<CFCoreTypes.MinimalTransaction> {
this.assertSignatures();
const multisigInput = this.getTransactionDetails();
const hash = this.hashToSign();
const signaturesList = sortSignaturesBySignerAddress(hash, this.signatures);
const signaturesList = await sortSignaturesBySignerAddress(hash, this.signatures);

const txData = new Interface(MinimumViableMultisig.abi).functions.execTransaction.encode([
multisigInput.to,
Expand Down
13 changes: 8 additions & 5 deletions modules/cf-core/src/ethereum/set-state-commitment.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Interface, keccak256, solidityPack } from "ethers/utils";
import { sortSignaturesBySignerAddress } from "@connext/types";

import { ChallengeRegistry } from "../contracts";
import {
Expand All @@ -8,7 +9,6 @@ import {
SignedStateHashUpdate,
SetStateCommitmentJSON,
} from "../types";
import { sortSignaturesBySignerAddress } from "../utils";

import { appIdentityToHash } from "./utils";

Expand Down Expand Up @@ -55,12 +55,15 @@ export class SetStateCommitment implements EthereumCommitment {
return keccak256(this.encode());
}

public getSignedTransaction(): CFCoreTypes.MinimalTransaction {
public async getSignedTransaction(): Promise<CFCoreTypes.MinimalTransaction> {
this.assertSignatures();
return {
to: this.challengeRegistryAddress,
value: 0,
data: iface.functions.setState.encode([this.appIdentity, this.getSignedStateHashUpdate()]),
data: iface.functions.setState.encode([
this.appIdentity,
await this.getSignedStateHashUpdate(),
]),
};
}

Expand Down Expand Up @@ -88,14 +91,14 @@ export class SetStateCommitment implements EthereumCommitment {
);
}

private getSignedStateHashUpdate(): SignedStateHashUpdate {
private async getSignedStateHashUpdate(): Promise<SignedStateHashUpdate> {
this.assertSignatures();
const hash = this.hashToSign();
return {
appStateHash: this.appStateHash,
versionNumber: this.versionNumber,
timeout: this.timeout,
signatures: sortSignaturesBySignerAddress(hash, this.signatures),
signatures: await sortSignaturesBySignerAddress(hash, this.signatures),
};
}

Expand Down
4 changes: 2 additions & 2 deletions modules/cf-core/src/machine/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export class MiddlewareContainer {
public async run(opCode: Opcode, args: any[]) {
const middleware = this.middlewares[opCode][0];

if (middleware === undefined) {
throw Error(`Attempted to run middleware for opcode ${opCode} but none existed`);
if (typeof middleware === "undefined") {
throw new Error(`Attempted to run middleware for opcode ${opCode} but none existed`);
}

return middleware(args);
Expand Down
6 changes: 3 additions & 3 deletions modules/cf-core/src/machine/protocol-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function firstRecipientFromProtocolName(protocolName: Protocol) {
) {
return "responderXpub";
}
throw Error(`Unknown protocolName ${protocolName} passed to firstRecipientFromProtocolName`);
throw new Error(`Unknown protocolName ${protocolName} passed to firstRecipientFromProtocolName`);
}

export class ProtocolRunner {
Expand All @@ -76,8 +76,8 @@ export class ProtocolRunner {
public async runProtocolWithMessage(msg: ProtocolMessage) {
const protocol = getProtocolFromName(msg.protocol);
const step = protocol[msg.seq];
if (step === undefined) {
throw Error(`Received invalid seq ${msg.seq} for protocol ${msg.protocol}`);
if (typeof step === "undefined") {
throw new Error(`Received invalid seq ${msg.seq} for protocol ${msg.protocol}`);
}
return this.runProtocol(step, msg);
}
Expand Down
Loading