Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send encrypted secret on metadata #2853

Merged
merged 11 commits into from
Jul 26, 2021
Merged
2 changes: 2 additions & 0 deletions raiden-ts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
## [Unreleased]
### Added
- [#2766] Add `Capabilities.IMMUTABLE_METADATA` (true on LC, fallback to falsy for backwards compatibility) to allow opting in of not prunning metadata.route and allowing to pass it through mediators unchanged
- [#2730] Add `config.encryptSecret` and `Raiden.transfer`'s `encryptSecret` boolean option, to allow sending secret to target on LockedTransfer's metadata, encrypted with ECIES over their publicKey, skipping SecretRequest/Reveal and speeding up transfers.

### Fixed
- [#2831] Force PFS to acknowledge our capabilities updates

[#2730]: https://github.com/raiden-network/light-client/issues/2730
[#2766]: https://github.com/raiden-network/light-client/pull/2766
[#2831]: https://github.com/raiden-network/light-client/issues/2831

Expand Down
2 changes: 2 additions & 0 deletions raiden-ts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"@ethersproject/bytes": "^5.4.0",
"@ethersproject/constants": "^5.4.0",
"@ethersproject/contracts": "^5.4.0",
"@ethersproject/hash": "^5.4.0",
"@ethersproject/keccak256": "^5.4.0",
"@ethersproject/networks": "^5.4.1",
"@ethersproject/properties": "^5.4.0",
Expand All @@ -96,6 +97,7 @@
"@ethersproject/wallet": "^5.4.0",
"abort-controller": "^3.0.0",
"decimal.js": "^10.3.1",
"eciesjs": "^0.3.11",
"ethers": "^5.4.1",
"fp-ts": "^2.10.5",
"io-ts": "^2.2.16",
Expand Down
4 changes: 3 additions & 1 deletion raiden-ts/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const RTCIceServer = t.type({ urls: t.union([t.string, t.array(t.string)]) });
* validated and decoded by [[FeeModel.decodeConfig]].
* - gasPriceFactor - Multiplier to be applied over `eth_gasPrice` for initial transactions gas prices;
* 1.1 means gasPrice returned by ETH node is added of 10% for every transaction sent
* - encryptSecret - Whether to send secret encrypted to target by default on transfers
weilbith marked this conversation as resolved.
Show resolved Hide resolved
* - matrixServer? - Specify a matrix server to use.
* - subkey? - When using subkey, this sets the behavior when { subkey } option isn't explicitly
* set in on-chain method calls. false (default) = use main key; true = use subkey
Expand Down Expand Up @@ -93,6 +94,7 @@ export const RaidenConfig = t.readonly(
autoUDCWithdraw: t.boolean,
mediationFees: t.unknown,
gasPriceFactor: t.number,
encryptSecret: t.boolean,
}),
t.partial({
matrixServer: t.string,
Expand Down Expand Up @@ -125,7 +127,6 @@ export function makeDefaultConfig(
network.chainId === 1
? 'https://raw.githubusercontent.com/raiden-network/raiden-service-bundle/master/known_servers/known_servers-production-v1.2.0.json'
: 'https://raw.githubusercontent.com/raiden-network/raiden-service-bundle/master/known_servers/known_servers-development-v1.2.0.json';

// merge caps independently
const caps =
overwrites?.caps === null
Expand Down Expand Up @@ -160,6 +161,7 @@ export function makeDefaultConfig(
autoUDCWithdraw: true,
mediationFees: {},
gasPriceFactor: 1.0,
encryptSecret: true,
...overwrites,
caps, // merged caps overwrites 'overwrites.caps'
};
Expand Down
48 changes: 17 additions & 31 deletions raiden-ts/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,56 +695,42 @@ export function waitChannelSettleable$(
/**
* Helper function to create the RaidenEpicDeps dependencies object for Raiden Epics
*
* @param signer - Signer holding raiden account connected to a JsonRpcProvider
* @param contractsInfo - Object holding deployment information from Raiden contracts on current network
* @param state - Initial/previous RaidenState
* @param config - defaultConfig overwrites
* @param opts - Options
* @param opts.state - Initial/previous RaidenState
* @param opts.signer - Signer holding raiden account connected to a JsonRpcProvider
* @param opts.contractsInfo - Object holding deployment information from Raiden contracts on
* current network
* @param opts.db - Database instance
* @param opts.config - defaultConfig overwrites
* @param opts.main - Main account object, set when using a subkey as raiden signer
* @param opts.main.address - Address of main signer
* @param opts.main.signer - Signer instance from which the subkey used raiden account was derived
* @returns Constructed epics dependencies object
*/
export function makeDependencies(
signer: Signer,
contractsInfo: ContractsInfo,
state: RaidenState,
config: PartialRaidenConfig | undefined,
{
signer,
contractsInfo,
db,
state,
config,
main,
}: {
state: RaidenState;
db: RaidenDatabase;
config?: PartialRaidenConfig;
main?: { address: Address; signer: Signer };
},
}: Pick<RaidenEpicDeps, 'signer' | 'contractsInfo' | 'db' | 'main'>,
): RaidenEpicDeps {
const provider = signer.provider;
assert(
provider && provider instanceof JsonRpcProvider && provider.network,
signer.provider && signer.provider instanceof JsonRpcProvider && signer.provider.network,
'Signer must be connected to a JsonRpcProvider',
);
const network = provider.network;
const latest$ = new ReplaySubject<Latest>(1);
const config$ = latest$.pipe(pluckDistinct('config'));
const matrix$ = new AsyncSubject<MatrixClient>();

const address = state.address;
const defaultConfig = makeDefaultConfig({ network }, config);
const log = logging.getLogger(`raiden:${address}`);

return {
latest$,
config$,
matrix$,
provider,
network,
config$: latest$.pipe(pluckDistinct('config')),
matrix$: new AsyncSubject<MatrixClient>(),
signer,
provider: signer.provider,
network: signer.provider.network,
address: state.address,
log,
defaultConfig,
log: logging.getLogger(`raiden:${state.address}`),
defaultConfig: makeDefaultConfig({ network: signer.provider.network }, config),
contractsInfo,
registryContract: TokenNetworkRegistry__factory.connect(
contractsInfo.TokenNetworkRegistry.address,
Expand Down
51 changes: 42 additions & 9 deletions raiden-ts/src/messages/utils.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
import type { Signer } from '@ethersproject/abstract-signer';
import { arrayify, concat as concatBytes, hexlify } from '@ethersproject/bytes';
import { HashZero } from '@ethersproject/constants';
import { hashMessage } from '@ethersproject/hash';
import { keccak256 } from '@ethersproject/keccak256';
import { encode as rlpEncode } from '@ethersproject/rlp';
import { recoverPublicKey } from '@ethersproject/signing-key';
import { toUtf8Bytes } from '@ethersproject/strings';
import { computeAddress } from '@ethersproject/transactions';
import { verifyMessage } from '@ethersproject/wallet';
import type * as t from 'io-ts';
import { canonicalize } from 'json-canonicalize';
import logging from 'loglevel';

import type { BalanceProof } from '../channels/types';
import { Capabilities, LocksrootZero } from '../constants';
import type { matrixPresence } from '../transport/actions';
import { matrixPresence } from '../transport/actions';
import type { Caps } from '../transport/types';
import { getCap } from '../transport/utils';
import { getCap, parseCaps } from '../transport/utils';
import type { RaidenEpicDeps } from '../types';
import { assert } from '../utils';
import { encode, jsonParse, jsonStringify } from '../utils/data';
import { ErrorCodes } from '../utils/error';
import type { Address, Hash, HexString } from '../utils/types';
import type { Address, Hash, HexString, PublicKey } from '../utils/types';
import { decode, Signature, Signed } from '../utils/types';
import { messageReceived } from './actions';
import type { AddressMetadata, EnvelopeMessage } from './types';
Expand Down Expand Up @@ -419,23 +422,53 @@ export function isMessageReceivedOfType<C extends t.Mixed>(messageCodecs: C | C[
: messageCodecs.is(action.payload.message));
}

/**
* @param metadata - to convert to presence
* @returns presence for metadata, assuming node is available
*/
export function metadataToPresence(metadata: AddressMetadata): matrixPresence.success {
const pubkey = recoverPublicKey(
arrayify(hashMessage(metadata.user_id)),
metadata.displayname,
) as PublicKey;
const address = computeAddress(pubkey) as Address;
return matrixPresence.success(
{
userId: metadata.user_id,
available: true,
ts: Date.now(),
caps: parseCaps(metadata.capabilities),
pubkey,
},
{ address },
);
}

/**
* Validates metadata was signed by address
*
* @param metadata - Peer's metadata
* @param address - Peer's address
* @param opts - Options
* @param opts.log - Logger instance
* @returns Metadata iff it's valid and was signed by address
* @returns presence iff metadata is valid and was signed by address
*/
export function validateAddressMetadata(
metadata: AddressMetadata | undefined,
address: Address,
{ log }: { log: logging.Logger } = { log: logging },
): AddressMetadata | undefined {
if (metadata && verifyMessage(metadata.user_id, metadata.displayname) === address)
return metadata;
else if (metadata) log?.warn('Invalid address metadata', { address, metadata });
{ log }: Partial<Pick<RaidenEpicDeps, 'log'>> = {},
): matrixPresence.success | undefined {
if (!metadata) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we were logging a warning here, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anything was logged if !metadata. The log call was only issued if metadata existed, so in the spirit of the happy case to the left, I check the undefined case early and just skip in that case.

try {
const presence = metadataToPresence(metadata);
assert(presence.meta.address === address, [
'Wrong signature',
{ expected: address, recovered: presence.meta.address },
]);
return presence;
} catch (error) {
log?.warn('Invalid address metadata', { address, metadata, error });
}
}

/**
Expand Down
104 changes: 34 additions & 70 deletions raiden-ts/src/raiden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { createLogger } from 'redux-logger';
import type { EpicMiddleware } from 'redux-observable';
import { createEpicMiddleware } from 'redux-observable';
import type { Observable } from 'rxjs';
import { defer, EMPTY, from, merge, of, throwError } from 'rxjs';
import { EMPTY, from, of, throwError } from 'rxjs';
import { fromFetch } from 'rxjs/fetch';
import {
catchError,
Expand Down Expand Up @@ -83,7 +83,6 @@ import {
getSecrethash,
makePaymentId,
makeSecret,
metadataFromPaths,
raidenTransfer,
transferKey,
transferKeyToMeta,
Expand All @@ -92,7 +91,7 @@ import { matrixPresence } from './transport/actions';
import type { ContractsInfo, OnChange, RaidenEpicDeps } from './types';
import { EventTypes } from './types';
import { assert } from './utils';
import { asyncActionToPromise, isActionOf, isResponseOf } from './utils/actions';
import { asyncActionToPromise, isActionOf } from './utils/actions';
import { jsonParse } from './utils/data';
import { ErrorCodes, RaidenError } from './utils/error';
import { getLogsByChunk$ } from './utils/ethers';
Expand Down Expand Up @@ -347,7 +346,7 @@ export class Raiden {
);
const cleanConfig = config && decode(PartialRaidenConfig, omitBy(config, isUndefined));

const deps = makeDependencies(signer, contractsInfo, { db, state, config: cleanConfig, main });
const deps = makeDependencies(state, cleanConfig, { signer, contractsInfo, db, main });
return new this(state, deps) as InstanceType<R>;
}

Expand Down Expand Up @@ -854,6 +853,8 @@ export class Raiden {
* disabled (null), use it if set or if undefined (auto mode), fetches the best
* PFS from ServiceRegistry and automatically fetch routes from it.
* @param options.lockTimeout - Specify a lock timeout for transfer; default is 2 * revealTimeout
* @param options.encryptSecret - Whether to force encrypting the secret or not,
* if target supports it
* @returns A promise to transfer's unique key (id) when it's accepted
*/
public async transfer(
Expand All @@ -867,6 +868,7 @@ export class Raiden {
paths?: RaidenPaths;
pfs?: RaidenPFS;
lockTimeout?: number;
encryptSecret?: boolean;
weilbith marked this conversation as resolved.
Show resolved Hide resolved
} = {},
): Promise<string> {
assert(Address.is(token), [ErrorCodes.DTA_INVALID_ADDRESS, { token }], this.log.info);
Expand All @@ -879,16 +881,9 @@ export class Raiden {
options.paymentId !== undefined
? decode(UInt(8), options.paymentId, ErrorCodes.DTA_INVALID_PAYMENT_ID, this.log.info)
: makePaymentId();
const paths = !options.paths
? undefined
: decode(Paths, options.paths, ErrorCodes.DTA_INVALID_PATH, this.log.info);
const pfs = !options.pfs
? undefined
: decode(PFS, options.pfs, ErrorCodes.DTA_INVALID_PFS, this.log.info);
// if undefined, default expiration is calculated at locked's [[makeAndSignTransfer$]]
const expiration = !options.lockTimeout
? undefined
: this.state.blockNumber + options.lockTimeout;
const paths =
options.paths && decode(Paths, options.paths, ErrorCodes.DTA_INVALID_PATH, this.log.info);
const pfs = options.pfs && decode(PFS, options.pfs, ErrorCodes.DTA_INVALID_PFS, this.log.info);

assert(
options.secret === undefined || Secret.is(options.secret),
Expand All @@ -902,73 +897,42 @@ export class Raiden {
);

// use provided secret or create one if no secrethash was provided
const secret = options.secret
? options.secret
: !options.secrethash
? makeSecret()
: undefined;
const secret = options.secret || (options.secrethash ? undefined : makeSecret());
const secrethash = options.secrethash || getSecrethash(secret!);
assert(
!secret || getSecrethash(secret) === secrethash,
ErrorCodes.RDN_SECRET_SECRETHASH_MISMATCH,
this.log.info,
);

const pathFindMeta = { tokenNetwork, target, value: decodedValue };
return merge(
// wait for pathFind response
this.action$.pipe(
first(isResponseOf(pathFind, pathFindMeta)),
const promise = this.action$
.pipe(
filter(isActionOf([transferSigned, transfer.failure])),
first(({ meta }) => meta.direction === Direction.SENT && meta.secrethash === secrethash),
map((action) => {
if (pathFind.failure.is(action)) throw action.payload;
return action.payload.paths;
if (transfer.failure.is(action)) throw action.payload;
return transferKey(action.meta);
}),
),
// request pathFind; even if paths were provided, send it again for validation
// this is done at 'merge' subscription time (i.e. when above action filter is subscribed)
defer(() => {
this.store.dispatch(pathFind.request({ paths, pfs }, pathFindMeta));
return EMPTY;
}),
)
.pipe(
mergeMap((paths) =>
merge(
// wait for transfer response
this.action$.pipe(
filter(isActionOf([transferSigned, transfer.failure])),
first(
(action) =>
action.meta.direction === Direction.SENT &&
action.meta.secrethash === secrethash,
),
map((action) => {
if (transfer.failure.is(action)) throw action.payload;
return transferKey(action.meta);
}),
),
// request transfer with returned/validated paths at 'merge' subscription time
defer(() => {
this.store.dispatch(
transfer.request(
{
tokenNetwork,
target,
value: decodedValue,
paymentId,
secret,
expiration,
...metadataFromPaths(paths),
},
{ secrethash, direction: Direction.SENT },
),
);
return EMPTY;
}),
),
),
)
.toPromise();
this.store.dispatch(
transfer.request(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be nicer to have a new action for this, instead of adding this resolved parameter? Something like resolveTransfer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but decided for the resolved tagged union because the whole metadata and most of the payload would overlap, and I'm not sure it's worth it to create a mostly similar copy of this action in order to split those. But I don't have hard feelings for it, it was mostly about code deduplication, but if you think it'd be better to have the copy, I can do it.

{
tokenNetwork,
target,
value: decodedValue,
paymentId,
secret,
resolved: false,
paths,
pfs,
lockTimeout: options.lockTimeout,
encryptSecret: options.encryptSecret,
},
{ secrethash, direction: Direction.SENT },
),
);
return promise;
}

/**
Expand Down
Loading