From d0f715baabe9b037cdfcc874e96ed13e8008bde5 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Fri, 16 Apr 2021 12:18:31 +0200 Subject: [PATCH 1/5] :pencil2: [AssetHolder] Fix insufficient funds error message Signed-off-by: Matthias Geihs --- contracts/AssetHolder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/AssetHolder.sol b/contracts/AssetHolder.sol index f3e7133..7252f2b 100644 --- a/contracts/AssetHolder.sol +++ b/contracts/AssetHolder.sol @@ -157,7 +157,7 @@ abstract contract AssetHolder { require(settled[authorization.channelID], "channel not settled"); require(Sig.verify(abi.encode(authorization), signature, authorization.participant), "signature verification failed"); bytes32 id = calcFundingID(authorization.channelID, authorization.participant); - require(holdings[id] >= authorization.amount, "insufficient ETH for withdrawal"); + require(holdings[id] >= authorization.amount, "insufficient funds"); withdrawCheck(authorization, signature); holdings[id] = holdings[id].sub(authorization.amount); withdrawEnact(authorization, signature); From b241243f589516a1d2de5284c20f3891fca8f4d4 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Fri, 16 Apr 2021 12:33:37 +0200 Subject: [PATCH 2/5] :recycle: [Adjudicator] Rename function `channelID` to `calcChannelID` `channelID` is a convenient variable name, so we rename the function to prevent shadowing. Signed-off-by: Matthias Geihs --- contracts/Adjudicator.sol | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/contracts/Adjudicator.sol b/contracts/Adjudicator.sol index c9dbaf5..7b71218 100644 --- a/contracts/Adjudicator.sol +++ b/contracts/Adjudicator.sol @@ -209,7 +209,7 @@ contract Adjudicator { * @param params The parameters of the channel. * @return The ID of the channel. */ - function channelID(Channel.Params memory params) public pure returns (bytes32) { + function calcChannelID(Channel.Params memory params) public pure returns (bytes32) { return keccak256(Channel.encodeParams(params)); } @@ -231,7 +231,7 @@ contract Adjudicator { Channel.Params memory params, Channel.State memory state) internal pure { - require(state.channelID == channelID(params), "invalid params"); + require(state.channelID == calcChannelID(params), "invalid params"); } /** @@ -453,8 +453,8 @@ contract Adjudicator { * @dev Returns the dispute state for the given channelID. The second return * value indicates whether the given channel has been registered yet. */ - function getDispute(bytes32 _channelID) internal view returns (Dispute memory, bool) { - Dispute memory dispute = disputes[_channelID]; + function getDispute(bytes32 channelID) internal view returns (Dispute memory, bool) { + Dispute memory dispute = disputes[channelID]; return (dispute, dispute.stateHash != bytes32(0)); } @@ -462,8 +462,8 @@ contract Adjudicator { * @dev Returns the dispute state for the given channelID. Reverts if the * channel has not been registered yet. */ - function requireGetDispute(bytes32 _channelID) internal view returns (Dispute memory) { - (Dispute memory dispute, bool registered) = getDispute(_channelID); + function requireGetDispute(bytes32 channelID) internal view returns (Dispute memory) { + (Dispute memory dispute, bool registered) = getDispute(channelID); require(registered, "not registered"); return dispute; } @@ -472,8 +472,8 @@ contract Adjudicator { * @dev Sets the dispute state for the given channelID. Emits event * ChannelUpdate. */ - function setDispute(bytes32 _channelID, Dispute memory dispute) internal { - disputes[_channelID] = dispute; - emit ChannelUpdate(_channelID, dispute.version, dispute.phase, dispute.timeout); + function setDispute(bytes32 channelID, Dispute memory dispute) internal { + disputes[channelID] = dispute; + emit ChannelUpdate(channelID, dispute.version, dispute.phase, dispute.timeout); } } From 4a7d4c070a491861db24aefdaa7f6d17ccdcdcc7 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Fri, 16 Apr 2021 17:30:19 +0200 Subject: [PATCH 3/5] DRAFT: deposit recovery Signed-off-by: Matthias Geihs --- contracts/Adjudicator.sol | 77 ++++++++++++++++++++++++++++++++++----- contracts/AssetHolder.sol | 58 +++++++++++++++++++---------- 2 files changed, 106 insertions(+), 29 deletions(-) diff --git a/contracts/Adjudicator.sol b/contracts/Adjudicator.sol index 7b71218..97785e2 100644 --- a/contracts/Adjudicator.sol +++ b/contracts/Adjudicator.sol @@ -46,6 +46,7 @@ contract Adjudicator { uint64 version; bool hasApp; uint8 phase; + bool depositRecovery; // Indicates whether this is a deposit recovery. bytes32 stateHash; } @@ -87,13 +88,53 @@ contract Adjudicator { // If registered, require newer version and refutation timeout not passed. (Dispute memory dispute, bool registered) = getDispute(state.channelID); if (registered) { - require(dispute.version < state.version, "invalid version"); + // We skip the version check if the previous registration was a deposit recovery. + require(dispute.version < state.version || dispute.depositRecovery == true, "invalid version"); require(dispute.phase == uint8(DisputePhase.DISPUTE), "incorrect phase"); // solhint-disable-next-line not-rely-on-time require(block.timestamp < dispute.timeout, "refutation timeout passed"); } - storeChallenge(params, state, DisputePhase.DISPUTE); + storeChallenge(params, state, DisputePhase.DISPUTE, false); + } + + /** + * @notice registerDepositRecovery initiates the recovery of the deposited + * funds. It can be used if the channel state is lost. + * + * @param params The channel parameters. + * @param assets The assets to be recovered. + * @param sig A signature on the state. + * @param partIdx The participant index of the signer. + */ + function registerDepositRecovery( + Channel.Params memory params, + address[] memory assets, + bytes memory sig, + uint64 partIdx) + external + { + bytes32 channelID = calcChannelID(params); + // We authenticate the caller to protect against griefing. + require(Sig.verify(abi.encode(channelID, assets), sig, params.participants[partIdx]), "invalid signature"); + + (Dispute memory dispute, bool registered) = getDispute(channelID); + if (registered) { + require(dispute.depositRecovery == true, "already registered"); + require(dispute.phase == uint8(DisputePhase.DISPUTE), "wrong phase"); + } + + storeChallenge(params, Channel.State({ + channelID: channelID, + version: 0, + outcome: Channel.Allocation({ + assets: assets, + balances: zeroBalances(assets.length, params.participants.length), + locked: new Channel.SubAlloc[](0) + }), + appData: "", + isFinal: false + }), DisputePhase.DISPUTE, true); } /** @@ -119,6 +160,7 @@ contract Adjudicator { external { Dispute memory dispute = requireGetDispute(state.channelID); + require(dispute.depositRecovery == false, "deposit recovery"); // We do not allow state progression for deposit recoveries. if(dispute.phase == uint8(DisputePhase.DISPUTE)) { // solhint-disable-next-line not-rely-on-time require(block.timestamp >= dispute.timeout, "timeout not passed"); @@ -136,7 +178,7 @@ contract Adjudicator { require(Sig.verify(Channel.encodeState(state), sig, params.participants[actorIdx]), "invalid signature"); requireValidTransition(params, stateOld, state, actorIdx); - storeChallenge(params, state, DisputePhase.FORCEEXEC); + storeChallenge(params, state, DisputePhase.FORCEEXEC, false); } /** @@ -166,7 +208,7 @@ contract Adjudicator { requireValidParams(params, state); ensureTreeConcluded(state, subStates); - pushOutcome(state, subStates, params.participants); + pushOutcome(state, subStates, params.participants, dispute.depositRecovery); } /** @@ -198,10 +240,10 @@ contract Adjudicator { require(dispute.phase != uint8(DisputePhase.CONCLUDED), "channel already concluded"); } - storeChallenge(params, state, DisputePhase.CONCLUDED); + storeChallenge(params, state, DisputePhase.CONCLUDED, false); Channel.State[] memory subStates = new Channel.State[](0); - pushOutcome(state, subStates, params.participants); + pushOutcome(state, subStates, params.participants, false); } /** @@ -244,7 +286,8 @@ contract Adjudicator { function storeChallenge( Channel.Params memory params, Channel.State memory state, - DisputePhase disputePhase) + DisputePhase disputePhase, + bool depositRecovery) internal { (Dispute memory dispute, bool registered) = getDispute(state.channelID); @@ -253,6 +296,7 @@ contract Adjudicator { dispute.version = state.version; dispute.hasApp = params.app != address(0); dispute.phase = uint8(disputePhase); + dispute.depositRecovery = depositRecovery; dispute.stateHash = hashState(state); // Compute timeout. @@ -423,7 +467,8 @@ contract Adjudicator { function pushOutcome( Channel.State memory state, Channel.State[] memory subStates, - address[] memory participants) + address[] memory participants, + bool depositRecovery) internal { address[] memory assets = state.outcome.assets; @@ -445,7 +490,7 @@ contract Adjudicator { } // push accumulated outcome - AssetHolder(assets[a]).setOutcome(state.channelID, participants, outcome); + AssetHolder(assets[a]).setOutcome(state.channelID, participants, outcome, depositRecovery); } } @@ -476,4 +521,18 @@ contract Adjudicator { disputes[channelID] = dispute; emit ChannelUpdate(channelID, dispute.version, dispute.phase, dispute.timeout); } + + /** + * @notice zeroBalances creates a zero-balance array with the specified + * dimensions. + * + * @param m The length of the first dimension. + * @param m The length of the second dimension. + */ + function zeroBalances(uint m, uint n) internal pure returns (uint256[][] memory balances) { + balances = new uint256[][](m); + for (uint i = 0; i < m; i++) { + balances[i] = new uint256[](n); + } + } } diff --git a/contracts/AssetHolder.sol b/contracts/AssetHolder.sol index 7252f2b..41cb8fe 100644 --- a/contracts/AssetHolder.sol +++ b/contracts/AssetHolder.sol @@ -86,33 +86,38 @@ abstract contract AssetHolder { function setOutcome( bytes32 channelID, address[] calldata parts, - uint256[] calldata newBals) + uint256[] calldata newBals, + bool depositRecovery) external onlyAdjudicator { require(parts.length == newBals.length, "participants length should equal balances"); // solhint-disable-line reason-string require(settled[channelID] == false, "trying to set already settled channel"); // solhint-disable-line reason-string - // The channelID itself might already be funded - uint256 sumHeld = holdings[channelID]; - holdings[channelID] = 0; - uint256 sumOutcome = 0; - - bytes32[] memory fundingIDs = new bytes32[](parts.length); - for (uint256 i = 0; i < parts.length; i++) { - bytes32 id = calcFundingID(channelID, parts[i]); - // Save calculated ids to save gas. - fundingIDs[i] = id; - // Compute old balances. - sumHeld = sumHeld.add(holdings[id]); - // Compute new balances. - sumOutcome = sumOutcome.add(newBals[i]); - } + // We only redistribute assets if this is not a deposit recovery. + if (depositRecovery == false) { + // The channelID itself might already be funded + uint256 sumHeld = holdings[channelID]; + holdings[channelID] = 0; + uint256 sumOutcome = 0; - // We allow overfunding channels, who overfunds looses their funds. - if (sumHeld >= sumOutcome) { + bytes32[] memory fundingIDs = new bytes32[](parts.length); for (uint256 i = 0; i < parts.length; i++) { - holdings[fundingIDs[i]] = newBals[i]; + bytes32 id = calcFundingID(channelID, parts[i]); + // Save calculated ids to save gas. + fundingIDs[i] = id; + // Compute old balances. + sumHeld = sumHeld.add(holdings[id]); + // Compute new balances. + sumOutcome = sumOutcome.add(newBals[i]); + } + + // We allow overfunding channels, who overfunds looses their funds. + if (sumHeld >= sumOutcome) { + for (uint256 i = 0; i < parts.length; i++) { + holdings[fundingIDs[i]] = newBals[i]; + } } } + settled[channelID] = true; emit OutcomeSet(channelID); } @@ -130,13 +135,26 @@ abstract contract AssetHolder { * Calculated as the hash of the channel id and the participant address. * @param amount Amount of money that should be deposited. */ - function deposit(bytes32 fundingID, uint256 amount) external payable { + function deposit(bytes32 fundingID, uint256 amount) public payable { depositCheck(fundingID, amount); holdings[fundingID] = holdings[fundingID].add(amount); depositEnact(fundingID, amount); emit Deposited(fundingID, amount); } + /** + * @notice depositChannelParticipant deposits the given amount of assets + * at the specified channel for the specified participant. + * + * @param channelID Channel identifier. + * @param participant Channel participant. + * @param amount Deposit amount. + */ + function depositChannelParticipant(bytes32 channelID, address participant, uint256 amount) external payable { + bytes32 fundingID = calcFundingID(channelID, participant); + deposit(fundingID, amount); + } + /** * @notice Sends money from authorization.participant to authorization.receiver. * @dev Generic function which uses the virtual functions `withdrawCheck` and From ca7970219f2693082752a8224f97a5c5ea32d77a Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Fri, 16 Apr 2021 22:33:54 +0200 Subject: [PATCH 4/5] DRAFT: deposit recovery test Signed-off-by: Matthias Geihs --- src/test/Adjudicator.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/Adjudicator.ts b/src/test/Adjudicator.ts index 91db655..0b237a5 100644 --- a/src/test/Adjudicator.ts +++ b/src/test/Adjudicator.ts @@ -191,6 +191,8 @@ contract("Adjudicator", async (accounts) => { initialDeposit(B); }); + //TODO test deposit recovery + describeWithBlockRevert("register and refute", () => { const testsRegister = [ { From a2161c686e995fd8d7ed236cac03f306a3ab8399 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Sat, 17 Apr 2021 00:15:26 +0200 Subject: [PATCH 5/5] Fixup: test setOutcome call Signed-off-by: Matthias Geihs --- src/test/AssetHolder.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/AssetHolder.ts b/src/test/AssetHolder.ts index baa89c5..3ea5008 100644 --- a/src/test/AssetHolder.ts +++ b/src/test/AssetHolder.ts @@ -101,26 +101,26 @@ export function genericAssetHolderTest(setup: AssetHolderSetup) { it("wrong parts length", async () => { const wrongParts = [setup.parts[setup.A]] await truffleAssert.reverts( - setup.ah.setOutcome(setup.channelID, wrongParts, finalBalance, { from: setup.adj }), + setup.ah.setOutcome(setup.channelID, wrongParts, finalBalance, false, { from: setup.adj }), ); }); it("wrong balances length", async () => { const wrongBals = [ether(1)] await truffleAssert.reverts( - setup.ah.setOutcome(setup.channelID, setup.parts, wrongBals, { from: setup.adj }), + setup.ah.setOutcome(setup.channelID, setup.parts, wrongBals, false, { from: setup.adj }), ); }); it("wrong sender", async () => { await truffleAssert.reverts( - setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, { from: setup.txSender }), + setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, false, { from: setup.txSender }), ); }); it("correct sender", async () => { truffleAssert.eventEmitted( - await setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, { from: setup.adj }), + await setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, false, { from: setup.adj }), 'OutcomeSet', (ev: any) => { return ev.channelID == setup.channelID } ); @@ -133,7 +133,7 @@ export function genericAssetHolderTest(setup: AssetHolderSetup) { it("correct sender (twice)", async () => { await truffleAssert.reverts( - setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, { from: setup.adj }) + setup.ah.setOutcome(setup.channelID, setup.parts, finalBalance, false, { from: setup.adj }) ); }); }) @@ -187,7 +187,7 @@ export function genericAssetHolderTest(setup: AssetHolderSetup) { it("set outcome of the asset holder with deposit refusal", async () => { assert(await setup.ah.settled.call(channelID) == false); truffleAssert.eventEmitted( - await setup.ah.setOutcome(channelID, setup.parts, finalBalance, { from: setup.adj }), + await setup.ah.setOutcome(channelID, setup.parts, finalBalance, false, { from: setup.adj }), 'OutcomeSet', (ev: any) => { return ev.channelID == channelID; } );