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

Check recipient in nettingchannel #602

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
30 changes: 24 additions & 6 deletions raiden/smart_contracts/NettingChannelLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ library NettingChannelLibrary {
bytes memory transfer_raw;
uint64 nonce;
address transfer_address;
address recipient;
bytes32 locksroot;
uint256 transferred_amount;

Expand Down Expand Up @@ -161,13 +162,18 @@ library NettingChannelLibrary {
// by the closing node
Participant storage counterparty = self.participants[counterparty_index];

(nonce, locksroot, transferred_amount) = decodeTransfer(transfer_raw);
(nonce, recipient, locksroot, transferred_amount) = decodeTransfer(transfer_raw);

// only accept messages with a valid nonce
if (!isValidNonce(self, nonce)) {
throw;
}

// the registered message recipient should be the closing party
if (recipient != self.closing_address) {
throw;
}

counterparty.nonce = nonce;
counterparty.locksroot = locksroot;
counterparty.transferred_amount = transferred_amount;
Expand All @@ -183,6 +189,7 @@ library NettingChannelLibrary {
stillTimeout(self)
{
address transfer_address;
address recipient;
bytes32 locksroot;
bytes memory transfer_raw;
uint256 transferred_amount;
Expand Down Expand Up @@ -215,13 +222,21 @@ library NettingChannelLibrary {
// counterparty
closer_index = 1 - caller_index;

(nonce, locksroot, transferred_amount) = decodeTransfer(transfer_raw);
(nonce, recipient, locksroot, transferred_amount) = decodeTransfer(transfer_raw);

// only accept messages with a valid nonce
if (!isValidNonce(self, nonce)) {
throw;
}

// the registered recipient in the message should be us
// Note: could have taken msg.sender here but trying to be future-proof
// for when we allow third party updates
Participant storage updating_party = self.participants[caller_index];
if (updating_party.node_address != recipient) {
throw;
}

self.participants[closer_index].nonce = nonce;
self.participants[closer_index].locksroot = locksroot;
self.participants[closer_index].transferred_amount = transferred_amount;
Expand Down Expand Up @@ -444,7 +459,7 @@ library NettingChannelLibrary {

function decodeTransfer(bytes transfer_raw)
internal
returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount)
returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount)
{
uint cmdid = uint(transfer_raw[0]);

Expand All @@ -461,7 +476,7 @@ library NettingChannelLibrary {

function decodeDirectTransfer(bytes memory message)
private
returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount)
returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount)
{
// size of the raw message without the signature
if (message.length != 124) {
Expand All @@ -479,14 +494,15 @@ library NettingChannelLibrary {
// [92:124] optional_locksroot
assembly {
nonce := mload(add(message, 12))
recipient := mload(add(message, 60))
transferred_amount := mload(add(message, 92))
locksroot := mload(add(message, 124))
}
}

function decodeMediatedTransfer(bytes memory message)
private
returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount)
returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount)
{
// size of the raw message without the signature
if (message.length != 268) {
Expand All @@ -510,14 +526,15 @@ library NettingChannelLibrary {
// [236:268] fee
assembly {
nonce := mload(add(message, 12))
recipient := mload(add(message, 68))
locksroot := mload(add(message, 140))
transferred_amount := mload(add(message, 204))
}
}

function decodeRefundTransfer(bytes memory message)
private
returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount)
returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount)
{
// size of the raw message without the signature
if (message.length != 196) {
Expand All @@ -538,6 +555,7 @@ library NettingChannelLibrary {
// [164:196] hashlock
assembly {
nonce := mload(add(message, 12))
recipient := mload(add(message, 68))
locksroot := mload(add(message, 100))
transferred_amount := mload(add(message, 132))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract DecoderTester {
return NettingChannelLibrary.getTransferRawAddress(signed_transfer);
}

function decodeTransfer(bytes transfer_raw) returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) {
function decodeTransfer(bytes transfer_raw) returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) {
return NettingChannelLibrary.decodeTransfer(transfer_raw);
}
}
27 changes: 26 additions & 1 deletion raiden/tests/smart_contracts/netting_channel/test_close.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ def test_close_accepts_only_transfer_from_participants(tester_channels, private_
""" Close must not accept a transfer from a non participant. """
pkey0, _, nettingchannel, channel0, _ = tester_channels[0]
nonparticipant_key = private_keys[2]
opened_block = nettingchannel.opened(sender=pkey0)

# make a transfer where pkey0 is the target
transfer_nonparticipant = DirectTransfer(
identifier=1,
nonce=1,
nonce=1 + (opened_block * (2 ** 32)),
token=channel0.token_address,
transferred_amount=10,
recipient=channel0.our_address,
Expand All @@ -102,6 +103,30 @@ def test_close_accepts_only_transfer_from_participants(tester_channels, private_
nettingchannel.close(transfer_nonparticipant_data, sender=pkey0)


@pytest.mark.parametrize('number_of_nodes', [3])
def test_close_wrong_recipient(tester_channels, private_keys):
""" Close must not accept a transfer aimed at a non recipient. """
pkey0, pkey1, nettingchannel, channel0, _ = tester_channels[0]
opened_block = nettingchannel.opened(sender=pkey0)
nonparticipant_address = privatekey_to_address(private_keys[2])

# make a transfer where the recipient is totally wrong
transfer_wrong_recipient = DirectTransfer(
identifier=1,
nonce=1 + (opened_block * (2 ** 32)),
token=channel0.token_address,
transferred_amount=10,
recipient=nonparticipant_address,
locksroot='',
)

transfer_wrong_recipient.sign(PrivateKey(pkey1), privatekey_to_address(pkey1))
transfer_wrong_recipient_data = str(transfer_wrong_recipient.packed().data)

with pytest.raises(TransactionFailed):
nettingchannel.close(transfer_wrong_recipient_data, sender=pkey0)


def test_close_called_multiple_times(tester_state, tester_nettingcontracts):
""" A channel can be closed only once. """
pkey0, pkey1, nettingchannel = tester_nettingcontracts[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ def assert_decoder_results(message, decoder):

(
nonce_decoded,
recipient_address_decoded,
locksroot_decoded,
transferred_amount_decoded
) = decoder.decodeTransfer(transfer_raw)

assert message.nonce == nonce_decoded
assert message.recipient == address_decoder(recipient_address_decoded)
assert message.transferred_amount == transferred_amount_decoded
assert message.locksroot == locksroot_decoded

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,12 @@ def test_update_must_fail_with_a_nonparticipant_transfer(tester_channels, privat
""" updateTransfer must not accept a transfer from a non participant. """
pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0]
nonparticipant_key = private_keys[2]
opened_block = nettingchannel.opened(sender=pkey0)

# make a transfer where pkey1 is the target
transfer_nonparticipant = DirectTransfer(
identifier=1,
nonce=1,
nonce=1 + (opened_block * (2 ** 32)),
token=channel0.token_address,
transferred_amount=10,
recipient=channel1.our_address,
Expand All @@ -106,6 +107,35 @@ def test_update_must_fail_with_a_nonparticipant_transfer(tester_channels, privat
nettingchannel.updateTransfer(transfer_nonparticipant_data, sender=pkey1)


@pytest.mark.parametrize('number_of_nodes', [3])
def test_update_must_fail_with_a_wrong_recipient(tester_channels, private_keys):
""" updateTransfer must not accept a transfer from a non participant. """
pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0]
opened_block = nettingchannel.opened(sender=pkey0)
nonparticipant_address = privatekey_to_address(private_keys[2])

# make a transfer where pkey1 is the target
transfer_wrong_recipient = DirectTransfer(
identifier=1,
nonce=1 + (opened_block * (2 ** 32)),
token=channel0.token_address,
transferred_amount=10,
recipient=nonparticipant_address,
locksroot='',
)

our_address = privatekey_to_address(pkey0)
our_sign_key = PrivateKey(pkey0)

transfer_wrong_recipient.sign(our_sign_key, our_address)
transfer_wrong_recipient_data = str(transfer_wrong_recipient.packed().data)

nettingchannel.close('', sender=pkey0)

with pytest.raises(TransactionFailed):
nettingchannel.updateTransfer(transfer_wrong_recipient_data, sender=pkey1)


def test_update_called_multiple_times_same_transfer(tester_channels):
""" updateTransfer can be called only once. """
pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0]
Expand Down