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

internal/ethapi: fix signer in GetTransactionReceiptsByBlock method #1107

Conversation

bbaktaeho
Copy link
Contributor

@bbaktaeho bbaktaeho commented Dec 16, 2023

Description

In GetTransactionReceiptsByBlock, the value from is sometimes zero address. Therefore, I changed the code that creates the signer by referring to the code implemented in GetTransactionReceipt.

Looking at the code, only the signer of legacy transaction with type 0 is being made.

The block used as an example:

  • blockNumber - 51178987

  • rpc - eth_getTransactionReceiptsByBlock

  • result (transactionHash, from, to)

    0x8af449a3dc8214a3b3fea2bdaef6afd8857c326902b25f8254aa1df46d988e33 0x0000000000000000000000000000000000000000 0x91b431a541872d0b3c55f3381cb391d9347f1552
    0x5d4e69d161ff90da3a67a303029574130159248b69c51cbe559d0230558fc1f9 0x0000000000000000000000000000000000000000 0xa3fb72cbf2e07dc1b34aea569ed40755fd978bd8
    0x92f1b7e86b2ce03a01c1c0f7ca211843af96710666986b4eb743dcefcd7eec75 0x0000000000000000000000000000000000000000 0x726d86ff7861e94a34e7fd6260b4fef5b9c354e4
    0xb536148036238c5c57b22114ca081847a48dc3ba2e54fb2b04e155ec7a2c47e6 0x0000000000000000000000000000000000000000 0x17414e39c2a386ded00710f208d9e20657d6f38b
    0x493360753b864445ea2bbb8eb8c5b4f50f43047a1d62a09ac1f7fe1608006c19 0x0000000000000000000000000000000000000000 0x2aad1825c233944109dbe418e21645ad79106350
    0x8bdc5e7b787f7a06ef113e1f18592d34c64fecb94e78f29d99f22ff51ee8d645 0x0000000000000000000000000000000000000000 0x2a51882259e03190a07ab8a83871e41348328b9c
    0x1ebd243a7789bdc6bc19882a583ff71e80dabe9671f4110bc6568218f2587306 0x0000000000000000000000000000000000000000 0xc2132d05d31c914a87c6611c10748aeb04b58e8f
    0xac0b9efc8d929d09f6f763a996d0b3f4e276bd89dbf7d745c0043bb4297ca97e 0x0000000000000000000000000000000000000000 0x907e5638d6d6d933088a860d2803e762a6f2acbd
    0x31448309e41bdcba6531726e8f66bda2d19091a3a578e1048df4353bbb144a11 0x0000000000000000000000000000000000000000 0xc2132d05d31c914a87c6611c10748aeb04b58e8f
    0x912c5adb53f4d8530cad5ea3c55b0a003eabbaabe1a7b0310ce9f2b9e2029828 0x0000000000000000000000000000000000000000 0x2ef766b59e4603250265ecc468cf38a6a00b84b3
    0x0bd0500ed346d245f471aecf7f187f949759e850ac836fcfe4212d7c8b70222f 0x0000000000000000000000000000000000000000 0xdf6fee057222d2f7933c215c11e5150bd2efc53e
    0x0ae7036249c92bc93a18d7b3efc07e51a8de15e321d4da0614b4a0b822fc9292 0x0000000000000000000000000000000000000000 0xdf6fee057222d2f7933c215c11e5150bd2efc53e
    0x629fe926421f1d0e0f6d54c6a7fd919ff8a6c785bda44dc56ee483141c811d8b 0x0000000000000000000000000000000000000000 0xdf6fee057222d2f7933c215c11e5150bd2efc53e
    0x16da38de43bc3d791949bf35289b6931421f047fe9332eb1afbcc45e030657cb 0x0000000000000000000000000000000000000000 0xdf6fee057222d2f7933c215c11e5150bd2efc53e
    0x14d28879acac93f091b8c9e19163b016d84a2e2235b0de5fafbf6fa2b2bd22a8 0x0000000000000000000000000000000000000000 0x3f2f4aa023c6ed149d342229afac6e140e149114
    0xe5fdf1793a55a55f31ededebe1d3e576d285a0f34ff0281c108d3ca6acf534c6 0x0000000000000000000000000000000000000000 0x3f2f4aa023c6ed149d342229afac6e140e149114
    0x95c543d67f8b6885f57eff591f7a6deef8c1dfbb14fee1be768d64f8c9f15fc8 0x0000000000000000000000000000000000000000 0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789
    0x35546e964adf712e2540abad70b7b2a394c702ab3676d4d0b1d5403545223b75 0x0000000000000000000000000000000000000000 0x0e1f20075c90ab31fc2dd91e536e6990262cf76d
    0x91671e5da65801123b2d65e29144d5becf8e25f58125d8546c17f813268391de 0x0000000000000000000000000000000000000000 0x7573933eb12fa15d5557b74fdaff845b3baf0ba2
    0x84ff81caa3c9c76587aef0bbbd73182825004b879b98eed2ed2edc625e7f3bf8 0x0000000000000000000000000000000000000000 0x5dee49d03e8587ef90562748204022cb39b12551
    0x745aa6b148cce3b0a6724e6308d6674b72079ca699db06c217ba9471a24adc58 0x0000000000000000000000000000000000000000 0x2133770d31949d00c64a65f43ecbc4ba4239d466
    0x8d18f1cf12b3268e747321975abdfb3b2117a91efa9c859db4cf0988147efb44 0x0000000000000000000000000000000000000000 0x057653a77e5c007e5a06aaa4cc075e75cc7db15f
    0xd3cb92ce50b78355277fb0298539698325916ed3e3f8c006080d52b579e728b1 0x0000000000000000000000000000000000000000 0x9ca01969afd393f4c5208ffceb1a606e380b55a8
    0x0fd3d137990ea5e828aaaa99ad47a3a191052f3e4703767e24170a5208f2306d 0x0000000000000000000000000000000000000000 0xdef1c0ded9bec7f1a1670819833240f027b25eff
    0x479a1a96ac7ee85f14932afe6b7dd3a37be2e846a4db9923784dbd5bef2f5875 0x0000000000000000000000000000000000000000 0x7d7aaf03ea51bc1658e935a8ad09d77f89a1f250
    0x4f3e16adcf4a962e134f9709feac63ffad00d0df57aa4fd52820effd162a5411 0x0000000000000000000000000000000000000000 0x1aaec58ddbcd1038c3c8a5959c555e40e036a808
    0x9814fb229e52365ee467d0b336b28b08deb89426784b9e03b5bfc006630f05a3 0x0000000000000000000000000000000000000000 0x7b36dfd5304562952e2b4de9c8048ed155c6115d
    0x84c675d3bba2c298b2be6f401bdeff89caaa497634b6094065a088af4e30caeb 0x0000000000000000000000000000000000000000 0xfdf8785e2171d1bd201e975e7ac7e10c1635c5c9
    

    All transactions in issues are the receipt of the EIP-1559 transaction (type=0x2).

