From dd0ead2fc3625aed07732a24e850c91892e21e3e Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Thu, 27 Sep 2018 11:45:12 -0300 Subject: [PATCH 1/7] fix: don't use blockgas limit as startgas Integration tests are flaky because the current code base is using a percentage of the block gas limit as the transaction startgas (cached for 600 seconds). This is a problem when there are too few transactions and the gas limit decreases by more than 20%, making the transaction sent invalid and consectuvily the ethereum node rejects it with a 'exceeds block gas limit' message. For the version v0.4.0 of the smart contract these functions have a predictable gas profile and can use hard coded constats: - EndpointRegistry:registerEndpoint - TokenNetworkRegistry:createERC20TokenNetwork - TokenNetwork:closeChannel - TokenNetwork:updateNonClosingBalanceProof - SecretRegistry:registerSecret - SecretRegistry:registerSecretBatch For these functions we call external smart contracts that can have a variable gas usage, therefore must be estimated. - TokenNetwork:openChannel -> uses token.balanceOf - TokenNetwork:setTotalDeposit -> uses token.balanceOf and token.transferFrom - TokenNetwork:setTotalWithdraw -> uses token.transfer - TokenNetwork:settleChannel -> uses token.transfer - TokenNetwork:unlock -> uses token.transfer - TokenNetwork:cooperativeSettle -> uses token.transfer The proxies were updated to use a gas estimation when it makes sense, similar to what was introduced by 454671a64fdff090049a32fcea2f3fd8cbb19925. The estimate_and_transact function was broken by the byzantium release and fixed with the max gas limit by 1f18640253cc30f1ed21c9d618ceb06a1d764a5d, which was removed from the code base by f1d055c296e4d5dd4fff8781c2c2a1590076a830 and implicitely changed the code to use the block gas limit. --- raiden/constants.py | 11 ++- raiden/network/proxies/discovery.py | 10 ++- raiden/network/proxies/secret_registry.py | 16 +++- raiden/network/proxies/token.py | 4 + raiden/network/proxies/token_network.py | 74 +++++++++++++++++++ .../network/proxies/token_network_registry.py | 11 ++- raiden/network/rpc/client.py | 8 +- raiden/network/rpc/smartcontract_proxy.py | 3 +- .../tests/integration/rpc/test_assumptions.py | 36 ++++----- .../integration/test_endpointregistry.py | 12 ++- .../scenario-player/scenario_player/runner.py | 5 +- tools/transfer_eth.py | 7 +- 12 files changed, 160 insertions(+), 37 deletions(-) diff --git a/raiden/constants.py b/raiden/constants.py index e394ecc77e..6356e88c41 100644 --- a/raiden/constants.py +++ b/raiden/constants.py @@ -1,3 +1,4 @@ +import math from enum import Enum from eth_utils import to_checksum_address @@ -31,12 +32,15 @@ class EthClient(Enum): START_QUERY_BLOCK_KEY = 'DefaultStartBlock' SNAPSHOT_STATE_CHANGES_COUNT = 500 +GAS_LIMIT_UPPER_BOUND = int(0.4 * 3141592) +GAS_FACTOR = 1.3 + # The more pending transfers there are, the more computationally complex # it becomes to unlock them. Lest an unlocking operation fails because # not enough gas is available, we define a gas limit for unlock calls # and limit the number of pending transfers per channel so it is not # exceeded. The limit is inclusive. -TRANSACTION_GAS_LIMIT = int(0.4 * 3141592) +UNLOCK_TX_GAS_LIMIT = GAS_LIMIT_UPPER_BOUND MAXIMUM_PENDING_TRANSFERS = 160 @@ -44,3 +48,8 @@ class Environment(Enum): """Environment configurations that can be chosen on the command line.""" PRODUCTION = 'production' DEVELOPMENT = 'development' + +GAS_REQUIRED_PER_SECRET_IN_BATCH = math.ceil(UNLOCK_TX_GAS_LIMIT / MAXIMUM_PENDING_TRANSFERS) + +# Value is verified in the test_endpointregistry_gas +GAS_REQUIRED_FOR_ENDPOINT_REGISTER = 76000 diff --git a/raiden/network/proxies/discovery.py b/raiden/network/proxies/discovery.py index 0b05e40e6d..aa98c0db86 100644 --- a/raiden/network/proxies/discovery.py +++ b/raiden/network/proxies/discovery.py @@ -1,9 +1,14 @@ import structlog from eth_utils import is_binary_address, to_checksum_address, to_normalized_address -from raiden.constants import NULL_ADDRESS -from raiden.exceptions import TransactionThrew, UnknownAddress from raiden.network.proxies.utils import compare_contract_versions +from raiden.constants import GAS_FACTOR, GAS_REQUIRED_FOR_ENDPOINT_REGISTER, NULL_ADDRESS +from raiden.exceptions import ( + AddressWrongContract, + ContractVersionMismatch, + TransactionThrew, + UnknownAddress, +) from raiden.network.rpc.client import check_address_has_code from raiden.network.rpc.smartcontract_proxy import ContractProxy from raiden.network.rpc.transactions import check_transaction_threw @@ -63,6 +68,7 @@ def register_endpoint(self, node_address, endpoint): transaction_hash = self.proxy.transact( 'registerEndpoint', + int(GAS_REQUIRED_FOR_ENDPOINT_REGISTER * GAS_FACTOR), endpoint, ) diff --git a/raiden/network/proxies/secret_registry.py b/raiden/network/proxies/secret_registry.py index a175517ba5..b77c6b5d98 100644 --- a/raiden/network/proxies/secret_registry.py +++ b/raiden/network/proxies/secret_registry.py @@ -4,9 +4,14 @@ from eth_utils import encode_hex, event_abi_to_log_topic, is_binary_address, to_normalized_address from gevent.event import AsyncResult -from raiden.constants import GENESIS_BLOCK_NUMBER -from raiden.exceptions import InvalidAddress, TransactionThrew from raiden.network.proxies.utils import compare_contract_versions +from raiden.constants import GAS_FACTOR, GAS_REQUIRED_PER_SECRET_IN_BATCH, GENESIS_BLOCK_NUMBER +from raiden.exceptions import ( + AddressWrongContract, + ContractVersionMismatch, + InvalidAddress, + TransactionThrew, +) from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw from raiden.utils import pex, privatekey_to_address, sha3, typing @@ -98,7 +103,12 @@ def register_secret_batch(self, secrets: List[typing.Secret]): self.open_secret_transactions.pop(secret, None) def _register_secret_batch(self, secrets): - transaction_hash = self.proxy.transact('registerSecretBatch', secrets) + gas_limit = self.proxy.estimate_gas('registerSecretBatch', secrets) + gas_limit = max( + gas_limit, + int(len(secrets) * GAS_REQUIRED_PER_SECRET_IN_BATCH * GAS_FACTOR), + ) + transaction_hash = self.proxy.transact('registerSecretBatch', gas_limit, secrets) self.client.poll(transaction_hash) receipt_or_none = check_transaction_threw(self.client, transaction_hash) diff --git a/raiden/network/proxies/token.py b/raiden/network/proxies/token.py index 96e8a3f12d..bbac6b5959 100644 --- a/raiden/network/proxies/token.py +++ b/raiden/network/proxies/token.py @@ -58,8 +58,10 @@ def approve(self, allowed_address, allowance): } log.debug('approve called', **log_details) + startgas = 100000 transaction_hash = self.proxy.transact( 'approve', + startgas, to_checksum_address(allowed_address), allowance, ) @@ -122,8 +124,10 @@ def transfer(self, to_address, amount): } log.debug('transfer called', **log_details) + startgas = 100000 transaction_hash = self.proxy.transact( 'transfer', + startgas, to_checksum_address(to_address), amount, ) diff --git a/raiden/network/proxies/token_network.py b/raiden/network/proxies/token_network.py index d606b4db79..5a4b6504d5 100644 --- a/raiden/network/proxies/token_network.py +++ b/raiden/network/proxies/token_network.py @@ -195,8 +195,17 @@ def _new_netting_channel(self, partner: typing.Address, settle_timeout: int): if self.channel_exists_and_not_settled(self.node_address, partner): raise DuplicatedChannelError('Channel with given partner address already exists') + gas_limit = self.proxy.estimate_gas( + 'openChannel', + self.node_address, + partner, + settle_timeout, + ) + gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_OPEN_CHANNEL) + transaction_hash = self.proxy.transact( 'openChannel', + int(gas_limit * GAS_FACTOR), self.node_address, partner, settle_timeout, @@ -589,8 +598,18 @@ def set_total_deposit( # making the second deposit fail. token.approve(self.address, amount_to_deposit) + gas_limit = self.proxy.estimate_gas( + 'setTotalDeposit', + channel_identifier, + self.node_address, + total_deposit, + partner, + ) + gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT) + transaction_hash = self.proxy.transact( 'setTotalDeposit', + int(gas_limit * GAS_FACTOR), channel_identifier, self.node_address, total_deposit, @@ -601,6 +620,12 @@ def set_total_deposit( receipt_or_none = check_transaction_threw(self.client, transaction_hash) if receipt_or_none: + latest_deposit = self.detail_participant( + channel_identifier, + self.node_address, + partner, + ).deposit + if token.allowance(self.node_address, self.address) < amount_to_deposit: log_msg = ( 'setTotalDeposit failed. The allowance is insufficient, ' @@ -609,6 +634,8 @@ def set_total_deposit( ) elif token.balance_of(self.node_address) < amount_to_deposit: log_msg = 'setTotalDeposit failed. The address doesnt have funds' + elif latest_deposit < total_deposit: + log_msg = 'setTotalDeposit failed. The tokens were not transferred' else: log_msg = 'setTotalDeposit failed' @@ -668,6 +695,7 @@ def close( with self.channel_operations_lock[partner]: transaction_hash = self.proxy.transact( 'closeChannel', + int(GAS_REQUIRED_FOR_CLOSE_CHANNEL * GAS_FACTOR), channel_identifier, partner, balance_hash, @@ -771,6 +799,7 @@ def update_transfer( transaction_hash = self.proxy.transact( 'updateNonClosingBalanceProof', + int(GAS_REQUIRED_FOR_UPDATE_TRANSFER * GAS_FACTOR), channel_identifier, partner, self.node_address, @@ -856,8 +885,13 @@ def withdraw( raise ValueError(msg) with self.channel_operations_lock[partner]: + # gaslimit below must be defined + raise NotImplementedError('feature temporarily disabled') + + gaslimit = None transaction_hash = self.proxy.transact( 'setTotalWithdraw', + gaslimit, channel_identifier, self.node_address, total_withdraw, @@ -902,8 +936,18 @@ def unlock( leaves_packed = b''.join(lock.encoded for lock in merkle_tree_leaves) + gas_limit = self.proxy.estimate_gas( + 'unlock', + channel_identifier, + self.node_address, + partner, + leaves_packed, + ) + gas_limit = max(gas_limit or 0, UNLOCK_TX_GAS_LIMIT) + transaction_hash = self.proxy.transact( 'unlock', + int(gas_limit * GAS_FACTOR), channel_identifier, self.node_address, partner, @@ -974,8 +1018,23 @@ def settle( our_bp_is_larger = our_maximum > partner_maximum if our_bp_is_larger: + gas_limit = self.proxy.estimate_gas( + 'settleChannel', + channel_identifier, + partner, + partner_transferred_amount, + partner_locked_amount, + partner_locksroot, + self.node_address, + transferred_amount, + locked_amount, + locksroot, + ) + gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + transaction_hash = self.proxy.transact( 'settleChannel', + int(gas_limit * GAS_FACTOR), channel_identifier, partner, partner_transferred_amount, @@ -987,8 +1046,23 @@ def settle( locksroot, ) else: + gas_limit = self.proxy.estimate_gas( + 'settleChannel', + channel_identifier, + self.node_address, + transferred_amount, + locked_amount, + locksroot, + partner, + partner_transferred_amount, + partner_locked_amount, + partner_locksroot, + ) + gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + transaction_hash = self.proxy.transact( 'settleChannel', + int(gas_limit * GAS_FACTOR), channel_identifier, self.node_address, transferred_amount, diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index a101d98505..b91cd7ce93 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -11,9 +11,15 @@ to_normalized_address, ) -from raiden.constants import GENESIS_BLOCK_NUMBER, NULL_ADDRESS -from raiden.exceptions import InvalidAddress, RaidenRecoverableError, TransactionThrew from raiden.network.proxies.utils import compare_contract_versions +from raiden.constants import GAS_FACTOR, GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK, NULL_ADDRESS, GENESIS_BLOCK_NUMBER +from raiden.exceptions import ( + AddressWrongContract, + ContractVersionMismatch, + InvalidAddress, + RaidenRecoverableError, + TransactionThrew, +) from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw from raiden.utils import pex, privatekey_to_address, typing @@ -83,6 +89,7 @@ def add_token(self, token_address: typing.TokenAddress): transaction_hash = self.proxy.transact( 'createERC20TokenNetwork', + int(GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK * GAS_FACTOR), token_address, ) diff --git a/raiden/network/rpc/client.py b/raiden/network/rpc/client.py index 79a5bf500a..76fb2e32fc 100644 --- a/raiden/network/rpc/client.py +++ b/raiden/network/rpc/client.py @@ -369,7 +369,8 @@ def deploy_solidity_contract( dependency_contract['bin'] = bytecode transaction_hash = self.send_transaction( - to=typing.Address(b''), + to=Address(b''), + startgas=int(self.gaslimit() * 0.8), data=bytecode, ) @@ -426,10 +427,10 @@ def deploy_solidity_contract( def send_transaction( self, - to: typing.Address, + to: Address, + startgas: int, value: int = 0, data: bytes = b'', - startgas: int = None, ) -> bytes: """ Helper to send signed messages. @@ -442,7 +443,6 @@ def send_transaction( with self._nonce_lock: nonce = self._available_nonce - startgas = self.check_startgas(startgas) gas_price = self.gas_price() transaction = { diff --git a/raiden/network/rpc/smartcontract_proxy.py b/raiden/network/rpc/smartcontract_proxy.py index 4ca9260b42..0af10744e6 100644 --- a/raiden/network/rpc/smartcontract_proxy.py +++ b/raiden/network/rpc/smartcontract_proxy.py @@ -66,12 +66,13 @@ def __init__( self.jsonrpc_client = jsonrpc_client self.contract = contract - def transact(self, function_name: str, *args, **kargs): + def transact(self, function_name: str, startgas: int, *args, **kargs): data = ContractProxy.get_transaction_data(self.contract.abi, function_name, args) try: txhash = self.jsonrpc_client.send_transaction( to=self.contract.address, + startgas=startgas, value=kargs.pop('value', 0), data=decode_hex(data), **kargs, diff --git a/raiden/tests/integration/rpc/test_assumptions.py b/raiden/tests/integration/rpc/test_assumptions.py index 6a9f677b29..6079af2172 100644 --- a/raiden/tests/integration/rpc/test_assumptions.py +++ b/raiden/tests/integration/rpc/test_assumptions.py @@ -131,11 +131,11 @@ def test_duplicated_transaction_same_gas_price_raises(deploy_client): contract_proxy.contract_address, ) - gas = contract_proxy.estimate_gas('ret') * 2 + startgas = contract_proxy.estimate_gas('ret') * 2 with pytest.raises(TransactionAlreadyPending): - second_proxy.transact('ret', startgas=gas) - contract_proxy.transact('ret', startgas=gas) + second_proxy.transact('ret', startgas) + contract_proxy.transact('ret', startgas) def test_duplicated_transaction_different_gas_price_raises(deploy_client): @@ -157,11 +157,11 @@ def test_duplicated_transaction_different_gas_price_raises(deploy_client): contract_proxy.contract_address, ) - gas = contract_proxy.estimate_gas('ret') * 2 + startgas = contract_proxy.estimate_gas('ret') * 2 with pytest.raises(ReplacementTransactionUnderpriced): - second_proxy.transact('ret', startgas=gas) - contract_proxy.transact('ret', startgas=gas) + second_proxy.transact('ret', startgas) + contract_proxy.transact('ret', startgas) def test_transact_opcode(deploy_client): @@ -171,9 +171,9 @@ def test_transact_opcode(deploy_client): address = contract_proxy.contract_address assert len(deploy_client.web3.eth.getCode(to_checksum_address(address))) > 0 - gas = contract_proxy.estimate_gas('ret') * 2 + startgas = contract_proxy.estimate_gas('ret') * 2 - transaction = contract_proxy.transact('ret', startgas=gas) + transaction = contract_proxy.transact('ret', startgas) deploy_client.poll(transaction) assert check_transaction_threw(deploy_client, transaction) is None, 'must be empty' @@ -186,9 +186,9 @@ def test_transact_throws_opcode(deploy_client): address = contract_proxy.contract_address assert len(deploy_client.web3.eth.getCode(to_checksum_address(address))) > 0 - gas = deploy_client.gaslimit() + startgas = deploy_client.gaslimit() - transaction = contract_proxy.transact('fail', startgas=gas) + transaction = contract_proxy.transact('fail', startgas) deploy_client.poll(transaction) assert check_transaction_threw(deploy_client, transaction), 'must not be empty' @@ -201,9 +201,9 @@ def test_transact_opcode_oog(deploy_client): address = contract_proxy.contract_address assert len(deploy_client.web3.eth.getCode(to_checksum_address(address))) > 0 - gas = min(contract_proxy.estimate_gas('loop', 1000) // 2, deploy_client.gaslimit()) + startgas = min(contract_proxy.estimate_gas('loop', 1000) // 2, deploy_client.gaslimit()) - transaction = contract_proxy.transact('loop', 1000, startgas=gas) + transaction = contract_proxy.transact('loop', 1000, startgas) deploy_client.poll(transaction) assert check_transaction_threw(deploy_client, transaction), 'must not be empty' @@ -214,10 +214,10 @@ def test_filter_start_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - gas = contract_proxy.estimate_gas('createEvent', 1) * 2 - transaction_1 = contract_proxy.transact('createEvent', 1, startgas=gas) + startgas = contract_proxy.estimate_gas('createEvent', 1) * 2 + transaction_1 = contract_proxy.transact('createEvent', 1, startgas) deploy_client.poll(transaction_1) - transaction_2 = contract_proxy.transact('createEvent', 2, startgas=gas) + transaction_2 = contract_proxy.transact('createEvent', 2, startgas) deploy_client.poll(transaction_2) result_1 = deploy_client.get_filter_events(contract_proxy.contract_address) @@ -246,10 +246,10 @@ def test_filter_end_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - gas = contract_proxy.estimate_gas('createEvent', 1) * 2 - transaction_1 = contract_proxy.transact('createEvent', 1, startgas=gas) + startgas = contract_proxy.estimate_gas('createEvent', 1) * 2 + transaction_1 = contract_proxy.transact('createEvent', 1, startgas) deploy_client.poll(transaction_1) - transaction_2 = contract_proxy.transact('createEvent', 2, startgas=gas) + transaction_2 = contract_proxy.transact('createEvent', 2, startgas) deploy_client.poll(transaction_2) result_1 = deploy_client.get_filter_events(contract_proxy.contract_address) diff --git a/raiden/tests/integration/test_endpointregistry.py b/raiden/tests/integration/test_endpointregistry.py index 24f3456a71..dc3dd3bfad 100644 --- a/raiden/tests/integration/test_endpointregistry.py +++ b/raiden/tests/integration/test_endpointregistry.py @@ -1,5 +1,6 @@ import pytest +from raiden.constants import GAS_REQUIRED_FOR_ENDPOINT_REGISTER from raiden.exceptions import UnknownAddress from raiden.network.discovery import ContractDiscovery from raiden.tests.utils.factories import make_address @@ -46,15 +47,20 @@ def test_endpointregistry(private_keys, blockchain_services, contract_manager): @pytest.mark.parametrize('number_of_nodes', [1]) def test_endpointregistry_gas(endpoint_discovery_services): - """ GAS_REQUIRED_FOR_ENDPOINT_REGISTER value must be equal to the gas required to call + """ GAS_REQUIRED_FOR_ENDPOINT_REGISTER value must be equal to the gas requried to call registerEndpoint. """ contract_discovery = endpoint_discovery_services[0] discovery_proxy = contract_discovery.discovery_proxy endpoint = host_port_to_endpoint('127.0.0.1', 44444) - transaction_hash = discovery_proxy.proxy.transact('registerEndpoint', endpoint) + transaction_hash = discovery_proxy.proxy.transact( + 'registerEndpoint', + GAS_REQUIRED_FOR_ENDPOINT_REGISTER, + endpoint, + ) discovery_proxy.client.poll(transaction_hash) receipt = discovery_proxy.client.get_transaction_receipt(transaction_hash) - assert receipt['gasUsed'] <= GAS_REQUIRED_FOR_ENDPOINT_REGISTER + msg = 'the transaction failed, check if it was because of the gas being too low' + assert receipt['status'] != 0, msg diff --git a/tools/scenario-player/scenario_player/runner.py b/tools/scenario-player/scenario_player/runner.py index 21422791dd..d877b5d1e5 100644 --- a/tools/scenario-player/scenario_player/runner.py +++ b/tools/scenario-player/scenario_player/runner.py @@ -227,9 +227,10 @@ def run_scenario(self): for address in addresses: balance = token_ctr.contract.functions.balanceOf(address).call() if balance < token_balance_min: - mint_amount = token_balance_fund - balance + mint_amount = token_balance_min - balance + startgas = 100000 log.debug("Minting tokens for", address=address, amount=mint_amount) - mint_tx.append(token_ctr.transact('mintFor', mint_amount, address)) + mint_tx.append(token_ctr.transact('mintFor', startgas, mint_amount, address)) elif balance > token_balance_min: log.warning("Node is overfunded", address=address, balance=balance) diff --git a/tools/transfer_eth.py b/tools/transfer_eth.py index f26348eb39..4f87fb27e8 100755 --- a/tools/transfer_eth.py +++ b/tools/transfer_eth.py @@ -44,7 +44,12 @@ def main(keystore_file, password, rpc_url, eth_amount, targets_file): print("Sending {} eth to:".format(eth_amount)) for target in targets: print(" - {}".format(target)) - client.send_transaction(to=target, value=eth_amount * WEI_TO_ETH) + startgas = 100000 + client.send_transaction( + to=target, + startgas=startgas, + value=eth_amount * WEI_TO_ETH, + ) if __name__ == "__main__": From ce7016ce53af78e829397af0e965e322c1156a30 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 28 Sep 2018 09:15:29 -0300 Subject: [PATCH 2/7] pr review --- raiden/network/proxies/token.py | 7 ++++++- raiden/network/rpc/client.py | 2 +- tools/transfer_eth.py | 3 +-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/raiden/network/proxies/token.py b/raiden/network/proxies/token.py index bbac6b5959..d723aa3afc 100644 --- a/raiden/network/proxies/token.py +++ b/raiden/network/proxies/token.py @@ -58,7 +58,12 @@ def approve(self, allowed_address, allowance): } log.debug('approve called', **log_details) - startgas = 100000 + startgas = self.proxy.estimate_gas( + 'approve', + to_checksum_address(allowed_address), + allowance, + ) + transaction_hash = self.proxy.transact( 'approve', startgas, diff --git a/raiden/network/rpc/client.py b/raiden/network/rpc/client.py index 76fb2e32fc..1dcbd17a66 100644 --- a/raiden/network/rpc/client.py +++ b/raiden/network/rpc/client.py @@ -370,7 +370,7 @@ def deploy_solidity_contract( transaction_hash = self.send_transaction( to=Address(b''), - startgas=int(self.gaslimit() * 0.8), + startgas=self.gaslimit(), data=bytecode, ) diff --git a/tools/transfer_eth.py b/tools/transfer_eth.py index 4f87fb27e8..8629d56ca0 100755 --- a/tools/transfer_eth.py +++ b/tools/transfer_eth.py @@ -44,10 +44,9 @@ def main(keystore_file, password, rpc_url, eth_amount, targets_file): print("Sending {} eth to:".format(eth_amount)) for target in targets: print(" - {}".format(target)) - startgas = 100000 client.send_transaction( to=target, - startgas=startgas, + startgas=21000, value=eth_amount * WEI_TO_ETH, ) From 7d18985a2263451659f47e9810d14ea3a01bc64f Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Tue, 6 Nov 2018 13:04:07 +0100 Subject: [PATCH 3/7] Rebase fallout --- raiden/constants.py | 5 ++--- raiden/network/proxies/discovery.py | 14 ++++++-------- raiden/network/proxies/secret_registry.py | 9 ++------- raiden/network/proxies/token_network.py | 9 +++++++-- raiden/network/proxies/token_network_registry.py | 15 +++++++-------- raiden/network/rpc/client.py | 4 ++-- raiden/tests/integration/rpc/test_assumptions.py | 16 +++++++++------- .../tests/integration/test_endpointregistry.py | 1 - raiden/utils/gas_reserve.py | 4 ++-- tools/scenario-player/scenario_player/runner.py | 2 +- 10 files changed, 38 insertions(+), 41 deletions(-) diff --git a/raiden/constants.py b/raiden/constants.py index 6356e88c41..038444d177 100644 --- a/raiden/constants.py +++ b/raiden/constants.py @@ -49,7 +49,6 @@ class Environment(Enum): PRODUCTION = 'production' DEVELOPMENT = 'development' -GAS_REQUIRED_PER_SECRET_IN_BATCH = math.ceil(UNLOCK_TX_GAS_LIMIT / MAXIMUM_PENDING_TRANSFERS) -# Value is verified in the test_endpointregistry_gas -GAS_REQUIRED_FOR_ENDPOINT_REGISTER = 76000 +GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK = 3234716 +GAS_REQUIRED_PER_SECRET_IN_BATCH = math.ceil(UNLOCK_TX_GAS_LIMIT / MAXIMUM_PENDING_TRANSFERS) diff --git a/raiden/network/proxies/discovery.py b/raiden/network/proxies/discovery.py index aa98c0db86..c386820d1e 100644 --- a/raiden/network/proxies/discovery.py +++ b/raiden/network/proxies/discovery.py @@ -1,19 +1,17 @@ import structlog from eth_utils import is_binary_address, to_checksum_address, to_normalized_address +from raiden.constants import GAS_FACTOR, NULL_ADDRESS +from raiden.exceptions import TransactionThrew, UnknownAddress from raiden.network.proxies.utils import compare_contract_versions -from raiden.constants import GAS_FACTOR, GAS_REQUIRED_FOR_ENDPOINT_REGISTER, NULL_ADDRESS -from raiden.exceptions import ( - AddressWrongContract, - ContractVersionMismatch, - TransactionThrew, - UnknownAddress, -) from raiden.network.rpc.client import check_address_has_code from raiden.network.rpc.smartcontract_proxy import ContractProxy from raiden.network.rpc.transactions import check_transaction_threw from raiden.utils import pex, privatekey_to_address -from raiden_contracts.constants import CONTRACT_ENDPOINT_REGISTRY +from raiden_contracts.constants import ( + CONTRACT_ENDPOINT_REGISTRY, + GAS_REQUIRED_FOR_ENDPOINT_REGISTER, +) from raiden_contracts.contract_manager import ContractManager log = structlog.get_logger(__name__) # pylint: disable=invalid-name diff --git a/raiden/network/proxies/secret_registry.py b/raiden/network/proxies/secret_registry.py index b77c6b5d98..e5f5bf9752 100644 --- a/raiden/network/proxies/secret_registry.py +++ b/raiden/network/proxies/secret_registry.py @@ -4,14 +4,9 @@ from eth_utils import encode_hex, event_abi_to_log_topic, is_binary_address, to_normalized_address from gevent.event import AsyncResult -from raiden.network.proxies.utils import compare_contract_versions from raiden.constants import GAS_FACTOR, GAS_REQUIRED_PER_SECRET_IN_BATCH, GENESIS_BLOCK_NUMBER -from raiden.exceptions import ( - AddressWrongContract, - ContractVersionMismatch, - InvalidAddress, - TransactionThrew, -) +from raiden.exceptions import InvalidAddress, TransactionThrew +from raiden.network.proxies.utils import compare_contract_versions from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw from raiden.utils import pex, privatekey_to_address, sha3, typing diff --git a/raiden/network/proxies/token_network.py b/raiden/network/proxies/token_network.py index 5a4b6504d5..ac521105a3 100644 --- a/raiden/network/proxies/token_network.py +++ b/raiden/network/proxies/token_network.py @@ -12,7 +12,7 @@ from gevent.event import AsyncResult from gevent.lock import RLock, Semaphore -from raiden.constants import GENESIS_BLOCK_NUMBER +from raiden.constants import GAS_FACTOR, GENESIS_BLOCK_NUMBER, UNLOCK_TX_GAS_LIMIT from raiden.exceptions import ( ChannelOutdatedError, DepositMismatch, @@ -33,6 +33,11 @@ from raiden.utils import pex, privatekey_to_address, typing from raiden_contracts.constants import ( CONTRACT_TOKEN_NETWORK, + GAS_REQUIRED_FOR_CLOSE_CHANNEL, + GAS_REQUIRED_FOR_OPEN_CHANNEL, + GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT, + GAS_REQUIRED_FOR_SETTLE_CHANNEL, + GAS_REQUIRED_FOR_UPDATE_BALANCE_PROOF, ChannelInfoIndex, ChannelState, ParticipantInfoIndex, @@ -799,7 +804,7 @@ def update_transfer( transaction_hash = self.proxy.transact( 'updateNonClosingBalanceProof', - int(GAS_REQUIRED_FOR_UPDATE_TRANSFER * GAS_FACTOR), + int(GAS_REQUIRED_FOR_UPDATE_BALANCE_PROOF * GAS_FACTOR), channel_identifier, partner, self.node_address, diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index b91cd7ce93..eaf3f1b91b 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -11,15 +11,14 @@ to_normalized_address, ) -from raiden.network.proxies.utils import compare_contract_versions -from raiden.constants import GAS_FACTOR, GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK, NULL_ADDRESS, GENESIS_BLOCK_NUMBER -from raiden.exceptions import ( - AddressWrongContract, - ContractVersionMismatch, - InvalidAddress, - RaidenRecoverableError, - TransactionThrew, +from raiden.constants import ( + GAS_FACTOR, + GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK, + GENESIS_BLOCK_NUMBER, + NULL_ADDRESS, ) +from raiden.exceptions import InvalidAddress, RaidenRecoverableError, TransactionThrew +from raiden.network.proxies.utils import compare_contract_versions from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw from raiden.utils import pex, privatekey_to_address, typing diff --git a/raiden/network/rpc/client.py b/raiden/network/rpc/client.py index 1dcbd17a66..b20f9e889c 100644 --- a/raiden/network/rpc/client.py +++ b/raiden/network/rpc/client.py @@ -369,7 +369,7 @@ def deploy_solidity_contract( dependency_contract['bin'] = bytecode transaction_hash = self.send_transaction( - to=Address(b''), + to=typing.Address(b''), startgas=self.gaslimit(), data=bytecode, ) @@ -427,7 +427,7 @@ def deploy_solidity_contract( def send_transaction( self, - to: Address, + to: typing.Address, startgas: int, value: int = 0, data: bytes = b'', diff --git a/raiden/tests/integration/rpc/test_assumptions.py b/raiden/tests/integration/rpc/test_assumptions.py index 6079af2172..89d957fe20 100644 --- a/raiden/tests/integration/rpc/test_assumptions.py +++ b/raiden/tests/integration/rpc/test_assumptions.py @@ -1,8 +1,10 @@ +import math import os import pytest from eth_utils import decode_hex, to_checksum_address +from raiden.constants import GAS_FACTOR from raiden.exceptions import ReplacementTransactionUnderpriced, TransactionAlreadyPending from raiden.network.rpc.client import JSONRPCClient from raiden.network.rpc.transactions import check_transaction_threw @@ -203,7 +205,7 @@ def test_transact_opcode_oog(deploy_client): startgas = min(contract_proxy.estimate_gas('loop', 1000) // 2, deploy_client.gaslimit()) - transaction = contract_proxy.transact('loop', 1000, startgas) + transaction = contract_proxy.transact('loop', startgas, 1000) deploy_client.poll(transaction) assert check_transaction_threw(deploy_client, transaction), 'must not be empty' @@ -214,10 +216,10 @@ def test_filter_start_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - startgas = contract_proxy.estimate_gas('createEvent', 1) * 2 - transaction_1 = contract_proxy.transact('createEvent', 1, startgas) + startgas = math.ceil(contract_proxy.estimate_gas('createEvent', 1) * GAS_FACTOR) + transaction_1 = contract_proxy.transact('createEvent', startgas, 1) deploy_client.poll(transaction_1) - transaction_2 = contract_proxy.transact('createEvent', 2, startgas) + transaction_2 = contract_proxy.transact('createEvent', startgas, 2) deploy_client.poll(transaction_2) result_1 = deploy_client.get_filter_events(contract_proxy.contract_address) @@ -246,10 +248,10 @@ def test_filter_end_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - startgas = contract_proxy.estimate_gas('createEvent', 1) * 2 - transaction_1 = contract_proxy.transact('createEvent', 1, startgas) + startgas = math.ceil(contract_proxy.estimate_gas('createEvent', 1) * GAS_FACTOR) + transaction_1 = contract_proxy.transact('createEvent', startgas, 1) deploy_client.poll(transaction_1) - transaction_2 = contract_proxy.transact('createEvent', 2, startgas) + transaction_2 = contract_proxy.transact('createEvent', startgas, 2) deploy_client.poll(transaction_2) result_1 = deploy_client.get_filter_events(contract_proxy.contract_address) diff --git a/raiden/tests/integration/test_endpointregistry.py b/raiden/tests/integration/test_endpointregistry.py index dc3dd3bfad..bc44f21f73 100644 --- a/raiden/tests/integration/test_endpointregistry.py +++ b/raiden/tests/integration/test_endpointregistry.py @@ -1,6 +1,5 @@ import pytest -from raiden.constants import GAS_REQUIRED_FOR_ENDPOINT_REGISTER from raiden.exceptions import UnknownAddress from raiden.network.discovery import ContractDiscovery from raiden.tests.utils.factories import make_address diff --git a/raiden/utils/gas_reserve.py b/raiden/utils/gas_reserve.py index 367a28fd00..d2cae8b9cf 100644 --- a/raiden/utils/gas_reserve.py +++ b/raiden/utils/gas_reserve.py @@ -1,6 +1,6 @@ from typing import Tuple -from raiden.constants import TRANSACTION_GAS_LIMIT +from raiden.constants import GAS_LIMIT_UPPER_BOUND from raiden.transfer import views from raiden_contracts.constants import ( GAS_REQUIRED_FOR_CLOSE_CHANNEL, @@ -10,7 +10,7 @@ ) GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_SETTLE = ( - TRANSACTION_GAS_LIMIT + GAS_LIMIT_UPPER_BOUND ) GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_CLOSE = ( GAS_REQUIRED_FOR_SETTLE_CHANNEL + GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_SETTLE diff --git a/tools/scenario-player/scenario_player/runner.py b/tools/scenario-player/scenario_player/runner.py index d877b5d1e5..6c3d40fc44 100644 --- a/tools/scenario-player/scenario_player/runner.py +++ b/tools/scenario-player/scenario_player/runner.py @@ -227,7 +227,7 @@ def run_scenario(self): for address in addresses: balance = token_ctr.contract.functions.balanceOf(address).call() if balance < token_balance_min: - mint_amount = token_balance_min - balance + mint_amount = token_balance_fund - balance startgas = 100000 log.debug("Minting tokens for", address=address, amount=mint_amount) mint_tx.append(token_ctr.transact('mintFor', startgas, mint_amount, address)) From ce710afd561c59ff9d0137256c46943d65eeea6c Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Tue, 6 Nov 2018 15:48:52 +0100 Subject: [PATCH 4/7] Review fixes --- raiden/constants.py | 5 +++-- raiden/network/proxies/token.py | 3 ++- raiden/network/proxies/token_network.py | 19 ++++++++++++------- .../scenario-player/scenario_player/runner.py | 3 ++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/raiden/constants.py b/raiden/constants.py index 038444d177..e54e6ab11a 100644 --- a/raiden/constants.py +++ b/raiden/constants.py @@ -32,7 +32,7 @@ class EthClient(Enum): START_QUERY_BLOCK_KEY = 'DefaultStartBlock' SNAPSHOT_STATE_CHANGES_COUNT = 500 -GAS_LIMIT_UPPER_BOUND = int(0.4 * 3141592) +GAS_LIMIT_UPPER_BOUND = int(0.4 * 3_141_592) GAS_FACTOR = 1.3 # The more pending transfers there are, the more computationally complex @@ -50,5 +50,6 @@ class Environment(Enum): DEVELOPMENT = 'development' -GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK = 3234716 +GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK = 3_234_716 GAS_REQUIRED_PER_SECRET_IN_BATCH = math.ceil(UNLOCK_TX_GAS_LIMIT / MAXIMUM_PENDING_TRANSFERS) +GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL = 100_000 diff --git a/raiden/network/proxies/token.py b/raiden/network/proxies/token.py index d723aa3afc..c77a08a7e1 100644 --- a/raiden/network/proxies/token.py +++ b/raiden/network/proxies/token.py @@ -1,6 +1,7 @@ import structlog from eth_utils import is_binary_address, to_checksum_address, to_normalized_address +from raiden.constants import GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL from raiden.exceptions import TransactionThrew from raiden.network.rpc.client import check_address_has_code from raiden.network.rpc.smartcontract_proxy import ContractProxy @@ -129,7 +130,7 @@ def transfer(self, to_address, amount): } log.debug('transfer called', **log_details) - startgas = 100000 + startgas = GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL transaction_hash = self.proxy.transact( 'transfer', startgas, diff --git a/raiden/network/proxies/token_network.py b/raiden/network/proxies/token_network.py index ac521105a3..cf0df59c00 100644 --- a/raiden/network/proxies/token_network.py +++ b/raiden/network/proxies/token_network.py @@ -48,6 +48,11 @@ log = structlog.get_logger(__name__) # pylint: disable=invalid-name +def get_gas_limit(given, precalculated): + """ Even though it's not documented, it does happen that estimate_gas returns `None`. """ + return max(given or 0, precalculated) + + class ChannelData(NamedTuple): channel_identifier: typing.ChannelID settle_block_number: typing.BlockNumber @@ -206,7 +211,7 @@ def _new_netting_channel(self, partner: typing.Address, settle_timeout: int): partner, settle_timeout, ) - gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_OPEN_CHANNEL) + gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_OPEN_CHANNEL) transaction_hash = self.proxy.transact( 'openChannel', @@ -610,7 +615,7 @@ def set_total_deposit( total_deposit, partner, ) - gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT) + gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT) transaction_hash = self.proxy.transact( 'setTotalDeposit', @@ -893,10 +898,10 @@ def withdraw( # gaslimit below must be defined raise NotImplementedError('feature temporarily disabled') - gaslimit = None + gas_limit = None transaction_hash = self.proxy.transact( 'setTotalWithdraw', - gaslimit, + gas_limit, channel_identifier, self.node_address, total_withdraw, @@ -948,7 +953,7 @@ def unlock( partner, leaves_packed, ) - gas_limit = max(gas_limit or 0, UNLOCK_TX_GAS_LIMIT) + gas_limit = get_gas_limit(gas_limit, UNLOCK_TX_GAS_LIMIT) transaction_hash = self.proxy.transact( 'unlock', @@ -1035,7 +1040,7 @@ def settle( locked_amount, locksroot, ) - gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) transaction_hash = self.proxy.transact( 'settleChannel', @@ -1063,7 +1068,7 @@ def settle( partner_locked_amount, partner_locksroot, ) - gas_limit = max(gas_limit or 0, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) transaction_hash = self.proxy.transact( 'settleChannel', diff --git a/tools/scenario-player/scenario_player/runner.py b/tools/scenario-player/scenario_player/runner.py index 6c3d40fc44..ebea2e83be 100644 --- a/tools/scenario-player/scenario_player/runner.py +++ b/tools/scenario-player/scenario_player/runner.py @@ -13,6 +13,7 @@ from web3 import HTTPProvider, Web3 from raiden.accounts import Account +from raiden.constants import GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL from raiden.network.rpc.client import JSONRPCClient from scenario_player.exceptions import NodesUnreachableError, ScenarioError, TokenRegistrationError from scenario_player.utils import ( @@ -228,7 +229,7 @@ def run_scenario(self): balance = token_ctr.contract.functions.balanceOf(address).call() if balance < token_balance_min: mint_amount = token_balance_fund - balance - startgas = 100000 + startgas = GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL log.debug("Minting tokens for", address=address, amount=mint_amount) mint_tx.append(token_ctr.transact('mintFor', startgas, mint_amount, address)) elif balance > token_balance_min: From 47901f0fb327cbac40f2814c922d9c8891b3e627 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Tue, 6 Nov 2018 17:55:33 +0100 Subject: [PATCH 5/7] PR review --- raiden/constants.py | 3 +++ raiden/network/proxies/token_network.py | 20 ++++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/raiden/constants.py b/raiden/constants.py index e54e6ab11a..c88a21c15d 100644 --- a/raiden/constants.py +++ b/raiden/constants.py @@ -32,7 +32,10 @@ class EthClient(Enum): START_QUERY_BLOCK_KEY = 'DefaultStartBlock' SNAPSHOT_STATE_CHANGES_COUNT = 500 +# An arbitrary limit for transaction size in Raiden, added in PR #1990 GAS_LIMIT_UPPER_BOUND = int(0.4 * 3_141_592) + +# Used to add a 30% security margin to gas estimations in case the calculations are off GAS_FACTOR = 1.3 # The more pending transfers there are, the more computationally complex diff --git a/raiden/network/proxies/token_network.py b/raiden/network/proxies/token_network.py index cf0df59c00..bff0ff2e00 100644 --- a/raiden/network/proxies/token_network.py +++ b/raiden/network/proxies/token_network.py @@ -48,9 +48,13 @@ log = structlog.get_logger(__name__) # pylint: disable=invalid-name -def get_gas_limit(given, precalculated): - """ Even though it's not documented, it does happen that estimate_gas returns `None`. """ - return max(given or 0, precalculated) +def get_gas_limit(given: int = None, precalculated: int = 0) -> int: + """ Even though it's not documented, it does happen that estimate_gas returns `None`. + + This function takes care of this and adds a security margin as well. + """ + calculated_limit = max(given or 0, precalculated) + return int(calculated_limit * GAS_FACTOR) class ChannelData(NamedTuple): @@ -215,7 +219,7 @@ def _new_netting_channel(self, partner: typing.Address, settle_timeout: int): transaction_hash = self.proxy.transact( 'openChannel', - int(gas_limit * GAS_FACTOR), + gas_limit, self.node_address, partner, settle_timeout, @@ -619,7 +623,7 @@ def set_total_deposit( transaction_hash = self.proxy.transact( 'setTotalDeposit', - int(gas_limit * GAS_FACTOR), + gas_limit, channel_identifier, self.node_address, total_deposit, @@ -957,7 +961,7 @@ def unlock( transaction_hash = self.proxy.transact( 'unlock', - int(gas_limit * GAS_FACTOR), + gas_limit, channel_identifier, self.node_address, partner, @@ -1044,7 +1048,7 @@ def settle( transaction_hash = self.proxy.transact( 'settleChannel', - int(gas_limit * GAS_FACTOR), + gas_limit, channel_identifier, partner, partner_transferred_amount, @@ -1072,7 +1076,7 @@ def settle( transaction_hash = self.proxy.transact( 'settleChannel', - int(gas_limit * GAS_FACTOR), + gas_limit, channel_identifier, self.node_address, transferred_amount, From 27c9c97d1b0978d13327f6bce4882dd4afd12ac1 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Wed, 7 Nov 2018 11:50:56 +0100 Subject: [PATCH 6/7] PR review 2 --- raiden/constants.py | 4 +-- raiden/network/proxies/discovery.py | 6 ++--- raiden/network/proxies/secret_registry.py | 6 ++--- raiden/network/proxies/token_network.py | 27 +++++++------------ .../network/proxies/token_network_registry.py | 5 ++-- .../tests/integration/rpc/test_assumptions.py | 11 ++++---- .../integration/test_endpointregistry.py | 2 +- raiden/utils/__init__.py | 10 +++++++ raiden/utils/gas_reserve.py | 4 +-- 9 files changed, 37 insertions(+), 38 deletions(-) diff --git a/raiden/constants.py b/raiden/constants.py index c88a21c15d..808c6db0cd 100644 --- a/raiden/constants.py +++ b/raiden/constants.py @@ -33,7 +33,7 @@ class EthClient(Enum): SNAPSHOT_STATE_CHANGES_COUNT = 500 # An arbitrary limit for transaction size in Raiden, added in PR #1990 -GAS_LIMIT_UPPER_BOUND = int(0.4 * 3_141_592) +TRANSACTION_GAS_LIMIT_UPPER_BOUND = int(0.4 * 3_141_592) # Used to add a 30% security margin to gas estimations in case the calculations are off GAS_FACTOR = 1.3 @@ -43,7 +43,7 @@ class EthClient(Enum): # not enough gas is available, we define a gas limit for unlock calls # and limit the number of pending transfers per channel so it is not # exceeded. The limit is inclusive. -UNLOCK_TX_GAS_LIMIT = GAS_LIMIT_UPPER_BOUND +UNLOCK_TX_GAS_LIMIT = TRANSACTION_GAS_LIMIT_UPPER_BOUND MAXIMUM_PENDING_TRANSFERS = 160 diff --git a/raiden/network/proxies/discovery.py b/raiden/network/proxies/discovery.py index c386820d1e..5d05b754f0 100644 --- a/raiden/network/proxies/discovery.py +++ b/raiden/network/proxies/discovery.py @@ -1,13 +1,13 @@ import structlog from eth_utils import is_binary_address, to_checksum_address, to_normalized_address -from raiden.constants import GAS_FACTOR, NULL_ADDRESS +from raiden.constants import NULL_ADDRESS from raiden.exceptions import TransactionThrew, UnknownAddress from raiden.network.proxies.utils import compare_contract_versions from raiden.network.rpc.client import check_address_has_code from raiden.network.rpc.smartcontract_proxy import ContractProxy from raiden.network.rpc.transactions import check_transaction_threw -from raiden.utils import pex, privatekey_to_address +from raiden.utils import pex, privatekey_to_address, safe_gas_limit from raiden_contracts.constants import ( CONTRACT_ENDPOINT_REGISTRY, GAS_REQUIRED_FOR_ENDPOINT_REGISTER, @@ -66,7 +66,7 @@ def register_endpoint(self, node_address, endpoint): transaction_hash = self.proxy.transact( 'registerEndpoint', - int(GAS_REQUIRED_FOR_ENDPOINT_REGISTER * GAS_FACTOR), + safe_gas_limit(GAS_REQUIRED_FOR_ENDPOINT_REGISTER), endpoint, ) diff --git a/raiden/network/proxies/secret_registry.py b/raiden/network/proxies/secret_registry.py index e5f5bf9752..09397be836 100644 --- a/raiden/network/proxies/secret_registry.py +++ b/raiden/network/proxies/secret_registry.py @@ -4,12 +4,12 @@ from eth_utils import encode_hex, event_abi_to_log_topic, is_binary_address, to_normalized_address from gevent.event import AsyncResult -from raiden.constants import GAS_FACTOR, GAS_REQUIRED_PER_SECRET_IN_BATCH, GENESIS_BLOCK_NUMBER +from raiden.constants import GAS_REQUIRED_PER_SECRET_IN_BATCH, GENESIS_BLOCK_NUMBER from raiden.exceptions import InvalidAddress, TransactionThrew from raiden.network.proxies.utils import compare_contract_versions from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw -from raiden.utils import pex, privatekey_to_address, sha3, typing +from raiden.utils import pex, privatekey_to_address, safe_gas_limit, sha3, typing from raiden_contracts.constants import CONTRACT_SECRET_REGISTRY, EVENT_SECRET_REVEALED from raiden_contracts.contract_manager import ContractManager @@ -101,7 +101,7 @@ def _register_secret_batch(self, secrets): gas_limit = self.proxy.estimate_gas('registerSecretBatch', secrets) gas_limit = max( gas_limit, - int(len(secrets) * GAS_REQUIRED_PER_SECRET_IN_BATCH * GAS_FACTOR), + safe_gas_limit(len(secrets) * GAS_REQUIRED_PER_SECRET_IN_BATCH), ) transaction_hash = self.proxy.transact('registerSecretBatch', gas_limit, secrets) self.client.poll(transaction_hash) diff --git a/raiden/network/proxies/token_network.py b/raiden/network/proxies/token_network.py index bff0ff2e00..bd7f08c614 100644 --- a/raiden/network/proxies/token_network.py +++ b/raiden/network/proxies/token_network.py @@ -12,7 +12,7 @@ from gevent.event import AsyncResult from gevent.lock import RLock, Semaphore -from raiden.constants import GAS_FACTOR, GENESIS_BLOCK_NUMBER, UNLOCK_TX_GAS_LIMIT +from raiden.constants import GENESIS_BLOCK_NUMBER, UNLOCK_TX_GAS_LIMIT from raiden.exceptions import ( ChannelOutdatedError, DepositMismatch, @@ -30,7 +30,7 @@ from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw from raiden.transfer.balance_proof import pack_balance_proof -from raiden.utils import pex, privatekey_to_address, typing +from raiden.utils import pex, privatekey_to_address, safe_gas_limit, typing from raiden_contracts.constants import ( CONTRACT_TOKEN_NETWORK, GAS_REQUIRED_FOR_CLOSE_CHANNEL, @@ -48,15 +48,6 @@ log = structlog.get_logger(__name__) # pylint: disable=invalid-name -def get_gas_limit(given: int = None, precalculated: int = 0) -> int: - """ Even though it's not documented, it does happen that estimate_gas returns `None`. - - This function takes care of this and adds a security margin as well. - """ - calculated_limit = max(given or 0, precalculated) - return int(calculated_limit * GAS_FACTOR) - - class ChannelData(NamedTuple): channel_identifier: typing.ChannelID settle_block_number: typing.BlockNumber @@ -215,7 +206,7 @@ def _new_netting_channel(self, partner: typing.Address, settle_timeout: int): partner, settle_timeout, ) - gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_OPEN_CHANNEL) + gas_limit = safe_gas_limit(gas_limit, GAS_REQUIRED_FOR_OPEN_CHANNEL) transaction_hash = self.proxy.transact( 'openChannel', @@ -619,7 +610,7 @@ def set_total_deposit( total_deposit, partner, ) - gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT) + gas_limit = safe_gas_limit(gas_limit, GAS_REQUIRED_FOR_SET_TOTAL_DEPOSIT) transaction_hash = self.proxy.transact( 'setTotalDeposit', @@ -709,7 +700,7 @@ def close( with self.channel_operations_lock[partner]: transaction_hash = self.proxy.transact( 'closeChannel', - int(GAS_REQUIRED_FOR_CLOSE_CHANNEL * GAS_FACTOR), + safe_gas_limit(GAS_REQUIRED_FOR_CLOSE_CHANNEL), channel_identifier, partner, balance_hash, @@ -813,7 +804,7 @@ def update_transfer( transaction_hash = self.proxy.transact( 'updateNonClosingBalanceProof', - int(GAS_REQUIRED_FOR_UPDATE_BALANCE_PROOF * GAS_FACTOR), + safe_gas_limit(GAS_REQUIRED_FOR_UPDATE_BALANCE_PROOF), channel_identifier, partner, self.node_address, @@ -957,7 +948,7 @@ def unlock( partner, leaves_packed, ) - gas_limit = get_gas_limit(gas_limit, UNLOCK_TX_GAS_LIMIT) + gas_limit = safe_gas_limit(gas_limit, UNLOCK_TX_GAS_LIMIT) transaction_hash = self.proxy.transact( 'unlock', @@ -1044,7 +1035,7 @@ def settle( locked_amount, locksroot, ) - gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + gas_limit = safe_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) transaction_hash = self.proxy.transact( 'settleChannel', @@ -1072,7 +1063,7 @@ def settle( partner_locked_amount, partner_locksroot, ) - gas_limit = get_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) + gas_limit = safe_gas_limit(gas_limit, GAS_REQUIRED_FOR_SETTLE_CHANNEL) transaction_hash = self.proxy.transact( 'settleChannel', diff --git a/raiden/network/proxies/token_network_registry.py b/raiden/network/proxies/token_network_registry.py index eaf3f1b91b..03ad27b581 100644 --- a/raiden/network/proxies/token_network_registry.py +++ b/raiden/network/proxies/token_network_registry.py @@ -12,7 +12,6 @@ ) from raiden.constants import ( - GAS_FACTOR, GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK, GENESIS_BLOCK_NUMBER, NULL_ADDRESS, @@ -21,7 +20,7 @@ from raiden.network.proxies.utils import compare_contract_versions from raiden.network.rpc.client import StatelessFilter, check_address_has_code from raiden.network.rpc.transactions import check_transaction_threw -from raiden.utils import pex, privatekey_to_address, typing +from raiden.utils import pex, privatekey_to_address, safe_gas_limit, typing from raiden_contracts.constants import CONTRACT_TOKEN_NETWORK_REGISTRY, EVENT_TOKEN_NETWORK_CREATED from raiden_contracts.contract_manager import ContractManager @@ -88,7 +87,7 @@ def add_token(self, token_address: typing.TokenAddress): transaction_hash = self.proxy.transact( 'createERC20TokenNetwork', - int(GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK * GAS_FACTOR), + safe_gas_limit(GAS_REQUIRED_FOR_CREATE_ERC20_TOKEN_NETWORK), token_address, ) diff --git a/raiden/tests/integration/rpc/test_assumptions.py b/raiden/tests/integration/rpc/test_assumptions.py index 89d957fe20..55c27d9a23 100644 --- a/raiden/tests/integration/rpc/test_assumptions.py +++ b/raiden/tests/integration/rpc/test_assumptions.py @@ -1,13 +1,12 @@ -import math import os import pytest from eth_utils import decode_hex, to_checksum_address -from raiden.constants import GAS_FACTOR from raiden.exceptions import ReplacementTransactionUnderpriced, TransactionAlreadyPending from raiden.network.rpc.client import JSONRPCClient from raiden.network.rpc.transactions import check_transaction_threw +from raiden.utils import safe_gas_limit from raiden.utils.solc import compile_files_cwd # pylint: disable=unused-argument,protected-access @@ -133,7 +132,7 @@ def test_duplicated_transaction_same_gas_price_raises(deploy_client): contract_proxy.contract_address, ) - startgas = contract_proxy.estimate_gas('ret') * 2 + startgas = safe_gas_limit(contract_proxy.estimate_gas('ret')) with pytest.raises(TransactionAlreadyPending): second_proxy.transact('ret', startgas) @@ -159,7 +158,7 @@ def test_duplicated_transaction_different_gas_price_raises(deploy_client): contract_proxy.contract_address, ) - startgas = contract_proxy.estimate_gas('ret') * 2 + startgas = safe_gas_limit(contract_proxy.estimate_gas('ret')) with pytest.raises(ReplacementTransactionUnderpriced): second_proxy.transact('ret', startgas) @@ -216,7 +215,7 @@ def test_filter_start_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - startgas = math.ceil(contract_proxy.estimate_gas('createEvent', 1) * GAS_FACTOR) + startgas = safe_gas_limit(contract_proxy.estimate_gas('createEvent', 1)) transaction_1 = contract_proxy.transact('createEvent', startgas, 1) deploy_client.poll(transaction_1) transaction_2 = contract_proxy.transact('createEvent', startgas, 2) @@ -248,7 +247,7 @@ def test_filter_end_block_inclusive(deploy_client): contract_proxy = deploy_rpc_test_contract(deploy_client) # call the create event function twice and wait for confirmation each time - startgas = math.ceil(contract_proxy.estimate_gas('createEvent', 1) * GAS_FACTOR) + startgas = safe_gas_limit(contract_proxy.estimate_gas('createEvent', 1)) transaction_1 = contract_proxy.transact('createEvent', startgas, 1) deploy_client.poll(transaction_1) transaction_2 = contract_proxy.transact('createEvent', startgas, 2) diff --git a/raiden/tests/integration/test_endpointregistry.py b/raiden/tests/integration/test_endpointregistry.py index bc44f21f73..ac04f718cd 100644 --- a/raiden/tests/integration/test_endpointregistry.py +++ b/raiden/tests/integration/test_endpointregistry.py @@ -46,7 +46,7 @@ def test_endpointregistry(private_keys, blockchain_services, contract_manager): @pytest.mark.parametrize('number_of_nodes', [1]) def test_endpointregistry_gas(endpoint_discovery_services): - """ GAS_REQUIRED_FOR_ENDPOINT_REGISTER value must be equal to the gas requried to call + """ GAS_REQUIRED_FOR_ENDPOINT_REGISTER value must be equal to the gas required to call registerEndpoint. """ contract_discovery = endpoint_discovery_services[0] diff --git a/raiden/utils/__init__.py b/raiden/utils/__init__.py index 81947be62e..12d2458b5c 100644 --- a/raiden/utils/__init__.py +++ b/raiden/utils/__init__.py @@ -261,3 +261,13 @@ def optional_address_to_string(address: typing.Address = None) -> typing.Optiona return None return to_checksum_address(address) + + +def safe_gas_limit(*estimates) -> int: + """ Calculates a safe gas limit for a number of estimates. + + Even though it's not documented, it does happen that estimate_gas returns `None`. + This function takes care of this and adds a security margin as well. + """ + calculated_limit = max(gas or 0 for gas in estimates) + return int(calculated_limit * constants.GAS_FACTOR) diff --git a/raiden/utils/gas_reserve.py b/raiden/utils/gas_reserve.py index d2cae8b9cf..78056e6037 100644 --- a/raiden/utils/gas_reserve.py +++ b/raiden/utils/gas_reserve.py @@ -1,6 +1,6 @@ from typing import Tuple -from raiden.constants import GAS_LIMIT_UPPER_BOUND +from raiden.constants import UNLOCK_TX_GAS_LIMIT from raiden.transfer import views from raiden_contracts.constants import ( GAS_REQUIRED_FOR_CLOSE_CHANNEL, @@ -10,7 +10,7 @@ ) GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_SETTLE = ( - GAS_LIMIT_UPPER_BOUND + UNLOCK_TX_GAS_LIMIT ) GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_CLOSE = ( GAS_REQUIRED_FOR_SETTLE_CHANNEL + GAS_REQUIRED_FOR_CHANNEL_LIFECYCLE_AFTER_SETTLE From 3da0baded2863b07462a0c79d1ce7d147d7efb45 Mon Sep 17 00:00:00 2001 From: Paul Lange Date: Wed, 7 Nov 2018 12:22:26 +0100 Subject: [PATCH 7/7] PR review 3 --- raiden/network/proxies/secret_registry.py | 5 +---- raiden/network/proxies/token.py | 6 +++--- raiden/network/rpc/client.py | 12 ++---------- raiden/tests/integration/rpc/test_assumptions.py | 6 ++++-- raiden/tests/integration/test_endpointregistry.py | 4 ++-- 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/raiden/network/proxies/secret_registry.py b/raiden/network/proxies/secret_registry.py index 09397be836..14104fac26 100644 --- a/raiden/network/proxies/secret_registry.py +++ b/raiden/network/proxies/secret_registry.py @@ -99,10 +99,7 @@ def register_secret_batch(self, secrets: List[typing.Secret]): def _register_secret_batch(self, secrets): gas_limit = self.proxy.estimate_gas('registerSecretBatch', secrets) - gas_limit = max( - gas_limit, - safe_gas_limit(len(secrets) * GAS_REQUIRED_PER_SECRET_IN_BATCH), - ) + gas_limit = safe_gas_limit(gas_limit, len(secrets) * GAS_REQUIRED_PER_SECRET_IN_BATCH) transaction_hash = self.proxy.transact('registerSecretBatch', gas_limit, secrets) self.client.poll(transaction_hash) receipt_or_none = check_transaction_threw(self.client, transaction_hash) diff --git a/raiden/network/proxies/token.py b/raiden/network/proxies/token.py index c77a08a7e1..47b2acbc9b 100644 --- a/raiden/network/proxies/token.py +++ b/raiden/network/proxies/token.py @@ -6,7 +6,7 @@ from raiden.network.rpc.client import check_address_has_code from raiden.network.rpc.smartcontract_proxy import ContractProxy from raiden.network.rpc.transactions import check_transaction_threw -from raiden.utils import pex, privatekey_to_address +from raiden.utils import pex, privatekey_to_address, safe_gas_limit from raiden_contracts.constants import CONTRACT_HUMAN_STANDARD_TOKEN from raiden_contracts.contract_manager import ContractManager @@ -67,7 +67,7 @@ def approve(self, allowed_address, allowance): transaction_hash = self.proxy.transact( 'approve', - startgas, + safe_gas_limit(startgas), to_checksum_address(allowed_address), allowance, ) @@ -133,7 +133,7 @@ def transfer(self, to_address, amount): startgas = GAS_LIMIT_FOR_TOKEN_CONTRACT_CALL transaction_hash = self.proxy.transact( 'transfer', - startgas, + safe_gas_limit(startgas), to_checksum_address(to_address), amount, ) diff --git a/raiden/network/rpc/client.py b/raiden/network/rpc/client.py index b20f9e889c..bd33faaeed 100644 --- a/raiden/network/rpc/client.py +++ b/raiden/network/rpc/client.py @@ -257,10 +257,6 @@ def balance(self, account: typing.Address): """ Return the balance of the account of given address. """ return self.web3.eth.getBalance(to_checksum_address(account), 'pending') - def gaslimit(self, location='latest') -> int: - gas_limit = self.web3.eth.getBlock(location)['gasLimit'] - return gas_limit * 8 // 10 - def gas_price(self) -> int: # generateGasPrice takes the transaction to be send as an optional argument # but both strategies that we are using (time-based and rpc-based) don't make @@ -268,11 +264,6 @@ def gas_price(self) -> int: # This needs to be reevaluated if we use different gas price strategies return int(self.web3.eth.generateGasPrice()) - def check_startgas(self, startgas): - if not startgas: - return self.gaslimit() - return startgas - def new_contract_proxy(self, contract_interface, contract_address: typing.Address): """ Return a proxy for interacting with a smart contract. @@ -368,9 +359,10 @@ def deploy_solidity_contract( dependency_contract['bin'] = bytecode + gas_limit = self.web3.eth.getBlock('latest')['gasLimit'] * 8 // 10 transaction_hash = self.send_transaction( to=typing.Address(b''), - startgas=self.gaslimit(), + startgas=gas_limit, data=bytecode, ) diff --git a/raiden/tests/integration/rpc/test_assumptions.py b/raiden/tests/integration/rpc/test_assumptions.py index 55c27d9a23..cc5a5055ba 100644 --- a/raiden/tests/integration/rpc/test_assumptions.py +++ b/raiden/tests/integration/rpc/test_assumptions.py @@ -187,7 +187,8 @@ def test_transact_throws_opcode(deploy_client): address = contract_proxy.contract_address assert len(deploy_client.web3.eth.getCode(to_checksum_address(address))) > 0 - startgas = deploy_client.gaslimit() + # the gas estimation returns 0 here, so hardcode a value + startgas = safe_gas_limit(22_000) transaction = contract_proxy.transact('fail', startgas) deploy_client.poll(transaction) @@ -202,7 +203,8 @@ def test_transact_opcode_oog(deploy_client): address = contract_proxy.contract_address assert len(deploy_client.web3.eth.getCode(to_checksum_address(address))) > 0 - startgas = min(contract_proxy.estimate_gas('loop', 1000) // 2, deploy_client.gaslimit()) + # divide the estimate by 2 to run into out-of-gas + startgas = safe_gas_limit(contract_proxy.estimate_gas('loop', 1000)) // 2 transaction = contract_proxy.transact('loop', startgas, 1000) deploy_client.poll(transaction) diff --git a/raiden/tests/integration/test_endpointregistry.py b/raiden/tests/integration/test_endpointregistry.py index ac04f718cd..ea63825f4b 100644 --- a/raiden/tests/integration/test_endpointregistry.py +++ b/raiden/tests/integration/test_endpointregistry.py @@ -4,7 +4,7 @@ from raiden.network.discovery import ContractDiscovery from raiden.tests.utils.factories import make_address from raiden.tests.utils.smartcontracts import deploy_contract_web3 -from raiden.utils import host_port_to_endpoint, privatekey_to_address +from raiden.utils import host_port_to_endpoint, privatekey_to_address, safe_gas_limit from raiden_contracts.constants import ( CONTRACT_ENDPOINT_REGISTRY, GAS_REQUIRED_FOR_ENDPOINT_REGISTER, @@ -55,7 +55,7 @@ def test_endpointregistry_gas(endpoint_discovery_services): transaction_hash = discovery_proxy.proxy.transact( 'registerEndpoint', - GAS_REQUIRED_FOR_ENDPOINT_REGISTER, + safe_gas_limit(GAS_REQUIRED_FOR_ENDPOINT_REGISTER), endpoint, ) discovery_proxy.client.poll(transaction_hash)