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

[main branch] Exclude only shell receipts from eth endpoints #1877

Merged
merged 10 commits into from
Oct 1, 2024

Conversation

jewei1997
Copy link
Contributor

@jewei1997 jewei1997 commented Oct 1, 2024

Describe your changes and provide context

In the eth_ endpoints, we actually do want to include synthetic logs because these have replaced EVM logs for ERC20 and ERC721 transfer/approval events. We will just exclude shell txs in eth_ endpoints and include both in sei_ endpoints.

Testing performed to validate your change

hardhat tests

@jewei1997 jewei1997 changed the title Exclude only shell receipts from eth endpoints main [for main branch] Exclude only shell receipts from eth endpoints Oct 1, 2024
@jewei1997 jewei1997 changed the title [for main branch] Exclude only shell receipts from eth endpoints [main branch] Exclude only shell receipts from eth endpoints Oct 1, 2024
@jewei1997 jewei1997 requested a review from codchen October 1, 2024 04:15
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

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

Project coverage is 61.23%. Comparing base (a2c4683) to head (9ebb227).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
evmrpc/block.go 57.14% 1 Missing and 2 partials ⚠️
evmrpc/filter.go 33.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1877      +/-   ##
==========================================
- Coverage   61.26%   61.23%   -0.03%     
==========================================
  Files         263      263              
  Lines       23272    23275       +3     
==========================================
- Hits        14257    14253       -4     
- Misses       8010     8014       +4     
- Partials     1005     1008       +3     
Files with missing lines Coverage Δ
evmrpc/filter.go 75.28% <33.33%> (-0.77%) ⬇️
evmrpc/block.go 71.31% <57.14%> (-1.72%) ⬇️

contracts/test/ERC20toCW20PointerTest.js Outdated Show resolved Hide resolved
contracts/test/ERC20toCW20PointerTest.js Outdated Show resolved Hide resolved
Comment on lines +133 to +134
expect(ethReceipts.length).to.equal(1);
expect(seiReceipts.length).to.equal(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we do any verification on the seiReceipt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking that the 2 tx hashes are the same between ethReceipts and seiReceipts

@jewei1997 jewei1997 merged commit da7280d into main Oct 1, 2024
48 of 49 checks passed
@jewei1997 jewei1997 deleted the exclude-only-shell-receipts-from-eth-endpoints-main branch October 1, 2024 18:09
jewei1997 added a commit that referenced this pull request Oct 1, 2024
* exclude only shell receipts from eth endpoints

* minor fix

* remove print statements

* put panic to see if shell receipts are being used

* fix test

* fix block receipts test

* Revert "put panic to see if shell receipts are being used"

This reverts commit 1ea23fd.

* minor fixes

* check that ethReceipt and seiReceipt have the same tx hash
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.

2 participants