From ca8690501206062ade34972e19a037c5b6edf48d Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Fri, 16 Dec 2016 13:23:10 +0100 Subject: [PATCH 1/9] adding nonce ranges --- raiden/channel.py | 4 +++ .../smart_contracts/NettingChannelLibrary.sol | 28 ++++++++++++++++--- raiden/tests/utils/tester_client.py | 1 + 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/raiden/channel.py b/raiden/channel.py index a4d52cb9f1..346a459c3e 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -520,6 +520,10 @@ def __init__(self, our_state, partner_state, external_state, self.settle_timeout = settle_timeout self.external_state = external_state + # use nonce ranges + self.our_state.nonce = our_state.nonce * (external_state.opened_block * (2 ** 32)) + self.partner_state.nonce = partner_state.nonce * (external_state.opened_block * (2 ** 32)) + self.open_event = Event() self.close_event = Event() self.settle_event = Event() diff --git a/raiden/smart_contracts/NettingChannelLibrary.sol b/raiden/smart_contracts/NettingChannelLibrary.sol index 288c500e25..1e953c8d5d 100644 --- a/raiden/smart_contracts/NettingChannelLibrary.sol +++ b/raiden/smart_contracts/NettingChannelLibrary.sol @@ -64,6 +64,14 @@ library NettingChannelLibrary { _; } + modifier inNonceRange(Data storage self, bytes message) { + uint64 nonce; + nonce = getNonce(message); + if (nonce < self.opened * (2 ** 32) || nonce >= (self.opened + 1) * (2 ** 32)) + throw; + _; + } + /// @notice deposit(uint) to deposit amount to channel. /// @dev Deposit an amount to the channel. At least one of the participants /// must deposit before the channel is opened. @@ -198,7 +206,7 @@ library NettingChannelLibrary { } // else we are closing a channel that has received transfers - their_sender = processTransfer(node1, node2, their_transfer); + their_sender = processTransfer(self, node1, node2, their_transfer); if (their_sender == caller_address) { // the sender of "their" transaction can't be ourselves throw; @@ -207,7 +215,7 @@ library NettingChannelLibrary { if (our_transfer.length != 0) { address our_sender; // we also provided a courtesy update of our own latest transfer - our_sender = processTransfer(node1, node2, our_transfer); + our_sender = processTransfer(self, node1, node2, our_transfer); if (our_sender != caller_address) { // we have to be the sender of our own transaction throw; @@ -215,7 +223,11 @@ library NettingChannelLibrary { } } - function processTransfer(Participant storage node1, Participant storage node2, bytes transfer) internal returns (address) { + function processTransfer(Data storage self, Participant storage node1, Participant storage node2, bytes transfer) + inNonceRange(self, transfer) + internal + returns (address) + { bytes memory transfer_raw; address transfer_address; @@ -267,7 +279,7 @@ library NettingChannelLibrary { Participant storage node1 = participants[0]; Participant storage node2 = participants[1]; - processTransfer(node1, node2, their_transfer); + processTransfer(self, node1, node2, their_transfer); // TODO check if tampered and penalize // TODO check if outdated and penalize @@ -572,6 +584,14 @@ library NettingChannelLibrary { } } + // Get nonce from a message + function getNonce(bytes message) private returns (uint64 nonce) { + // don't care about length of message since nonce is always at a fixed position + assembly { + nonce := mload(add(message, 12)) + } + } + function signatureSplit(bytes signature) private returns (bytes32 r, bytes32 s, uint8 v) { // The signature format is a compact form of: // {bytes32 r}{bytes32 s}{uint8 v} diff --git a/raiden/tests/utils/tester_client.py b/raiden/tests/utils/tester_client.py index 9fbf082cdc..222b23556a 100644 --- a/raiden/tests/utils/tester_client.py +++ b/raiden/tests/utils/tester_client.py @@ -121,6 +121,7 @@ def __init__(self, tester_state, private_key, address): def get_block_number(self): return self.tester_state.block.number + @property def opened_block(self): return self.proxy.opened() From 3c93da9e99412aaf38a9ecafb0cf2bbe03b09f9f Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Fri, 16 Dec 2016 16:44:05 +0100 Subject: [PATCH 2/9] adding test for re opening of a channel plus nonce ranges --- .../smart_contracts/test_channel_manager.py | 82 ++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/raiden/tests/smart_contracts/test_channel_manager.py b/raiden/tests/smart_contracts/test_channel_manager.py index 431c6f5043..44a3c5fa5d 100644 --- a/raiden/tests/smart_contracts/test_channel_manager.py +++ b/raiden/tests/smart_contracts/test_channel_manager.py @@ -3,8 +3,10 @@ from ethereum import tester from ethereum.utils import encode_hex, sha3 -from raiden.utils import get_contract_path +from raiden.utils import get_contract_path, privatekey_to_address +from raiden.encoding.signing import GLOBAL_CTX from ethereum.tester import ABIContract, ContractTranslator, TransactionFailed +from secp256k1 import PrivateKey from raiden.tests.utils.tester import new_channelmanager @@ -146,3 +148,81 @@ def test_channelmanager(tester_state, tester_token, tester_events, # assert k1 == sha3(vs[0] + vs[1]) # with pytest.raises(TransactionFailed): # channel_manager.key(sha3('address1')[:20], sha3('address1')[:20]) + +@pytest.mark.xfail +def test_reopen_channel(tester_state, tester_channelmanager, tester_channels, settle_timeout, + netting_channel_abi): + privatekey0_raw, privatekey1_raw, nettingchannel, channel0, _ = tester_channels[0] + + privatekey0 = PrivateKey(privatekey0_raw, ctx=GLOBAL_CTX, raw=True) + address0 = privatekey_to_address(privatekey0_raw) + address1 = privatekey_to_address(privatekey1_raw) + address2 = tester.a2 + + # We need to close the channel before it can be deleted, to do so we need + # one transfer to call closeSingleTransfer(0 + transfer_amount = 10 + identifier = 1 + direct_transfer = channel0.create_directtransfer( + transfer_amount, + identifier, + ) + direct_transfer.sign(privatekey0, address0) + direct_transfer_data = str(direct_transfer.packed().data) + + should_be_nonce = nettingchannel.opened(sender=privatekey0_raw) * (2**32) + should_be_nonce_plus_one = (nettingchannel.opened(sender=privatekey0_raw) + 1) * (2**32) + assert should_be_nonce <= direct_transfer.nonce < should_be_nonce_plus_one + + # settle the channel should not change the channel manager state + nettingchannel.closeSingleTransfer( + direct_transfer_data, + sender=privatekey1_raw, + ) + tester_state.mine(number_of_blocks=settle_timeout + 1) + + # delete the channel needs to update the manager's state + number_of_channels = len(tester_channelmanager.getChannelsAddresses(sender=privatekey0_raw)) + + nettingchannel.settle(sender=privatekey0_raw) + + tester_state.mine(1) + + # now a single new channel can be open + # if channel with address is settled a new can be opened + # old entry will be deleted when calling newChannel + netting_channel_address1_hex = tester_channelmanager.newChannel( + address1, + settle_timeout, + sender=privatekey0_raw, + ) + + netting_channel_translator = ContractTranslator(netting_channel_abi) + + netting_contract_proxy1 = ABIContract( + tester_state, + netting_channel_translator, + netting_channel_address1_hex, + ) + + # transfer not in nonce range + with pytest.raises(TransactionFailed): + netting_contract_proxy1.closeSingleTransfer( + direct_transfer_data, + sender=privatekey0_raw, + ) + + # channel already exists + with pytest.raises(TransactionFailed): + tester_channelmanager.newChannel( + address1, + settle_timeout, + sender=privatekey0_raw, + ) + + # opening a new channel that did not exist before + netting_channel_address2_hex = tester_channelmanager.newChannel( + address2, + settle_timeout, + sender=privatekey0_raw, + ) From 2b46459816d2ee7e54bc87cb03fc54d037b74d62 Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Mon, 19 Dec 2016 13:23:29 +0100 Subject: [PATCH 3/9] put each test method argument on its own line --- .../smart_contracts/test_channel_manager.py | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/raiden/tests/smart_contracts/test_channel_manager.py b/raiden/tests/smart_contracts/test_channel_manager.py index 44a3c5fa5d..a93b804735 100644 --- a/raiden/tests/smart_contracts/test_channel_manager.py +++ b/raiden/tests/smart_contracts/test_channel_manager.py @@ -11,8 +11,12 @@ from raiden.tests.utils.tester import new_channelmanager -def test_channelnew_event(settle_timeout, tester_state, tester_events, - tester_registry, tester_token): +def test_channelnew_event( + settle_timeout, + tester_state, + tester_events, + tester_registry, + tester_token): privatekey0 = tester.DEFAULT_KEY address0 = tester.DEFAULT_ACCOUNT @@ -42,10 +46,13 @@ def test_channelnew_event(settle_timeout, tester_state, tester_events, } -def test_channelmanager(tester_state, tester_token, tester_events, - tester_channelmanager_library_address, settle_timeout, - netting_channel_abi): - # pylint: disable=too-many-locals,too-many-statements +def test_channelmanager( + tester_state, + tester_token, + tester_events, + tester_channelmanager_library_address, + settle_timeout, + netting_channel_abi): # pylint: disable=too-many-locals,too-many-statements address0 = tester.DEFAULT_ACCOUNT address1 = tester.a1 @@ -150,8 +157,13 @@ def test_channelmanager(tester_state, tester_token, tester_events, # channel_manager.key(sha3('address1')[:20], sha3('address1')[:20]) @pytest.mark.xfail -def test_reopen_channel(tester_state, tester_channelmanager, tester_channels, settle_timeout, - netting_channel_abi): +def test_reopen_channel( + tester_state, + tester_channelmanager, + tester_channels, + settle_timeout, + netting_channel_abi): + privatekey0_raw, privatekey1_raw, nettingchannel, channel0, _ = tester_channels[0] privatekey0 = PrivateKey(privatekey0_raw, ctx=GLOBAL_CTX, raw=True) From 3e672dd0db0ce5c401b5a7bee1b87497254683bb Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Mon, 19 Dec 2016 15:12:05 +0100 Subject: [PATCH 4/9] fixing typos --- raiden/tests/smart_contracts/test_channel_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/raiden/tests/smart_contracts/test_channel_manager.py b/raiden/tests/smart_contracts/test_channel_manager.py index a93b804735..61b92503b3 100644 --- a/raiden/tests/smart_contracts/test_channel_manager.py +++ b/raiden/tests/smart_contracts/test_channel_manager.py @@ -172,7 +172,7 @@ def test_reopen_channel( address2 = tester.a2 # We need to close the channel before it can be deleted, to do so we need - # one transfer to call closeSingleTransfer(0 + # one transfer to call closeSingleTransfer() transfer_amount = 10 identifier = 1 direct_transfer = channel0.create_directtransfer( @@ -193,14 +193,14 @@ def test_reopen_channel( ) tester_state.mine(number_of_blocks=settle_timeout + 1) - # delete the channel needs to update the manager's state + # deleting the channel needs to update the manager's state number_of_channels = len(tester_channelmanager.getChannelsAddresses(sender=privatekey0_raw)) nettingchannel.settle(sender=privatekey0_raw) tester_state.mine(1) - # now a single new channel can be open + # now a single new channel can be opened # if channel with address is settled a new can be opened # old entry will be deleted when calling newChannel netting_channel_address1_hex = tester_channelmanager.newChannel( From 95090805f4c99ba622fa04b19ae9be1a1cf7a155 Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Mon, 19 Dec 2016 16:34:21 +0100 Subject: [PATCH 5/9] move nonce range statement to ChannelEndState constructor --- raiden/assetmanager.py | 2 ++ raiden/channel.py | 8 ++------ raiden/tests/utils/tester.py | 2 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/raiden/assetmanager.py b/raiden/assetmanager.py index 595ca6b6d7..5a18b7e63e 100644 --- a/raiden/assetmanager.py +++ b/raiden/assetmanager.py @@ -119,10 +119,12 @@ def register_channel(self, netting_channel, reveal_timeout): our_state = ChannelEndState( channel_details['our_address'], channel_details['our_balance'], + self.raiden.chain.block_number, ) partner_state = ChannelEndState( channel_details['partner_address'], channel_details['partner_balance'], + self.raiden.chain.block_number, ) external_state = ChannelExternalState( diff --git a/raiden/channel.py b/raiden/channel.py index 346a459c3e..e2b7810bd3 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -268,7 +268,7 @@ def compute_proof_for_lock(self, secret, lock): class ChannelEndState(object): """ Tracks the state of one of the participants in a channel. """ - def __init__(self, participant_address, participant_balance): + def __init__(self, participant_address, participant_balance, get_block_number): # since ethereum only uses integral values we cannot use float/Decimal if not isinstance(participant_balance, (int, long)): raise ValueError('participant_balance must be an integer.') @@ -282,7 +282,7 @@ def __init__(self, participant_address, participant_balance): # sequential nonce, current value has not been used. # 0 is used in the netting contract to represent the lack of a # transfer, so this value must start at 1 - self.nonce = 1 + self.nonce = 1 * (get_block_number * (2 ** 32)) # contains the last known message with a valid signature and # transferred_amount, the secrets revealed since that transfer, and the @@ -520,10 +520,6 @@ def __init__(self, our_state, partner_state, external_state, self.settle_timeout = settle_timeout self.external_state = external_state - # use nonce ranges - self.our_state.nonce = our_state.nonce * (external_state.opened_block * (2 ** 32)) - self.partner_state.nonce = partner_state.nonce * (external_state.opened_block * (2 ** 32)) - self.open_event = Event() self.close_event = Event() self.settle_event = Event() diff --git a/raiden/tests/utils/tester.py b/raiden/tests/utils/tester.py index 0247073731..db77bc7ec3 100644 --- a/raiden/tests/utils/tester.py +++ b/raiden/tests/utils/tester.py @@ -104,10 +104,12 @@ def channel_from_nettingcontract(our_key, netting_contract, external_state, reve our_state = ChannelEndState( our_address, our_balance, + external_state.opened_block, ) partner_state = ChannelEndState( partner_address, partner_balance, + external_state.opened_block, ) channel = Channel( From 29f3d90d1bf1189dee3e1b47414d689cedae6ea4 Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Mon, 19 Dec 2016 19:33:12 +0100 Subject: [PATCH 6/9] handle int and instancemethod for block number --- raiden/channel.py | 5 ++++- raiden/tests/unit/test_channel.py | 15 +++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/raiden/channel.py b/raiden/channel.py index e2b7810bd3..754ec6fb7f 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -282,7 +282,10 @@ def __init__(self, participant_address, participant_balance, get_block_number): # sequential nonce, current value has not been used. # 0 is used in the netting contract to represent the lack of a # transfer, so this value must start at 1 - self.nonce = 1 * (get_block_number * (2 ** 32)) + if isinstance(get_block_number, int): + self.nonce = 1 * (get_block_number * (2 ** 32)) + else: + self.nonce = 1 * (get_block_number() * (2 ** 32)) # contains the last known message with a valid signature and # transferred_amount, the secrets revealed since that transfer, and the diff --git a/raiden/tests/unit/test_channel.py b/raiden/tests/unit/test_channel.py index cab33fc81b..c10c9ea0ce 100644 --- a/raiden/tests/unit/test_channel.py +++ b/raiden/tests/unit/test_channel.py @@ -43,6 +43,7 @@ def make_external_state(): def test_end_state(): + netting_channel = NettingChannelMock() asset_address = make_address() privkey1, address1 = make_privkey_address() address2 = make_address() @@ -55,8 +56,8 @@ def test_end_state(): lock_expiration = 10 lock_hashlock = sha3(lock_secret) - state1 = ChannelEndState(address1, balance1) - state2 = ChannelEndState(address2, balance2) + state1 = ChannelEndState(address1, balance1, netting_channel.opened) + state2 = ChannelEndState(address2, balance2, netting_channel.opened) assert state1.contract_balance == balance1 assert state2.contract_balance == balance2 @@ -187,6 +188,7 @@ def test_end_state(): def test_invalid_timeouts(): + netting_channel = NettingChannelMock() asset_address = make_address() reveal_timeout = 5 settle_timeout = 15 @@ -196,8 +198,8 @@ def test_invalid_timeouts(): balance1 = 10 balance2 = 10 - our_state = ChannelEndState(address1, balance1) - partner_state = ChannelEndState(address2, balance2) + our_state = ChannelEndState(address1, balance1, netting_channel.opened) + partner_state = ChannelEndState(address2, balance2, netting_channel.opened) external_state = make_external_state() # do not allow a reveal timeout larger than the settle timeout @@ -237,6 +239,7 @@ def test_invalid_timeouts(): def test_python_channel(): + netting_channel = NettingChannelMock() asset_address = make_address() privkey1, address1 = make_privkey_address() address2 = make_address() @@ -247,8 +250,8 @@ def test_python_channel(): reveal_timeout = 5 settle_timeout = 15 - our_state = ChannelEndState(address1, balance1) - partner_state = ChannelEndState(address2, balance2) + our_state = ChannelEndState(address1, balance1, netting_channel.opened) + partner_state = ChannelEndState(address2, balance2, netting_channel.opened) external_state = make_external_state() test_channel = Channel( From dacda6f953b65818da482ae799fd0b2373b16fdc Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Mon, 19 Dec 2016 23:49:54 +0100 Subject: [PATCH 7/9] use opened block instead of current block --- raiden/assetmanager.py | 4 ++-- raiden/channel.py | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/raiden/assetmanager.py b/raiden/assetmanager.py index 5a18b7e63e..7993bfd913 100644 --- a/raiden/assetmanager.py +++ b/raiden/assetmanager.py @@ -119,12 +119,12 @@ def register_channel(self, netting_channel, reveal_timeout): our_state = ChannelEndState( channel_details['our_address'], channel_details['our_balance'], - self.raiden.chain.block_number, + netting_channel.opened, ) partner_state = ChannelEndState( channel_details['partner_address'], channel_details['partner_balance'], - self.raiden.chain.block_number, + netting_channel.opened, ) external_state = ChannelExternalState( diff --git a/raiden/channel.py b/raiden/channel.py index 754ec6fb7f..44caaa0861 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -283,8 +283,14 @@ def __init__(self, participant_address, participant_balance, get_block_number): # 0 is used in the netting contract to represent the lack of a # transfer, so this value must start at 1 if isinstance(get_block_number, int): + # if get_block_number == 0: + # self.nonce = 1 * (1 * (2 ** 32)) + # else: self.nonce = 1 * (get_block_number * (2 ** 32)) else: + # if get_block_number() == 0: + # self.nonce = 1 * (1 * (2 ** 32)) + # else: self.nonce = 1 * (get_block_number() * (2 ** 32)) # contains the last known message with a valid signature and From 5603d6fea0cf38a49154e3efa782630cae2b14f2 Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Tue, 20 Dec 2016 12:51:21 +0100 Subject: [PATCH 8/9] removing unused outcommented code --- raiden/channel.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/raiden/channel.py b/raiden/channel.py index 44caaa0861..754ec6fb7f 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -283,14 +283,8 @@ def __init__(self, participant_address, participant_balance, get_block_number): # 0 is used in the netting contract to represent the lack of a # transfer, so this value must start at 1 if isinstance(get_block_number, int): - # if get_block_number == 0: - # self.nonce = 1 * (1 * (2 ** 32)) - # else: self.nonce = 1 * (get_block_number * (2 ** 32)) else: - # if get_block_number() == 0: - # self.nonce = 1 * (1 * (2 ** 32)) - # else: self.nonce = 1 * (get_block_number() * (2 ** 32)) # contains the last known message with a valid signature and From 6d6b4fde1ff614865533ed1d2f487a2fce264666 Mon Sep 17 00:00:00 2001 From: Jacob Stenum Czepluch Date: Wed, 21 Dec 2016 14:41:10 +0100 Subject: [PATCH 9/9] update comment describing the nonce range --- raiden/channel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/raiden/channel.py b/raiden/channel.py index 754ec6fb7f..d75be09ddd 100644 --- a/raiden/channel.py +++ b/raiden/channel.py @@ -281,7 +281,8 @@ def __init__(self, participant_address, participant_balance, get_block_number): # sequential nonce, current value has not been used. # 0 is used in the netting contract to represent the lack of a - # transfer, so this value must start at 1 + # the nonce value must be inside the netting channel allowed range + # that is defined in terms of the opened block if isinstance(get_block_number, int): self.nonce = 1 * (get_block_number * (2 ** 32)) else: