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

masp gas limit doesn't apply for non-native gas tokens #4041

Open
McDaan opened this issue Nov 16, 2024 · 4 comments
Open

masp gas limit doesn't apply for non-native gas tokens #4041

McDaan opened this issue Nov 16, 2024 · 4 comments

Comments

@McDaan
Copy link
Contributor

McDaan commented Nov 16, 2024

masp_fee_payment_gas_limit parameter value is set to 100000 in the current dry-run testnet.

Could be that the protocol parameter only applies when the native token is used as gas token? I mean, I only seen these logs involving OSMO as gas token while performing shielded to shielded transfers and unshielding IBC transfers, along with --gas-spending-key and --disposable-gas-payer flags.

In both cases it complained about transaction gas limit being exceeded:

Encountered error while broadcasting transaction: StringTracer: server error: {"codespace":"","code":9,"data":"","log":"Mempool validation failed: Error trying to apply a transaction: Gas error: Transaction gas limit exceeded","hash":"0DF185F5C936696430B963DD754E145494BEBF26BAF0BE7C7B283BA6FD6C7966"}

I had to add --gas-limit 80000 so as to troubleshoot it and make them functional.

Setting the same gas limit value as the protocol parameter value for non-native tokens would allow to perform above actions without having to set --gas-limit flag.

@grarco
Copy link
Contributor

grarco commented Nov 18, 2024

Hi @McDaan. The protocol limit is applied every time the transaction needs to pay gas fees via the MASP, regardless of the token involved. The error message you get only states that your transaction exceeded its gas limit, not the protocol one and indeed you were able to make the transaction pass by just using a gas limit of 80_000 which is less than the protocol param.

The protocol parameter is a limit that we enforce on masp fee payment, but it does not override the gas limit of your transaction: the protocol has some logic to check that neither your tx limit nor the protocol parameter are exceeded. As such, it is important to set the gas limit of transactions correctly. Right now we have a default gas limit value when the user does not provide one manually but we cannot raise it too much cause that will force simpler transactions to pay more gas fees than needed. We have an issue for this #1625 that we'll probably update and hopefully work on in the future

@McDaan
Copy link
Contributor Author

McDaan commented Nov 18, 2024

Hey @grarco, thank you so much for the clarification. I now understand somewhat better the logic behind it. So, in the meantime, interfaces or the extension will need the option to tweak gas limit so as to address these situations, or perhaps also setting a sufficient gas limit directly by default in the transaction construction would work better in terms of UX? It would give the user both the choice to tweak the gas limit (just in case) and a priori a sufficient gas limit by default to process the transaction smoothly without having the user spending time tweaking gas limits. What do you think?

Hope to see gas estimation in client in the future!

@grarco
Copy link
Contributor

grarco commented Nov 19, 2024

Maybe we could come up with another constant in the SDK for gas-demanding transactions (such as MASP and others) set to a higher value so that it could ease the UX (at least for now). @Fraccaman I believe you found that also the redelegate tx can be quite intense on gas sometimes, what do you think of this solution?

@McDaan please note that masp transactions are variable in their gas usage, so using a higher preset value could still lead to failures from time-to-time: this would not be a one-time fix but rather a temporary workaround while we wait for a better gas-estimation logic in the client

@Fraccaman
Copy link
Member

right now the interface is querying an endpoint from the indexer to "estimate" the gas needed for a specific transaction i.e https://github.com/anoma/namada-indexer/blob/main/swagger.yml#L467-L476. Indeed some transactions consume more gas and this is reflected in the returned values from the endpoint

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

3 participants