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.

Sign up for GitHub

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

Use ABI V2 to shorten settle arg list #1392

Merged
merged 5 commits into from
Jun 9, 2020
Merged

Use ABI V2 to shorten settle arg list #1392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
adbcbde
Use ABI V2 to shorten settle arg list
karlb Jun 2, 2020
67cf44e
Shorten `ParticipantSettleInput` to `SettleInput`
karlb Jun 9, 2020
4deabfd
Use `SettleInput` in `verifyBalanceHashData`
karlb Jun 9, 2020
65a977e
Use `SettleInput` in `storeUnlockData`
karlb Jun 9, 2020
ad0a198
Compile contracts
karlb Jun 9, 2020
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
392 changes: 342 additions & 50 deletions raiden_contracts/data/contracts.json
View file
Open in desktop

Large diffs are not rendered by default.

114 changes: 68 additions & 46 deletions raiden_contracts/data/source/raiden/TokenNetwork.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pragma solidity 0.6.4;
pragma experimental ABIEncoderV2;

import "lib/ECVerify.sol";
import "raiden/Token.sol";
Expand Down Expand Up @@ -124,6 +125,13 @@ contract TokenNetwork is Utils {
uint256 locked_amount;
}

struct SettleInput {
address participant;
uint256 transferred_amount;
uint256 locked_amount;
bytes32 locksroot;
}

