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

fix gas arguments #1500

Open
dpaiton opened this issue May 29, 2024 · 1 comment
Open

fix gas arguments #1500

dpaiton opened this issue May 29, 2024 · 1 comment
Labels
enhancement A feature or refactor request

Comments

@dpaiton
Copy link
Member

dpaiton commented May 29, 2024

Problem

We currently expose three different variables for limiting gas:

txn_options_gas=gas_limit,
txn_options_base_fee_multiple=txn_options_base_fee_multiple,
txn_options_priority_fee_multiple=txn_options_priority_fee_multiple,

They were added incrementally as attempts to debug an ongoing intermittent problem related to insufficient gas. This is confusing and in some ways not correct.

web3py (everyone, really) uses these parameters for a transaction:

TRANSACTION_DEFAULTS = {
    "value": 0,
    "data": b"",
    "gas": lambda w3, tx: w3.eth.estimate_gas(tx),
    "gasPrice": lambda w3, tx: w3.eth.generate_gas_price(tx),
    "maxFeePerGas": (
        lambda w3, tx: w3.eth.max_priority_fee
        + (2 * w3.eth.get_block("latest")["baseFeePerGas"])
    ),
    "maxPriorityFeePerGas": lambda w3, tx: w3.eth.max_priority_fee,
    "chainId": lambda w3, tx: w3.eth.chain_id,
}

If we override gas we are saying "this is the max gas (in units of gas) that I will pay for the transaction." It defaults to an estimate based on the tx details. This is problematic because the user does not know how much gas any given transaction should cost. It's possible to calculate this, but that calculation is tedious and requires a good amount of expert knowledge, including knowing where to look for the fn in the smart contracts.

Additionally, the way we implement the other two parameters (base fee & priority fee max) was incorrect and partially fixed in #1496.

Partial solution

We want to specify this limit in units of base, so the user can say "I don't want to pay more than X base for this transaction". if we assign that value to a new config, e.g. transaction_gas_fee_limit, and also include a similar base denominated parameter for priority fee, then we can back out the tx parameters as such:

# user specifies the most they will pay on the txn gas and the priority fee ("tip") for any tx
config.transaction_fee_limit: FixedPoint # provided by the user, in units of base
config.priority_fee_limit: FixedPoint # provided by the user, in units of base

# get these values from the web3 estimates for the given tx
gas = web3.eth.estimate_gas(tx) # the estimated gas cost for this transaction
gas_price = web3.eth.generate_gas_price(tx) # the current price (in base) of one gas, aka baseFeePerGas
total_gas_fee = gas * gas_price # total amount that the tx will cost, in base [gas * base/gas = base]

# set these values from user preferences
max_priority_fee_per_gas = config.priority_fee_limit / gas # the user's max priority fee per gas in units of base
transaction_fee_limit_per_gas = config.transaction_fee_limit / gas # the user's max transaction fee per gas in units of base

# add a check to ensure they're allowing for enough gas fee
assert transaction_fee_limit_per_gas >= 2 * gas

max_fee_per_gas = max_priority_fee_per_gas + transaction_fee_limit_per_gas # the maximum fee the user will spend per gas on this tx

TRANSACTION_DEFAULTS = {
    "value": 0,
    "data": b"",
    "gas": gas,
    "gasPrice": gas_price,
    "maxFeePerGas": max_fee_per_gas,
    "maxPriorityFeePerGas": max_priority_fee_per_gas,
    "chainId": lambda w3, tx: w3.eth.chain_id,
}
@dpaiton
Copy link
Member Author

dpaiton commented May 30, 2024

After discussing with @slundqui and @wakamex we decided that we do not want to be so opinionated about what units the user wants to use to specify these constraints. Instead, we think it is best to explicitly expose all of these parameters:

gas_limit # "gas" argument
max_fee_per_gas
max_priority_fee_per_gas
timeout

We haven't settled on what level of the hierarchy we want for these arguments. They maybe shouldn't be the same. We may want to give the user access to defaults (via chain or pool config) but then also have the ability to override the default on any relevant function call as a function argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or refactor request
Projects
None yet
Development

No branches or pull requests

1 participant