Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IS-388 Improve transaction method #544

Merged
merged 8 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
35 changes: 12 additions & 23 deletions skale/contracts/base_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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()
Expand Down
57 changes: 21 additions & 36 deletions skale/transactions/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 69 in skale/transactions/result.py

View check run for this annotation

Codecov / codecov/patch

skale/transactions/result.py#L69

Added line #L69 was not covered by tests
19 changes: 12 additions & 7 deletions skale/transactions/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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
Expand All @@ -49,6 +49,7 @@
f'wallet: {skale.wallet.__class__.__name__}, '
f'value: {value}, '
)
estimated_gas = 0

try:
if gas_limit:
Expand All @@ -59,12 +60,14 @@
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={})

Check warning on line 67 in skale/transactions/tools.py

View check run for this annotation

Codecov / codecov/patch

skale/transactions/tools.py#L65-L67

Added lines #L65 - L67 were not covered by tests

return {'status': 1, 'payload': estimated_gas}
return TxCallResult(status=TxStatus.SUCCESS, error='',
message='success', data={'gas': estimated_gas})


def estimate_gas(web3, method, opts):
Expand Down Expand Up @@ -207,4 +210,6 @@
)
if raise_for_status:
raise error
if tx_res is not None:
tx_res.attempts = attempt
return tx_res
43 changes: 29 additions & 14 deletions skale/wallets/redis_wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import logging
import os
import time
from enum import Enum
from typing import Dict, Optional, Tuple

from redis import Redis
Expand Down Expand Up @@ -59,6 +60,15 @@
pass


class TxRecordStatus(str, Enum):
DROPPED = 'DROPPED'
SUCCESS = 'SUCCESS'
FAILED = 'FAILED'

def __str__(self) -> str:
return str.__str__(self)

Check warning on line 69 in skale/wallets/redis_wallet.py

View check run for this annotation

Codecov / codecov/patch

skale/wallets/redis_wallet.py#L69

Added line #L69 was not covered by tests


class RedisWalletAdapter(BaseWallet):
ID_SIZE = 16

Expand Down Expand Up @@ -172,23 +182,28 @@
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}')
15 changes: 8 additions & 7 deletions tests/manager/base_contract_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand Down
Loading
Loading