delvtech / agent0

Analysis & simulation repo for Delv
https://agent0.readthedocs.io/en/latest/
Apache License 2.0
49 stars 21 forks source link

fix gas arguments #1500

Open dpaiton opened 4 months ago

dpaiton commented 4 months ago

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 https://github.com/delvtech/agent0/pull/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 commented 4 months ago

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.