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

Estimation for contract call gas amount is too low #5156

Closed
cmichi opened this issue Apr 28, 2021 · 10 comments
Closed

Estimation for contract call gas amount is too low #5156

cmichi opened this issue Apr 28, 2021 · 10 comments

Comments

@cmichi
Copy link
Contributor

cmichi commented Apr 28, 2021

When executing the method of some contracts the default Max Gas Allowed value (the estimated one) is estimated significantly too low.

How to reproduce

We've found that at least the ink! examples delegator and contract-terminate are affected. contract-terminate is the easiest to reproduce this. I've attached the contract which can be used for deployment here: contract_terminate.contract.zip.

The contract has only one method terminateMe, the UI suggests a default Max Gas Allowed of 1417, but the contract then fails with system.ExtrinsicFailed: contracts.OutOfGas.

It works if the max gas is set manually to 2500.

screenshot-polkadot js org-2021 04 28-10_51_34

Debugging

We've used canvas-node with the latest Substrate HEAD. canvas-ui is also affected by this.

It might be that the actual bug is in Substrate, but I don't know how the logic of polkadot-js works here. Might be that we need to move the issue over to Substrate.

@jacogr
Copy link
Member

jacogr commented Apr 28, 2021

On the estimation, it uses the contracts.call RPC and then uses the gasConsumed value as the estimated gas passed to the actual call execute tx. So if this is very low (and in this case it seems off by a huge margin), something is really weird in the contracts call RPC in these specific cases.

The UI itself does round the retrieved value upwards, so the passed value is slightly more than the gasConsumed already, but there may be a case to add an additional safety-factor in there as well, say 5% above the gasConsumed estimation, but certainly not 50%

@jacogr
Copy link
Member

jacogr commented May 1, 2021

Logged the actual gasConsumed value on this specific contract, it is 1,416,325,397 (The UI adjust this upwards and passes 1,417,000,000 with the mega-gas rounding)

Added a 1% factor above that, it still fails. Even at 10%/15%/25%/35%/50% above the received gasConsumed value it still fails.

In this specific case, quite probably due to the termination itself?

@cmichi
Copy link
Contributor Author

cmichi commented May 1, 2021

Thanks @jacogr! I'm cc'ing @athei.

@jacogr
Copy link
Member

jacogr commented May 1, 2021

I tried to see if I'm doing something stoopid, but def. do call the read RPC with the selected account, i.e. same one that will execute the actual tx as well.

@athei
Copy link

athei commented May 1, 2021

Thanks for your investigation. Looks like a a bug in the pallet_contracts then. I will look into it.

@jacogr
Copy link
Member

jacogr commented May 1, 2021

Happy to hear if I'm doing something wrong here, just cannot find out what it is atm. So eyes elsewhere would help - there is indeed an issue "somewhere".

@jacogr jacogr added @app-contracts Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. labels May 2, 2021
@jacogr jacogr added substrate and removed @app-contracts Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. labels May 28, 2021
@polkadot-js-bot
Copy link

This issue has been open for 21 days with no activity and is not labelled as an enhancement. It will be closed in 7 days.

@cmichi
Copy link
Contributor Author

cmichi commented Jun 19, 2021

There is paritytech/substrate#8976 now, which should fix this.

@jacogr
Copy link
Member

jacogr commented Jun 19, 2021

Closing, tracked in Substrate.

@jacogr jacogr closed this as completed Jun 19, 2021
@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants