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.
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
fix(evm): commit temporary state on precompile start, avoiding bug stemming from uncommitted, dirty journal entries in the EVM StateDB #2086
fix(evm): commit temporary state on precompile start, avoiding bug stemming from uncommitted, dirty journal entries in the EVM StateDB #2086
Changes from 4 commits
8d8a249
06f0fa2
bd08702
55f75c1
516aece
0374dfd
8d7fcd3
eb87c3e
efbe244
52f1cdd
62a1b5f
7cc070b
cc7a802
f9e73d7
be2c0a2
0e6afd8
65b2cd6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 61 in x/evm/evmtest/test_deps.go
Codecov / codecov/patch
x/evm/evmtest/test_deps.go#L60-L61
Check warning on line 131 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L129-L131
Check warning on line 161 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L161
Check warning on line 166 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L166
Check warning on line 171 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L170-L171
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error Handling Improvement
In the
DeployContract
function, error handling can be streamlined by checking for the error immediately after the function call.Apply this diff to enhance error handling:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: codecov/patch
Check warning on line 174 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L174
Check warning on line 184 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L184
Check warning on line 222 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L220-L222
Check warning on line 225 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L225
Check warning on line 248 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L237-L248
Check warning on line 255 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L250-L255
Check warning on line 261 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L257-L261
Check warning on line 264 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L263-L264
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Streamline the
CallContractTx
FunctionThe
CallContractTx
function contains redundant state commits and error handling that can be improved for clarity and efficiency.Consider the following refactor:
EthereumTx
.Apply this diff to implement the refactor:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: codecov/patch
Check warning on line 280 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L280
Check warning on line 283 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L283
Check warning on line 295 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L295
Check warning on line 301 in x/evm/evmtest/tx.go
Codecov / codecov/patch
x/evm/evmtest/tx.go#L299-L301
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure
TxIndex
andLogIndex
are correctly setYou have assigned both
txConfig.TxIndex
andtxConfig.LogIndex
tok.EvmState.BlockLogSize.GetOr(ctx, 0)
. Typically,TxIndex
represents the index of the transaction within the block, whileLogIndex
refers to the index of the log within the transaction. Using the same value for both may not accurately reflect their distinct purposes. Please verify that this alignment is intentional and consistent with how these indices are used elsewhere in the EVM state management.Check warning on line 67 in x/evm/precompile/funtoken.go
Codecov / codecov/patch
x/evm/precompile/funtoken.go#L66-L67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add unit tests for critical error handling paths
Static analysis indicates that lines 66-67 and 69-70 are not covered by tests. These error handling paths are crucial for ensuring robustness. Please consider adding unit tests to cover these scenarios and prevent potential regressions.
Would you like assistance in creating unit tests for these error conditions?
Also applies to: 69-70
🧰 Tools
🪛 GitHub Check: codecov/patch
Check warning on line 70 in x/evm/precompile/funtoken.go
Codecov / codecov/patch
x/evm/precompile/funtoken.go#L69-L70