Fallback gas estimates to be set according to the blockchain

[OjusWiZard]

Describe the bug
We use these fallback estimates which are used when gas estimation fails. These are hardcoded values, but we can't use the same values for every chain. For one chain "maxFeePerGas": to_wei(20, GWEI) should be fine, but for some other chain it could be totally off a reasonable number.

At the moment if gas estimation fails, the whole transaction fails because it may ask for ~44 ETH for the gas. But of course if you do have > 44 ETH the gas estimation fails, but the transaction goes through.

To Reproduce
Steps to reproduce the behavior:
Here we estimate gas for a safe deployment transaction on Gnosis and Base. It works on Gnosis but fails on Base, due to the above issue.

import json

from aea.crypto.registries import make_ledger_api
from aea_ledger_ethereum import EthereumCrypto
from autonomy.chain.config import ChainType
from autonomy.chain.base import registry_contracts


rpcs = {
    ChainType.GNOSIS: '<A_TENDERLY_RPC>',
    ChainType.BASE: '<A_TENDERLY_RPC>',
}


def main():
    for chain, rpc in rpcs.items():
        print(f'Testing {chain}...')

        # estimating gas for creating a gnosis safe
        ledger_api = make_ledger_api(
            "ethereum",
            address=rpc,
            chain_id=chain.id,
        )
        crypto = EthereumCrypto(
            private_key_path='.operate/wallets/ethereum.txt',
            password=''
        )
        tx = registry_contracts.gnosis_safe.get_deploy_transaction(
            ledger_api=ledger_api,
            deployer_address=crypto.address,
            owners=[crypto.address],
            threshold=1,
            salt_nonce=0,
        )

        print(json.dumps(tx, indent=2))
        assert tx['gas'] > 0, "Gas estimate should be greater than 0"


if __name__ == "__main__":
    main()

Expected behavior
The gas should be estimated in the transaction on Base above.

Screenshots
N/A

Desktop (please complete the following information):

• OS: linux
• AEA Version 1.60.0

Additional context
N/A

[Adamantios]

The fallback can be overridden per chain:

open-aea/packages/valory/connections/ledger/connection.yaml

Line 42 in 6e4ba0c

maxFeePerGas: 20000000000

Please try this solution instead in valory-xyz/olas-operate-app#667. You should not have to update the DEFAULT_GAS_PRICE_STRATEGIES constant to do this.

Ok, I see that you are using the ledger directly without a connection. Therefore, you can pass the gas_price_strategies when instantiating the ledger object to override this:

open-aea/plugins/aea-ledger-ethereum/aea_ledger_ethereum/ethereum.py

Lines 904 to 906 in 6e4ba0c

	self._gas_price_strategies: Dict[str, Dict] = kwargs.pop(
	"gas_price_strategies", DEFAULT_GAS_PRICE_STRATEGIES
	)

Let me know when you try this and if this does not work.

[OjusWiZard]

yeah right @Adamantios! I already did that here and it worked!
But I mean, that it falling back to an unrealistic default value creates an issue that is extremely complicated to solve. I think having chain-specific default values, would improve the seperation of concern, because then the user of open-aea would not have to deal with gas estimation/transaction related stuff.
Or at least we can improve the warning there, that if the gas estimation fails then we are using these A and B values which can be adjusted this way. Wdyt?

[Adamantios]

• The solution applied on the PR is not exactly what I had in mind. When calling make_ledger_api you can pass kwargs. One of these kwargs can be gas_price_strategies. There, you can override whatever is already in the DEFAULT_GAS_PRICE_STRATEGIES constant:

  open-aea/plugins/aea-ledger-ethereum/aea_ledger_ethereum/ethereum.py

  Lines 904 to 906 in 6e4ba0c

  	self._gas_price_strategies: Dict[str, Dict] = kwargs.pop(
  	"gas_price_strategies", DEFAULT_GAS_PRICE_STRATEGIES
  	)

• I agree that falling back to an unrealistic default value creates an issue that is extremely complicated to solve. I also agree we can improve the warning, it is not helpful at all and we should update it.

• Regarding having chain-specific default values, I do agree that it would improve the separation of concerns because then the users of open-aea would not have to deal with gas estimation/transaction-related stuff. However, I think it is not the responsibility of the ledger, as it is instantiated for a single chain, therefore, an upper-level component should take care of creating a mapping of chain -> default values, such as the connection does for example.

[OjusWiZard]

• It is done in the PR, sorry I don't see the difference ':D

The rest of the two points makes sense to me! So, is there any actionable for this issue to keep it open, or should we close it? I see just to improve the warning?

[Adamantios]

Yes, let's keep it open for the warning improvements and also to revisit the third point above.

[OjusWiZard]

Basically we need to have a common default values logic to do these transactions in any chain, and the question is that is this logic already present anywhere? Or if not then where should we have that?
Perhaps we can have a wrapper of make_ledger_api in open-aea which is responsible to be seamlessly supported on all the chains.