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 1 commit
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
316 changes: 292 additions & 24 deletions raiden_contracts/data/contracts.json
View file
Open in desktop

Large diffs are not rendered by default.

86 changes: 60 additions & 26 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 ParticipantSettleInput {
Copy link
Contributor

Choose a reason for hiding this comment

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

imho we could go with a shorter name here:
SettleInput (having a mandatory participant argument) should be completely self-explanatory :)

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,
ParticipantSettleInput({
participant: participant1,
transferred_amount: participant1_transferred_amount,
locked_amount: participant1_locked_amount,
locksroot: participant1_locksroot
}),
ParticipantSettleInput({
participant: participant2,
transferred_amount: participant2_transferred_amount,
locked_amount: participant2_locked_amount,
locksroot: participant2_locksroot
})
);
}

function settleChannel2(
uint256 channel_identifier,
ParticipantSettleInput memory participant1_settlement,
ParticipantSettleInput 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,16 @@ 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.transferred_amount,
participant1_settlement.locked_amount,
participant1_settlement.locksroot
));

require(verifyBalanceHashData(
participant2_state,
participant2_transferred_amount,
participant2_locked_amount,
participant2_locksroot
participant2_settlement.transferred_amount,
participant2_settlement.locked_amount,
participant2_settlement.locksroot
));

// We are calculating the final token amounts that need to be
Expand All @@ -752,17 +786,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 @@ -779,32 +813,32 @@ contract TokenNetwork is Utils {
channel_identifier,
participant1,
participant2,
participant1_locked_amount,
participant1_locksroot
participant1_settlement.locked_amount,
participant1_settlement.locksroot
);
storeUnlockData(
channel_identifier,
participant2,
participant1,
participant2_locked_amount,
participant2_locksroot
participant2_settlement.locked_amount,
participant2_settlement.locksroot
);

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
1 change: 1 addition & 0 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
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