oasisprotocol / cli

Official CLI for the Oasis Network.
Apache License 2.0
12 stars 1 forks source link

Wrong gas estimates for encrypted ParaTime transfers #248

Open matevz opened 1 month ago

matevz commented 1 month ago

Trying to transfer tokens from the secp256k1 account on Sapphire and providing --encrypted incorrectly estimates gas:

  1. first attempt (no --gas-limit provided):

    Broadcasting transaction...
    Error: Transaction check failed with error: module: core code: 12 message: out of gas (limit: 2318 wanted: 11388)
  2. second attempt --gas-limit 11388:

Broadcasting transaction...
Error: Transaction check failed with error: module: core code: 12 message: out of gas (limit: 11388 wanted: 11389)
  1. third attempt --gas-limit 11389:
Broadcasting transaction...
Error: Transaction check failed with error: module: core code: 12 message: out of gas (limit: 11389 wanted: 11390)
  1. fourth attempt --gas-limit 11390:
Broadcasting transaction...
Transaction included in block successfully.
Round:            3995161
Transaction hash: 1dc1246a30e854dede27d48cb1220b4c19d46521ade38622bbda3ea8c2a4e738
                  (Transaction result is encrypted.)
Error: Execution failed with error: module: core code: 12 message: core: out of gas (limit: 11390 wanted: 12389)

^^ the transaction was actually submitted and gas was spent, see: https://explorer.oasis.io/mainnet/sapphire/tx/1dc1246a30e854dede27d48cb1220b4c19d46521ade38622bbda3ea8c2a4e738

  1. fifth attempt --gas-limit 12389 finally worked:
Broadcasting transaction...
Transaction included in block successfully.
Round:            3995186
Transaction hash: 7224d59e6b74d9b2caf55ab40dfe0757cef4f71c66d3a7aba3de1d267d2ea409
                  (Transaction result is encrypted.)
Execution successful.

Since gas estimates are standard for transactions such as ParaTime transfers and withdrawals, we could probably just hardcode those?

kostko commented 1 month ago

The problem is that currently the way CLI does gas estimation ignores encryption-related costs, because it assumes that the node it is talking to does not have access to the key manager committee so it estimates a plain transaction and uses that estimate.

To avoid changing this assumption, it could probably at least better guess the costs, for example by querying how much encryption-related stuff costs and adding that plus some extra per-byte costs due to encrypted transactions being slightly larger.