Fix estimate_gas result differs widely from the used_gas
#1239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I know you are doing some refactoring to the core evm and how the current frontier works. Would you mind to reviewing this when you have time. This is a bug fix and improvement. Maybe things will be different after your refactor, but before that, we need to solve this issue in the main branch. @sorpaas |
sorpaas
approved these changes
Nov 10, 2023
|
Sorry. It just took me a while to review the other parts of your rewrites. The EVM refactoring won't block anything here -- it'll still take a while (sorry I probably should allocate more time doing reviews). But indeed, at the end we'll be able to move away from the |
icodezjb
pushed a commit
to btclayer2/frontier
that referenced
this pull request
Nov 10, 2023
…t-evm#1239) * Fix add `size_limit` to the proof_size of weight info * Fix issues in the record dynamic opcode * Debug test case * Debug test case * Fix test cases * Fix clippy * Sload cache item (cherry picked from commit 176cb34)
boundless-forest
added a commit
to darwinia-network/frontier
that referenced
this pull request
Nov 24, 2023
…t-evm#1239) * Fix add `size_limit` to the proof_size of weight info * Fix issues in the record dynamic opcode * Debug test case * Debug test case * Fix test cases * Fix clippy * Sload cache item
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We found an issue last week where the estimated gas result for some transactions was significantly higher than the actual used_gas in the receipt. See this darwinia-network/darwinia#1301 for details. After some investment, the root cause was in the proof size calculation in the
record_external_operationandrecord_external_dynamic_opcode_cost:In the case of not hitting the account metadata, the weight info recorder tried to record the
create_contract_limitvalue, which is a very large amount 24576. This made it easy to trigger the transaction proof size limit and then out of gas. It fooled the binary research gas estimater to make the gas higher than before. Take https://crab.subscan.io/tx/0x313ba47341cdf5b6b5ce05cc2317ee6e3b285312b6a01d62fba4c39f51ab8c34 for example, the actual used_gas is 23135, but the estimated gas result over 160000~ This PR mainly fixes this issue. Beside this, the currentrecord_external_dynamic_opcode_costis very messy, I have rewritten it.