Problem: precompiles required-gas need adjustments

[thomas-nguy]

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

Using some base line value for estimation, may need adjustment in the future

Open for suggestions. Keeping this open as it is important to have this fixed before an official release.

PR Checklist:

• Have you read the CONTRIBUTING.md?
• Does your PR follow the C4 patch requirements?
• Have you rebased your work on top of the latest master?
• Have you checked your code compiles? (make)
• Have you included tests for any non-trivial functionality?
• Have you checked your code passes the unit tests? (make test)
• Have you checked your code formatting is correct? (go fmt)
• Have you checked your basic code style is fine? (golangci-lint run)
• If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
• If your changes affect the client infrastructure, have you run the integration test?
• If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
• If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
• If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

[codecov]

Codecov Report

Merging #1178 (2e220af) into main (071bc21) will increase coverage by 20.71%.
The diff coverage is 51.76%.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1178       +/-   ##
===========================================
+ Coverage   16.25%   36.97%   +20.71%     
===========================================
  Files          79      115       +36     
  Lines        5698    10235     +4537     
===========================================
+ Hits          926     3784     +2858     
- Misses       4694     6078     +1384     
- Partials       78      373      +295     

Files	Coverage Δ
x/cronos/keeper/precompiles/relayer.go	25.80% <62.50%> (+16.57%)	⬆️
x/cronos/keeper/precompiles/ica.go	13.47% <46.15%> (+6.80%)	⬆️
x/cronos/keeper/precompiles/bank.go	9.35% <44.44%> (+6.85%)	⬆️

... and 54 files with indirect coverage changes

[JayT106]

Not sure how the caller handles the return 0 from RequiredGas. Assume the caller called the contract function with no RequiredGas implementation. Should we return the max of uint64 instead of 0 for the error and the default cases?

[JayT106]

Not sure how the caller handles the return 0 from RequiredGas. Assume the caller called the contract function with no RequiredGas implementation. Should we return the max of uint64 instead of 0 for the error and the default cases?

During the execution, the handler would revert because of wrong method name. But it may also consume the gas. Lets say we set to max uint64. This may be a bit too punitive?

On the other hand, this could also be detectable it during the gas estimation phase if not skipped

So the handler would reverts and consumes some gas (like transaction base cost) but no addition cost for calling this pre-compiled contract, right?

Yes, the gas estimation can detect the error but cannot prevent attack. So my question is should we charge some gas for the contract (if we don't charge extra gas for entering the precompile contract, or we do?) if their is any executing errors or revert, perhaps can charge gas like half of the minimum gas cost of method in the contract?

[thomas-nguy]

 perhaps can charge gas like half of the minimum gas cost of method in the contract?

Half the minimum of which method? In case method does not exists, there will be no execution on cosmos side. I think charging the base cost should be enough