Merge pull request #543 from skalenetwork/fix-retry-tx

IS-388 Fix retry_tx decorator

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)
 

skale/transactions/exceptions.py

@@ -2,28 +2,78 @@
 
 
 class TransactionError(Exception):
+    """
+    Base exception for transaction related errors
+    """
     pass
 
 
-class DryRunFailedError(TransactionError):
+class ChainIdError(TransactionError):
+    """
+    Raised when chainId is missing or incorrect
+    """
     pass
 
 
-class InsufficientBalanceError(TransactionError):
+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 TransactionFailedError(TransactionError):
+class TransactionWaitError(TimeoutError, TransactionError):
+    """
+    Raised when error occurred during waiting for transaction
+    to be included in block
+    """
     pass
 
 
-class RevertError(TransactionError, ContractLogicError):
+class TransactionLogicError(TransactionError, ContractLogicError):
+    """
+    Raised when transaction executed with error
+    """
+    pass
+
+
+class DryRunFailedError(TransactionLogicError):
+    """
+    Raised when error occurred during dry run call
+    """
+    pass
+
+
+class TransactionFailedError(TransactionLogicError):
+    """
+    Raised when transaction included in the block failed during execution
+    """
+    pass
+
+
+class TransactionRevertError(TransactionFailedError):
+    """
+    Raised when transaction included in the block failed with revert
+    """
+    pass
+
+
+class DryRunRevertError(DryRunFailedError):
+    """
+    Raised when transaction reverted during dry run call
+    """
     pass

skale/transactions/result.py

@@ -17,9 +17,12 @@
 #   You should have received a copy of the GNU Affero General Public License
 #   along with SKALE.py.  If not, see <https://www.gnu.org/licenses/>.
 
+from typing import Optional
+
 from skale.transactions.exceptions import (
     DryRunFailedError,
-    RevertError,
+    DryRunRevertError,
+    TransactionRevertError,
     TransactionFailedError
 )
 
@@ -34,8 +37,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 not result:
+        return False
+    error = result.get('error', None)
+    return error == 'revert'
 
 
 class TxRes:
@@ -63,11 +69,16 @@ 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():
+        if self.receipt is not None:
+            if not is_success(self.receipt):
+                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['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 DryRunRevertError(error_msg)
+            else:
+                raise DryRunFailedError(error_msg)

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}
@@ -169,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 (RedisAdapterError, 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 + 1, 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 + 1, 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

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

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

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 RedisWalletAdapter
-from skale.wallets.rpc_wallet import RPCWallet
 from skale.wallets.sgx_wallet import SgxWallet
 from skale.wallets.web3_wallet import Web3Wallet

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):

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,
@@ -157,14 +159,16 @@ 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)
-        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 +179,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 (ValueError, Web3Exception) as e:
+            raise TransactionNotSentError(e)
 
     def sign_hash(self, unsigned_hash: str):
         raise NotImplementedError(