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

Each wallet type should be able to accept gas parameters #3

Open
Schwartz10 opened this issue Dec 4, 2024 · 4 comments
Open

Each wallet type should be able to accept gas parameters #3

Schwartz10 opened this issue Dec 4, 2024 · 4 comments
Assignees

Comments

@Schwartz10
Copy link
Contributor

It's come to my attention that the CLI can take arbitrarily long for transactions to confirm when the network is congested. I realized we seem to just lean on geth/abigen to set the gas parameters for us, but we should also be able to set these ourselves.

Instead of passing each argument on its own, we should create a CustomGas struct that has the necessary gas fields, and a nonce. This way, if we decide to add more custom fields, we don't break the compiler in really annoying ways, we just update a type and it should work fine

This ticket represents just the work needed to be done in the go-wallet-utils repo

@jimpick
Copy link
Collaborator

jimpick commented Dec 9, 2024

Here is some code that calls walletutils.NewEthWalletTransactor():

_, auth, err = walletutils.NewEthWalletTransactor(wallet, &account, passphrase, big.NewInt(chainID), ethClient)
if err != nil {
  logFatal(err)
}

(from https://github.com/glifio/glif/blob/jim_update_go_wallet_utils/cmd/utils.go#L369 )

The auth returned is of type *bind.TransactOpts from go-ethereum:

https://pkg.go.dev/github.com/ethereum/go-ethereum/accounts/abi/bind#TransactOpts

type TransactOpts struct {
	From   common.Address // Ethereum account to send the transaction from
	Nonce  *big.Int       // Nonce to use for the transaction execution (nil = use pending state)
	Signer SignerFn       // Method to use for signing the transaction (mandatory)

	Value      *big.Int         // Funds to transfer along the transaction (nil = 0 = no funds)
	GasPrice   *big.Int         // Gas price to use for the transaction execution (nil = gas price oracle)
	GasFeeCap  *big.Int         // Gas fee cap to use for the 1559 transaction execution (nil = gas price oracle)
	GasTipCap  *big.Int         // Gas priority fee cap to use for the 1559 transaction execution (nil = gas price oracle)
	GasLimit   uint64           // Gas limit to set for the transaction execution (0 = estimate)
	AccessList types.AccessList // Access list to set for the transaction execution (nil = no access list)

	Context context.Context // Network context to support cancellation and timeouts (nil = no timeout)

	NoSend bool // Do all transact steps but do not send the transaction
}

When writing to smart contracts using go-ethereum APIs, it is possible to set these values.

eg. From https://goethereumbook.org/smart-contract-write/

auth.Nonce = big.NewInt(int64(nonce))
auth.Value = big.NewInt(0)     // in wei
auth.GasLimit = uint64(300000) // in units
auth.GasPrice = gasPrice

If these are left unset, go-ethereum will make calls to PendingNonceAt() (to get the Nonce) and EstimateGas() (to get the estimated gas values). In go-wallet-utils, we created "shims" so we can put in alternate implementations (eg. when using Filecoin native ETH-RPC): https://github.com/glifio/go-wallet-utils/blob/primary/eth_eoa_transactor.go#L53

So there appears to be a way of doing this already.

Perhaps the CustomGas struct could be passed in somewhere to adjust the values?

@Schwartz10
Copy link
Contributor Author

Got it, so basically the idea would be the estimate the gas, and then set custom gas or nonce on the auth object, when its a ETH EOA transactor.

What do you think we could/should do to have the consuming software (i.e. CLI, admin-cli) not need to know the difference between eth eoa and fil eoa? How can we create the same behavior for fil eoa? and fil msig proposer?

Let's spec it out before we write code

@jimpick
Copy link
Collaborator

jimpick commented Dec 9, 2024

As I understand it, both eth eoa and fil eoa use the same gas params, just with slightly different naming. We already use the gas params from the auth object when constructing the fil messages:

All the transactors will estimate the gas if it isn't set.

Looking at it again, for the msig case, I notice we're setting the gas params on the propose message. When it is actually executed, I'm not sure if it uses the gas params from the propose message or the final approve message (in situations where there are more than one signature). I'd guess that the final approve message needs to have correct gas parameters.

Apart from msig approvals, my guess is that we can handle the gas the same way for all the transactors by setting the values on the auth objects.

@Schwartz10
Copy link
Contributor Author

my guess is that we can handle the gas the same way for all the transactors by setting the values on the auth objects

Hard for me to tell if this is true...

filMsg := &lotustypes.Message{
. I believe the fil eoa and fil msig propose wallet types get their gas params inside the Signer method that creates the filecoin message type. They're using the tx object to get the gas data - is that tx object getting its gas params from the auth object?

If so, then that might work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants