Merge pull request #2425 from cygnusv/butanooo

More work tracker improvements. Adds --gas-price option to nucypher stake
pull/2432/head
David Núñez 2020-11-09 01:00:21 +01:00 committed by GitHub
commit 7b7dea1962
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 159 additions and 76 deletions

View File

@ -0,0 +1 @@
Fixes logical bug in ``WorkTracker`` to ensure commitment transactions can only be issued once per period.

View File

@ -0,0 +1 @@
Now ``BlockchainInterface.gas_strategy`` always has a value; previously it was possible to pass ``None`` via the constructor (e.g. if the config file had an explicit ``"null"`` value).

View File

@ -0,0 +1 @@
Added support for a user-provided gas price to the ``nucypher stake`` command, using ``--gas-price GWEI``.

View File

@ -22,7 +22,7 @@ import sys
import time
import traceback
from decimal import Decimal
from typing import Callable
from typing import Callable, Union
from typing import Dict, Iterable, List, Optional, Tuple
import click
@ -31,6 +31,7 @@ from constant_sorrow.constants import FULL, WORKER_NOT_RUNNING
from eth_tester.exceptions import TransactionFailed as TestTransactionFailed
from eth_typing import ChecksumAddress
from eth_utils import to_canonical_address, to_checksum_address
from hexbytes import HexBytes
from web3 import Web3
from web3.exceptions import ValidationError
from web3.types import TxReceipt
@ -1632,7 +1633,7 @@ class Worker(NucypherTokenActor):
@only_me
@save_receipt # saves txhash instead of receipt if `fire_and_forget` is True
def commit_to_next_period(self, fire_and_forget: bool = True) -> TxReceipt:
def commit_to_next_period(self, fire_and_forget: bool = True) -> Union[TxReceipt, HexBytes]:
"""For each period that the worker makes a commitment, the staker is rewarded"""
txhash_or_receipt = self.staking_agent.commit_to_next_period(worker_address=self.__worker_address,
fire_and_forget=fire_and_forget)

View File

@ -583,11 +583,11 @@ class StakingEscrowAgent(EthereumContractAgent):
For each period that the worker makes a commitment, the staker is rewarded.
"""
contract_function: ContractFunction = self.contract.functions.commitToNextPeriod()
receipt: TxReceipt = self.blockchain.send_transaction(contract_function=contract_function,
sender_address=worker_address,
gas_estimation_multiplier=1.5, # TODO: Workaround for #2337
fire_and_forget=fire_and_forget)
return receipt
txhash_or_receipt = self.blockchain.send_transaction(contract_function=contract_function,
sender_address=worker_address,
gas_estimation_multiplier=1.5, # TODO: Workaround for #2337
fire_and_forget=fire_and_forget)
return txhash_or_receipt
@contract_api(TRANSACTION)
def mint(self, staker_address: ChecksumAddress) -> TxReceipt:

View File

@ -39,7 +39,6 @@ from hexbytes.main import HexBytes
from web3 import Web3, middleware
from web3.contract import Contract, ContractConstructor, ContractFunction
from web3.exceptions import TimeExhausted, ValidationError
from web3.gas_strategies import time_based
from web3.middleware import geth_poa_middleware
from web3.providers import BaseProvider
from web3.types import TxReceipt
@ -157,7 +156,7 @@ class BlockchainInterface:
provider_process=NO_PROVIDER_PROCESS,
provider_uri: str = NO_BLOCKCHAIN_CONNECTION,
provider: BaseProvider = NO_BLOCKCHAIN_CONNECTION,
gas_strategy: Union[str, Callable] = DEFAULT_GAS_STRATEGY):
gas_strategy: Optional[Union[str, Callable]] = None):
"""
TODO: #1502 - Move to API docs.
@ -231,7 +230,7 @@ class BlockchainInterface:
self.client = NO_BLOCKCHAIN_CONNECTION # type: EthereumClient
self.transacting_power = READ_ONLY_INTERFACE
self.is_light = light
self.gas_strategy = gas_strategy
self.gas_strategy = gas_strategy or self.DEFAULT_GAS_STRATEGY
def __repr__(self):
r = '{name}({uri})'.format(name=self.__class__.__name__, uri=self.provider_uri)
@ -280,21 +279,26 @@ class BlockchainInterface:
self.log.debug('Injecting POA middleware at layer 0')
self.client.inject_middleware(geth_poa_middleware, layer=0)
# Gas Price Strategy:
# Bundled web3 strategies are too expensive for Infura (it takes ~1 minute to get a price),
# so we use external gas price oracles, instead (see #2139)
if isinstance(self.client, InfuraClient):
self.client.add_middleware(middleware.time_based_cache_middleware)
# self.client.add_middleware(middleware.latest_block_based_cache_middleware) # TODO: This line causes failed tests and nonce reuse in tests. See #2348.
self.client.add_middleware(middleware.simple_cache_middleware)
self.set_gas_strategy()
def set_gas_strategy(self, gas_strategy: Optional[Callable] = None):
if gas_strategy:
reported_gas_strategy = f"fixed/{gas_strategy.name}"
elif isinstance(self.client, InfuraClient):
gas_strategy = datafeed_fallback_gas_price_strategy
self.gas_strategy = 'fast' # FIXME
self.gas_strategy = 'fast'
reported_gas_strategy = "datafeed/fast"
else:
reported_gas_strategy = f"web3/{self.gas_strategy}"
gas_strategy = self.get_gas_strategy(self.gas_strategy)
self.client.set_gas_strategy(gas_strategy=gas_strategy)
gwei_gas_price = Web3.fromWei(self.client.gas_price_for_transaction(), 'gwei')
self.log.debug(f"Currently, our gas strategy returns a gas price of {gwei_gas_price} gwei")
self.client.add_middleware(middleware.time_based_cache_middleware)
# self.client.add_middleware(middleware.latest_block_based_cache_middleware)
self.client.add_middleware(middleware.simple_cache_middleware)
self.log.debug(f"Using gas strategy '{reported_gas_strategy}'. "
f"Currently, it returns a gas price of {gwei_gas_price} gwei")
def connect(self):
@ -655,7 +659,7 @@ class BlockchainInterface:
gas_estimation_multiplier: Optional[float] = None,
confirmations: int = 0,
fire_and_forget: bool = False, # do not wait for receipt. See #2385
) -> dict:
) -> Union[TxReceipt, HexBytes]:
if fire_and_forget:
if confirmations > 0:

View File

@ -14,14 +14,14 @@ GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with nucypher. If not, see <https://www.gnu.org/licenses/>.
"""
import random
from _pydecimal import Decimal
from collections import UserList
from enum import Enum
from typing import Callable, Dict, Union, List
from typing import Callable, Dict, Union
import maya
import random
from constant_sorrow.constants import (
EMPTY_STAKING_SLOT,
NEW_STAKE,
@ -632,7 +632,7 @@ class WorkTracker:
def pending(self) -> Dict[int, HexBytes]:
return self.__pending.copy()
def __tracking_consistency_check(self) -> bool:
def __commitments_tracker_is_consistent(self) -> bool:
worker_address = self.worker.worker_address
tx_count_pending = self.client.get_transaction_count(account=worker_address, pending=True)
tx_count_latest = self.client.get_transaction_count(account=worker_address, pending=False)
@ -641,11 +641,8 @@ class WorkTracker:
return True # OK!
if txs_in_mempool > len(self.__pending): # We're missing some pending TXs
return False
# TODO: Not sure what to do in this case, but let's do this for the moment
# Note that the block my have changed since the previous query
# elif txs_in_mempool < len(self.__pending): # Our tracking is somehow outdated
# return False
else: # TODO #2429: What to do when txs_in_mempool < len(self.__pending)? What does this imply?
return True
def __track_pending_commitments(self) -> bool:
# TODO: Keep a purpose-built persistent log of worker transaction history
@ -665,11 +662,11 @@ class WorkTracker:
del self.__pending[tx_firing_block_number]
if unmined_transactions:
pluralize = "s" if len(unmined_transactions) > 1 else ""
self.log.info(f'{len(unmined_transactions)} pending commitment transaction{pluralize} detected.')
s = "s" if len(unmined_transactions) > 1 else ""
self.log.info(f'{len(unmined_transactions)} pending commitment transaction{s} detected.')
inconsistency = self.__tracking_consistency_check() is False
if inconsistency:
inconsistent_tracker = not self.__commitments_tracker_is_consistent()
if inconsistent_tracker:
# If we detect there's a mismatch between the number of internally tracked and
# pending block transactions, create a special pending TX that accounts for this.
# TODO: Detect if this untracked pending transaction is a commitment transaction at all.
@ -685,23 +682,25 @@ class WorkTracker:
def __handle_replacement_commitment(self, current_block_number: int) -> None:
tx_firing_block_number, txhash = list(sorted(self.pending.items()))[0]
self.log.info(f'Waiting for pending commitment transaction to be mined ({txhash}).')
# If the transaction is still not mined after a max confirmation time
# (based on current gas strategy) issue a replacement transaction.
wait_time_in_blocks = current_block_number - tx_firing_block_number
wait_time_in_seconds = wait_time_in_blocks * AVERAGE_BLOCK_TIME_IN_SECONDS
if wait_time_in_seconds > self.max_confirmation_time():
if txhash is UNTRACKED_PENDING_TRANSACTION:
# TODO: Detect if this untracked pending transaction is a commitment transaction at all.
message = f"We've an untracked pending transaction. Issuing a replacement transaction."
if txhash is UNTRACKED_PENDING_TRANSACTION:
# TODO: Detect if this untracked pending transaction is a commitment transaction at all.
message = f"We have an untracked pending transaction. Issuing a replacement transaction."
else:
# If the transaction is still not mined after a max confirmation time
# (based on current gas strategy) issue a replacement transaction.
wait_time_in_blocks = current_block_number - tx_firing_block_number
wait_time_in_seconds = wait_time_in_blocks * AVERAGE_BLOCK_TIME_IN_SECONDS
if wait_time_in_seconds < self.max_confirmation_time():
self.log.info(f'Waiting for pending commitment transaction to be mined ({txhash.hex()}).')
return
else:
message = f"We've waited for {wait_time_in_seconds}, but max time is {self.max_confirmation_time()}" \
f" for {self.gas_strategy} gas strategy. Issuing a replacement transaction."
self.log.info(message)
self.__fire_replacement_commitment(current_block_number=current_block_number,
tx_firing_block_number=tx_firing_block_number)
# Send a replacement transaction
self.log.info(message)
self.__fire_replacement_commitment(current_block_number=current_block_number,
tx_firing_block_number=tx_firing_block_number)
def _do_work(self) -> None:
"""
@ -711,17 +710,6 @@ class WorkTracker:
# Call once here, and inject later for temporal consistency
current_block_number = self.client.block_number
# Commitment tracking
unmined_transactions = self.__track_pending_commitments()
if unmined_transactions:
self.__handle_replacement_commitment(current_block_number=current_block_number)
# while there are known pending transactions, remain in fast interval mode
self._tracking_task.interval = self.INTERVAL_FLOOR
return # This cycle is finished.
else:
# Randomize the next task interval over time, within bounds.
self._tracking_task.interval = self.random_interval()
# Update on-chain status
self.log.info(f"Checking for new period. Current period is {self.__current_period}")
onchain_period = self.staking_agent.get_current_period() # < -- Read from contract
@ -741,6 +729,17 @@ class WorkTracker:
# TODO: #1516 Follow-up actions for missed commitments
self.log.warn(f"MISSED COMMITMENTS - {interval} missed staking commitments detected.")
# Commitment tracking
unmined_transactions = self.__track_pending_commitments()
if unmined_transactions:
self.__handle_replacement_commitment(current_block_number=current_block_number)
# while there are known pending transactions, remain in fast interval mode
self._tracking_task.interval = self.INTERVAL_FLOOR
return # This cycle is finished.
else:
# Randomize the next task interval over time, within bounds.
self._tracking_task.interval = self.random_interval()
# Only perform work this round if the requirements are met
if not self.__work_requirement_is_satisfied():
self.log.warn(f'COMMIT PREVENTED (callable: "{self.__requirement.__name__}") - '
@ -756,7 +755,7 @@ class WorkTracker:
transacting_power = self.worker.transacting_power
with transacting_power:
txhash = self.worker.commit_to_next_period(fire_and_forget=True) # < --- blockchain WRITE
self.log.info(f"Making a commitment to period {self.current_period} - TxHash: {txhash}")
self.log.info(f"Making a commitment to period {self.current_period} - TxHash: {txhash.hex()}")
return txhash

View File

@ -87,8 +87,8 @@ from nucypher.cli.options import (
option_provider_uri,
option_registry_filepath,
option_signer_uri,
option_staking_address
)
option_staking_address,
option_gas_price)
from nucypher.cli.painting.staking import (
paint_min_rate, paint_staged_stake,
paint_staged_stake_division,
@ -101,9 +101,11 @@ from nucypher.cli.painting.transactions import paint_receipt_summary
from nucypher.cli.types import (
EIP55_CHECKSUM_ADDRESS,
EXISTING_READABLE_FILE,
GWEI,
DecimalRange)
from nucypher.cli.utils import setup_emitter
from nucypher.config.characters import StakeHolderConfiguration
from nucypher.utilities.gas_strategies import construct_fixed_price_gas_strategy
option_value = click.option('--value', help="Token value of stake", type=click.INT)
option_lock_periods = click.option('--lock-periods', help="Duration of stake in periods.", type=click.INT)
@ -221,11 +223,12 @@ class TransactingStakerOptions:
__option_name__ = 'transacting_staker_options'
def __init__(self, staker_options: StakerOptions, hw_wallet, beneficiary_address, allocation_filepath):
def __init__(self, staker_options: StakerOptions, hw_wallet, beneficiary_address, allocation_filepath, gas_price):
self.staker_options = staker_options
self.hw_wallet = hw_wallet
self.beneficiary_address = beneficiary_address
self.allocation_filepath = allocation_filepath
self.gas_price = gas_price
def create_character(self, emitter, config_file):
@ -269,7 +272,11 @@ class TransactingStakerOptions:
)
def get_blockchain(self):
return self.staker_options.get_blockchain()
blockchain = self.staker_options.get_blockchain()
if self.gas_price: # TODO: Consider performing this step in the init of EthereumClient
fixed_price_strategy = construct_fixed_price_gas_strategy(gas_price=self.gas_price, denomination="gwei")
blockchain.set_gas_strategy(fixed_price_strategy)
return blockchain
group_transacting_staker_options = group_options(
@ -278,6 +285,7 @@ group_transacting_staker_options = group_options(
hw_wallet=option_hw_wallet,
beneficiary_address=click.option('--beneficiary-address', help="Address of a pre-allocation beneficiary", type=EIP55_CHECKSUM_ADDRESS),
allocation_filepath=click.option('--allocation-filepath', help="Path to individual allocation file", type=EXISTING_READABLE_FILE),
gas_price=option_gas_price,
)
@ -1295,7 +1303,7 @@ def events(general_config, staker_options, config_file, event_name):
@option_config_file
@option_force
@group_general_config
@click.option('--min-rate', help="Minimum acceptable fee rate (in GWEI), set by staker", type=DecimalRange(min=0))
@click.option('--min-rate', help="Minimum acceptable fee rate (in GWEI), set by staker", type=GWEI)
def set_min_rate(general_config: GroupGeneralConfig,
transacting_staker_options: TransactingStakerOptions,
config_file, force, min_rate):

View File

@ -24,6 +24,7 @@ from nucypher.blockchain.eth.constants import NUCYPHER_CONTRACT_NAMES
from nucypher.cli.types import (
EIP55_CHECKSUM_ADDRESS,
EXISTING_READABLE_FILE,
GWEI,
NETWORK_PORT,
NuCypherNetworkName,
WEI
@ -42,6 +43,7 @@ option_etherscan = click.option('--etherscan/--no-etherscan', help="Enable/disab
option_event_name = click.option('--event-name', help="Specify an event by name", type=click.STRING)
option_federated_only = click.option('--federated-only/--decentralized', '-F', help="Connect only to federated nodes", is_flag=True, default=None)
option_force = click.option('--force', help="Don't ask for confirmation", is_flag=True)
option_gas_price = click.option('--gas-price', help="Use this gas price (in GWEI)", type=GWEI)
option_geth = click.option('--geth', '-G', help="Run using the built-in geth node", is_flag=True)
option_hw_wallet = click.option('--hw-wallet/--no-hw-wallet')
option_light = click.option('--light', help="Indicate that node is light", is_flag=True, default=None)

View File

@ -104,6 +104,7 @@ class NuCypherNetworkName(click.ParamType):
# Ethereum
EIP55_CHECKSUM_ADDRESS = ChecksumAddress()
WEI = click.IntRange(min=1, clamp=False) # TODO: Better validation for ether and wei values?
GWEI = DecimalRange(min=0)
# Filesystem
EXISTING_WRITABLE_DIRECTORY = click.Path(exists=True, dir_okay=True, file_okay=False, writable=True)

View File

@ -15,8 +15,7 @@
along with nucypher. If not, see <https://www.gnu.org/licenses/>.
"""
import datetime
import functools
from typing import Callable
from typing import Callable, Optional
from web3 import Web3
from web3.exceptions import ValidationError
@ -64,23 +63,44 @@ __RAW_WEB3_GAS_STRATEGIES = {
}
def wrap_web3_gas_strategy(web3_gas_strategy: Callable):
def wrap_web3_gas_strategy(speed: Optional[str] = None):
"""
Enriches the web3 exceptions thrown by gas strategies
"""
@functools.wraps(web3_gas_strategy)
web3_gas_strategy = __RAW_WEB3_GAS_STRATEGIES[speed]
def _wrapper(*args, **kwargs):
try:
return web3_gas_strategy(*args, **kwargs)
except ValidationError as e:
raise GasStrategyError("Calling the web3 gas strategy failed, probably due to an unsynced chain.") from e
raise GasStrategyError(f"Calling the '{speed}' web3 gas strategy failed. "
f"Verify your Ethereum provider connection and syncing status.") from e
_wrapper.name = speed
return _wrapper
WEB3_GAS_STRATEGIES = {speed: wrap_web3_gas_strategy(strategy) for speed, strategy in __RAW_WEB3_GAS_STRATEGIES.items()}
WEB3_GAS_STRATEGIES = {speed: wrap_web3_gas_strategy(speed) for speed in __RAW_WEB3_GAS_STRATEGIES}
EXPECTED_CONFIRMATION_TIME_IN_SECONDS = {
'slow': int(datetime.timedelta(hours=1).total_seconds()),
'medium': int(datetime.timedelta(minutes=5).total_seconds()),
'fast': 60
}
#
# Fixed-price gas strategy
#
def construct_fixed_price_gas_strategy(gas_price, denomination: str = "wei") -> Callable:
gas_price_in_wei = Web3.toWei(gas_price, denomination)
def _fixed_price_strategy(web3: Web3, transaction_params: TxParams = None) -> Wei:
return gas_price_in_wei
_fixed_price_strategy.name = f"{round(Web3.fromWei(gas_price_in_wei, 'gwei'))}gwei"
return _fixed_price_strategy

View File

@ -143,9 +143,10 @@ def test_ursula_and_local_keystore_signer_integration(click_runner,
ursula_config.attach_keyring(checksum_address=worker_account.address)
ursula_config.keyring.unlock(password=password)
# Produce an ursula with a Keystore signer correctly derived from the signer URI, and dont do anything else!
# Produce an Ursula with a Keystore signer correctly derived from the signer URI, and don't do anything else!
mocker.patch.object(StakeList, 'refresh', autospec=True)
ursula = ursula_config.produce(client_password=password,
start_working_now=False,
block_until_ready=False)
try:
@ -157,7 +158,7 @@ def test_ursula_and_local_keystore_signer_integration(click_runner,
# Show that we can produce the exact same signer as pre-config...
assert pre_config_signer.path == ursula.signer.path
# ...and that transactions are signed by the keytore signer
# ...and that transactions are signed by the keystore signer
txhash = ursula.commit_to_next_period()
receipt = testerchain.wait_for_receipt(txhash)
transaction_data = testerchain.client.w3.eth.getTransaction(receipt['transactionHash'])

View File

@ -142,7 +142,8 @@ def test_stakeholder_configuration(test_emitter, test_registry, mock_testerchain
transacting_staker_options = TransactingStakerOptions(staker_options=staker_options,
hw_wallet=None,
beneficiary_address=None,
allocation_filepath=None)
allocation_filepath=None,
gas_price=None)
stakeholder_from_configuration = transacting_staker_options.create_character(emitter=test_emitter, config_file=None)
client_account, staking_address = select_client_account_for_staking(emitter=test_emitter,
stakeholder=stakeholder_from_configuration,
@ -157,7 +158,8 @@ def test_stakeholder_configuration(test_emitter, test_registry, mock_testerchain
transacting_staker_options = TransactingStakerOptions(staker_options=staker_options,
hw_wallet=None,
beneficiary_address=None,
allocation_filepath=None)
allocation_filepath=None,
gas_price=None)
stakeholder_from_configuration = transacting_staker_options.create_character(emitter=None, config_file=None)
client_account, staking_address = select_client_account_for_staking(emitter=test_emitter,
stakeholder=stakeholder_from_configuration,

View File

@ -108,8 +108,12 @@ def mock_token_agent(mock_testerchain, token_economics, mock_contract_agency):
@pytest.fixture(scope='function', autouse=True)
def mock_staking_agent(mock_testerchain, token_economics, mock_contract_agency):
def mock_staking_agent(mock_testerchain, token_economics, mock_contract_agency, mocker):
mock_agent = mock_contract_agency.get_agent(StakingEscrowAgent)
# Handle the special case of commit_to_next_period, which returns a txhash due to the fire_and_forget option
mock_agent.commit_to_next_period = mocker.Mock(return_value=MockContractAgent.FAKE_TX_HASH)
yield mock_agent
mock_agent.reset()

View File

@ -40,7 +40,9 @@ CURRENT_BLOCK = MOCK_TESTERCHAIN.w3.eth.getBlock('latest')
class MockContractAgent:
FAKE_RECEIPT = {'transactionHash': HexBytes(b'FAKE29890FAKE8349804'),
FAKE_TX_HASH = HexBytes(b'FAKE29890FAKE8349804')
FAKE_RECEIPT = {'transactionHash': FAKE_TX_HASH,
'gasUsed': 1,
'blockNumber': CURRENT_BLOCK.number,
'blockHash': HexBytes(b'FAKE43434343FAKE43443434')}

View File

@ -0,0 +1,35 @@
"""
This file is part of nucypher.
nucypher 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.
nucypher 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 nucypher. If not, see <https://www.gnu.org/licenses/>.
"""
from nucypher.utilities.gas_strategies import construct_fixed_price_gas_strategy
def test_fixed_price_gas_strategy():
strategy = construct_fixed_price_gas_strategy(gas_price=42)
assert 42 == strategy("web3", "tx")
assert 42 == strategy("web3", "tx")
assert 42 == strategy("web3", "tx")
assert "0gwei" == strategy.name
strategy = construct_fixed_price_gas_strategy(gas_price=12.34, denomination="gwei")
assert 12340000000 == strategy("web3", "tx")
assert 12340000000 == strategy("web3", "tx")
assert 12340000000 == strategy("web3", "tx")
assert "12gwei" == strategy.name

View File

@ -106,8 +106,9 @@ def make_decentralized_ursulas(ursula_config: UrsulaConfiguration,
worker_address=worker_address,
db_filepath=tempfile.mkdtemp(),
rest_port=port + 100,
# start_working_now=commit_to_next_period, # FIXME: 2424
**ursula_overrides)
if commit_to_next_period:
if commit_to_next_period: # FIXME: 2424
# TODO: Is _crypto_power trying to be public? Or is there a way to expose *something* public about TransactingPower?
# Do we need to revisit the concept of "public material"? Or does this rightly belong as a method?
tx_power = ursula._crypto_power.power_ups(TransactingPower)