Merge pull request #544 from skalenetwork/improve-transaction-method

IS-388 Improve transaction method

setup.py

@@ -33,7 +33,7 @@
 
 setup(
     name='skale.py',
-    version='6.0',
+    version='6.1',
     description='SKALE client tools',
     long_description_markdown_filename='README.md',
     author='SKALE Labs',

skale/contracts/base_contract.py

@@ -25,12 +25,8 @@
 from web3 import Web3
 
 import skale.config as config
-from skale.transactions.result import (
-    TxRes,
-    is_success,
-    is_success_or_not_performed
-)
-from skale.transactions.tools import make_dry_run_call, transaction_from_method
+from skale.transactions.result import TxRes
+from skale.transactions.tools import make_dry_run_call, transaction_from_method, TxStatus
 from skale.utils.web3_utils import (
     DEFAULT_BLOCKS_TO_WAIT,
     get_eth_nonce,
@@ -44,14 +40,6 @@
 logger = logging.getLogger(__name__)
 
 
-def execute_dry_run(skale, method, custom_gas_limit, value=0) -> tuple:
-    dry_run_result = make_dry_run_call(skale, method, custom_gas_limit, value)
-    estimated_gas_limit = None
-    if is_success(dry_run_result):
-        estimated_gas_limit = dry_run_result['payload']
-    return dry_run_result, estimated_gas_limit
-
-
 def transaction_method(transaction):
     @wraps(transaction)
     def wrapper(
@@ -76,22 +64,23 @@ def wrapper(
         **kwargs
     ):
         method = transaction(self, *args, **kwargs)
-        dry_run_result, tx_hash, receipt = None, None, None
 
         nonce = get_eth_nonce(self.skale.web3, self.skale.wallet.address)
 
-        estimated_gas_limit = None
+        call_result, tx_hash, receipt = None, None, None
         should_dry_run = not skip_dry_run and not config.DISABLE_DRY_RUN
 
         if should_dry_run:
-            dry_run_result, estimated_gas_limit = execute_dry_run(self.skale,
-                                                                  method, gas_limit, value)
+            call_result = make_dry_run_call(self.skale, method, gas_limit, value)
+            if call_result.status == TxStatus.SUCCESS:
+                gas_limit = gas_limit or call_result.data['gas']
 
-        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
+        should_send = not dry_run_only and \
+            (not should_dry_run or call_result.status == TxStatus.SUCCESS)
 
         if should_send:
+            gas_limit = gas_limit or config.DEFAULT_GAS_LIMIT
+            gas_price = gas_price or config.DEFAULT_GAS_PRICE_WEI or self.skale.gas_price
             tx = transaction_from_method(
                 method=method,
                 gas_limit=gas_limit,
@@ -114,11 +103,11 @@ def wrapper(
         if should_wait:
             receipt = self.skale.wallet.wait(tx_hash)
 
-        should_confirm = receipt and confirmation_blocks > 0
+        should_confirm = receipt is not None 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)
+        tx_res = TxRes(call_result, tx_hash, receipt)
 
         if raise_for_status:
             tx_res.raise_for_status()

skale/transactions/result.py

@@ -17,68 +17,53 @@
 #   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
+import enum
+from typing import NamedTuple
 
 from skale.transactions.exceptions import (
     DryRunFailedError,
     DryRunRevertError,
-    TransactionRevertError,
     TransactionFailedError
 )
 
-SUCCESS_STATUS = 1
 
+class TxStatus(int, enum.Enum):
+    FAILED = 0
+    SUCCESS = 1
 
-def is_success(result: dict) -> bool:
-    return result.get('status') == SUCCESS_STATUS
 
-
-def is_success_or_not_performed(result: dict) -> bool:
-    return result is None or is_success(result)
-
-
-def is_revert_error(result: Optional[dict]) -> bool:
-    if not result:
-        return False
-    error = result.get('error', None)
-    return error == 'revert'
+class TxCallResult(NamedTuple):
+    status: TxStatus
+    error: str
+    message: str
+    data: dict
 
 
 class TxRes:
-    def __init__(self, dry_run_result=None, tx_hash=None, receipt=None):
-        self.dry_run_result = dry_run_result
+    def __init__(self, tx_call_result=None, tx_hash=None, receipt=None, revert=None):
+        self.tx_call_result = tx_call_result
         self.tx_hash = tx_hash
         self.receipt = receipt
+        self.attempts = 0
 
     def __str__(self) -> str:
         return (
-            f'TxRes hash: {self.tx_hash}, dry_run_result {self.dry_run_result}, '
+            f'TxRes hash: {self.tx_hash}, tx_call_result {self.tx_call_result}, '
             f'receipt {self.receipt}'
         )
 
     def __repr__(self) -> str:
         return (
-            f'TxRes hash: {self.tx_hash}, dry_run_result {self.dry_run_result}, '
+            f'TxRes hash: {self.tx_hash}, tx_call_result {self.tx_call_result}, '
             f'receipt {self.receipt}'
         )
 
-    def dry_run_failed(self) -> bool:
-        return not is_success_or_not_performed(self.dry_run_result)
-
-    def tx_failed(self) -> bool:
-        return not is_success_or_not_performed(self.receipt)
-
     def raise_for_status(self) -> None:
         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 DryRunRevertError(error_msg)
+            if self.receipt['status'] == TxStatus.FAILED:
+                raise TransactionFailedError(self.receipt)
+        elif self.tx_call_result is not None and self.tx_call_result.status == TxStatus.FAILED:
+            if self.tx_call_result.error == 'revert':
+                raise DryRunRevertError(self.tx_call_result.message)
             else:
-                raise DryRunFailedError(error_msg)
+                raise DryRunFailedError(self.tx_call_result.message)

skale/transactions/tools.py

@@ -28,7 +28,7 @@
 
 import skale.config as config
 from skale.transactions.exceptions import TransactionError
-from skale.transactions.result import TxRes
+from skale.transactions.result import TxCallResult, TxRes, TxStatus
 from skale.utils.web3_utils import get_eth_nonce
 
 
@@ -38,7 +38,7 @@
 DEFAULT_ETH_SEND_GAS_LIMIT = 22000
 
 
-def make_dry_run_call(skale, method, gas_limit=None, value=0) -> dict:
+def make_dry_run_call(skale, method, gas_limit=None, value=0) -> TxCallResult:
     opts = {
         'from': skale.wallet.address,
         'value': value
@@ -49,6 +49,7 @@ def make_dry_run_call(skale, method, gas_limit=None, value=0) -> dict:
         f'wallet: {skale.wallet.__class__.__name__}, '
         f'value: {value}, '
     )
+    estimated_gas = 0
 
     try:
         if gas_limit:
@@ -59,12 +60,14 @@ def make_dry_run_call(skale, method, gas_limit=None, value=0) -> dict:
             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 %s failed', method, exc_info=err)
-        return {'status': 0, 'error': str(err)}
+        return TxCallResult(status=TxStatus.FAILED,
+                            error='revert', message=e.message, data=e.data)
+    except (Web3Exception, ValueError) as e:
+        logger.exception('Dry run for %s failed', method)
+        return TxCallResult(status=TxStatus.FAILED, error='exception', message=str(e), data={})
 
-    return {'status': 1, 'payload': estimated_gas}
+    return TxCallResult(status=TxStatus.SUCCESS, error='',
+                        message='success', data={'gas': estimated_gas})
 
 
 def estimate_gas(web3, method, opts):
@@ -207,4 +210,6 @@ def run_tx_with_retry(transaction, *args, max_retries=3,
         )
         if raise_for_status:
             raise error
+    if tx_res is not None:
+        tx_res.attempts = attempt
     return tx_res

skale/wallets/redis_wallet.py

@@ -22,6 +22,7 @@
 import logging
 import os
 import time
+from enum import Enum
 from typing import Dict, Optional, Tuple
 
 from redis import Redis
@@ -59,6 +60,15 @@ class RedisWalletWaitError(RedisWalletError, TransactionWaitError):
     pass
 
 
+class TxRecordStatus(str, Enum):
+    DROPPED = 'DROPPED'
+    SUCCESS = 'SUCCESS'
+    FAILED = 'FAILED'
+
+    def __str__(self) -> str:
+        return str.__str__(self)
+
+
 class RedisWalletAdapter(BaseWallet):
     ID_SIZE = 16
 
@@ -172,23 +182,28 @@ def wait(
         timeout: int = MAX_WAITING_TIME
     ) -> Dict:
         start_ts = time.time()
-        status = None
-
-        while time.time() - start_ts < timeout:
+        status, result = None, None
+        while status not in [
+            TxRecordStatus.DROPPED,
+            TxRecordStatus.SUCCESS,
+            TxRecordStatus.FAILED
+        ] and time.time() - start_ts < timeout:
             try:
-                status = self.get_status(tx_id)
-                if status == 'DROPPED':
-                    break
-                if status in ('SUCCESS', 'FAILED'):
-                    r = self.get_record(tx_id)
-                    return get_receipt(self.wallet._web3, r['tx_hash'])
-            except Exception as err:
-                logger.exception(f'Waiting for tx {tx_id} errored')
-                raise RedisWalletWaitError(err)
+                record = self.get_record(tx_id)
+                if record is not None:
+                    status = record.get('status')
+                    if status in (TxRecordStatus.SUCCESS, TxRecordStatus.FAILED):
+                        result = get_receipt(self.wallet._web3, record['tx_hash'])
+            except Exception as e:
+                logger.exception('Waiting for tx %s errored', tx_id)
+                raise RedisWalletWaitError(e)
+
+        if result:
+            return result
 
         if status is None:
-            raise RedisWalletEmptyStatusError('Tx status is None')
-        if status == 'DROPPED':
+            raise RedisWalletEmptyStatusError(f'Tx status is {status}')
+        elif status == TxRecordStatus.DROPPED:
             raise RedisWalletDroppedError('Tx was dropped after max retries')
         else:
             raise RedisWalletWaitError(f'Tx finished with status {status}')

tests/manager/base_contract_test.py

@@ -5,6 +5,7 @@
 import pytest
 import skale.config as config
 from skale.transactions.exceptions import TransactionNotSentError
+from skale.transactions.result import TxStatus
 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
@@ -26,8 +27,8 @@ def test_dry_run(skale):
     balance_to_before = skale.token.get_balance(address_to)
     amount = 10 * ETH_IN_WEI
     tx_res = skale.token.transfer(address_to, amount, dry_run_only=True)
-    assert isinstance(tx_res.dry_run_result['payload'], int)
-    assert tx_res.dry_run_result['status'] == 1
+    assert isinstance(tx_res.tx_call_result.data['gas'], int)
+    assert tx_res.tx_call_result.status == TxStatus.SUCCESS
     tx_res.raise_for_status()
 
     balance_from_after = skale.token.get_balance(address_from)
@@ -54,7 +55,7 @@ def test_disable_dry_run_env(skale, disable_dry_run_env):
     address_to = account['address']
     amount = 10 * ETH_IN_WEI
     with mock.patch(
-        'skale.contracts.base_contract.execute_dry_run'
+        'skale.contracts.base_contract.make_dry_run_call'
     ) as dry_run_mock:
         skale.token.transfer(address_to, amount)
         dry_run_mock.assert_not_called()
@@ -76,7 +77,7 @@ def test_skip_dry_run(skale):
     )
     assert tx_res.tx_hash is not None, tx_res
     assert tx_res.receipt is not None
-    assert tx_res.dry_run_result is None
+    assert tx_res.tx_call_result is None
     balance_from_after = skale.token.get_balance(address_from)
     assert balance_from_after == balance_from_before - amount
     balance_to_after = skale.token.get_balance(address_to)
@@ -96,8 +97,8 @@ def test_wait_for_false(skale):
     tx_res = skale.token.transfer(address_to, amount, wait_for=False)
     assert tx_res.tx_hash is not None
     assert tx_res.receipt is None
-    assert isinstance(tx_res.dry_run_result['payload'], int)
-    assert tx_res.dry_run_result['status'] == 1
+    assert isinstance(tx_res.tx_call_result.data['gas'], int)
+    assert tx_res.tx_call_result.status == TxStatus.SUCCESS
 
     tx_res.receipt = wait_for_receipt_by_blocks(skale.web3, tx_res.tx_hash)
     tx_res.raise_for_status()
@@ -113,7 +114,7 @@ def test_tx_res_dry_run(skale):
     token_amount = 10
     tx_res = skale.token.transfer(
         account['address'], token_amount, dry_run_only=True)
-    assert tx_res.dry_run_result is not None
+    assert tx_res.tx_call_result is not None
     assert tx_res.tx_hash is None
     assert tx_res.receipt is None
     tx_res.raise_for_status()