Gas Exactimation v2

[fedejinich]

Description

This is a new gas estimation algorithm, where ESTIMATED_GAS = GAS_USED + GAS_DEDUCTED_REFUNDS

Motivation and Context

This is a second step of @SergioDemianLerner's old PR:

How Has This Been Tested?

Unit tests
DSL tests

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• Tests for the changes have been added (for bug fixes / features)
• Requires Activation Code (Hard Fork)

• Other information:

[joaquinlpereyra]

👋🏿 Hello.

So I've been looking at this and it seems that:

• For calls without value, this is the exact gas estimation that was present previously.
• For calls with value, this is the gas estimation previously plus the STIPEND_COST constant.

This is because res.getDeductedRefund() will return zero always for local calls, as marked by this comment.

This whole addDeductedRefund logic is unused.

As the PR stands, we still won't actually take into account refunds to calculate the necessary gasLimit someone should send.

So all in all, most of the changes are still unimplemented: to take advantage of these this comment needs to addressed.

Also a comment itself warns of the possibility of an overflow. It is true that the user itself would be the only one affected by the overflow by providing a wrong gasLimit, but there's no reason to perform a simple sanity check and failing early at the beginning of an estimateGas RPC call if that were the case, specially as gasValid in the VM is not called for local calls.

Anyway, all in all, I think to truly take advantage of the PR it needs a bit of work still.

[joaquinlpereyra]

Also seems like a great time to implement something like Geth's gascap .

[fedejinich]

I guess there is another inconsistency here, the produced exception it's never added to the ProgramResult

[fedejinich]

Currently this solution it's overestimating by a small difference. This is "good" because it improves the old algorithm (taking account gas "refunds"), and it won't produce failures if you run a transaction with the estimated value.
On this last steps we're working to match exactly the gasNeeded to perform a transaction.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

94.5% Coverage
0.0% Duplication