From accc02d1a1991760e205f06da0262b9b7e9322e5 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 30 Aug 2019 12:22:00 +0200 Subject: [PATCH 1/3] Treat trying to deposit on a closed channel as recoverable error Maybe fix #4451 If a channel is not open, or is in a closing state (still open but waiting on the closing transaction result) the `set_total_deposit` function was throwing a `ValueError`. That is a valid and recoverable race condition so this commit introduces an appropriate error and handles it in all places where it can be thrown. --- raiden/api/python.py | 5 +++-- raiden/api/rest.py | 5 ++++- raiden/connection_manager.py | 2 ++ raiden/exceptions.py | 4 ++++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/raiden/api/python.py b/raiden/api/python.py index e3a518fbdb..ee05d18e02 100644 --- a/raiden/api/python.py +++ b/raiden/api/python.py @@ -8,6 +8,7 @@ from raiden.constants import GENESIS_BLOCK_NUMBER, UINT256_MAX from raiden.exceptions import ( AlreadyRegisteredTokenAddress, + ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -571,6 +572,7 @@ def set_total_channel_deposit( AddressWithoutCode: The channel was settled during the deposit execution. DepositOverLimit: The total deposit amount is higher than the limit. + ChannelNotOpenError: The channel is no longer in an open state. """ chain_state = views.state_from_raiden(self.raiden) @@ -642,8 +644,7 @@ def set_total_channel_deposit( is_channel_open = channel.get_status(channel_state) == ChannelState.STATE_OPENED if not is_channel_open: - msg = "Channel is not in an open state." - raise ValueError(msg) + raise ChannelNotOpenError("Channel is not in an open state.") if safety_deprecation_switch: msg = ( diff --git a/raiden/api/rest.py b/raiden/api/rest.py index cc19378813..06cf67c5b5 100644 --- a/raiden/api/rest.py +++ b/raiden/api/rest.py @@ -59,6 +59,7 @@ AddressWithoutCode, AlreadyRegisteredTokenAddress, APIServerPortInUseError, + ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -646,7 +647,7 @@ def open( return api_error(errors=str(e), status_code=HTTPStatus.PAYMENT_REQUIRED) except (NonexistingChannel, UnknownTokenAddress) as e: return api_error(errors=str(e), status_code=HTTPStatus.BAD_REQUEST) - except (DepositOverLimit, DepositMismatch) as e: + except (DepositOverLimit, DepositMismatch, ChannelNotOpenError) as e: return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) channel_state = views.get_channelstate_for( @@ -1092,6 +1093,8 @@ def _deposit( return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) except TokenNetworkDeprecated as e: return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) + except ChannelNotOpenError as e: + return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) updated_channel_state = self.raiden_api.get_channel( registry_address, channel_state.token_address, channel_state.partner_state.address diff --git a/raiden/connection_manager.py b/raiden/connection_manager.py index 6552d2b47a..091da58b93 100644 --- a/raiden/connection_manager.py +++ b/raiden/connection_manager.py @@ -9,6 +9,7 @@ from raiden.api.python import RaidenAPI from raiden.constants import Environment from raiden.exceptions import ( + ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -30,6 +31,7 @@ InsufficientFunds, RaidenRecoverableError, TransactionThrew, + ChannelNotOpenError, ) diff --git a/raiden/exceptions.py b/raiden/exceptions.py index d075abe45c..430d1fb921 100644 --- a/raiden/exceptions.py +++ b/raiden/exceptions.py @@ -189,6 +189,10 @@ class DuplicatedChannelError(RaidenRecoverableError): """Raised if someone tries to create a channel that already exists.""" +class ChannelNotOpenError(RaidenRecoverableError): + """Raised if someone tries to work on a channel that is no longer in an open state.""" + + class ContractCodeMismatch(RaidenError): """Raised if the onchain code of the contract differs.""" From 63080298c7302e68d72fbba2bbf5be8819e61769 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 30 Aug 2019 13:01:30 +0200 Subject: [PATCH 2/3] Add test to see that the channel not open error is raised --- raiden/tests/integration/api/test_pythonapi.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/raiden/tests/integration/api/test_pythonapi.py b/raiden/tests/integration/api/test_pythonapi.py index 5819545de7..7db2f251c5 100644 --- a/raiden/tests/integration/api/test_pythonapi.py +++ b/raiden/tests/integration/api/test_pythonapi.py @@ -2,7 +2,12 @@ from eth_utils import to_checksum_address from raiden.api.python import RaidenAPI -from raiden.exceptions import DepositMismatch, TokenNotRegistered, UnknownTokenAddress +from raiden.exceptions import ( + ChannelNotOpenError, + DepositMismatch, + TokenNotRegistered, + UnknownTokenAddress, +) from raiden.tests.utils.detect_failure import raise_on_failure from raiden.tests.utils.events import must_have_event, wait_for_state_change from raiden.tests.utils.transfer import get_channelstate @@ -205,6 +210,11 @@ def run_test_raidenapi_channel_lifecycle(raiden_network, token_addresses, deposi ) assert channel.get_status(channel12) == ChannelState.STATE_CLOSED + with pytest.raises(ChannelNotOpenError): + api1.set_total_channel_deposit( + registry_address, token_address, api2.address, deposit + 100 + ) + assert wait_for_state_change( node1.raiden, ContractReceiveChannelSettled, From 781ad646dc52cfa7137331b408201b2ccdf0ce1c Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 30 Aug 2019 15:55:13 +0200 Subject: [PATCH 3/3] ChannelNotOpenError -> UnexpectedChannelState --- raiden/api/python.py | 6 +++--- raiden/api/rest.py | 6 +++--- raiden/connection_manager.py | 4 ++-- raiden/exceptions.py | 4 ++-- raiden/tests/integration/api/test_pythonapi.py | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/raiden/api/python.py b/raiden/api/python.py index ee05d18e02..c887383381 100644 --- a/raiden/api/python.py +++ b/raiden/api/python.py @@ -8,7 +8,6 @@ from raiden.constants import GENESIS_BLOCK_NUMBER, UINT256_MAX from raiden.exceptions import ( AlreadyRegisteredTokenAddress, - ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -22,6 +21,7 @@ RaidenRecoverableError, TokenNetworkDeprecated, TokenNotRegistered, + UnexpectedChannelState, UnknownTokenAddress, WithdrawMismatch, ) @@ -572,7 +572,7 @@ def set_total_channel_deposit( AddressWithoutCode: The channel was settled during the deposit execution. DepositOverLimit: The total deposit amount is higher than the limit. - ChannelNotOpenError: The channel is no longer in an open state. + UnexpectedChannelState: The channel is no longer in an open state. """ chain_state = views.state_from_raiden(self.raiden) @@ -644,7 +644,7 @@ def set_total_channel_deposit( is_channel_open = channel.get_status(channel_state) == ChannelState.STATE_OPENED if not is_channel_open: - raise ChannelNotOpenError("Channel is not in an open state.") + raise UnexpectedChannelState("Channel is not in an open state.") if safety_deprecation_switch: msg = ( diff --git a/raiden/api/rest.py b/raiden/api/rest.py index 06cf67c5b5..bd95db966a 100644 --- a/raiden/api/rest.py +++ b/raiden/api/rest.py @@ -59,7 +59,6 @@ AddressWithoutCode, AlreadyRegisteredTokenAddress, APIServerPortInUseError, - ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -79,6 +78,7 @@ TokenNetworkDeprecated, TokenNotRegistered, TransactionThrew, + UnexpectedChannelState, UnknownTokenAddress, WithdrawMismatch, ) @@ -647,7 +647,7 @@ def open( return api_error(errors=str(e), status_code=HTTPStatus.PAYMENT_REQUIRED) except (NonexistingChannel, UnknownTokenAddress) as e: return api_error(errors=str(e), status_code=HTTPStatus.BAD_REQUEST) - except (DepositOverLimit, DepositMismatch, ChannelNotOpenError) as e: + except (DepositOverLimit, DepositMismatch, UnexpectedChannelState) as e: return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) channel_state = views.get_channelstate_for( @@ -1093,7 +1093,7 @@ def _deposit( return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) except TokenNetworkDeprecated as e: return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) - except ChannelNotOpenError as e: + except UnexpectedChannelState as e: return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT) updated_channel_state = self.raiden_api.get_channel( diff --git a/raiden/connection_manager.py b/raiden/connection_manager.py index 091da58b93..019192b537 100644 --- a/raiden/connection_manager.py +++ b/raiden/connection_manager.py @@ -9,7 +9,6 @@ from raiden.api.python import RaidenAPI from raiden.constants import Environment from raiden.exceptions import ( - ChannelNotOpenError, DepositMismatch, DepositOverLimit, DuplicatedChannelError, @@ -19,6 +18,7 @@ RaidenRecoverableError, RaidenUnrecoverableError, TransactionThrew, + UnexpectedChannelState, ) from raiden.transfer import views from raiden.utils import typing @@ -31,7 +31,7 @@ InsufficientFunds, RaidenRecoverableError, TransactionThrew, - ChannelNotOpenError, + UnexpectedChannelState, ) diff --git a/raiden/exceptions.py b/raiden/exceptions.py index 430d1fb921..44f437b643 100644 --- a/raiden/exceptions.py +++ b/raiden/exceptions.py @@ -189,8 +189,8 @@ class DuplicatedChannelError(RaidenRecoverableError): """Raised if someone tries to create a channel that already exists.""" -class ChannelNotOpenError(RaidenRecoverableError): - """Raised if someone tries to work on a channel that is no longer in an open state.""" +class UnexpectedChannelState(RaidenRecoverableError): + """Raised if an operation is attempted on a channel while it is in an unexpected state.""" class ContractCodeMismatch(RaidenError): diff --git a/raiden/tests/integration/api/test_pythonapi.py b/raiden/tests/integration/api/test_pythonapi.py index 7db2f251c5..3c611a1106 100644 --- a/raiden/tests/integration/api/test_pythonapi.py +++ b/raiden/tests/integration/api/test_pythonapi.py @@ -3,9 +3,9 @@ from raiden.api.python import RaidenAPI from raiden.exceptions import ( - ChannelNotOpenError, DepositMismatch, TokenNotRegistered, + UnexpectedChannelState, UnknownTokenAddress, ) from raiden.tests.utils.detect_failure import raise_on_failure @@ -210,7 +210,7 @@ def run_test_raidenapi_channel_lifecycle(raiden_network, token_addresses, deposi ) assert channel.get_status(channel12) == ChannelState.STATE_CLOSED - with pytest.raises(ChannelNotOpenError): + with pytest.raises(UnexpectedChannelState): api1.set_total_channel_deposit( registry_address, token_address, api2.address, deposit + 100 )