From 9edcc074500fc28bd8281fb47dcabe0a01655767 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 11:00:23 +0200 Subject: [PATCH 01/11] Add precondition checks for the token network registry --- .../network/proxies/token_network_registry.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 3b2e76381c..1435dfdd47 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -30,6 +30,7 @@ Address, BlockSpecification, Dict, + SecretRegistryAddress, T_TargetAddress, TokenAddress, TokenAmount, @@ -74,6 +75,35 @@ def __init__( self.node_address = self.client.address self.proxy = proxy + def get_chain_id(self, block_identifier: BlockSpecification) -> int: + return self.proxy.contract.functions.chain_id().call(block_identifier=block_identifier) + + def get_settlement_timeout_min(self, block_identifier: BlockSpecification) -> int: + return self.proxy.contract.functions.settlement_timeout_min().call( + block_identifier=block_identifier + ) + + def get_settlement_timeout_max(self, block_identifier: BlockSpecification) -> int: + return self.proxy.contract.functions.settlement_timeout_max().call( + block_identifier=block_identifier + ) + + def get_secret_registry_address( + self, block_identifier: BlockSpecification + ) -> SecretRegistryAddress: + return SecretRegistryAddress( + self.proxy.contract.functions.secret_registry_address().call( + block_identifier=block_identifier + ) + ) + + def get_deprecation_executor(self, block_identifier: BlockSpecification) -> Address: + return Address( + self.proxy.contract.functions.deprecation_executor().call( + block_identifier=block_identifier + ) + ) + def get_token_network( self, token_address: TokenAddress, block_identifier: BlockSpecification ) -> Optional[TokenNetworkAddress]: @@ -118,6 +148,17 @@ def add_token( already_registered = self.get_token_network( token_address=token_address, block_identifier=block_identifier ) + deprecation_executor = self.get_deprecation_executor(block_identifier=block_identifier) + settlement_timeout_min = self.get_settlement_timeout_min( + block_identifier=block_identifier + ) + settlement_timeout_max = self.get_settlement_timeout_max( + block_identifier=block_identifier + ) + chain_id = self.get_chain_id(block_identifier=block_identifier) + secret_registry_address = self.get_secret_registry_address( + block_identifier=block_identifier + ) except ValueError: # If `block_identifier` has been pruned the checks cannot be performed pass @@ -129,6 +170,39 @@ def add_token( "The token is already registered in the TokenNetworkRegistry." ) + if deprecation_executor == NULL_ADDRESS: + raise BrokenPreconditionError( + "The deprecation executor property for the TokenNetworkRegistry is invalid." + ) + + if chain_id == 0: + raise BrokenPreconditionError( + "The chain ID property for the TokenNetworkRegistry is invalid." + ) + + if secret_registry_address == NULL_ADDRESS: + raise BrokenPreconditionError( + "The secret registry address for the token network is invalid." + ) + + if settlement_timeout_min == 0: + raise BrokenPreconditionError( + "The minimum settlement timeout for the token network " + "should be larger than zero." + ) + + if settlement_timeout_min == 0: + raise BrokenPreconditionError( + "The minimum settlement timeout for the token network " + "should be larger than zero." + ) + + if settlement_timeout_max <= settlement_timeout_min: + raise BrokenPreconditionError( + "The maximum settlement timeout for the token network " + "should be larger than the minimum settlement timeout." + ) + return self._add_token( token_address=token_address, channel_participant_deposit_limit=channel_participant_deposit_limit, From a02ea7eb9f6b3e462997c8c600aaf3b07f517615 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 11:39:47 +0200 Subject: [PATCH 02/11] Refactor the token network registry - Follow the token network pruned blocks handling pattern - Added transaction / gas estimation failure checks --- .../network/proxies/token_network_registry.py | 248 +++++++++++++----- 1 file changed, 186 insertions(+), 62 deletions(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 1435dfdd47..70865e11d4 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -143,8 +143,9 @@ def add_token( if token_address == NULL_ADDRESS_BYTES: raise InvalidTokenAddress("The call to register a token at 0x00..00 will fail.") - # check preconditions + token_proxy = self.blockchain_service.token(token_address) try: + token_supply = token_proxy.total_supply(block_identifier=block_identifier) already_registered = self.get_token_network( token_address=token_address, block_identifier=block_identifier ) @@ -165,6 +166,11 @@ def add_token( except BadFunctionCallOutput: raise_on_call_returned_empty(block_identifier) else: + if token_supply == "": + raise InvalidToken( + "Given token address does not follow the " + "ERC20 standard (missing `totalSupply()`)" + ) if already_registered: raise BrokenPreconditionError( "The token is already registered in the TokenNetworkRegistry." @@ -203,91 +209,209 @@ def add_token( "should be larger than the minimum settlement timeout." ) - return self._add_token( - token_address=token_address, - channel_participant_deposit_limit=channel_participant_deposit_limit, - token_network_deposit_limit=token_network_deposit_limit, - ) + log_details = { + "node": to_checksum_address(self.node_address), + "contract": to_checksum_address(self.address), + "token_address": to_checksum_address(token_address), + } + with log_transaction(log, "add_token", log_details): + return self._add_token( + token_address=token_address, + channel_participant_deposit_limit=channel_participant_deposit_limit, + token_network_deposit_limit=token_network_deposit_limit, + log_details=log_details, + ) def _add_token( self, token_address: TokenAddress, channel_participant_deposit_limit: TokenAmount, token_network_deposit_limit: TokenAmount, + log_details: Dict[Any, Any], ) -> TokenNetworkAddress: - if not is_binary_address(token_address): - raise ValueError("Expected binary address format for token") - - token_proxy = self.blockchain_service.token(token_address) + token_network_address = None - if token_proxy.total_supply() == "": - raise InvalidToken( - "Given token address does not follow the ERC20 standard (missing `totalSupply()`)" - ) + checking_block = self.client.get_checking_block() - log_details: Dict[str, Any] = { - "node": to_checksum_address(self.node_address), - "contract": to_checksum_address(self.address), - "token_address": to_checksum_address(token_address), + kwargs = { + "_token_address": token_address, + "_channel_participant_deposit_limit": channel_participant_deposit_limit, + "_token_network_deposit_limit": token_network_deposit_limit, } + gas_limit = self.proxy.estimate_gas(checking_block, "createERC20TokenNetwork", **kwargs) - failed_receipt = None - with log_transaction(log, "add_token", log_details): - checking_block = self.client.get_checking_block() - error_prefix = "Call to createERC20TokenNetwork will fail" - - kwarguments = { - "_token_address": token_address, - "_channel_participant_deposit_limit": channel_participant_deposit_limit, - "_token_network_deposit_limit": token_network_deposit_limit, - } - gas_limit = self.proxy.estimate_gas( - checking_block, "createERC20TokenNetwork", **kwarguments + if gas_limit: + gas_limit = safe_gas_limit( + gas_limit, self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] ) + log_details["gas_limit"] = gas_limit + transaction_hash = self.proxy.transact("createERC20TokenNetwork", gas_limit, **kwargs) - if gas_limit: - error_prefix = "Call to createERC20TokenNetwork failed" - gas_limit = safe_gas_limit( - gas_limit, - self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"], + receipt = self.client.poll(transaction_hash) + failed_receipt = check_transaction_threw(receipt=receipt) + + if failed_receipt: + failed_at_blocknumber = failed_receipt["blockNumber"] + + already_registered = self.get_token_network( + token_address=token_address, block_identifier=failed_at_blocknumber + ) + deprecation_executor = self.get_deprecation_executor( + block_identifier=failed_at_blocknumber + ) + settlement_timeout_min = self.get_settlement_timeout_min( + block_identifier=failed_at_blocknumber + ) + settlement_timeout_max = self.get_settlement_timeout_max( + block_identifier=failed_at_blocknumber + ) + chain_id = self.get_chain_id(block_identifier=failed_at_blocknumber) + secret_registry_address = self.get_secret_registry_address( + block_identifier=failed_at_blocknumber + ) + + if failed_receipt["cumulativeGasUsed"] == gas_limit: + msg = ( + f"createERC20TokenNetwork failed and all gas was used " + f"({gas_limit}). Estimate gas may have underestimated " + f"createERC20TokenNetwork, or succeeded even though an assert is " + f"triggered, or the smart contract code has an " + f"conditional assert." + ) + raise RaidenRecoverableError(msg) + + if already_registered: + # Race condition lost, the token network was created in a different + # transaction which got mined first. + raise RaidenRecoverableError( + "The token was already registered in the TokenNetworkRegistry." + ) + + if deprecation_executor == NULL_ADDRESS: + raise RaidenUnrecoverableError( + "The deprecation executor property for the " + "TokenNetworkRegistry is invalid." + ) + + if chain_id == 0: + raise RaidenUnrecoverableError( + "The chain ID property for the TokenNetworkRegistry is invalid." + ) + + if secret_registry_address == NULL_ADDRESS: + raise RaidenUnrecoverableError( + "The secret registry address for the token network is invalid." + ) + + if settlement_timeout_min == 0: + raise RaidenUnrecoverableError( + "The minimum settlement timeout for the token network " + "should be larger than zero." + ) + + if settlement_timeout_min == 0: + raise RaidenUnrecoverableError( + "The minimum settlement timeout for the token network " + "should be larger than zero." + ) + + if settlement_timeout_max <= settlement_timeout_min: + raise RaidenUnrecoverableError( + "The maximum settlement timeout for the token network " + "should be larger than the minimum settlement timeout." + ) + + # At this point, the TokenNetworkRegistry fails to instantiate + # a new TokenNetwork. + raise RaidenUnrecoverableError( + "createERC20TokenNetwork failed for an unknown reason" ) - log_details["gas_limit"] = gas_limit - transaction_hash = self.proxy.transact( - "createERC20TokenNetwork", gas_limit, **kwarguments + + token_network_address = self.get_token_network(token_address, receipt["blockHash"]) + if token_network_address is None: + msg = "createERC20TokenNetwork succeeded but token network address is Null" + raise RaidenUnrecoverableError(msg) + else: + # The latest block can not be used reliably because of reorgs, + # therefore every call using this block has to handle pruned data. + failed_at = self.proxy.jsonrpc_client.get_block("latest") + failed_at_blocknumber = failed_at["number"] + + already_registered = self.get_token_network( + token_address=token_address, block_identifier=failed_at_blocknumber + ) + deprecation_executor = self.get_deprecation_executor( + block_identifier=failed_at_blocknumber + ) + settlement_timeout_min = self.get_settlement_timeout_min( + block_identifier=failed_at_blocknumber + ) + settlement_timeout_max = self.get_settlement_timeout_max( + block_identifier=failed_at_blocknumber + ) + chain_id = self.get_chain_id(block_identifier=failed_at_blocknumber) + secret_registry_address = self.get_secret_registry_address( + block_identifier=failed_at_blocknumber + ) + + required_gas = ( + gas_limit + if gas_limit + else self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] + ) + self.proxy.jsonrpc_client.check_for_insufficient_eth( + transaction_name="createERC20TokenNetwork", + transaction_executed=False, + required_gas=required_gas, + block_identifier=failed_at_blocknumber, + ) + + if already_registered: + # Race condition lost, the token network was created in a different + # transaction which got mined first. + raise RaidenRecoverableError( + "The token was already registered in the TokenNetworkRegistry." ) - receipt = self.client.poll(transaction_hash) - failed_receipt = check_transaction_threw(receipt=receipt) + if deprecation_executor == NULL_ADDRESS: + raise RaidenUnrecoverableError( + "The deprecation executor property for the " "TokenNetworkRegistry is invalid." + ) - transaction_executed = gas_limit is not None - if not transaction_executed or failed_receipt: - if failed_receipt: - block = failed_receipt["blockNumber"] - else: - block = checking_block + if chain_id == 0: + raise RaidenUnrecoverableError( + "The chain ID property for the TokenNetworkRegistry is invalid." + ) - required_gas = ( - gas_limit - if gas_limit - else self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] + if secret_registry_address == NULL_ADDRESS: + raise RaidenUnrecoverableError( + "The secret registry address for the token network is invalid." ) - self.proxy.jsonrpc_client.check_for_insufficient_eth( - transaction_name="createERC20TokenNetwork", - transaction_executed=transaction_executed, - required_gas=required_gas, - block_identifier=block, + + if settlement_timeout_min == 0: + raise RaidenUnrecoverableError( + "The minimum settlement timeout for the token network " + "should be larger than zero." ) - if self.get_token_network(token_address, block): - raise RaidenRecoverableError(f"{error_prefix}. Token already registered") + if settlement_timeout_min == 0: + raise RaidenUnrecoverableError( + "The minimum settlement timeout for the token network " + "should be larger than zero." + ) - raise RaidenUnrecoverableError(error_prefix) + if settlement_timeout_max <= settlement_timeout_min: + raise RaidenUnrecoverableError( + "The maximum settlement timeout for the token network " + "should be larger than the minimum settlement timeout." + ) - token_network_address = self.get_token_network(token_address, "latest") - if token_network_address is None: - msg = "createERC20TokenNetwork succeeded but token network address is Null" - raise RuntimeError(msg) + if self.get_token_network(token_address, failed_at_blocknumber): + raise RaidenRecoverableError("Token already registered") + # At this point, the TokenNetworkRegistry fails to instantiate + # a new TokenNetwork. + raise RaidenUnrecoverableError("createERC20TokenNetwork failed for an unknown reason") return token_network_address def tokenadded_filter( From 325970cfe9cc482bc2133a19ae28145be1bc15c4 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 11:40:29 +0200 Subject: [PATCH 03/11] Remove string matching from exception in tests (error prone) --- .../integration/network/proxies/test_token_network_registry.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/raiden/tests/integration/network/proxies/test_token_network_registry.py b/raiden/tests/integration/network/proxies/test_token_network_registry.py index cbea4ae6bf..fcfbaa7e06 100644 --- a/raiden/tests/integration/network/proxies/test_token_network_registry.py +++ b/raiden/tests/integration/network/proxies/test_token_network_registry.py @@ -91,8 +91,7 @@ def test_token_network_registry( # Re-registering the same token should fail with a recoverable error # because it is a race condition. - match = "Token already registered" - with pytest.raises(RaidenRecoverableError, match=match): + with pytest.raises(RaidenRecoverableError): token_network_registry_proxy.add_token( token_address=test_token_address, channel_participant_deposit_limit=TokenAmount(UINT256_MAX), From 560d6ba2242e5e5977f3099c4f731a554cd5215a Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 13:12:56 +0200 Subject: [PATCH 04/11] Fix gas limit keys --- raiden/network/proxies/token_network_registry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 70865e11d4..8ed1b90ee8 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -242,7 +242,7 @@ def _add_token( if gas_limit: gas_limit = safe_gas_limit( - gas_limit, self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] + gas_limit, self.gas_measurements["TokenNetworkRegistry.createERC20TokenNetwork"] ) log_details["gas_limit"] = gas_limit transaction_hash = self.proxy.transact("createERC20TokenNetwork", gas_limit, **kwargs) @@ -357,7 +357,7 @@ def _add_token( required_gas = ( gas_limit if gas_limit - else self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] + else self.gas_measurements["TokenNetworkRegistry.createERC20TokenNetwork"] ) self.proxy.jsonrpc_client.check_for_insufficient_eth( transaction_name="createERC20TokenNetwork", From 49b9c551394dbaa00a5e2dbab353900d7c7c3623 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 13:13:13 +0200 Subject: [PATCH 05/11] Check that chain id matches the one Raiden is running on --- .../network/proxies/token_network_registry.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 8ed1b90ee8..8e924dcb4f 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -4,7 +4,6 @@ from eth_utils import ( encode_hex, event_abi_to_log_topic, - is_binary_address, is_same_address, to_canonical_address, to_checksum_address, @@ -186,6 +185,12 @@ def add_token( "The chain ID property for the TokenNetworkRegistry is invalid." ) + if chain_id != self.blockchain_service.network_id: + raise BrokenPreconditionError( + "The provided chain ID {chain_id} does not match the " + "network Raiden is running on: {self.blockchain_service.network_id}." + ) + if secret_registry_address == NULL_ADDRESS: raise BrokenPreconditionError( "The secret registry address for the token network is invalid." @@ -298,6 +303,12 @@ def _add_token( "The chain ID property for the TokenNetworkRegistry is invalid." ) + if chain_id != self.blockchain_service.network_id: + raise RaidenUnrecoverableError( + "The provided chain ID {chain_id} does not match the " + "network Raiden is running on: {self.blockchain_service.network_id}." + ) + if secret_registry_address == NULL_ADDRESS: raise RaidenUnrecoverableError( "The secret registry address for the token network is invalid." @@ -383,6 +394,12 @@ def _add_token( "The chain ID property for the TokenNetworkRegistry is invalid." ) + if chain_id != self.blockchain_service.network_id: + raise RaidenUnrecoverableError( + "The provided chain ID {chain_id} does not match the " + "network Raiden is running on: {self.blockchain_service.network_id}." + ) + if secret_registry_address == NULL_ADDRESS: raise RaidenUnrecoverableError( "The secret registry address for the token network is invalid." From b5a03132fd09298524212106671612aec0da9bf0 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Fri, 20 Sep 2019 13:23:37 +0200 Subject: [PATCH 06/11] use gas_measurements.get to protect against typos --- raiden/network/proxies/token_network_registry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 8e924dcb4f..c12cc0eede 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -247,7 +247,7 @@ def _add_token( if gas_limit: gas_limit = safe_gas_limit( - gas_limit, self.gas_measurements["TokenNetworkRegistry.createERC20TokenNetwork"] + gas_limit, self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork") ) log_details["gas_limit"] = gas_limit transaction_hash = self.proxy.transact("createERC20TokenNetwork", gas_limit, **kwargs) @@ -368,7 +368,7 @@ def _add_token( required_gas = ( gas_limit if gas_limit - else self.gas_measurements["TokenNetworkRegistry.createERC20TokenNetwork"] + else self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork") ) self.proxy.jsonrpc_client.check_for_insufficient_eth( transaction_name="createERC20TokenNetwork", From 2b019cae085294a818c40101bc9cd5dc996edd3f Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Sep 2019 11:24:29 +0200 Subject: [PATCH 07/11] Introduce exceptions for invalid deposit limits --- raiden/exceptions.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/raiden/exceptions.py b/raiden/exceptions.py index dc33fe25b4..bc007d4935 100644 --- a/raiden/exceptions.py +++ b/raiden/exceptions.py @@ -161,6 +161,18 @@ class InvalidTokenAddress(RaidenError): """ Raised if the token address is invalid """ +class InvalidTokenNetworkDepositLimit(RaidenError): + """ Raised when an invalid token network deposit + limit is passed to the token network registry proxy. + """ + + +class InvalidChannelParticipantDepositLimit(RaidenError): + """ Raised when an invalid channel participant + deposit limit is passed to the token network registry proxy. + """ + + class STUNUnavailableException(RaidenError): pass From 434779cccfac1610e24a1e70cd166301baaa9f3f Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Sep 2019 11:25:22 +0200 Subject: [PATCH 08/11] Remove redundant settlement timeout methods, use existing --- .../network/proxies/token_network_registry.py | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index c12cc0eede..96dbc668e5 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -74,35 +74,6 @@ def __init__( self.node_address = self.client.address self.proxy = proxy - def get_chain_id(self, block_identifier: BlockSpecification) -> int: - return self.proxy.contract.functions.chain_id().call(block_identifier=block_identifier) - - def get_settlement_timeout_min(self, block_identifier: BlockSpecification) -> int: - return self.proxy.contract.functions.settlement_timeout_min().call( - block_identifier=block_identifier - ) - - def get_settlement_timeout_max(self, block_identifier: BlockSpecification) -> int: - return self.proxy.contract.functions.settlement_timeout_max().call( - block_identifier=block_identifier - ) - - def get_secret_registry_address( - self, block_identifier: BlockSpecification - ) -> SecretRegistryAddress: - return SecretRegistryAddress( - self.proxy.contract.functions.secret_registry_address().call( - block_identifier=block_identifier - ) - ) - - def get_deprecation_executor(self, block_identifier: BlockSpecification) -> Address: - return Address( - self.proxy.contract.functions.deprecation_executor().call( - block_identifier=block_identifier - ) - ) - def get_token_network( self, token_address: TokenAddress, block_identifier: BlockSpecification ) -> Optional[TokenNetworkAddress]: @@ -463,24 +434,49 @@ def filter_token_added_events(self) -> List[Dict[str, Any]]: return events - def settlement_timeout_min(self) -> int: + def get_chain_id(self, block_identifier: BlockSpecification) -> int: + return self.proxy.contract.functions.chain_id().call(block_identifier=block_identifier) + + def get_secret_registry_address( + self, block_identifier: BlockSpecification + ) -> SecretRegistryAddress: + return SecretRegistryAddress( + self.proxy.contract.functions.secret_registry_address().call( + block_identifier=block_identifier + ) + ) + + def get_deprecation_executor(self, block_identifier: BlockSpecification) -> Address: + return Address( + self.proxy.contract.functions.deprecation_executor().call( + block_identifier=block_identifier + ) + ) + + def settlement_timeout_min(self, block_identifier: BlockSpecification) -> int: """ Returns the minimal settlement timeout for the token network registry. """ - return self.proxy.contract.functions.settlement_timeout_min().call() + return self.proxy.contract.functions.settlement_timeout_min().call( + block_identifier=block_identifier + ) - def settlement_timeout_max(self) -> int: + def settlement_timeout_max(self, block_identifier: BlockSpecification) -> int: """ Returns the maximal settlement timeout for the token network registry. """ - return self.proxy.contract.functions.settlement_timeout_max().call() + return self.proxy.contract.functions.settlement_timeout_max().call( + block_identifier=block_identifier + ) - def get_token_network_created(self, to_block: BlockSpecification) -> int: + def get_token_network_created(self, block_identifier: BlockSpecification) -> int: """ Returns the number of TokenNetwork contracts created so far in the token network registry. """ return self.proxy.contract.functions.token_network_created().call( - block_identifier=to_block + block_identifier=block_identifier ) - def get_max_token_networks(self, to_block: BlockSpecification) -> int: + def get_max_token_networks(self, block_identifier: BlockSpecification) -> int: """ Returns the maximal number of TokenNetwork contracts that the token network registry. """ - return self.proxy.contract.functions.max_token_networks().call(block_identifier=to_block) + return self.proxy.contract.functions.max_token_networks().call( + block_identifier=block_identifier + ) From ace16c8745760dbbe368736192d47f94141b7227 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Sep 2019 11:26:00 +0200 Subject: [PATCH 09/11] Validate deposit limits and max token networks vs created --- .../network/proxies/token_network_registry.py | 63 +++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index 96dbc668e5..eb2a1415e3 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -14,8 +14,10 @@ from raiden.constants import NULL_ADDRESS, NULL_ADDRESS_BYTES from raiden.exceptions import ( BrokenPreconditionError, + InvalidChannelParticipantDepositLimit, InvalidToken, InvalidTokenAddress, + InvalidTokenNetworkDepositLimit, RaidenRecoverableError, RaidenUnrecoverableError, ) @@ -113,6 +115,16 @@ def add_token( if token_address == NULL_ADDRESS_BYTES: raise InvalidTokenAddress("The call to register a token at 0x00..00 will fail.") + if token_network_deposit_limit <= 0: + raise InvalidTokenNetworkDepositLimit( + f"Token network deposit limit of {token_network_deposit_limit} is invalid" + ) + + if channel_participant_deposit_limit >= token_network_deposit_limit: + raise InvalidChannelParticipantDepositLimit( + f"Channel participant deposit limit of {channel_participant_deposit_limit} is invalid" + ) + token_proxy = self.blockchain_service.token(token_address) try: token_supply = token_proxy.total_supply(block_identifier=block_identifier) @@ -120,22 +132,27 @@ def add_token( token_address=token_address, block_identifier=block_identifier ) deprecation_executor = self.get_deprecation_executor(block_identifier=block_identifier) - settlement_timeout_min = self.get_settlement_timeout_min( - block_identifier=block_identifier - ) - settlement_timeout_max = self.get_settlement_timeout_max( - block_identifier=block_identifier - ) + settlement_timeout_min = self.settlement_timeout_min(block_identifier=block_identifier) + settlement_timeout_max = self.settlement_timeout_max(block_identifier=block_identifier) chain_id = self.get_chain_id(block_identifier=block_identifier) secret_registry_address = self.get_secret_registry_address( block_identifier=block_identifier ) + max_token_networks = self.get_max_token_networks(block_identifier=block_identifier) + token_networks_created = self.get_token_network_created( + block_identifier=block_identifier + ) except ValueError: # If `block_identifier` has been pruned the checks cannot be performed pass except BadFunctionCallOutput: raise_on_call_returned_empty(block_identifier) else: + if token_networks_created + 1 >= max_token_networks: + raise BrokenPreconditionError( + f"Number of token networks will exceed the max of {max_token_networks}" + ) + if token_supply == "": raise InvalidToken( "Given token address does not follow the " @@ -218,7 +235,8 @@ def _add_token( if gas_limit: gas_limit = safe_gas_limit( - gas_limit, self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork") + gas_limit, + self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork"), ) log_details["gas_limit"] = gas_limit transaction_hash = self.proxy.transact("createERC20TokenNetwork", gas_limit, **kwargs) @@ -229,16 +247,22 @@ def _add_token( if failed_receipt: failed_at_blocknumber = failed_receipt["blockNumber"] + max_token_networks = self.get_max_token_networks( + block_identifier=failed_at_blocknumber + ) + token_networks_created = self.get_token_network_created( + block_identifier=failed_at_blocknumber + ) already_registered = self.get_token_network( token_address=token_address, block_identifier=failed_at_blocknumber ) deprecation_executor = self.get_deprecation_executor( block_identifier=failed_at_blocknumber ) - settlement_timeout_min = self.get_settlement_timeout_min( + settlement_timeout_min = self.settlement_timeout_min( block_identifier=failed_at_blocknumber ) - settlement_timeout_max = self.get_settlement_timeout_max( + settlement_timeout_max = self.settlement_timeout_max( block_identifier=failed_at_blocknumber ) chain_id = self.get_chain_id(block_identifier=failed_at_blocknumber) @@ -256,6 +280,11 @@ def _add_token( ) raise RaidenRecoverableError(msg) + if token_networks_created >= max_token_networks: + raise RaidenRecoverableError( + "The number of existing token networks reached the maximum allowed" + ) + if already_registered: # Race condition lost, the token network was created in a different # transaction which got mined first. @@ -319,16 +348,23 @@ def _add_token( failed_at = self.proxy.jsonrpc_client.get_block("latest") failed_at_blocknumber = failed_at["number"] + max_token_networks = self.get_max_token_networks( + block_identifier=failed_at_blocknumber + ) + token_networks_created = self.get_token_network_created( + block_identifier=failed_at_blocknumber + ) + already_registered = self.get_token_network( token_address=token_address, block_identifier=failed_at_blocknumber ) deprecation_executor = self.get_deprecation_executor( block_identifier=failed_at_blocknumber ) - settlement_timeout_min = self.get_settlement_timeout_min( + settlement_timeout_min = self.settlement_timeout_min( block_identifier=failed_at_blocknumber ) - settlement_timeout_max = self.get_settlement_timeout_max( + settlement_timeout_max = self.settlement_timeout_max( block_identifier=failed_at_blocknumber ) chain_id = self.get_chain_id(block_identifier=failed_at_blocknumber) @@ -348,6 +384,11 @@ def _add_token( block_identifier=failed_at_blocknumber, ) + if token_networks_created >= max_token_networks: + raise RaidenRecoverableError( + "The number of existing token networks reached the maximum allowed" + ) + if already_registered: # Race condition lost, the token network was created in a different # transaction which got mined first. From 28615396a7bd9e17272c62c5501f17206bf41781 Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Sep 2019 11:26:25 +0200 Subject: [PATCH 10/11] Fix token network registry tests --- raiden/app.py | 10 ++++++---- raiden/network/proxies/token_network_registry.py | 10 +++++----- .../network/proxies/test_token_network_registry.py | 13 ++++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/raiden/app.py b/raiden/app.py index 7f7e3d5065..c2ed2baa22 100644 --- a/raiden/app.py +++ b/raiden/app.py @@ -101,9 +101,11 @@ def __init__( ) # check that the settlement timeout fits the limits of the contract + settlement_timeout_min = default_registry.settlement_timeout_min("latest") + settlement_timeout_max = default_registry.settlement_timeout_max("latest") invalid_settle_timeout = ( - config["settle_timeout"] < default_registry.settlement_timeout_min() - or config["settle_timeout"] > default_registry.settlement_timeout_max() + config["settle_timeout"] < settlement_timeout_min + or config["settle_timeout"] > settlement_timeout_max or config["settle_timeout"] < config["reveal_timeout"] * 2 ) if invalid_settle_timeout: @@ -113,8 +115,8 @@ def __init__( "be in range [{}, {}], is {}" ).format( to_checksum_address(default_registry.address), - default_registry.settlement_timeout_min(), - default_registry.settlement_timeout_max(), + settlement_timeout_min, + settlement_timeout_max, config["settle_timeout"], ) ) diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index eb2a1415e3..5922c6eb03 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -120,9 +120,10 @@ def add_token( f"Token network deposit limit of {token_network_deposit_limit} is invalid" ) - if channel_participant_deposit_limit >= token_network_deposit_limit: + if channel_participant_deposit_limit > token_network_deposit_limit: raise InvalidChannelParticipantDepositLimit( - f"Channel participant deposit limit of {channel_participant_deposit_limit} is invalid" + f"Channel participant deposit limit of " + f"{channel_participant_deposit_limit} is invalid" ) token_proxy = self.blockchain_service.token(token_address) @@ -235,8 +236,7 @@ def _add_token( if gas_limit: gas_limit = safe_gas_limit( - gas_limit, - self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork"), + gas_limit, self.gas_measurements["TokenNetworkRegistry createERC20TokenNetwork"] ) log_details["gas_limit"] = gas_limit transaction_hash = self.proxy.transact("createERC20TokenNetwork", gas_limit, **kwargs) @@ -375,7 +375,7 @@ def _add_token( required_gas = ( gas_limit if gas_limit - else self.gas_measurements.get("TokenNetworkRegistry.createERC20TokenNetwork") + else self.gas_measurements.get("TokenNetworkRegistry createERC20TokenNetwork") ) self.proxy.jsonrpc_client.check_for_insufficient_eth( transaction_name="createERC20TokenNetwork", diff --git a/raiden/tests/integration/network/proxies/test_token_network_registry.py b/raiden/tests/integration/network/proxies/test_token_network_registry.py index fcfbaa7e06..7cd9944002 100644 --- a/raiden/tests/integration/network/proxies/test_token_network_registry.py +++ b/raiden/tests/integration/network/proxies/test_token_network_registry.py @@ -39,9 +39,9 @@ def test_token_network_registry( token_network_registry_address ) - assert token_network_registry_proxy.settlement_timeout_min() == TEST_SETTLE_TIMEOUT_MIN - assert token_network_registry_proxy.settlement_timeout_max() == TEST_SETTLE_TIMEOUT_MAX - assert token_network_registry_proxy.get_token_network_created(to_block="latest") == 0 + assert token_network_registry_proxy.settlement_timeout_min("latest") == TEST_SETTLE_TIMEOUT_MIN + assert token_network_registry_proxy.settlement_timeout_max("latest") == TEST_SETTLE_TIMEOUT_MAX + assert token_network_registry_proxy.get_token_network_created(block_identifier="latest") == 0 bad_token_address = make_token_address() @@ -87,7 +87,7 @@ def test_token_network_registry( block_identifier=preblockhash, ) assert token_network_address - assert token_network_registry_proxy.get_token_network_created(to_block="latest") == 1 + assert token_network_registry_proxy.get_token_network_created(block_identifier="latest") == 1 # Re-registering the same token should fail with a recoverable error # because it is a race condition. @@ -138,7 +138,10 @@ def test_token_network_registry_max_token_networks( token_network_registry_proxy = blockchain_service.token_network_registry( to_canonical_address(token_network_registry_address) ) - assert token_network_registry_proxy.get_max_token_networks(to_block="latest") == UINT256_MAX + assert ( + token_network_registry_proxy.get_max_token_networks(block_identifier="latest") + == UINT256_MAX + ) def test_token_network_registry_with_zero_token_address( From 1efc4b658da3c832b8cf0f7432d742acdecdb09b Mon Sep 17 00:00:00 2001 From: Rakan Alhneiti Date: Mon, 23 Sep 2019 15:08:14 +0200 Subject: [PATCH 11/11] Wait for the block at which token was deployed to be confirmed --- raiden/tests/integration/api/test_restapi.py | 38 ++++++++++++- raiden/tests/integration/test_pythonapi.py | 56 +++++++++++++++++++- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/raiden/tests/integration/api/test_restapi.py b/raiden/tests/integration/api/test_restapi.py index 5f8cb38aba..0047f2598c 100644 --- a/raiden/tests/integration/api/test_restapi.py +++ b/raiden/tests/integration/api/test_restapi.py @@ -33,6 +33,7 @@ from raiden.utils import get_system_spec from raiden.waiting import ( TransferWaitResult, + wait_for_block, wait_for_received_transfer_result, wait_for_token_network, ) @@ -1221,7 +1222,12 @@ def test_register_token_mainnet( @pytest.mark.parametrize("channels_per_node", [0]) @pytest.mark.parametrize("environment_type", [Environment.DEVELOPMENT]) def test_register_token( - api_server_test_instance, token_amount, token_addresses, raiden_network, contract_manager + api_server_test_instance, + token_amount, + token_addresses, + raiden_network, + contract_manager, + retry_timeout, ): app0 = raiden_network[0] new_token_address = deploy_contract_web3( @@ -1237,6 +1243,20 @@ def test_register_token( constructor_arguments=(token_amount, 2, "raiden", "Rd"), ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + wait_for_block( + raiden=app0.raiden, + block_number=app0.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + wait_for_block( + raiden=app0.raiden, + block_number=app0.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + register_request = grequests.put( api_url_for( api_server_test_instance, @@ -1279,7 +1299,12 @@ def test_register_token( @pytest.mark.parametrize("channels_per_node", [0]) @pytest.mark.parametrize("environment_type", [Environment.DEVELOPMENT]) def test_get_token_network_for_token( - api_server_test_instance, token_amount, token_addresses, raiden_network, contract_manager + api_server_test_instance, + token_amount, + token_addresses, + raiden_network, + contract_manager, + retry_timeout, ): app0 = raiden_network[0] @@ -1290,6 +1315,15 @@ def test_get_token_network_for_token( constructor_arguments=(token_amount, 2, "raiden", "Rd"), ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + wait_for_block( + raiden=app0.raiden, + block_number=app0.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + # unregistered token returns 404 token_request = grequests.get( api_url_for( diff --git a/raiden/tests/integration/test_pythonapi.py b/raiden/tests/integration/test_pythonapi.py index 56bffaf56a..d1edd73451 100644 --- a/raiden/tests/integration/test_pythonapi.py +++ b/raiden/tests/integration/test_pythonapi.py @@ -14,6 +14,7 @@ InsufficientGasReserve, InvalidBinaryAddress, ) +from raiden.settings import DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS from raiden.storage.serialization import DictSerializer from raiden.tests.utils.client import burn_eth from raiden.tests.utils.detect_failure import raise_on_failure @@ -34,7 +35,7 @@ GAS_RESERVE_ESTIMATE_SECURITY_FACTOR, get_required_gas_estimate, ) -from raiden.utils.typing import List, TokenAddress, TokenAmount +from raiden.utils.typing import BlockNumber, List, TokenAddress, TokenAmount from raiden_contracts.constants import CONTRACT_HUMAN_STANDARD_TOKEN, ChannelEvent from raiden_contracts.contract_manager import ContractManager @@ -62,6 +63,15 @@ def test_register_token(raiden_network, token_amount, contract_manager, retry_ti constructor_arguments=(token_amount, 2, "raiden", "Rd"), ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + waiting.wait_for_block( + raiden=app1.raiden, + block_number=app1.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + api1 = RaidenAPI(app1.raiden) assert token_address not in api1.get_tokens_list(registry_address) @@ -96,7 +106,9 @@ def test_register_token(raiden_network, token_amount, contract_manager, retry_ti @pytest.mark.parametrize("number_of_nodes", [1]) @pytest.mark.parametrize("channels_per_node", [0]) @pytest.mark.parametrize("number_of_tokens", [1]) -def test_register_token_insufficient_eth(raiden_network, token_amount, contract_manager): +def test_register_token_insufficient_eth( + raiden_network, token_amount, contract_manager, retry_timeout +): app1 = raiden_network[0] registry_address = app1.raiden.default_registry.address @@ -108,6 +120,15 @@ def test_register_token_insufficient_eth(raiden_network, token_amount, contract_ constructor_arguments=(token_amount, 2, "raiden", "Rd"), ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + waiting.wait_for_block( + raiden=app1.raiden, + block_number=app1.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + api1 = RaidenAPI(app1.raiden) assert token_address not in api1.get_tokens_list(registry_address) @@ -153,6 +174,15 @@ def test_token_registered_race(raiden_chain, token_amount, retry_timeout, contra constructor_arguments=(token_amount, 2, "raiden", "Rd"), ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + waiting.wait_for_block( + raiden=app1.raiden, + block_number=app1.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1, + retry_timeout=retry_timeout, + ) + registry_address = app0.raiden.default_registry.address assert token_address not in api0.get_tokens_list(registry_address) assert token_address not in api1.get_tokens_list(registry_address) @@ -430,6 +460,17 @@ def test_participant_deposit_amount_must_be_smaller_than_the_limit( msg = "Token is not registered yet, it must not be in the token list." assert token_address not in api1.get_tokens_list(registry_address), msg + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + waiting.wait_for_block( + raiden=app1.raiden, + block_number=BlockNumber( + app1.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1 + ), + retry_timeout=retry_timeout, + ) + token_network_participant_deposit_limit = TokenAmount(100) api1.token_network_register( registry_address=registry_address, @@ -506,6 +547,17 @@ def test_deposit_amount_must_be_smaller_than_the_token_network_limit( ) ) + # Wait until Raiden can start using the token contract. + # Here, the block at which the contract was deployed should be confirmed by Raiden. + # Therefore, until that block is received. + waiting.wait_for_block( + raiden=app1.raiden, + block_number=BlockNumber( + app1.raiden.get_block_number() + DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS + 1 + ), + retry_timeout=retry_timeout, + ) + api1 = RaidenAPI(app1.raiden) msg = "Token is not registered yet, it must not be in the token list."