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 8 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions Makefile
Expand Up @@ -188,7 +188,7 @@ test-cf: cf-core
test-client: client
bash ops/test/client.sh

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

test-daicard:
Expand Down Expand Up @@ -285,12 +285,12 @@ 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))
client: cf-core types crypto contracts store 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)/$@

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 types 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)/$@
Expand Down Expand Up @@ -320,7 +320,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: cf-core types crypto contracts 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 Down Expand Up @@ -348,7 +348,7 @@ apps: node-modules types crypto cf-core $(shell find $(apps)/src $(find_options)
########################################
# Common Prerequisites

contracts: node-modules types contract-artifacts $(shell find $(contracts)/index.ts $(contracts)/test $(contracts)/tsconfig.json $(find_options))
contracts: node-modules types 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
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
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
@@ -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
@@ -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
8 changes: 7 additions & 1 deletion modules/apps/src/shared/validation.ts
@@ -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
Expand Up @@ -24,11 +24,11 @@ export default class GetAppInstanceDetailsController extends NodeController {
}

//TODO - This is very dumb, just add multisigAddress to the base app instance type to begin with
let appInstance = (await store.getAppInstance(appInstanceId)).toJson()
appInstance.multisigAddress = (await store.getMultisigAddressFromAppInstance(appInstanceId))
let appInstance = (await store.getAppInstance(appInstanceId)).toJson();
appInstance.multisigAddress = await store.getMultisigAddressFromAppInstance(appInstanceId);

return {
appInstance
appInstance,
};
}
}
5 changes: 3 additions & 2 deletions modules/cf-core/src/node.ts
Expand Up @@ -7,6 +7,7 @@ import {
AppInstanceProposal,
PersistAppType,
} from "@connext/types";
import { signDigest } from "@connext/crypto";
import { BaseProvider } from "ethers/providers";
import { SigningKey } from "ethers/utils";
import EventEmitter from "eventemitter3";
Expand All @@ -28,7 +29,7 @@ import {
NodeMessageWrappedProtocolMessage,
ProtocolMessage,
} from "./types";
import { timeout, signDigestWithEthers } from "./utils";
import { timeout } from "./utils";
import { Store } from "./store";
import {
ConditionalTransactionCommitment,
Expand Down Expand Up @@ -180,7 +181,7 @@ export class Node {
const privateKey = await this.privateKeyGetter.getPrivateKey(keyIndex);
const hash = commitment.hashToSign();

return signDigestWithEthers(privateKey, hash);
return await signDigest(privateKey, hash);
});

protocolRunner.register(Opcode.IO_SEND, async (args: [ProtocolMessage]) => {
Expand Down
9 changes: 7 additions & 2 deletions modules/cf-core/src/protocol/utils/signature-validator.ts
@@ -1,7 +1,12 @@
import { getAddress, recoverAddress, Signature } from "ethers/utils";
import { getAddress, recoverAddress } from "ethers/utils";
import { isHexString, addHexPrefix } from "@connext/crypto";

import { EthereumCommitment } from "../../types";

function sanitizeHexString(hex: string): string {
return isHexString(hex) ? addHexPrefix(hex) : hex;
}

export function assertIsValidSignature(
expectedSigner: string,
commitment?: EthereumCommitment,
Expand All @@ -18,7 +23,7 @@ export function assertIsValidSignature(
const hash = commitment.hashToSign();

// recoverAddress: 83 ms, hashToSign: 7 ms
const signer = recoverAddress(hash, signature);
const signer = recoverAddress(sanitizeHexString(hash), sanitizeHexString(signature));

if (getAddress(expectedSigner) !== signer) {
throw Error(
Expand Down