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

Treat trying to deposit on a closed channel as recoverable error #4693

Merged
merged 3 commits into from
Sep 3, 2019
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
5 changes: 3 additions & 2 deletions raiden/api/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from raiden.constants import GENESIS_BLOCK_NUMBER, UINT256_MAX
from raiden.exceptions import (
AlreadyRegisteredTokenAddress,
ChannelNotOpenError,
DepositMismatch,
DepositOverLimit,
DuplicatedChannelError,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 = (
Expand Down
5 changes: 4 additions & 1 deletion raiden/api/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
AddressWithoutCode,
AlreadyRegisteredTokenAddress,
APIServerPortInUseError,
ChannelNotOpenError,
DepositMismatch,
DepositOverLimit,
DuplicatedChannelError,
Expand Down Expand Up @@ -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:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure about handling it here. Since this is right after opening in the same function I don't see how a race with the channel being closed can happen here.

But I opted to handle it here too just to be safe.

return api_error(errors=str(e), status_code=HTTPStatus.CONFLICT)

channel_state = views.get_channelstate_for(
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions raiden/connection_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from raiden.api.python import RaidenAPI
from raiden.constants import Environment
from raiden.exceptions import (
ChannelNotOpenError,
DepositMismatch,
DepositOverLimit,
DuplicatedChannelError,
Expand All @@ -30,6 +31,7 @@
InsufficientFunds,
RaidenRecoverableError,
TransactionThrew,
ChannelNotOpenError,
)


Expand Down
4 changes: 4 additions & 0 deletions raiden/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ class DuplicatedChannelError(RaidenRecoverableError):
"""Raised if someone tries to create a channel that already exists."""


class ChannelNotOpenError(RaidenRecoverableError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: I would generalize this to something like InvalidChannelState

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm perhaps unexpected channel state? Invalid would mean that it's something which we can't even parse.

"""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."""

Expand Down
12 changes: 11 additions & 1 deletion raiden/tests/integration/api/test_pythonapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down