Skip to content
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

[Bug Fix] getLogs endpoint should check whether or not to include cosmos Txs based on namespace #1988

Merged
merged 9 commits into from
Dec 12, 2024

Conversation

yzang2019
Copy link
Contributor

@yzang2019 yzang2019 commented Dec 11, 2024

Describe your changes and provide context

Problem:
getEvmTxHashesFromBlock currently include both the EVM and cosmos txs, so it will complain for receipt not found when querying get_ethLogs.

Solution:
Add a validation logic to check whether we should include cosmos side txs based on the namespace.

Testing performed to validate your change

Tested on archive node

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 61.55%. Comparing base (e8e4b3b) to head (58b0dc9).
Report is 227 commits behind head on main.

Files with missing lines Patch % Lines
evmrpc/filter.go 40.00% 2 Missing and 1 partial ⚠️
evmrpc/utils.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1988      +/-   ##
==========================================
- Coverage   61.64%   61.55%   -0.10%     
==========================================
  Files         365      263     -102     
  Lines       26178    24349    -1829     
==========================================
- Hits        16138    14987    -1151     
+ Misses       8967     8248     -719     
- Partials     1073     1114      +41     
Files with missing lines Coverage Δ
evmrpc/block.go 75.10% <100.00%> (+7.07%) ⬆️
evmrpc/utils.go 66.21% <80.00%> (-9.83%) ⬇️
evmrpc/filter.go 79.37% <40.00%> (+1.90%) ⬆️

... and 219 files with indirect coverage changes

@yzang2019 yzang2019 changed the title Fix GetEthLogs: getEvmTxHashesFromBlock should exclude cosmos txs Fix GetEthLogs: getTxHashesFromBlock should check whether or not to include cosmosTxs based on namespace Dec 12, 2024
@yzang2019 yzang2019 changed the title Fix GetEthLogs: getTxHashesFromBlock should check whether or not to include cosmosTxs based on namespace Fix GetEthLogs should check whether or not to include cosmosTxs based on namespace Dec 12, 2024
@yzang2019 yzang2019 changed the title Fix GetEthLogs should check whether or not to include cosmosTxs based on namespace Fix getLogs endpoint should check whether or not to include cosmos Txs based on namespace Dec 12, 2024
@yzang2019 yzang2019 changed the title Fix getLogs endpoint should check whether or not to include cosmos Txs based on namespace [Bug Fix] getLogs endpoint should check whether or not to include cosmos Txs based on namespace Dec 12, 2024
@yzang2019 yzang2019 enabled auto-merge (squash) December 12, 2024 18:29
@yzang2019 yzang2019 merged commit 893568d into main Dec 12, 2024
49 checks passed
@yzang2019 yzang2019 deleted the yzang/fix-getEthLogs branch December 12, 2024 18:55
philipsu522 pushed a commit that referenced this pull request Dec 12, 2024
…mos Txs based on namespace (#1988)

* Fix GetEthLogs: getEvmTxHashesFromBlock should exclude cosmos txs

* Fix unit test

* Fix test failure and add a check for synthetic logs

* Fix test

* Fix log

* Increase fee

* Add fee

* Test
philipsu522 pushed a commit that referenced this pull request Dec 12, 2024
…mos Txs based on namespace (#1988)

* Fix GetEthLogs: getEvmTxHashesFromBlock should exclude cosmos txs

* Fix unit test

* Fix test failure and add a check for synthetic logs

* Fix test

* Fix log

* Increase fee

* Add fee

* Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants