From f432e0d63865ba4cb82dfdf75c56e3a8e96e2a5f Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 11 Oct 2023 20:39:39 +0000 Subject: [PATCH 01/11] Rework exceptions types. Rename RedisWalletAdapter -> RedisWallet --- skale/transactions/exceptions.py | 16 ++++++++++------ skale/wallets/redis_wallet.py | 28 +++++++++++++++++----------- tests/wallets/redis_adapter_test.py | 26 +++++++++++++------------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/skale/transactions/exceptions.py b/skale/transactions/exceptions.py index 2c35acfe..5aae8649 100644 --- a/skale/transactions/exceptions.py +++ b/skale/transactions/exceptions.py @@ -5,25 +5,29 @@ class TransactionError(Exception): pass -class DryRunFailedError(TransactionError): +class TransactionNotSentError(TransactionError): pass -class InsufficientBalanceError(TransactionError): +class TransactionNotMinedError(TimeoutError, TransactionError): pass -class TransactionNotSentError(TransactionError): +class TransactionWaitError(TimeoutError, TransactionError): pass -class TransactionNotMinedError(TimeoutError, TransactionError): +class TransactionLogicError(TransactionError): + pass + + +class DryRunFailedError(TransactionLogicError): pass -class TransactionFailedError(TransactionError): +class TransactionFailedError(TransactionLogicError): pass -class RevertError(TransactionError, ContractLogicError): +class RevertError(TransactionLogicError): pass diff --git a/skale/wallets/redis_wallet.py b/skale/wallets/redis_wallet.py index 150b2664..28c47a07 100644 --- a/skale/wallets/redis_wallet.py +++ b/skale/wallets/redis_wallet.py @@ -27,33 +27,39 @@ from redis import Redis import skale.config as config +from skale.transactions.exceptions import ( + TransactionError, + TransactionNotMinedError, + TransactionNotSentError, + TransactionWaitError +) from skale.utils.web3_utils import get_receipt, MAX_WAITING_TIME from skale.wallets import BaseWallet logger = logging.getLogger(__name__) -class RedisAdapterError(Exception): +class RedisWalletError(Exception): pass -class DroppedError(RedisAdapterError): +class RedisWalletDroppedError(RedisWalletError, TransactionNotMinedError): pass -class EmptyStatusError(RedisAdapterError): +class RedisWalletEmptyStatusError(RedisWalletError, TransactionError): pass -class AdapterSendError(RedisAdapterError): +class RedisWalletNotSentError(RedisWalletError, TransactionNotSentError): pass -class AdapterWaitError(RedisAdapterError): +class RedisWalletWaitError(RedisWalletError, TransactionWaitError): pass -class RedisWalletAdapter(BaseWallet): +class RedisWallet(BaseWallet): ID_SIZE = 16 def __init__( @@ -150,7 +156,7 @@ def sign_and_send( return self._to_id(raw_id) except Exception as err: logger.exception(f'Sending {tx} with redis wallet errored') - raise AdapterSendError(err) + raise RedisWalletNotSentError(err) def get_status(self, tx_id: str) -> str: return self.get_record(tx_id)['status'] @@ -178,11 +184,11 @@ def wait( return get_receipt(self.wallet._web3, r['tx_hash']) except Exception as err: logger.exception(f'Waiting for tx {tx_id} errored') - raise AdapterWaitError(err) + raise RedisWalletWaitError(err) if status is None: - raise EmptyStatusError('Tx status is None') + raise RedisWalletEmptyStatusError('Tx status is None') if status == 'DROPPED': - raise DroppedError('Tx was dropped after max retries') + raise RedisWalletDroppedError('Tx was dropped after max retries') else: - raise AdapterWaitError(f'Tx finished with status {status}') + raise RedisWalletWaitError(f'Tx finished with status {status}') diff --git a/tests/wallets/redis_adapter_test.py b/tests/wallets/redis_adapter_test.py index 030818e7..b9906d64 100644 --- a/tests/wallets/redis_adapter_test.py +++ b/tests/wallets/redis_adapter_test.py @@ -5,11 +5,11 @@ from freezegun import freeze_time from skale.wallets.redis_wallet import ( - AdapterSendError, - AdapterWaitError, - DroppedError, - EmptyStatusError, - RedisWalletAdapter + RedisWalletNotSentError, + RedisWalletWaitError, + RedisWalletDroppedError, + RedisWalletEmptyStatusError, + RedisWallet ) from tests.helper import in_time @@ -17,7 +17,7 @@ @pytest.fixture def rdp(skale): - return RedisWalletAdapter(mock.Mock(), 'transactions', skale.wallet) + return RedisWallet(mock.Mock(), 'transactions', skale.wallet) class RedisTestError(Exception): @@ -25,7 +25,7 @@ class RedisTestError(Exception): def test_make_raw_id(): - tx_id = RedisWalletAdapter._make_raw_id() + tx_id = RedisWallet._make_raw_id() assert tx_id.startswith(b'tx-') assert len(tx_id) == 19 @@ -34,7 +34,7 @@ def test_make_score(): cts = 1623233060 dt = datetime.utcfromtimestamp(cts) with freeze_time(dt): - score = RedisWalletAdapter._make_score(priority=5) + score = RedisWallet._make_score(priority=5) assert score == 51623233060 @@ -49,7 +49,7 @@ def test_make_record(): 'chainId': 1 } score = '51623233060' - tx_id, r = RedisWalletAdapter._make_record(tx, score, 2, method='createNode') + tx_id, r = RedisWallet._make_record(tx, score, 2, method='createNode') assert tx_id.startswith(b'tx-') and len(tx_id) == 19 assert r == b'{"status": "PROPOSED", "score": "51623233060", "multiplier": 2, "tx_hash": null, "method": "createNode", "meta": null, "from": "0x1", "to": "0x2", "value": 1, "gasPrice": 1, "gas": null, "nonce": 1, "chainId": 1}' # noqa @@ -68,7 +68,7 @@ def test_sign_and_send(rdp): assert tx_id.startswith('tx-') and len(tx_id) == 19 rdp.rs.pipeline = mock.Mock(side_effect=RedisTestError('rtest')) - with pytest.raises(AdapterSendError): + with pytest.raises(RedisWalletNotSentError): tx_id = rdp.sign_and_send(tx, multiplier=2, priority=5) @@ -76,17 +76,17 @@ def test_wait(rdp): tx_id = 'tx-tttttttttttttttt' rdp.get_status = mock.Mock(return_value=None) with in_time(3): - with pytest.raises(EmptyStatusError): + with pytest.raises(RedisWalletEmptyStatusError): rdp.wait(tx_id, timeout=2) rdp.get_status = mock.Mock(return_value='DROPPED') with in_time(2): - with pytest.raises(DroppedError): + with pytest.raises(RedisWalletDroppedError): rdp.wait(tx_id, timeout=100) rdp.get_status = mock.Mock(side_effect=RedisTestError('test')) with in_time(2): - with pytest.raises(AdapterWaitError): + with pytest.raises(RedisWalletWaitError): rdp.wait(tx_id, timeout=100) rdp.get_status = mock.Mock(return_value='SUCCESS') From 38414d264f6b74b2bf6353cae37ae04afb3a6ded Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 26 Oct 2023 16:09:47 +0000 Subject: [PATCH 02/11] Rework transaction pipeline exception handling --- skale/transactions/result.py | 27 ++++++++++++++++++--------- skale/transactions/tools.py | 9 +++++---- skale/wallets/__init__.py | 2 +- tests/conftest.py | 2 +- tests/manager/manager_test.py | 7 ++++--- tests/transaction_tools_test.py | 2 +- 6 files changed, 30 insertions(+), 19 deletions(-) diff --git a/skale/transactions/result.py b/skale/transactions/result.py index 1984e8f4..4b7f8b0e 100644 --- a/skale/transactions/result.py +++ b/skale/transactions/result.py @@ -17,6 +17,8 @@ # You should have received a copy of the GNU Affero General Public License # along with SKALE.py. If not, see . +from typing import Optional + from skale.transactions.exceptions import ( DryRunFailedError, RevertError, @@ -34,8 +36,11 @@ def is_success_or_not_performed(result: dict) -> bool: return result is None or is_success(result) -def is_revert_error(result: dict) -> bool: - return result and result.get('error', None) and 'reverted' in result['error'].lower() +def is_revert_error(result: Optional[dict]) -> bool: + if result: + return False + error = result.get('error', None) + return error == 'revert' class TxRes: @@ -63,11 +68,15 @@ def tx_failed(self) -> bool: return not is_success_or_not_performed(self.receipt) def raise_for_status(self) -> None: - if self.dry_run_failed(): + # Check if tx was sent + if self.receipt is not None: + # Check if tx errored + if not is_success(self.receipt): + raise TransactionFailedError(self.receipt['error']) + else: + # Check if revert if is_revert_error(self.dry_run_result): - raise RevertError(self.dry_run_result['error']) - raise DryRunFailedError(f'Dry run check failed. ' - f'See result {self.dry_run_result}') - if self.tx_failed(): - raise TransactionFailedError(f'Transaction failed. ' - f'See receipt {self.receipt}') + raise RevertError(self.dry_run_result['message']) + # Check if dry run errored due to other reason + if not is_success(self.dry_run_result): + raise DryRunFailedError(self.dry_run_result['error']) diff --git a/skale/transactions/tools.py b/skale/transactions/tools.py index 47144f0b..5294c24b 100644 --- a/skale/transactions/tools.py +++ b/skale/transactions/tools.py @@ -23,14 +23,13 @@ from typing import Dict, Optional from web3 import Web3 -from web3.exceptions import Web3Exception +from web3.exceptions import ContractLogicError, Web3Exception from web3._utils.transactions import get_block_gas_limit import skale.config as config from skale.transactions.exceptions import TransactionError from skale.transactions.result import TxRes from skale.utils.web3_utils import get_eth_nonce -from skale.wallets.redis_wallet import RedisAdapterError logger = logging.getLogger(__name__) @@ -59,8 +58,10 @@ def make_dry_run_call(skale, method, gas_limit=None, value=0) -> dict: else: estimated_gas = estimate_gas(skale.web3, method, opts) logger.info(f'Estimated gas for {method.fn_name}: {estimated_gas}') + except ContractLogicError as e: + return {'status': 0, 'error': 'revert', 'message': e.message, 'data': e.data} except (Web3Exception, ValueError) as err: - logger.error('Dry run for method failed with error', exc_info=err) + logger.error('Dry run for %s failed', method, exc_info=err) return {'status': 0, 'error': str(err)} return {'status': 1, 'payload': estimated_gas} @@ -178,7 +179,7 @@ def run_tx_with_retry(transaction, *args, max_retries=3, try: tx_res = transaction(*args, **kwargs) tx_res.raise_for_status() - except (RedisAdapterError, TransactionError) as err: + except TransactionError as err: logger.error(f'Tx attempt {attempt}/{max_retries} failed', exc_info=err) timeout = exp_timeout if retry_timeout < 0 else exp_timeout diff --git a/skale/wallets/__init__.py b/skale/wallets/__init__.py index a3b2cc4f..893fa8d3 100644 --- a/skale/wallets/__init__.py +++ b/skale/wallets/__init__.py @@ -2,7 +2,7 @@ from skale.wallets.common import BaseWallet from skale.wallets.ledger_wallet import LedgerWallet -from skale.wallets.redis_wallet import RedisWalletAdapter +from skale.wallets.redis_wallet import RedisWallet from skale.wallets.rpc_wallet import RPCWallet from skale.wallets.sgx_wallet import SgxWallet from skale.wallets.web3_wallet import Web3Wallet diff --git a/tests/conftest.py b/tests/conftest.py index 9192c561..488a1b3d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -122,7 +122,7 @@ def empty_account(): def failed_skale(skale): tmp_wait, tmp_sign_and_send = skale.wallet.wait, skale.wallet.sign_and_send skale.wallet.sign_and_send = mock.Mock(return_value='0x000000000') - skale.wallet.wait = mock.Mock(return_value={'status': 0}) + skale.wallet.wait = mock.Mock(return_value={'status': 0, 'error': 'Test error'}) try: yield skale finally: diff --git a/tests/manager/manager_test.py b/tests/manager/manager_test.py index df30b87a..a4a5e2d6 100644 --- a/tests/manager/manager_test.py +++ b/tests/manager/manager_test.py @@ -9,7 +9,7 @@ from unittest.mock import Mock from skale.wallets.web3_wallet import generate_wallet -from skale.transactions.result import TransactionFailedError +from skale.transactions.result import RevertError, TransactionFailedError from skale.utils.contracts_provision.main import ( generate_random_node_data, generate_random_schain_data @@ -144,6 +144,7 @@ def test_create_node_status_0(failed_skale): name, domain_name=DEFAULT_DOMAIN_NAME, wait_for=True, + skip_dry_run=True, raise_for_status=False ) assert tx_res.receipt['status'] == 0 @@ -162,8 +163,8 @@ def test_node_exit_with_no_schains(skale, nodes): def test_failed_node_exit(skale, block_in_seconds): # block_in_seconds fixuture to return transaction revert in a same way as geth does not_existed_node_id = 1 - with pytest.raises(TransactionFailedError): - skale.manager.node_exit(not_existed_node_id, skip_dry_run=True, + with pytest.raises(RevertError): + skale.manager.node_exit(not_existed_node_id, wait_for=True, gas_limit=TEST_GAS_LIMIT) diff --git a/tests/transaction_tools_test.py b/tests/transaction_tools_test.py index c13c383e..71f4f2cc 100644 --- a/tests/transaction_tools_test.py +++ b/tests/transaction_tools_test.py @@ -107,7 +107,7 @@ def test_run_tx_with_retry_insufficient_balance(skale): wait_for=True, gas_limit=TEST_GAS_LIMIT) retries_number = 5 - sender_skale.wallet.wait = mock.Mock() + sender_skale.wallet.wait = mock.MagicMock() run_tx_with_retry( sender_skale.token.transfer, skale.wallet.address, token_amount, wait_for=True, From c1a99b0ae1be4d7db0929f4f57e59a6c5a0a506e Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 26 Oct 2023 17:55:48 +0000 Subject: [PATCH 03/11] Fix tests --- skale/transactions/exceptions.py | 2 +- skale/transactions/result.py | 2 +- tests/manager/dkg_test.py | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/skale/transactions/exceptions.py b/skale/transactions/exceptions.py index 5aae8649..9ed2ef34 100644 --- a/skale/transactions/exceptions.py +++ b/skale/transactions/exceptions.py @@ -29,5 +29,5 @@ class TransactionFailedError(TransactionLogicError): pass -class RevertError(TransactionLogicError): +class RevertError(TransactionLogicError, ContractLogicError): pass diff --git a/skale/transactions/result.py b/skale/transactions/result.py index 4b7f8b0e..564b2754 100644 --- a/skale/transactions/result.py +++ b/skale/transactions/result.py @@ -37,7 +37,7 @@ def is_success_or_not_performed(result: dict) -> bool: def is_revert_error(result: Optional[dict]) -> bool: - if result: + if not result: return False error = result.get('error', None) return error == 'revert' diff --git a/tests/manager/dkg_test.py b/tests/manager/dkg_test.py index bb8312e7..170f1f5d 100644 --- a/tests/manager/dkg_test.py +++ b/tests/manager/dkg_test.py @@ -202,7 +202,6 @@ def test_complaint_bad_data(skale): send_tx_mock.return_value = b'hexstring' skale.dkg.complaint(group_index, from_node_index, to_node_index, wait_for=False, - skip_dry_run=True, gas_limit=gas_limit) send_tx_mock.assert_called_with(HexBytes(exp)) @@ -270,7 +269,7 @@ def test_alright_started_time(skale, schain): assert skale.dkg.get_alright_started_time(group_index) == 0 -def test_complaint_data(skale, schain): +def test_get_complaint_data(skale, schain): group_index = skale.web3.keccak(text=schain) assert skale.dkg.get_complaint_data(group_index) != [0, 0] From 3d73289ebaca8fd03aa59515ecacb0a4e995afd0 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 27 Oct 2023 16:43:11 +0000 Subject: [PATCH 04/11] Remove legacy RPCWallet --- skale/wallets/__init__.py | 1 - skale/wallets/rpc_wallet.py | 99 ---------------------- tests/wallets/rpc_test.py | 161 ------------------------------------ 3 files changed, 261 deletions(-) delete mode 100644 skale/wallets/rpc_wallet.py delete mode 100644 tests/wallets/rpc_test.py diff --git a/skale/wallets/__init__.py b/skale/wallets/__init__.py index 893fa8d3..0d93fd4a 100644 --- a/skale/wallets/__init__.py +++ b/skale/wallets/__init__.py @@ -3,6 +3,5 @@ from skale.wallets.common import BaseWallet from skale.wallets.ledger_wallet import LedgerWallet from skale.wallets.redis_wallet import RedisWallet -from skale.wallets.rpc_wallet import RPCWallet from skale.wallets.sgx_wallet import SgxWallet from skale.wallets.web3_wallet import Web3Wallet diff --git a/skale/wallets/rpc_wallet.py b/skale/wallets/rpc_wallet.py deleted file mode 100644 index 5def77cc..00000000 --- a/skale/wallets/rpc_wallet.py +++ /dev/null @@ -1,99 +0,0 @@ -# -*- coding: utf-8 -*- -# -# This file is part of SKALE.py -# -# Copyright (C) 2019-Present SKALE Labs -# -# SKALE.py is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# SKALE.py is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with SKALE.py. If not, see . - -import functools -import json -import time -import logging -import urllib - -import requests - -from skale.utils.exceptions import RPCWalletError -from skale.wallets.sgx_wallet import SgxWallet - - -logger = logging.getLogger(__name__) - - -ATTEMPTS = 100 -TIMEOUTS = [2 ** p for p in range(ATTEMPTS)] - - -def rpc_request(func): - @functools.wraps(func) - def wrapper(self, route, *args, **kwargs): - data, error, response = None, None, None - for i, timeout in enumerate(TIMEOUTS): - logger.info(f'Sending wallet rpc {route} request. Attempt {i}') - try: - response = func(self, route, *args, **kwargs).json() - data, error = response.get('data'), response.get('error') - except Exception as err: - error = 'RPC request failed' - logger.error(error, exc_info=err) - - if isinstance(error, str) and \ - error.startswith('Dry run failed') or \ - not error or not self._retry_if_failed: - break - - logger.info(f'Sleeping {timeout}s ...') - time.sleep(timeout) - - if error is not None: - raise RPCWalletError(error) - logger.info(f'Rpc wallet {route} request returned {data}') - return data - return wrapper - - -class RPCWallet(SgxWallet): - def __init__(self, url, sgx_endpoint, web3, key_name=None, - path_to_cert=None, retry_if_failed=False) -> None: - super().__init__(sgx_endpoint=sgx_endpoint, - web3=web3, key_name=key_name, - path_to_cert=path_to_cert) - self._url = url - self._retry_if_failed = retry_if_failed - - @classmethod - def _construct_url(cls, host, url): - return urllib.parse.urljoin(host, url) - - @rpc_request - def _post(self, route, data): - request_url = self._construct_url(self._url, route) - return requests.post(request_url, json=data) - - @rpc_request - def _get(self, route, data=None): - request_url = self._construct_url(self._url, route) - return requests.get(request_url, data=data) - - @classmethod - def _compose_tx_data(cls, tx_dict): - return { - 'transaction_dict': json.dumps(tx_dict) - } - - def sign_and_send(self, tx_dict): - data = self._post('/sign-and-send', - self._compose_tx_data(tx_dict)) - return data['transaction_hash'] diff --git a/tests/wallets/rpc_test.py b/tests/wallets/rpc_test.py deleted file mode 100644 index 8c566820..00000000 --- a/tests/wallets/rpc_test.py +++ /dev/null @@ -1,161 +0,0 @@ -""" SKALE RPCWallet test """ - - -from http import HTTPStatus - -import pytest -from unittest import mock - -from hexbytes import HexBytes -from skale.wallets import RPCWallet -from skale.utils.exceptions import RPCWalletError - -from tests.constants import ( - ENDPOINT, EMPTY_HEX_STR, ETH_PRIVATE_KEY, - NOT_EXISTING_RPC_WALLET_URL, TEST_SGX_ENDPOINT, TEST_RPC_WALLET_URL -) -from tests.helper import response_mock, request_mock -from tests.wallets.utils import SgxClient -from skale.utils.web3_utils import ( - init_web3, - private_key_to_address, private_key_to_public, - to_checksum_address -) - - -TX_DICT = { - 'to': '0x1057dc7277a319927D3eB43e05680B75a00eb5f4', - 'value': 9, - 'gas': 200000, - 'gasPrice': 1, - 'nonce': 7, - 'chainId': None, - 'data': '0x0' -} - -EMPTY_ETH_ACCOUNT = '0x0000000000000000000000000000000000000000' -TEST_MAX_RETRIES = 3 - - -@pytest.fixture -def web3(): - return init_web3(ENDPOINT) - - -@pytest.fixture -@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=SgxClient) -def wallet(web3): - return RPCWallet(TEST_RPC_WALLET_URL, TEST_SGX_ENDPOINT, web3, - retry_if_failed=True) - - -@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=SgxClient) -def test_rpc_not_available(web3): - wallet = RPCWallet(NOT_EXISTING_RPC_WALLET_URL, TEST_SGX_ENDPOINT, web3) - with pytest.raises(RPCWalletError): - wallet.sign_and_send({}) - - -def test_rpc_sign_and_send(wallet): - res_mock = response_mock(HTTPStatus.OK, - {'data': {'transaction_hash': EMPTY_HEX_STR}, - 'error': None}) - with mock.patch('requests.post', new=request_mock(res_mock)): - tx_hash = wallet.sign_and_send(TX_DICT) - assert tx_hash == EMPTY_HEX_STR - - -def test_rpc_sign_and_send_fails(wallet): - cnt = 0 - - def post_mock(*args, **kwargs): - nonlocal cnt - response_mock = mock.Mock() - if cnt < TEST_MAX_RETRIES: - rv = {'data': None, 'error': object()} - cnt += 1 - else: - rv = {'data': 'test', 'error': ''} - - response_mock.json = mock.Mock(return_value=rv) - return response_mock - - with mock.patch('requests.post', post_mock): - with pytest.raises(RPCWalletError): - wallet.sign_and_send(TX_DICT) - - assert cnt == TEST_MAX_RETRIES - - -@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=SgxClient) -def test_rpc_sign_and_send_sgx_unreachable_no_retries(web3): - res_mock = response_mock(HTTPStatus.BAD_REQUEST, - {'data': None, - 'error': 'Sgx server is unreachable'}) - wallet = RPCWallet(NOT_EXISTING_RPC_WALLET_URL, TEST_SGX_ENDPOINT, web3) - with mock.patch('requests.post', new=request_mock(res_mock)): - with pytest.raises(RPCWalletError): - wallet.sign_and_send(TX_DICT) - assert res_mock.call_count == 1 - - -def test_rpc_sign_and_send_sgx_unreachable(wallet): - cnt = 0 - - def post_mock(*args, **kwargs): - nonlocal cnt - response_mock = mock.Mock() - if cnt < TEST_MAX_RETRIES: - rv = {'data': None, 'error': 'Sgx server is unreachable'} - cnt += 1 - else: - rv = {'data': 'test', 'error': ''} - - response_mock.json = mock.Mock(return_value=rv) - return response_mock - - with mock.patch('requests.post', post_mock): - with pytest.raises(RPCWalletError): - wallet.sign_and_send(TX_DICT) - - assert cnt == TEST_MAX_RETRIES - - -def test_rpc_sign_hash(wallet): - unsigned_hash = '0x31323331' - signed_message = wallet.sign_hash(unsigned_hash) - assert signed_message.signature == HexBytes('0x6161616161613131313131') - - -def test_sign_and_send_dry_run(wallet): - cnt = 0 - - def post_mock(*args, **kwargs): - nonlocal cnt - response_mock = mock.Mock() - if cnt < TEST_MAX_RETRIES: - rv = {'data': None, 'error': 'Dry run failed: {"Test"}'} - cnt += 1 - else: - rv = {'data': 'test', 'error': ''} - - response_mock.json = mock.Mock(return_value=rv) - return response_mock - - with mock.patch('requests.post', post_mock): - with pytest.raises(RPCWalletError): - wallet.sign_and_send(TX_DICT) - - assert cnt == 1 - - -def test_rpc_address(wallet): - address = to_checksum_address( - private_key_to_address(ETH_PRIVATE_KEY) - ) - assert wallet.address == address - - -def test_rpc_public_key(wallet): - pk = private_key_to_public(ETH_PRIVATE_KEY) - assert wallet.public_key == pk From b8b6840d04746f23baf8479e62fe67472f9157cc Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 27 Oct 2023 18:27:47 +0000 Subject: [PATCH 05/11] Make wallet methods return proper exceptions --- skale/transactions/exceptions.py | 48 ++++++++++++++++++++++++++++- skale/utils/exceptions.py | 4 --- skale/utils/web3_utils.py | 4 +++ skale/wallets/__init__.py | 2 +- skale/wallets/common.py | 9 +++++- skale/wallets/ledger_wallet.py | 20 ++++++++---- skale/wallets/redis_wallet.py | 2 +- skale/wallets/sgx_wallet.py | 16 +++++++--- skale/wallets/web3_wallet.py | 38 +++++++++++++++-------- tests/manager/base_contract_test.py | 3 +- tests/wallets/common_test.py | 2 +- tests/wallets/redis_adapter_test.py | 10 +++--- 12 files changed, 121 insertions(+), 37 deletions(-) diff --git a/skale/transactions/exceptions.py b/skale/transactions/exceptions.py index 9ed2ef34..073f40a3 100644 --- a/skale/transactions/exceptions.py +++ b/skale/transactions/exceptions.py @@ -2,32 +2,78 @@ class TransactionError(Exception): + """ + Base exception for transaction related errors + """ + pass + + +class ChainIdError(TransactionError): + """ + Raised when chainId is missing or incorrect + """ + pass + + +class TransactionNotSignedError(TransactionError): + """ + Raised when transaction wasn't signed + """ pass class TransactionNotSentError(TransactionError): + """ + Raised when transaction wasn't sent + """ pass class TransactionNotMinedError(TimeoutError, TransactionError): + """ + Raised when transaction wasn't included in block within timeout + """ pass class TransactionWaitError(TimeoutError, TransactionError): + """ + Raised when error occurred during waiting for transaction + to be included in block + """ pass class TransactionLogicError(TransactionError): + """ + Raised when transaction executed with error + """ pass -class DryRunFailedError(TransactionLogicError): +class DryRunFailedError(TransactionError): + """ + Raised when error occurred during dry run call + """ pass class TransactionFailedError(TransactionLogicError): + """ + Raised when transaction included in a block failed during execution + """ pass class RevertError(TransactionLogicError, ContractLogicError): + """ + Raised when transaction was included in a block and reverted during execution + """ + pass + + +class RevertDryRunError(DryRunFailedError): + """ + Raised when transaction reverted during dry run call + """ pass diff --git a/skale/utils/exceptions.py b/skale/utils/exceptions.py index 3ebd6098..a59912ae 100644 --- a/skale/utils/exceptions.py +++ b/skale/utils/exceptions.py @@ -46,7 +46,3 @@ class InvalidNodeIdError(Exception): def __init__(self, node_id): message = f'Node with ID = {node_id} doesn\'t exist!' super().__init__(message) - - -class ChainIdError(ValueError): - """Raised when chainId is missing or incorrect""" diff --git a/skale/utils/web3_utils.py b/skale/utils/web3_utils.py index 66d60d0b..eae748ea 100644 --- a/skale/utils/web3_utils.py +++ b/skale/utils/web3_utils.py @@ -71,6 +71,10 @@ class EthClientOutdatedError(Exception): pass +class BlockWaitTimeoutError(Exception): + pass + + def get_last_known_block_number(state_path: str) -> int: if not os.path.isfile(state_path): return 0 diff --git a/skale/wallets/__init__.py b/skale/wallets/__init__.py index 0d93fd4a..a5d7b685 100644 --- a/skale/wallets/__init__.py +++ b/skale/wallets/__init__.py @@ -2,6 +2,6 @@ from skale.wallets.common import BaseWallet from skale.wallets.ledger_wallet import LedgerWallet -from skale.wallets.redis_wallet import RedisWallet +from skale.wallets.redis_wallet import RedisWalletAdapter from skale.wallets.sgx_wallet import SgxWallet from skale.wallets.web3_wallet import Web3Wallet diff --git a/skale/wallets/common.py b/skale/wallets/common.py index 2b5ba113..bb97ee8c 100644 --- a/skale/wallets/common.py +++ b/skale/wallets/common.py @@ -20,7 +20,7 @@ from abc import ABC, abstractmethod from typing import Dict, Optional -from skale.utils.exceptions import ChainIdError +from skale.transactions.exceptions import ChainIdError def ensure_chain_id(tx_dict, web3): @@ -30,6 +30,13 @@ def ensure_chain_id(tx_dict, web3): raise ChainIdError('chainId must be in tx_dict (see EIP-155)') +class MessageNotSignedError(Exception): + """ + Raised when signing message failed + """ + pass + + class BaseWallet(ABC): @abstractmethod def sign(self, tx): diff --git a/skale/wallets/ledger_wallet.py b/skale/wallets/ledger_wallet.py index 7f7f9249..1b1a5267 100644 --- a/skale/wallets/ledger_wallet.py +++ b/skale/wallets/ledger_wallet.py @@ -29,8 +29,10 @@ from eth_utils.crypto import keccak from rlp import encode +from web3.exceptions import Web3Exception import skale.config as config +from skale.transactions.exceptions import TransactionNotSentError, TransactionNotSignedError from skale.utils.web3_utils import ( get_eth_nonce, public_key_to_address, @@ -162,9 +164,12 @@ def sign(self, tx_dict): if tx_dict.get('nonce') is None: tx_dict['nonce'] = self._web3.eth.get_transaction_count(self.address) tx = tx_from_dict(tx_dict) - payload = self.make_payload(tx) - exchange_result = self.exchange_sign_payload_by_chunks(payload) - return LedgerWallet.parse_sign_result(tx, exchange_result) + try: + payload = self.make_payload(tx) + exchange_result = self.exchange_sign_payload_by_chunks(payload) + return LedgerWallet.parse_sign_result(tx, exchange_result) + except Exception as e: + raise TransactionNotSignedError(e) def sign_and_send( self, @@ -175,9 +180,12 @@ def sign_and_send( meta: Optional[Dict] = None ) -> str: signed_tx = self.sign(tx) - return self._web3.eth.send_raw_transaction( - signed_tx.rawTransaction - ).hex() + try: + return self._web3.eth.send_raw_transaction( + signed_tx.rawTransaction + ).hex() + except Web3Exception as e: + raise TransactionNotSentError(e) def sign_hash(self, unsigned_hash: str): raise NotImplementedError( diff --git a/skale/wallets/redis_wallet.py b/skale/wallets/redis_wallet.py index 28c47a07..8df872e7 100644 --- a/skale/wallets/redis_wallet.py +++ b/skale/wallets/redis_wallet.py @@ -59,7 +59,7 @@ class RedisWalletWaitError(RedisWalletError, TransactionWaitError): pass -class RedisWallet(BaseWallet): +class RedisWalletAdapter: ID_SIZE = 16 def __init__( diff --git a/skale/wallets/sgx_wallet.py b/skale/wallets/sgx_wallet.py index 53460b96..19d1f637 100644 --- a/skale/wallets/sgx_wallet.py +++ b/skale/wallets/sgx_wallet.py @@ -22,8 +22,10 @@ from sgx import SgxClient from web3 import Web3 +from web3.exceptions import Web3Exception import skale.config as config +from skale.transactions.exceptions import TransactionNotSentError, TransactionNotSignedError from skale.utils.web3_utils import get_eth_nonce, wait_for_receipt_by_blocks from skale.wallets.common import BaseWallet, ensure_chain_id @@ -45,7 +47,10 @@ def sign(self, tx_dict): if tx_dict.get('nonce') is None: tx_dict['nonce'] = get_eth_nonce(self._web3, self._address) ensure_chain_id(tx_dict, self._web3) - return self.sgx_client.sign(tx_dict, self.key_name) + try: + return self.sgx_client.sign(tx_dict, self.key_name) + except Exception as e: + raise TransactionNotSignedError(e) def sign_and_send( self, @@ -56,9 +61,12 @@ def sign_and_send( meta: Optional[Dict] = None ) -> str: signed_tx = self.sign(tx_dict) - return self._web3.eth.send_raw_transaction( - signed_tx.rawTransaction - ).hex() + try: + return self._web3.eth.send_raw_transaction( + signed_tx.rawTransaction + ).hex() + except Web3Exception as e: + raise TransactionNotSentError(e) def sign_hash(self, unsigned_hash: str): if unsigned_hash.startswith('0x'): diff --git a/skale/wallets/web3_wallet.py b/skale/wallets/web3_wallet.py index ab4fd860..f649e983 100644 --- a/skale/wallets/web3_wallet.py +++ b/skale/wallets/web3_wallet.py @@ -21,10 +21,15 @@ from eth_keys import keys from web3 import Web3 from eth_account import messages +from web3.exceptions import Web3Exception import skale.config as config +from skale.transactions.exceptions import ( + TransactionNotSignedError, + TransactionNotSentError +) from skale.utils.web3_utils import get_eth_nonce, wait_for_receipt_by_blocks -from skale.wallets.common import BaseWallet, ensure_chain_id +from skale.wallets.common import BaseWallet, ensure_chain_id, MessageNotSignedError def private_key_to_public(pr): @@ -65,17 +70,23 @@ def sign(self, tx_dict): if not tx_dict.get('nonce'): tx_dict['nonce'] = get_eth_nonce(self._web3, self._address) ensure_chain_id(tx_dict, self._web3) - return self._web3.eth.account.sign_transaction( - tx_dict, - private_key=self._private_key - ) + try: + return self._web3.eth.account.sign_transaction( + tx_dict, + private_key=self._private_key + ) + except Web3Exception as e: + raise TransactionNotSignedError(e) def sign_hash(self, unsigned_hash: str): unsigned_message = messages.encode_defunct(hexstr=unsigned_hash) - return self._web3.eth.account.sign_message( - unsigned_message, - private_key=self._private_key - ) + try: + return self._web3.eth.account.sign_message( + unsigned_message, + private_key=self._private_key + ) + except Web3Exception as e: + raise MessageNotSignedError(e) def sign_and_send( self, @@ -86,9 +97,12 @@ def sign_and_send( meta: Optional[Dict] = None ) -> str: signed_tx = self.sign(tx_dict) - return self._web3.eth.send_raw_transaction( - signed_tx.rawTransaction - ).hex() + try: + return self._web3.eth.send_raw_transaction( + signed_tx.rawTransaction + ).hex() + except Exception as e: + raise TransactionNotSentError(e) @property def address(self): diff --git a/tests/manager/base_contract_test.py b/tests/manager/base_contract_test.py index ccb7209d..e704ee80 100644 --- a/tests/manager/base_contract_test.py +++ b/tests/manager/base_contract_test.py @@ -4,6 +4,7 @@ from unittest import mock import pytest import skale.config as config +from skale.transactions.exceptions import TransactionNotSentError from skale.transactions.tools import estimate_gas from skale.utils.account_tools import generate_account from skale.utils.contracts_provision.utils import generate_random_schain_data @@ -144,7 +145,7 @@ def test_tx_res_with_insufficient_funds(skale): account = generate_account(skale.web3) token_amount = 9 huge_gas_price = 10 ** 22 - with pytest.raises(ValueError): + with pytest.raises(TransactionNotSentError): skale.token.transfer( account['address'], token_amount, diff --git a/tests/wallets/common_test.py b/tests/wallets/common_test.py index 93dd460d..688d617d 100644 --- a/tests/wallets/common_test.py +++ b/tests/wallets/common_test.py @@ -4,7 +4,7 @@ from skale.wallets import Web3Wallet from skale.wallets.common import ensure_chain_id -from skale.utils.exceptions import ChainIdError +from skale.transactions.exceptions import ChainIdError from skale.utils.web3_utils import init_web3 from tests.constants import ENDPOINT, ETH_PRIVATE_KEY diff --git a/tests/wallets/redis_adapter_test.py b/tests/wallets/redis_adapter_test.py index b9906d64..56642f29 100644 --- a/tests/wallets/redis_adapter_test.py +++ b/tests/wallets/redis_adapter_test.py @@ -9,7 +9,7 @@ RedisWalletWaitError, RedisWalletDroppedError, RedisWalletEmptyStatusError, - RedisWallet + RedisWalletAdapter ) from tests.helper import in_time @@ -17,7 +17,7 @@ @pytest.fixture def rdp(skale): - return RedisWallet(mock.Mock(), 'transactions', skale.wallet) + return RedisWalletAdapter(mock.Mock(), 'transactions', skale.wallet) class RedisTestError(Exception): @@ -25,7 +25,7 @@ class RedisTestError(Exception): def test_make_raw_id(): - tx_id = RedisWallet._make_raw_id() + tx_id = RedisWalletAdapter._make_raw_id() assert tx_id.startswith(b'tx-') assert len(tx_id) == 19 @@ -34,7 +34,7 @@ def test_make_score(): cts = 1623233060 dt = datetime.utcfromtimestamp(cts) with freeze_time(dt): - score = RedisWallet._make_score(priority=5) + score = RedisWalletAdapter._make_score(priority=5) assert score == 51623233060 @@ -49,7 +49,7 @@ def test_make_record(): 'chainId': 1 } score = '51623233060' - tx_id, r = RedisWallet._make_record(tx, score, 2, method='createNode') + tx_id, r = RedisWalletAdapter._make_record(tx, score, 2, method='createNode') assert tx_id.startswith(b'tx-') and len(tx_id) == 19 assert r == b'{"status": "PROPOSED", "score": "51623233060", "multiplier": 2, "tx_hash": null, "method": "createNode", "meta": null, "from": "0x1", "to": "0x2", "value": 1, "gasPrice": 1, "gas": null, "nonce": 1, "chainId": 1}' # noqa From c13cd3ed5ea923ec6665d75fff83570029e6420e Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 30 Oct 2023 19:07:38 +0000 Subject: [PATCH 06/11] Improve wallets tests --- skale/wallets/ledger_wallet.py | 5 ++-- skale/wallets/sgx_wallet.py | 17 +++++++------ skale/wallets/web3_wallet.py | 8 +++--- tests/wallets/ledger_test.py | 15 ++++++++++++ tests/wallets/sgx_test.py | 45 ++++++++++++++++++++++++++++++++-- tests/wallets/utils.py | 25 +++++++++++++++++++ 6 files changed, 99 insertions(+), 16 deletions(-) diff --git a/skale/wallets/ledger_wallet.py b/skale/wallets/ledger_wallet.py index 1b1a5267..6d2638e0 100644 --- a/skale/wallets/ledger_wallet.py +++ b/skale/wallets/ledger_wallet.py @@ -159,10 +159,9 @@ def exchange_sign_payload_by_chunks(self, payload): def sign(self, tx_dict): ensure_chain_id(tx_dict, self._web3) - if config.ENV == 'dev': # fix for big chainId in ganache - tx_dict['chainId'] = None if tx_dict.get('nonce') is None: tx_dict['nonce'] = self._web3.eth.get_transaction_count(self.address) + tx = tx_from_dict(tx_dict) try: payload = self.make_payload(tx) @@ -184,7 +183,7 @@ def sign_and_send( return self._web3.eth.send_raw_transaction( signed_tx.rawTransaction ).hex() - except Web3Exception as e: + except (ValueError, Web3Exception) as e: raise TransactionNotSentError(e) def sign_hash(self, unsigned_hash: str): diff --git a/skale/wallets/sgx_wallet.py b/skale/wallets/sgx_wallet.py index 19d1f637..c2053bce 100644 --- a/skale/wallets/sgx_wallet.py +++ b/skale/wallets/sgx_wallet.py @@ -27,7 +27,7 @@ import skale.config as config from skale.transactions.exceptions import TransactionNotSentError, TransactionNotSignedError from skale.utils.web3_utils import get_eth_nonce, wait_for_receipt_by_blocks -from skale.wallets.common import BaseWallet, ensure_chain_id +from skale.wallets.common import BaseWallet, ensure_chain_id, MessageNotSignedError logger = logging.getLogger(__name__) @@ -65,7 +65,7 @@ def sign_and_send( return self._web3.eth.send_raw_transaction( signed_tx.rawTransaction ).hex() - except Web3Exception as e: + except (ValueError, Web3Exception) as e: raise TransactionNotSentError(e) def sign_hash(self, unsigned_hash: str): @@ -77,11 +77,14 @@ def sign_hash(self, unsigned_hash: str): normalized_hash = header + body hash_to_sign = Web3.keccak(hexstr='0x' + normalized_hash.hex()) chain_id = None - return self.sgx_client.sign_hash( - hash_to_sign, - self._key_name, - chain_id - ) + try: + return self.sgx_client.sign_hash( + hash_to_sign, + self._key_name, + chain_id + ) + except Exception as e: + raise MessageNotSignedError(e) @property def address(self): diff --git a/skale/wallets/web3_wallet.py b/skale/wallets/web3_wallet.py index f649e983..64b9faf5 100644 --- a/skale/wallets/web3_wallet.py +++ b/skale/wallets/web3_wallet.py @@ -75,17 +75,17 @@ def sign(self, tx_dict): tx_dict, private_key=self._private_key ) - except Web3Exception as e: + except (TypeError, ValueError, Web3Exception) as e: raise TransactionNotSignedError(e) def sign_hash(self, unsigned_hash: str): - unsigned_message = messages.encode_defunct(hexstr=unsigned_hash) try: + unsigned_message = messages.encode_defunct(hexstr=unsigned_hash) return self._web3.eth.account.sign_message( unsigned_message, private_key=self._private_key ) - except Web3Exception as e: + except (TypeError, ValueError, Web3Exception) as e: raise MessageNotSignedError(e) def sign_and_send( @@ -101,7 +101,7 @@ def sign_and_send( return self._web3.eth.send_raw_transaction( signed_tx.rawTransaction ).hex() - except Exception as e: + except (ValueError, Web3Exception) as e: raise TransactionNotSentError(e) @property diff --git a/tests/wallets/ledger_test.py b/tests/wallets/ledger_test.py index 1d9e2f51..f10315e6 100644 --- a/tests/wallets/ledger_test.py +++ b/tests/wallets/ledger_test.py @@ -1,5 +1,7 @@ +import pytest from unittest import mock +from skale.transactions.exceptions import TransactionNotSentError from skale.utils.web3_utils import init_web3 from skale.wallets.ledger_wallet import LedgerWallet @@ -36,3 +38,16 @@ def test_hardware_sign_and_send(): 'data': b'\x9b\xd9\xbb\xc6\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x95qY\xc4i\xfc;\xba\xa8\xe3\x9e\xe0\xa3$\xc28\x8a\xd6Q\xe5\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\r\xe0\xb6\xb3\xa7d\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00`\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x006\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa8\xc0\x04/Rglamorous-kitalpha\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' # noqa } wallet.sign(tx_dict) + + tx_dict = { + 'to': '0x1057dc7277a319927D3eB43e05680B75a00eb5f4', + 'value': 9, + 'gas': 200000, + 'gasPrice': 1, + 'nonce': 1, + 'chainId': 2132, + 'data': b'\x9b\xd9\xbb\xc6\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x95qY\xc4i\xfc;\xba\xa8\xe3\x9e\xe0\xa3$\xc28\x8a\xd6Q\xe5\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\r\xe0\xb6\xb3\xa7d\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00`\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x006\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa8\xc0\x04/Rglamorous-kitalpha\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' # noqa + } + # No balance on this account, should fail + with pytest.raises(TransactionNotSentError): + wallet.sign_and_send(tx_dict) diff --git a/tests/wallets/sgx_test.py b/tests/wallets/sgx_test.py index ad7602fd..b1fe7132 100644 --- a/tests/wallets/sgx_test.py +++ b/tests/wallets/sgx_test.py @@ -1,7 +1,8 @@ from unittest import mock import pytest from hexbytes import HexBytes -from skale.wallets import SgxWallet +from skale.wallets.sgx_wallet import (SgxWallet, TransactionNotSentError, + TransactionNotSignedError, MessageNotSignedError) from skale.utils.web3_utils import ( init_web3, private_key_to_address, @@ -10,7 +11,7 @@ ) from tests.constants import ENDPOINT, ETH_PRIVATE_KEY, TEST_SGX_ENDPOINT -from tests.wallets.utils import SgxClient +from tests.wallets.utils import BadSgxClient, SgxClient ADDRESS = to_checksum_address( private_key_to_address(ETH_PRIVATE_KEY) @@ -88,3 +89,43 @@ def test_sgx_key_init(web3): assert wallet.key_name == 'TEST_KEY' assert wallet.address == ADDRESS assert wallet.public_key == pk + + +@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=SgxClient) +def test_not_sent_error(web3): + wallet = SgxWallet(TEST_SGX_ENDPOINT, web3, 'TEST_KEY') + tx_dict = { + 'to': '0x1057dc7277a319927D3eB43e05680B75a00eb5f4', + 'value': 9, + 'gas': 200000, + 'gasPrice': 1, + 'nonce': 1, # nonce too low + 'chainId': None, + 'data': b'\x9b\xd9\xbb\xc6\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x95qY\xc4i\xfc;\xba\xa8\xe3\x9e\xe0\xa3$\xc28\x8a\xd6Q\xe5\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\r\xe0\xb6\xb3\xa7d\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00`\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x006\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa8\xc0\x04/Rglamorous-kitalpha\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' # noqa + } + + with pytest.raises(TransactionNotSentError): + wallet.sign_and_send(tx_dict) + + +@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=BadSgxClient) +def test_sign_error(web3): + wallet = SgxWallet(TEST_SGX_ENDPOINT, web3, 'TEST_KEY') + tx_dict = { + 'to': '0x1057dc7277a319927D3eB43e05680B75a00eb5f4', + 'value': 9, + 'gas': 200000, + 'gasPrice': 1, + 'nonce': 7, + 'chainId': None, + 'data': b'\x9b\xd9\xbb\xc6\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x95qY\xc4i\xfc;\xba\xa8\xe3\x9e\xe0\xa3$\xc28\x8a\xd6Q\xe5\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\r\xe0\xb6\xb3\xa7d\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00`\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x006\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa8\xc0\x04/Rglamorous-kitalpha\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' # noqa + } + with pytest.raises(TransactionNotSignedError): + wallet.sign(tx_dict) + + +@mock.patch('skale.wallets.sgx_wallet.SgxClient', new=BadSgxClient) +def test_sign_hash_error(web3): + wallet = SgxWallet(TEST_SGX_ENDPOINT, web3, 'TEST_KEY') + with pytest.raises(MessageNotSignedError): + wallet.sign_hash('0x') diff --git a/tests/wallets/utils.py b/tests/wallets/utils.py index 4a9c322f..1b84d229 100644 --- a/tests/wallets/utils.py +++ b/tests/wallets/utils.py @@ -49,3 +49,28 @@ def sign_hash(self, message, key_name, chain_id): v=27, signature=HexBytes('0x6161616161613131313131') ) + + +class BadSgxClient: + def __init__(self, endpoint, path_to_cert=None): + pass + + def generate_key(self): + return Account( + name='NEK:aaabbb', + address=ADDRESS, + public_key=PUBLIC_KEY + ) + + def get_account(self, key_name): + return Account( + name='NEK:aaabbb', + address=ADDRESS, + public_key=PUBLIC_KEY + ) + + def sign(self, transaction_dict, key_name): + raise ValueError('Test Error') + + def sign_hash(self, message, key_name, chain_id): + raise ValueError('Test error') From b2a0e6da5577a333e9f08fb0ee7ccc958c7cfc3f Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 31 Oct 2023 17:45:04 +0000 Subject: [PATCH 07/11] Fix run_tx_with_retry --- skale/transactions/tools.py | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/skale/transactions/tools.py b/skale/transactions/tools.py index 5294c24b..c0abbfcb 100644 --- a/skale/transactions/tools.py +++ b/skale/transactions/tools.py @@ -170,29 +170,41 @@ def wrapper(*args, **kwargs): def run_tx_with_retry(transaction, *args, max_retries=3, retry_timeout=-1, + raise_for_status=True, **kwargs) -> TxRes: - success = False attempt = 0 tx_res = None exp_timeout = 1 - while not success and attempt < max_retries: + error = None + while attempt < max_retries: try: - tx_res = transaction(*args, **kwargs) + tx_res = transaction( + *args, + raise_for_status=raise_for_status, + **kwargs + ) tx_res.raise_for_status() - except TransactionError as err: - logger.error(f'Tx attempt {attempt}/{max_retries} failed', - exc_info=err) + except TransactionError as e: + error = e + logger.exception('Tx attempt %d/%d failed', attempt, max_retries) + timeout = exp_timeout if retry_timeout < 0 else exp_timeout time.sleep(timeout) exp_timeout *= 2 else: - success = True + error = None + break attempt += 1 - if success: - logger.info(f'Tx {transaction.__name__} completed ' - f'after {attempt}/{max_retries} retries') + if error is None: + logger.info( + 'Tx %s completed after %d/%d retries', + transaction.__name__, attempt, max_retries + ) else: logger.error( - f'Tx {transaction.__name__} failed after ' - f'{max_retries} retries') + 'Tx %s completed after %d retries', + transaction.__name__, max_retries + ) + if raise_for_status: + raise error return tx_res From 6bf2d82c282d3e1f4f7b0e2621a68fe44efb425f Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 1 Nov 2023 11:18:12 +0000 Subject: [PATCH 08/11] Make transaction_method cleaner --- skale/contracts/base_contract.py | 36 ++++++++++++++------------------ 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/skale/contracts/base_contract.py b/skale/contracts/base_contract.py index 13b0fc4d..092de5ec 100644 --- a/skale/contracts/base_contract.py +++ b/skale/contracts/base_contract.py @@ -80,23 +80,18 @@ def wrapper( nonce = get_eth_nonce(self.skale.web3, self.skale.wallet.address) - # Make dry_run and estimate gas limit estimated_gas_limit = None - if not skip_dry_run and not config.DISABLE_DRY_RUN: - dry_run_result, estimated_gas_limit = execute_dry_run( - self.skale, method, gas_limit, value - ) + should_dry_run = not skip_dry_run and not config.DISABLE_DRY_RUN - gas_limit = gas_limit or estimated_gas_limit or \ - config.DEFAULT_GAS_LIMIT + if should_dry_run: + dry_run_result, estimated_gas_limit = execute_dry_run(self.skale, + method, gas_limit, value) - gas_price = gas_price or config.DEFAULT_GAS_PRICE_WEI or \ - self.skale.gas_price - # Send transaction - should_send_transaction = not dry_run_only and \ - is_success_or_not_performed(dry_run_result) + should_send = not dry_run_only and is_success_or_not_performed(dry_run_result) + gas_limit = gas_limit or estimated_gas_limit or config.DEFAULT_GAS_LIMIT + gas_price = gas_price or config.DEFAULT_GAS_PRICE_WEI or self.skale.gas_price - if should_send_transaction: + if should_send: tx = transaction_from_method( method=method, gas_limit=gas_limit, @@ -114,13 +109,14 @@ def wrapper( method=method_name, meta=meta ) - if wait_for: - receipt = self.skale.wallet.wait(tx_hash) - if confirmation_blocks: - wait_for_confirmation_blocks( - self.skale.web3, - confirmation_blocks - ) + + should_wait = tx_hash is not None and wait_for + if should_wait: + receipt = self.skale.wallet.wait(tx_hash) + + should_confirm = receipt and confirmation_blocks > 0 + if should_confirm: + wait_for_confirmation_blocks(self.skale.web3, confirmation_blocks) tx_res = TxRes(dry_run_result, tx_hash, receipt) From da415250f6b6ef454c7c389dd773163124033f8e Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 2 Nov 2023 20:28:34 +0000 Subject: [PATCH 09/11] Improve TxRes --- skale/transactions/exceptions.py | 12 +++++----- skale/transactions/result.py | 23 +++++++++++-------- tests/manager/constants_test.py | 4 ++-- .../delegation/delegation_controller_test.py | 4 ++-- .../delegation/validator_service_test.py | 8 +++---- tests/manager/manager_test.py | 4 ++-- tests/manager/sync_manager_test.py | 14 +++++------ 7 files changed, 35 insertions(+), 34 deletions(-) diff --git a/skale/transactions/exceptions.py b/skale/transactions/exceptions.py index 073f40a3..19c8cbd7 100644 --- a/skale/transactions/exceptions.py +++ b/skale/transactions/exceptions.py @@ -44,14 +44,14 @@ class TransactionWaitError(TimeoutError, TransactionError): pass -class TransactionLogicError(TransactionError): +class TransactionLogicError(TransactionError, ContractLogicError): """ Raised when transaction executed with error """ pass -class DryRunFailedError(TransactionError): +class DryRunFailedError(TransactionLogicError): """ Raised when error occurred during dry run call """ @@ -60,19 +60,19 @@ class DryRunFailedError(TransactionError): class TransactionFailedError(TransactionLogicError): """ - Raised when transaction included in a block failed during execution + Raised when transaction included in the block failed during execution """ pass -class RevertError(TransactionLogicError, ContractLogicError): +class TransactionRevertError(TransactionFailedError): """ - Raised when transaction was included in a block and reverted during execution + Raised when transaction included in the block failed with revert """ pass -class RevertDryRunError(DryRunFailedError): +class DryRunRevertError(DryRunFailedError): """ Raised when transaction reverted during dry run call """ diff --git a/skale/transactions/result.py b/skale/transactions/result.py index 564b2754..330e9c4d 100644 --- a/skale/transactions/result.py +++ b/skale/transactions/result.py @@ -21,7 +21,8 @@ from skale.transactions.exceptions import ( DryRunFailedError, - RevertError, + DryRunRevertError, + TransactionRevertError, TransactionFailedError ) @@ -68,15 +69,17 @@ def tx_failed(self) -> bool: return not is_success_or_not_performed(self.receipt) def raise_for_status(self) -> None: - # Check if tx was sent + print(self.dry_run_result) if self.receipt is not None: - # Check if tx errored if not is_success(self.receipt): - raise TransactionFailedError(self.receipt['error']) - else: - # Check if revert + error_msg = self.receipt['error'] + if is_revert_error(self.receipt): + raise TransactionRevertError(error_msg) + else: + raise TransactionFailedError(error_msg) + elif self.dry_run_result is not None and not is_success(self.dry_run_result): + error_msg = self.dry_run_result['message'] if is_revert_error(self.dry_run_result): - raise RevertError(self.dry_run_result['message']) - # Check if dry run errored due to other reason - if not is_success(self.dry_run_result): - raise DryRunFailedError(self.dry_run_result['error']) + raise DryRunRevertError(error_msg) + else: + raise DryRunFailedError(error_msg) diff --git a/tests/manager/constants_test.py b/tests/manager/constants_test.py index e2de8047..4f804026 100644 --- a/tests/manager/constants_test.py +++ b/tests/manager/constants_test.py @@ -3,7 +3,7 @@ from tests.constants import NEW_REWARD_PERIOD, NEW_DELTA_PERIOD -from skale.transactions.result import RevertError +from skale.transactions.result import DryRunRevertError def test_get_set_periods(skale): @@ -42,7 +42,7 @@ def test_get_set_latency(skale): def test_get_set_launch_timestamp(skale): launch_ts = skale.constants_holder.get_launch_timestamp() assert isinstance(launch_ts, int) - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.constants_holder.set_launch_timestamp(launch_ts, wait_for=True) diff --git a/tests/manager/delegation/delegation_controller_test.py b/tests/manager/delegation/delegation_controller_test.py index a006db7b..e8ec4249 100644 --- a/tests/manager/delegation/delegation_controller_test.py +++ b/tests/manager/delegation/delegation_controller_test.py @@ -4,7 +4,7 @@ from skale.contracts.manager.delegation.delegation_controller import FIELDS from skale.transactions.exceptions import ContractLogicError -from skale.transactions.result import RevertError +from skale.transactions.result import DryRunRevertError from skale.utils.contracts_provision.main import _skip_evm_time from skale.utils.contracts_provision.utils import generate_random_name @@ -204,7 +204,7 @@ def test_request_undelegate(skale, validator): ) # Transaction failed if delegation period is in progress - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): tx_res = skale.delegation_controller.request_undelegation( delegation_id, wait_for=True, diff --git a/tests/manager/delegation/validator_service_test.py b/tests/manager/delegation/validator_service_test.py index f38a10d4..29e7fb9f 100644 --- a/tests/manager/delegation/validator_service_test.py +++ b/tests/manager/delegation/validator_service_test.py @@ -4,7 +4,7 @@ import pytest from skale.contracts.manager.delegation.validator_service import FIELDS -from skale.transactions.result import RevertError +from skale.transactions.result import DryRunRevertError from skale.utils.account_tools import send_eth from skale.wallets.web3_wallet import generate_wallet from skale.utils.contracts_provision.main import _skip_evm_time, enable_validator @@ -208,7 +208,7 @@ def test_is_accepting_new_requests(skale, validator): def test_register_existing_validator(skale): - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.validator_service.register_validator( name=D_VALIDATOR_NAME, description=D_VALIDATOR_DESC, @@ -319,7 +319,7 @@ def test_request_confirm_new_address(skale): validator = skale.validator_service.get(validator_id) assert validator['requested_address'] == '0x0000000000000000000000000000000000000000' - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.validator_service.request_for_new_address( new_validator_address=main_wallet.address, wait_for=True @@ -402,7 +402,7 @@ def test_revert_reason(skale): min_delegation_amount=D_VALIDATOR_MIN_DEL, wait_for=True ) - except RevertError as e: + except DryRunRevertError as e: assert no_validator_revert in e.message diff --git a/tests/manager/manager_test.py b/tests/manager/manager_test.py index a4a5e2d6..2c7349e9 100644 --- a/tests/manager/manager_test.py +++ b/tests/manager/manager_test.py @@ -9,7 +9,7 @@ from unittest.mock import Mock from skale.wallets.web3_wallet import generate_wallet -from skale.transactions.result import RevertError, TransactionFailedError +from skale.transactions.result import DryRunRevertError, TransactionFailedError from skale.utils.contracts_provision.main import ( generate_random_node_data, generate_random_schain_data @@ -163,7 +163,7 @@ def test_node_exit_with_no_schains(skale, nodes): def test_failed_node_exit(skale, block_in_seconds): # block_in_seconds fixuture to return transaction revert in a same way as geth does not_existed_node_id = 1 - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.manager.node_exit(not_existed_node_id, wait_for=True, gas_limit=TEST_GAS_LIMIT) diff --git a/tests/manager/sync_manager_test.py b/tests/manager/sync_manager_test.py index f1f13045..9bca9ca5 100644 --- a/tests/manager/sync_manager_test.py +++ b/tests/manager/sync_manager_test.py @@ -1,6 +1,4 @@ -from web3.exceptions import ContractLogicError - -from skale.transactions.exceptions import RevertError +from skale.transactions.exceptions import DryRunRevertError import pytest @@ -34,10 +32,10 @@ def test_add_get_remove_ip_range(skale, sync_manager_permissions, ip_range, bloc def test_add_bad_ip_range(skale, sync_manager_permissions, block_in_seconds): - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.sync_manager.add_ip_range('brange', '0.0.0.0', '1.1.1.1') - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.sync_manager.add_ip_range('brange', '2.2.2.2', '1.1.1.1') with pytest.raises(OSError): @@ -45,16 +43,16 @@ def test_add_bad_ip_range(skale, sync_manager_permissions, block_in_seconds): def test_remove_range_bad_params(skale, sync_manager_permissions, block_in_seconds): - with pytest.raises(RevertError): + with pytest.raises(DryRunRevertError): skale.sync_manager.remove_ip_range('phantom') def test_get_range_bad_params(skale, sync_manager_permissions, block_in_seconds): num = skale.sync_manager.get_ip_ranges_number() # TODO: Make dry run handle revert that has empty reason properly - with pytest.raises(ContractLogicError): + with pytest.raises(DryRunRevertError): r = skale.sync_manager.get_ip_range_by_index(num) - with pytest.raises(ContractLogicError): + with pytest.raises(DryRunRevertError): r = skale.sync_manager.get_ip_range_by_index(0) r = skale.sync_manager.get_ip_range_by_name('phantom') assert r.start_ip == '0.0.0.0' and r.end_ip == '0.0.0.0' From 00b1a52c5dc7017b21fc88e5f5c959418f2c852f Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 3 Nov 2023 12:42:43 +0000 Subject: [PATCH 10/11] Fix retry and sync_manager tests --- skale/transactions/result.py | 1 - tests/manager/sync_manager_test.py | 6 ++++-- tests/transaction_tools_test.py | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/skale/transactions/result.py b/skale/transactions/result.py index 330e9c4d..4bb65948 100644 --- a/skale/transactions/result.py +++ b/skale/transactions/result.py @@ -69,7 +69,6 @@ def tx_failed(self) -> bool: return not is_success_or_not_performed(self.receipt) def raise_for_status(self) -> None: - print(self.dry_run_result) if self.receipt is not None: if not is_success(self.receipt): error_msg = self.receipt['error'] diff --git a/tests/manager/sync_manager_test.py b/tests/manager/sync_manager_test.py index 9bca9ca5..f386717b 100644 --- a/tests/manager/sync_manager_test.py +++ b/tests/manager/sync_manager_test.py @@ -1,3 +1,5 @@ +from web3.exceptions import ContractLogicError + from skale.transactions.exceptions import DryRunRevertError import pytest @@ -50,9 +52,9 @@ def test_remove_range_bad_params(skale, sync_manager_permissions, block_in_secon def test_get_range_bad_params(skale, sync_manager_permissions, block_in_seconds): num = skale.sync_manager.get_ip_ranges_number() # TODO: Make dry run handle revert that has empty reason properly - with pytest.raises(DryRunRevertError): + with pytest.raises(ContractLogicError): r = skale.sync_manager.get_ip_range_by_index(num) - with pytest.raises(DryRunRevertError): + with pytest.raises(ContractLogicError): r = skale.sync_manager.get_ip_range_by_index(0) r = skale.sync_manager.get_ip_range_by_name('phantom') assert r.start_ip == '0.0.0.0' and r.end_ip == '0.0.0.0' diff --git a/tests/transaction_tools_test.py b/tests/transaction_tools_test.py index 71f4f2cc..551fdf03 100644 --- a/tests/transaction_tools_test.py +++ b/tests/transaction_tools_test.py @@ -60,7 +60,8 @@ def test_run_tx_with_retry_dry_run_failed(skale): dry_run_call_mock = mock.Mock( return_value={ 'status': 0, - 'error': 'Dry run failed' + 'message': 'Dry run test failure', + 'error': 'revert' } ) account = generate_account(skale.web3) From b0d4f9bebf11c84f60a3e1d86b0d9e989d5af8be Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 8 Nov 2023 17:08:17 +0000 Subject: [PATCH 11/11] Improve retry_tx logs --- skale/transactions/tools.py | 4 ++-- skale/wallets/redis_wallet.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/skale/transactions/tools.py b/skale/transactions/tools.py index c0abbfcb..5b2d987e 100644 --- a/skale/transactions/tools.py +++ b/skale/transactions/tools.py @@ -186,7 +186,7 @@ def run_tx_with_retry(transaction, *args, max_retries=3, tx_res.raise_for_status() except TransactionError as e: error = e - logger.exception('Tx attempt %d/%d failed', attempt, max_retries) + logger.exception('Tx attempt %d/%d failed', attempt + 1, max_retries) timeout = exp_timeout if retry_timeout < 0 else exp_timeout time.sleep(timeout) @@ -198,7 +198,7 @@ def run_tx_with_retry(transaction, *args, max_retries=3, if error is None: logger.info( 'Tx %s completed after %d/%d retries', - transaction.__name__, attempt, max_retries + transaction.__name__, attempt + 1, max_retries ) else: logger.error( diff --git a/skale/wallets/redis_wallet.py b/skale/wallets/redis_wallet.py index 8df872e7..305cc767 100644 --- a/skale/wallets/redis_wallet.py +++ b/skale/wallets/redis_wallet.py @@ -59,7 +59,7 @@ class RedisWalletWaitError(RedisWalletError, TransactionWaitError): pass -class RedisWalletAdapter: +class RedisWalletAdapter(BaseWallet): ID_SIZE = 16 def __init__(