event ChannelOpened(
uint256 indexed channel_identifier,
address indexed participant1,
Expand Down Expand Up @@ -692,6 +700,30 @@ contract TokenNetwork is Utils {
bytes32 participant2_locksroot
)
public
Copy link
Contributor

Choose a reason for hiding this comment

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

why wouldn't we change the signature for settleChannel as well?

function settleChannel(
  uint256 channel_identifier,
  SettleInput participant1,
  SettleInput participant2
) ...

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, expand-contract pattern...I see.

{
settleChannel2(
channel_identifier,
SettleInput({
participant: participant1,
transferred_amount: participant1_transferred_amount,
locked_amount: participant1_locked_amount,
locksroot: participant1_locksroot
}),
SettleInput({
participant: participant2,
transferred_amount: participant2_transferred_amount,
locked_amount: participant2_locked_amount,
locksroot: participant2_locksroot
})
);
}

function settleChannel2(
uint256 channel_identifier,
SettleInput memory participant1_settlement,
SettleInput memory participant2_settlement
)
public
{
// There are several requirements that this function MUST enforce:
// - it MUST never fail; therefore, any overflows or underflows must be
Expand All @@ -708,6 +740,8 @@ contract TokenNetwork is Utils {
// therefore it cannot ensure correctness. Users MUST use the official
// Raiden clients for signing balance proofs.

address participant1 = participant1_settlement.participant;
address participant2 = participant2_settlement.participant;
require(channel_identifier == getChannelIdentifier(participant1, participant2));

bytes32 pair_hash;
Expand All @@ -725,16 +759,12 @@ contract TokenNetwork is Utils {

require(verifyBalanceHashData(
Copy link
Contributor

Choose a reason for hiding this comment

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

verifyBalanceHashData is also never used outside the context of settlement, right? So it could use (Participant)SettleInput as the sole argument.

participant1_state,
participant1_transferred_amount,
participant1_locked_amount,
participant1_locksroot
participant1_settlement
));

require(verifyBalanceHashData(
participant2_state,
participant2_transferred_amount,
participant2_locked_amount,
participant2_locksroot
participant2_settlement
));

// We are calculating the final token amounts that need to be
Expand All @@ -752,17 +782,17 @@ contract TokenNetwork is Utils {
// We are reusing variables due to the local variables number limit.
// For better readability this can be refactored further.
(
participant1_transferred_amount,
participant2_transferred_amount,
participant1_locked_amount,
participant2_locked_amount
participant1_settlement.transferred_amount,
participant2_settlement.transferred_amount,
participant1_settlement.locked_amount,
participant2_settlement.locked_amount
) = getSettleTransferAmounts(
participant1_state,
participant1_transferred_amount,
participant1_locked_amount,
participant1_settlement.transferred_amount,
participant1_settlement.locked_amount,
participant2_state,
participant2_transferred_amount,
participant2_locked_amount
participant2_settlement.transferred_amount,
participant2_settlement.locked_amount
);

// Remove the channel data from storage
Expand All @@ -777,34 +807,30 @@ contract TokenNetwork is Utils {
// locked amounts remaining in the contract.
storeUnlockData(
channel_identifier,
participant1,
participant2,
participant1_locked_amount,
participant1_locksroot
participant1_settlement,
participant2
);
storeUnlockData(
channel_identifier,
participant2,
participant1,
participant2_locked_amount,
participant2_locksroot
participant2_settlement,
participant1
);

emit ChannelSettled(
channel_identifier,
participant1_transferred_amount,
participant1_locksroot,
participant2_transferred_amount,
participant2_locksroot
participant1_settlement.transferred_amount,
participant1_settlement.locksroot,
participant2_settlement.transferred_amount,
participant2_settlement.locksroot
);

// Do the actual token transfers
if (participant1_transferred_amount > 0) {
require(token.transfer(participant1, participant1_transferred_amount));
if (participant1_settlement.transferred_amount > 0) {
require(token.transfer(participant1, participant1_settlement.transferred_amount));
}

if (participant2_transferred_amount > 0) {
require(token.transfer(participant2, participant2_transferred_amount));
if (participant2_settlement.transferred_amount > 0) {
require(token.transfer(participant2, participant2_settlement.transferred_amount));
}
}

Expand Down Expand Up @@ -1173,23 +1199,21 @@ contract TokenNetwork is Utils {

function storeUnlockData(
uint256 channel_identifier,
address sender,
address receiver,
uint256 locked_amount,
bytes32 locksroot
SettleInput memory settle_input,
address receiver
)
internal
{
// If there are transfers to unlock, store the locksroot and total
// amount of tokens
if (locked_amount == 0) {
if (settle_input.locked_amount == 0) {
return;
}

bytes32 key = getUnlockIdentifier(channel_identifier, sender, receiver);
bytes32 key = getUnlockIdentifier(channel_identifier, settle_input.participant, receiver);
UnlockData storage unlock_data = unlock_identifier_to_unlock_data[key];
unlock_data.locksroot = locksroot;
unlock_data.locked_amount = locked_amount;
unlock_data.locksroot = settle_input.locksroot;
unlock_data.locked_amount = settle_input.locked_amount;
}

function getChannelAvailableDeposit(
Expand Down Expand Up @@ -1455,9 +1479,7 @@ contract TokenNetwork is Utils {

function verifyBalanceHashData(
Participant storage participant,
uint256 transferred_amount,
uint256 locked_amount,
bytes32 locksroot
SettleInput memory settle_input
)
internal
view
Expand All @@ -1466,8 +1488,8 @@ contract TokenNetwork is Utils {
// When no balance proof has been provided, we need to check this
// separately because hashing values of 0 outputs a value != 0
if (participant.balance_hash == 0 &&
transferred_amount == 0 &&
locked_amount == 0
settle_input.transferred_amount == 0 &&
settle_input.locked_amount == 0
/* locksroot is ignored. */
) {
return true;
Expand All @@ -1476,9 +1498,9 @@ contract TokenNetwork is Utils {
// Make sure the hash of the provided state is the same as the stored
// balance_hash
return participant.balance_hash == keccak256(abi.encodePacked(
transferred_amount,
locked_amount,
locksroot
settle_input.transferred_amount,
settle_input.locked_amount,
settle_input.locksroot
));
}

Expand Down
21 changes: 7 additions & 14 deletions raiden_contracts/data/source/test/TokenNetworkInternalsTest.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pragma solidity 0.6.4;
pragma experimental ABIEncoderV2;

import "raiden/TokenNetwork.sol";

Expand Down Expand Up @@ -44,19 +45,15 @@ contract TokenNetworkInternalStorageTest is TokenNetwork {

function updateUnlockDataPublic(
uint256 channel_identifier,
address participant,
address partner,
uint256 locked_amount,
bytes32 locksroot
SettleInput memory settle_input,
address partner
)
public
{
return storeUnlockData(
channel_identifier,
participant,
partner,
locked_amount,
locksroot
settle_input,
partner
);
}

Expand Down Expand Up @@ -100,9 +97,7 @@ contract TokenNetworkInternalStorageTest is TokenNetwork {
function verifyBalanceHashDataPublic(
address to_verify,
address partner,
uint256 transferred_amount,
uint256 locked_amount,
bytes32 locksroot
SettleInput memory settle_input
)
public
view
Expand All @@ -114,9 +109,7 @@ contract TokenNetworkInternalStorageTest is TokenNetwork {
Participant storage to_verify_state = channel.participants[to_verify];
return verifyBalanceHashData(
to_verify_state,
transferred_amount,
locked_amount,
locksroot
settle_input
);
}

Expand Down
60 changes: 60 additions & 0 deletions raiden_contracts/tests/test_channel_settle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,66 @@ def test_settle_no_bp_success(
assert custom_token.functions.balanceOf(B).call() == deposit_B


def test_settle2_no_bp_success(
web3: Web3,
custom_token: Contract,
token_network: Contract,
create_channel_and_deposit: Callable,
get_accounts: Callable,
create_close_signature_for_no_balance_proof: Callable,
) -> None:
""" The simplest settlement, tested against the V2 ABI settle """
(A, B) = get_accounts(2)
deposit_A = 10
deposit_B = 6
settle_timeout = TEST_SETTLE_TIMEOUT_MIN
channel_identifier = create_channel_and_deposit(A, B, deposit_A, deposit_B)
closing_sig = create_close_signature_for_no_balance_proof(A, channel_identifier)

# Close channel with no balance proof
call_and_transact(
token_network.functions.closeChannel(
channel_identifier,
B,
A,
EMPTY_BALANCE_HASH,
0,
EMPTY_ADDITIONAL_HASH,
EMPTY_SIGNATURE,
closing_sig,
),
{"from": A},
)

# Do not call updateNonClosingBalanceProof

# Settlement window must be over before settling the channel
mine_blocks(web3, settle_timeout + 1)

# Settling the channel should work with no balance proofs
call_and_transact(
token_network.functions.settleChannel2(
channel_identifier=channel_identifier,
participant1_settlement=dict(
participant=A,
transferred_amount=0,
locked_amount=0,
locksroot=LOCKSROOT_OF_NO_LOCKS,
),
participant2_settlement=dict(
participant=B,
transferred_amount=0,
locked_amount=0,
locksroot=LOCKSROOT_OF_NO_LOCKS,
),
),
{"from": A},
)

assert custom_token.functions.balanceOf(A).call() == deposit_A
assert custom_token.functions.balanceOf(B).call() == deposit_B


def test_settle_channel_state(
web3: Web3,
get_accounts: Callable,
Expand Down