This repository has been archived by the owner on May 28, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 38
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
203 changed files
with
5,150 additions
and
3,677 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
|
||
# Summary | ||
|
||
## TL;DR I want to make things faster, what do we do? | ||
|
||
- Sender is consistently slug in uninstalling their side of a transfer, in particular the transition between taking action & uninstalling. | ||
- We need better visibility into node's lock service to find out why it occasionally takes 4000ms for a client to get a lock (eg if locks are blocking each other, let us know) | ||
- Investigate replacing the `recoverAddress` call in `cf-core/src/protocol/utils/signature-validator.ts` w a crypto primitive that's not written in JS. | ||
|
||
## re On-chain confirmation times | ||
|
||
These benchmark numbers were recorded using ganache as the ethprovider set to auto-mine (aka zero-second block times). On-chain transactions generally take ~25 ms to be confirmed so their contribution is negligible. | ||
|
||
For example, breakdown of funding a client as part of test-runner's createClient() function: | ||
- Signed eth transfer tx in 137 | ||
- Signed token transfer tx in 145 | ||
- Eth + token transfer txs mined in 24 | ||
|
||
## Misc Observations | ||
|
||
Some steps have very high variability. Steps that sometimes run in 50ms take 2000ms on subsequent runs w/out anything being different. | ||
|
||
eg Acquiring a lock is generally a very fast operation, often the node responds in less than 5ms. This is one that sometimes takes > 1000ms (sometimes closer to 5000ms) though so there's something else going on that's blocking the node from responding as quickly as it could. Possibly, it's because another client has the lock and the node's response is blocked until that lock is released. But no one protocol takes 5000ms to run.. | ||
|
||
Requesting collateral stands out as a step that takes consistently > 1000ms.. Unexpected given that approximately none of the time is spent waiting for transactions to be mined. BUT this action will always be paired w an on-chain transaction in real-life situations so it's not super high-priority to optimize the off-chain side of things here. | ||
|
||
Even if we get variability down to be consistently best case.. The best case still won't be able to transfer more than once per second. Eg install & uninstall protocols consistently take 200-300ms and most of that time is spent in a call to `ethers.utils.recoverAddress` (40-80ms per address recovery & eg it's called 4 times in the install protocol), specifically in `cf-core/src/protocol/utils/signature-validator.ts` which, at it's core, is calling into the `elliptic` library, a pure-JS implementation of some EC crypto primitives. Is this the kind of thing we could replace w a call to a browser's Web Crypto API? | ||
|
||
## Closer look at transfers | ||
|
||
A one-off install, transfer, uninstall flow involves protocols: | ||
|
||
50 | ||
150 - sender propose | ||
250 | ||
250 - sender install | ||
400 | ||
100 - recipient propose | ||
25 | ||
300 - recipient install | ||
150 | ||
250 - recipient take action | ||
25 | ||
250 - recipient uninstall | ||
500 | ||
100 - sender take action | ||
650 | ||
150 - sender uninstall | ||
|
||
That's 1550ms spent in-protocol (including waiting for protocol messages) while the full flow usually takes a total of 3500-4500ms so we're spending less than half our time in protocol logic. | ||
|
||
Almost a full second is spent between the sender finishing & the recipient starting & vice versa. Would it be worth it to try to speed this up? | ||
|
||
# Example play-by-play metrics for full deposit, collateralize, transfer, withdraw flow. | ||
|
||
## Setup | ||
|
||
300 - create ethprovider & get config | ||
100 - setup protocol | ||
+ 200 - wait for channel to be available, setup subscriptions | ||
------ | ||
600 ms total for `client.connect()` | ||
|
||
## Deposit | ||
|
||
50 - acquire lock | ||
150 - propose protocol initiation | ||
100 - clean up locks/subscriptions | ||
350 - install protocol initiation | ||
100 - sign/send/mine deposit tx | ||
250 - uninstall protocol initiation | ||
+ 50 - release locks, check free balance | ||
------ | ||
1050 ms total for `client.deposit()` | ||
|
||
## Collateralize | ||
|
||
150 - setup locks, check free balance | ||
100 - propose protocol response | ||
75 - delay | ||
300 - install protocol response | ||
75 - delay | ||
+ 150 - uninstall protocol response | ||
------ | ||
850 ms total for `client.requestCollateral()` | ||
|
||
## Transfer (sender) | ||
|
||
50 - acquire lock | ||
200 - propose protocol initiation | ||
2000 - delay (highly variable, sometimes just 75ms) | ||
250 - install protocol response | ||
+ 150 - listener cleanup? | ||
------ | ||
2650 ms total for `client.transfer()` | ||
|
||
## Transfer (recipient) | ||
|
||
0 - delay between client.transfer() returning & recipient's subscription starting up | ||
15 - decrypt message (very fast crypto, great) | ||
85 - wait for event after sending resolve-linked message to node | ||
100 - propose protocol response | ||
15 - transition | ||
300 - install protocol initiation | ||
60 - transition | ||
200 - take action protocol initiation | ||
20 - transition | ||
250 - uninstall protocol initiation | ||
+ 20 - transition | ||
------ | ||
1100 ms total for client's linked transfer subscription callback | ||
|
||
## Transfer (sender again) | ||
|
||
85 - delay between recipient finished & sender starting cleanup | ||
100 - take action protocol response | ||
250 - delay waiting for node | ||
+ 150 - uninstall protocol response | ||
------ | ||
600 ms total for sender cleanup | ||
|
||
That's about 4000ms for full transfer flow | ||
|
||
## Withdraw (including state deposit holder deployment) | ||
|
||
20 - acquire lock & rescind deposit rights | ||
300 - deploy multisig | ||
100 - delay? | ||
800 - withdraw protocol initiation | ||
150 - wait for node response | ||
------ | ||
1450 ms total for withdraw | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
{ | ||
"name": "@connext/apps", | ||
"version": "5.1.0", | ||
"description": "Connext Counterfactual Apps", | ||
"main": "dist/index.js", | ||
"module": "dist/index.esm.js", | ||
"types": "dist/src/index.d.ts", | ||
"iife": "dist/index-iife.js", | ||
"files": ["dist", "src"], | ||
"scripts": { | ||
"build": "./node_modules/.bin/tsc -b . && ./node_modules/.bin/rollup -c", | ||
"lint": "../../node_modules/.bin/eslint -c '../../.eslintrc.js' --fix 'src/**/*'", | ||
"test": "echo \"Error: no test specified\" && exit 1", | ||
"rebuild": "npm run clean && npm run build", | ||
"clean": "rm -rf ./dist" | ||
}, | ||
"devDependencies": { | ||
"@connext/cf-core": "5.1.0", | ||
"@connext/types": "5.1.0", | ||
"ethers": "4.0.45", | ||
"rollup": "1.31.1", | ||
"rollup-plugin-typescript2": "0.26.0", | ||
"typescript": "3.7.5" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import typescript from "rollup-plugin-typescript2"; | ||
|
||
import pkg from "./package.json"; | ||
|
||
export default [ | ||
{ | ||
input: "./src/index.ts", | ||
output: [ | ||
{ | ||
file: pkg.main, | ||
format: "cjs", | ||
}, | ||
{ | ||
file: pkg.module, | ||
format: "esm", | ||
}, | ||
{ | ||
file: pkg.iife, | ||
format: "iife", | ||
name: "window.apps", | ||
}, | ||
], | ||
plugins: [typescript()], | ||
}, | ||
]; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { | ||
NumericTypeName, | ||
NumericTypes, | ||
getType, | ||
convertFields, | ||
CoinBalanceRefundAppState, | ||
} from "@connext/types"; | ||
|
||
export function convertCoinBalanceRefund<To extends NumericTypeName>( | ||
to: To, | ||
obj: CoinBalanceRefundAppState<any>, | ||
): CoinBalanceRefundAppState<NumericTypes[To]> { | ||
const fromType = getType(obj.threshold); | ||
return convertFields(fromType, to, ["threshold"], obj); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from "./convert"; | ||
export * from "./registry"; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { | ||
CoinBalanceRefundAppStateEncoding, | ||
OutcomeType, | ||
CoinBalanceRefundApp, | ||
} from "@connext/types"; | ||
|
||
import { AppRegistryInfo } from "../shared"; | ||
|
||
export const CoinBalanceRefundAppRegistryInfo: AppRegistryInfo = { | ||
allowNodeInstall: true, | ||
name: CoinBalanceRefundApp, | ||
outcomeType: OutcomeType.SINGLE_ASSET_TWO_PARTY_COIN_TRANSFER, | ||
stateEncoding: CoinBalanceRefundAppStateEncoding, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { | ||
getType, | ||
NumericTypeName, | ||
NumericTypes, | ||
convertFields, | ||
convertAmountField, | ||
FastSignedTransferParameters, | ||
FastSignedTransferAppState, | ||
} from "@connext/types"; | ||
|
||
export function convertFastSignedTransferParameters<To extends NumericTypeName>( | ||
to: To, | ||
obj: FastSignedTransferParameters<any>, | ||
): FastSignedTransferParameters<NumericTypes[To]> { | ||
const fromType = getType(obj.amount); | ||
return convertFields(fromType, to, ["maxAllocation", "amount"], obj); | ||
} | ||
|
||
export function convertFastSignedTransferAppState<To extends NumericTypeName>( | ||
to: To, | ||
obj: FastSignedTransferAppState<any>, | ||
): FastSignedTransferAppState<NumericTypes[To]> { | ||
return { | ||
coinTransfers: [ | ||
convertAmountField(to, obj.coinTransfers[0]), | ||
convertAmountField(to, obj.coinTransfers[1]), | ||
], | ||
...convertAmountField(to, obj), | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export * from "./convert"; | ||
export * from "./registry"; | ||
export * from "./validation"; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { | ||
OutcomeType, | ||
FastSignedTransferAppStateEncoding, | ||
FastSignedTransferAppActionEncoding, | ||
FastSignedTransferApp, | ||
} from "@connext/types"; | ||
|
||
import { AppRegistryInfo } from "../shared"; | ||
import {} from "../shared"; | ||
|
||
export const FastSignedTransferAppRegistryInfo: AppRegistryInfo = { | ||
allowNodeInstall: true, | ||
name: FastSignedTransferApp, | ||
outcomeType: OutcomeType.SINGLE_ASSET_TWO_PARTY_COIN_TRANSFER, | ||
stateEncoding: FastSignedTransferAppStateEncoding, | ||
actionEncoding: FastSignedTransferAppActionEncoding, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { CFCoreTypes, bigNumberifyObj, CoinTransfer, CoinTransferBigNumber } from "@connext/types"; | ||
import { xkeyKthAddress } from "@connext/cf-core"; | ||
import { BigNumber } from "ethers/utils"; | ||
|
||
import { unidirectionalCoinTransferValidation } from "../shared"; | ||
import { convertFastSignedTransferAppState } from "./convert"; | ||
import { HashZero } from "ethers/constants"; | ||
|
||
export const validateFastSignedTransferApp = ( | ||
params: CFCoreTypes.ProposeInstallParams, | ||
initiatorPublicIdentifier: string, | ||
responderPublicIdentifier: string, | ||
) => { | ||
const { responderDeposit, initiatorDeposit, initialState: initialStateBadType } = bigNumberifyObj( | ||
params, | ||
); | ||
const initialState = convertFastSignedTransferAppState("bignumber", initialStateBadType); | ||
|
||
initialState.coinTransfers = initialState.coinTransfers.map((transfer: CoinTransfer<BigNumber>) => | ||
bigNumberifyObj(transfer), | ||
) as any; | ||
|
||
if (initialState.paymentId !== HashZero) { | ||
throw new Error(`Cannot install with pre-populated paymentId`); | ||
} | ||
|
||
const initiatorFreeBalanceAddress = xkeyKthAddress(initiatorPublicIdentifier); | ||
const responderFreeBalanceAddress = xkeyKthAddress(responderPublicIdentifier); | ||
|
||
// initiator is sender | ||
const initiatorTransfer = initialState.coinTransfers.filter((transfer: CoinTransferBigNumber) => { | ||
return transfer.to === initiatorFreeBalanceAddress; | ||
})[0]; | ||
|
||
// responder is receiver | ||
const responderTransfer = initialState.coinTransfers.filter((transfer: CoinTransferBigNumber) => { | ||
return transfer.to === responderFreeBalanceAddress; | ||
})[0]; | ||
|
||
unidirectionalCoinTransferValidation( | ||
initiatorDeposit, | ||
responderDeposit, | ||
initiatorTransfer, | ||
responderTransfer, | ||
); | ||
}; |
Oops, something went wrong.