Changes

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Changes only for a subset of nodes

Breaking changes

Please complete this section if any breaking changes have been made, otherwise delete it

Nodes audience

In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)

Checklist

  • I have added at least 2 reviewer or the whole pos-v1 team
  • I have added sufficient documentation in code
  • I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply
  • Created a task in Jira and informed the team for implementation in Erigon client (if applicable)
  • Includes RPC methods changes, and the Notion documentation has been updated

Cross repository changes

  • This PR requires changes to heimdall
    • In case link the PR here:
  • This PR requires changes to matic-cli
    • In case link the PR here:

Testing

  • I have added unit tests
  • I have added tests to CI
  • I have tested this code manually on local environment
  • I have tested this code manually on remote devnet using express-cli
  • I have tested this code manually on mumbai
  • I have created new e2e tests into express-cli

Manual tests

Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it

Additional comments

Please post additional comments in this section if you have them, otherwise delete it

@bbaktaeho
Copy link
Contributor Author

HI @marcello33, Please PR review.

@marcello33
Copy link
Contributor

Hey @bbaktaeho, I triggered the CI run.
In the meantime, can you implement a unit test, and eventually expand integration tests for this new functionality?
Thank you.

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (8b2f024) 56.00% compared to head (789b614) 55.96%.

❗ Current head 789b614 differs from pull request most recent head 30b4e96. Consider uploading reports for the commit 30b4e96 to get more accurate results

Files Patch % Lines
internal/ethapi/api.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1107      +/-   ##
===========================================
- Coverage    56.00%   55.96%   -0.04%     
===========================================
  Files          658      658              
  Lines       114473   114470       -3     
===========================================
- Hits         64108    64067      -41     
- Misses       46506    46547      +41     
+ Partials      3859     3856       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbaktaeho
Copy link
Contributor Author

@marcello33, I have written unit test codes for several transaction types.

@marcello33
Copy link
Contributor

@marcello33, I have written unit test codes for several transaction types.

Ok, thanks.
Will request review from the team and run the CI.

@marcello33 marcello33 requested a review from a team December 18, 2023 13:47
Copy link
Member

@pratikspatil024 pratikspatil024 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@pratikspatil024 pratikspatil024 merged commit 937dd6a into maticnetwork:develop Dec 21, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants