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

Fix settling when one side closes/updates with outdated BalanceProof #1689

Merged
merged 6 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions raiden-ts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## [Unreleased]
### Fixed
- [#1514] Fix handling of expired LockedTransfer and WithdrawRequest
- [#1607] Fix settling when one side closes/updates with outdated BalanceProof
- [#1637] Fix depositToUDC failing if services already have withdrawn some fees
- [#1651] Fix PFS being disabled if passed an undefined default config

Expand All @@ -15,6 +16,7 @@

[#837]: https://github.com/raiden-network/light-client/issues/837
[#1514]: https://github.com/raiden-network/light-client/issues/1514
[#1607]: https://github.com/raiden-network/light-client/issues/1607
[#1637]: https://github.com/raiden-network/light-client/issues/1637
[#1642]: https://github.com/raiden-network/light-client/issues/1642
[#1649]: https://github.com/raiden-network/light-client/pull/1649
Expand Down
99 changes: 63 additions & 36 deletions raiden-ts/src/channels/epics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ import { isActionOf } from '../utils/actions';
import { pluckDistinct } from '../utils/rx';
import { fromEthersEvent, getEventsStream, getNetwork } from '../utils/ethers';
import { encode } from '../utils/data';
import { RaidenError, ErrorCodes } from '../utils/error';
import { RaidenError, ErrorCodes, assert } from '../utils/error';
import { createBalanceHash, MessageTypeId } from '../messages/utils';
import { TokenNetwork } from '../contracts/TokenNetwork';
import { findBalanceProofMatchingBalanceHash } from '../transfers/utils';
import { ChannelState, Channel } from './state';
import {
newBlock,
Expand Down Expand Up @@ -806,11 +807,7 @@ export const channelCloseEpic = (
}

const balanceProof = channel.partner.balanceProof;
const balanceHash = createBalanceHash(
balanceProof.transferredAmount,
balanceProof.lockedAmount,
balanceProof.locksroot,
);
const balanceHash = createBalanceHash(balanceProof);
const nonce = balanceProof.nonce;
const additionalHash = balanceProof.additionalHash;
const nonClosingSignature = balanceProof.signature;
Expand Down Expand Up @@ -900,11 +897,7 @@ export const channelUpdateEpic = (
);
const channel = state.channels[channelKey(action.meta)];

const balanceHash = createBalanceHash(
channel.partner.balanceProof.transferredAmount,
channel.partner.balanceProof.lockedAmount,
channel.partner.balanceProof.locksroot,
);
const balanceHash = createBalanceHash(channel.partner.balanceProof);
const nonce = channel.partner.balanceProof.nonce;
const additionalHash = channel.partner.balanceProof.additionalHash;
const closingSignature = channel.partner.balanceProof.signature;
Expand Down Expand Up @@ -974,7 +967,7 @@ export const channelSettleEpic = (
filter(isActionOf(channelSettle.request)),
withLatestFrom(state$, config$),
mergeMap(([action, state, { subkey: configSubkey }]) => {
const { tokenNetwork } = action.meta;
const { tokenNetwork, partner } = action.meta;
const { signer: onchainSigner } = chooseOnchainAccount(
{ signer, address, main },
action.payload?.subkey ?? configSubkey,
Expand All @@ -992,31 +985,65 @@ export const channelSettleEpic = (
return of(channelSettle.failure(error, action.meta));
}

let part1 = channel.own;
let part2 = channel.partner;

// part1 total amounts must be <= part2 total amounts on settleChannel call
if (
part2.balanceProof.transferredAmount
.add(part2.balanceProof.lockedAmount)
.lt(part1.balanceProof.transferredAmount.add(part1.balanceProof.lockedAmount))
)
[part1, part2] = [part2, part1];

// send settleChannel transaction
return from(
tokenNetworkContract.functions.settleChannel(
channel.id,
part1.address,
part1.balanceProof.transferredAmount,
part1.balanceProof.lockedAmount,
part1.balanceProof.locksroot,
part2.address,
part2.balanceProof.transferredAmount,
part2.balanceProof.lockedAmount,
part2.balanceProof.locksroot,
),
// fetch closing/updated balanceHash for each end
Promise.all([
tokenNetworkContract.functions.getChannelParticipantInfo(channel.id, address, partner),
tokenNetworkContract.functions.getChannelParticipantInfo(channel.id, partner, address),
]),
).pipe(
mergeMap(([{ 3: ownBH }, { 3: partnerBH }]) => {
let ownBP = channel.own.balanceProof;
if (ownBH !== createBalanceHash(ownBP)) {
// partner closed/updated the channel with a non-latest BP from us
// they would lose our later transfers, but to settle we must search transfer history
const bp = findBalanceProofMatchingBalanceHash(state.sent, ownBH as Hash, action.meta);
assert(bp, [
ErrorCodes.CNL_SETTLECHANNEL_INVALID_BALANCEHASH,
{ address, ownBalanceHash: ownBH },
]);
ownBP = bp;
}

let partnerBP = channel.partner.balanceProof;
if (partnerBH !== createBalanceHash(partnerBP)) {
// shouldn't happen: if we closed, we must have done so with partner's latest BP
const bp = findBalanceProofMatchingBalanceHash(
state.received,
partnerBH as Hash,
action.meta,
);
assert(bp, [
ErrorCodes.CNL_SETTLECHANNEL_INVALID_BALANCEHASH,
{ partner, partnerBalanceHash: partnerBH },
]);
partnerBP = bp;
}

let part1 = [address, ownBP] as const;
let part2 = [partner, partnerBP] as const;

// part1 total amounts must be <= part2 total amounts on settleChannel call
if (
part2[1].transferredAmount
.add(part2[1].lockedAmount)
.lt(part1[1].transferredAmount.add(part1[1].lockedAmount))
)
[part1, part2] = [part2, part1]; // swap

// send settleChannel transaction
return tokenNetworkContract.functions.settleChannel(
channel.id,
part1[0],
part1[1].transferredAmount,
part1[1].lockedAmount,
part1[1].locksroot,
part2[0],
part2[1].transferredAmount,
part2[1].lockedAmount,
part2[1].locksroot,
);
}),
assertTx('settleChannel', ErrorCodes.CNL_SETTLECHANNEL_FAILED, { log }),
// if succeeded, return a empty/completed observable
// actual ChannelSettledAction will be detected and handled by channelMonitoredEpic
Expand Down Expand Up @@ -1154,7 +1181,7 @@ function checkPendingAction(
/**
* Process new blocks and re-emit confirmed or removed actions
*
* @param action$ - Observable of channelSettle actions
* @param action$ - Observable of RaidenActions
* @param state$ - Observable of RaidenStates
* @param deps - RaidenEpicDeps members
* @param deps.config$ - Config observable
Expand Down
18 changes: 4 additions & 14 deletions raiden-ts/src/channels/reducer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Zero, AddressZero, HashZero, One } from 'ethers/constants';
import { Zero, AddressZero, One } from 'ethers/constants';

import { UInt, Address, Hash } from '../utils/types';
import { UInt, Address } from '../utils/types';
import { Reducer, createReducer, isActionOf } from '../utils/actions';
import { partialCombineReducers } from '../utils/redux';
import { RaidenState, initialState } from '../state';
import { SignatureZero } from '../constants';
import { RaidenAction, ConfirmableActions } from '../actions';
import { transferSecretRegister } from '../transfers/actions';
import { Direction } from '../transfers/state';
Expand All @@ -20,6 +19,7 @@ import {
} from './actions';
import { Channel, ChannelState, ChannelEnd } from './state';
import { channelKey, channelUniqueKey } from './utils';
import { BalanceProofZero } from './types';

// state.blockNumber specific reducer, handles only newBlock action
const blockNumber = createReducer(initialState.blockNumber).handle(
Expand Down Expand Up @@ -56,17 +56,7 @@ const emptyChannelEnd: ChannelEnd = {
deposit: Zero as UInt<32>,
withdraw: Zero as UInt<32>,
locks: [],
balanceProof: {
chainId: Zero as UInt<32>,
tokenNetworkAddress: AddressZero as Address,
channelId: Zero as UInt<32>,
nonce: Zero as UInt<8>,
transferredAmount: Zero as UInt<32>,
lockedAmount: Zero as UInt<32>,
locksroot: HashZero as Hash,
additionalHash: HashZero as Hash,
signature: SignatureZero,
},
balanceProof: BalanceProofZero,
withdrawRequests: [],
nextNonce: One as UInt<8>,
};
Expand Down
42 changes: 29 additions & 13 deletions raiden-ts/src/channels/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as t from 'io-ts';
import { Zero, AddressZero, HashZero } from 'ethers/constants';

import { Address, Hash, UInt } from '../utils/types';
import { SignatureZero } from '../constants';
import { Address, Hash, UInt, Signed } from '../utils/types';

// should these become brands?
export const ChannelKey = t.string;
Expand Down Expand Up @@ -28,16 +30,30 @@ export interface Lock extends t.TypeOf<typeof Lock> {}
* because BP signature requires the hash of the message, for authentication of data not included
* nor relevant for the smartcontract/BP itself, but so for the peers (e.g. payment_id)
*/
export const BalanceProof = t.type({
// channel data
chainId: UInt(32),
tokenNetworkAddress: Address,
channelId: UInt(32),
// balance proof data
nonce: UInt(8),
transferredAmount: UInt(32),
lockedAmount: UInt(32),
locksroot: Hash,
additionalHash: Hash,
});
export const BalanceProof = t.readonly(
t.type({
// channel data
chainId: UInt(32),
tokenNetworkAddress: Address,
channelId: UInt(32),
// balance proof data
nonce: UInt(8),
transferredAmount: UInt(32),
lockedAmount: UInt(32),
locksroot: Hash,
additionalHash: Hash,
}),
);
export interface BalanceProof extends t.TypeOf<typeof BalanceProof> {}

export const BalanceProofZero: Signed<BalanceProof> = {
chainId: Zero as UInt<32>,
tokenNetworkAddress: AddressZero as Address,
channelId: Zero as UInt<32>,
nonce: Zero as UInt<8>,
transferredAmount: Zero as UInt<32>,
lockedAmount: Zero as UInt<32>,
locksroot: HashZero as Hash,
additionalHash: HashZero as Hash,
signature: SignatureZero,
};
1 change: 1 addition & 0 deletions raiden-ts/src/errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"CNL_OPENCHANNEL_FAILED": "Token networks openChannel transaction failed.",
"CNL_SETTOTALDEPOSIT_FAILED": "Token networks setTotalDeposit transaction failed.",
"CNL_CLOSECHANNEL_FAILED": "Token networks closeChannel transaction failed.",
"CNL_SETTLECHANNEL_INVALID_BALANCEHASH": "Could not find BalanceProof matching on-chain closing data. This client may have had its state cleared and can't settle.",
"CNL_SETTLECHANNEL_FAILED": "Token networks settleChannel transaction failed.",
"CNL_UPDATE_NONCLOSING_BP_FAILED": "updateNonClosingBalanceProof transaction failed.",
"CNL_ONCHAIN_UNLOCK_FAILED": "on-chain unlock transaction failed.",
Expand Down
29 changes: 15 additions & 14 deletions raiden-ts/src/messages/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { HashZero } from 'ethers/constants';
import logging from 'loglevel';

import { assert } from '../utils';
import { Address, Hash, HexString, Signature, UInt, Signed, decode } from '../utils/types';
import { Address, Hash, HexString, Signature, Signed, decode } from '../utils/types';
import { encode, losslessParse, losslessStringify } from '../utils/data';
import { BalanceProof } from '../channels/types';
import { EnvelopeMessage, Message, MessageType, Metadata } from './types';
Expand Down Expand Up @@ -54,16 +54,17 @@ export function createMetadataHash(metadata: Metadata): Hash {
/**
* Returns a balance_hash from transferred&locked amounts & locksroot
*
* @param transferredAmount - EnvelopeMessage.transferred_amount
* @param lockedAmount - EnvelopeMessage.locked_amount
* @param locksroot - Hash of all current locks
* @param bp - BalanceProof-like object
* @param bp.transferredAmount - balanceProof's transferredAmount
* @param bp.lockedAmount - balanceProof's lockedAmount
* @param bp.locksroot - balanceProof's locksroot
* @returns Hash of the balance
*/
export function createBalanceHash(
transferredAmount: UInt<32>,
lockedAmount: UInt<32>,
locksroot: Hash,
): Hash {
export function createBalanceHash({
transferredAmount,
lockedAmount,
locksroot,
}: Pick<BalanceProof, 'transferredAmount' | 'lockedAmount' | 'locksroot'>): Hash {
return (transferredAmount.isZero() && lockedAmount.isZero() && locksroot === HashZero
? HashZero
: keccak256(
Expand Down Expand Up @@ -152,11 +153,11 @@ export function packMessage(message: Message) {
case MessageType.UNLOCK:
case MessageType.LOCK_EXPIRED: {
const additionalHash = createMessageHash(message),
balanceHash = createBalanceHash(
message.transferred_amount,
message.locked_amount,
message.locksroot,
);
balanceHash = createBalanceHash({
transferredAmount: message.transferred_amount,
lockedAmount: message.locked_amount,
locksroot: message.locksroot,
});
return hexlify(
concat([
encode(message.token_network_address, 20),
Expand Down
18 changes: 3 additions & 15 deletions raiden-ts/src/migration/3.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,8 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { AddressZero, Zero, HashZero } from 'ethers/constants';
import { Zero } from 'ethers/constants';
import findKey from 'lodash/findKey';

import { SignatureZero } from '../constants';

const emptyBP = {
chainId: Zero,
tokenNetworkAddress: AddressZero,
channelId: Zero,
nonce: Zero,
transferredAmount: Zero,
lockedAmount: Zero,
locksroot: HashZero,
additionalHash: HashZero,
signature: SignatureZero,
};
import { BalanceProofZero } from '../channels';

function migrateChannelEnd(channel: any, endsAddr: any) {
for (const [prop, addr] of Object.entries<any>(endsAddr)) {
Expand All @@ -25,7 +13,7 @@ function migrateChannelEnd(channel: any, endsAddr: any) {
delete channel[prop].balanceProof.messageHash;
delete channel[prop].balanceProof.sender;
} else {
Object.assign(channel[prop], { balanceProof: emptyBP, locks: [] });
Object.assign(channel[prop], { balanceProof: BalanceProofZero, locks: [] });
}
Object.assign(channel[prop], { address: addr });
if (!('withdraw' in channel[prop])) {
Expand Down
6 changes: 1 addition & 5 deletions raiden-ts/src/services/epics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,11 +607,7 @@ function makeMonitoringRequest$({
take(1), // take/act on first time all conditions above pass
mergeMap(([, { monitoringReward, monitoringRoom }]) => {
const balanceProof = channel.partner.balanceProof;
const balanceHash = createBalanceHash(
balanceProof.transferredAmount,
balanceProof.lockedAmount,
balanceProof.locksroot,
);
const balanceHash = createBalanceHash(balanceProof);

const nonClosingMessage = concat([
encode(channel.tokenNetwork, 20),
Expand Down
4 changes: 0 additions & 4 deletions raiden-ts/src/transfers/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ export const transferRefunded = createAction(
);
export interface transferRefunded extends ActionType<typeof transferRefunded> {}

/** A pending transfer isn't needed anymore and should be cleared from state */
export const transferClear = createAction('transfer/clear', undefined, TransferId);
export interface transferClear extends ActionType<typeof transferClear> {}

// Withdraw actions

const WithdrawId = t.type({
Expand Down
Loading