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

ethclient: fix tx sender cache miss detection #23877

Merged
merged 4 commits into from Nov 17, 2021

Conversation

PlasmaPower
Copy link
Contributor

types.Sender caches signers' recovered addresses for transactions. senderFromServer takes advantage of that with two different invocations:

  • It can be used with both blockhash and addr specified to populate the cache. senderFromServer will return the already known address, which types.Sender will remember and attach to the transaction.
  • It can be used with only blockhash specified to query the cache. senderFromServer's Equal implementation only checks the blockhash, so if the address was previously cached for this block hash, types.Sender will remember that and return it. However, if the cache misses, senderFromServer should return an error, as it doesn't know the address. That's what this PR fixes. Previously, the check for a cache miss was if blockhash was empty, which is never the case. This PR fixes that and changes the check to if addr is empty, which is the case when senderFromServer is being used to query the cache instead of populate it. Now, when that happens but types.Sender's cache misses, senderFromServer will correctly return an error.

This mechanism is used by *ethclient.Client's TransactionSender method. This bug manifested as that method returning an empty address when the cache missed, instead of querying the server as intended.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, I think.

@ligi
Copy link
Member

ligi commented Nov 16, 2021

@fjl will write a test for it

@fjl fjl removed the status:triage label Nov 16, 2021
@fjl fjl removed their assignment Nov 16, 2021
@fjl fjl added this to the 1.10.13 milestone Nov 16, 2021
@fjl fjl merged commit 16341e0 into ethereum:master Nov 17, 2021
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Nov 17, 2021
This fixes a bug in TransactionSender where it would return the
zero address for transactions where the sender address wasn't
cached already.

Co-authored-by: Felix Lange <fjl@twurst.com>
yongjun925 pushed a commit to DODOEX/go-ethereum that referenced this pull request Dec 3, 2022
This fixes a bug in TransactionSender where it would return the
zero address for transactions where the sender address wasn't
cached already.

Co-authored-by: Felix Lange <fjl@twurst.com>
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.

None yet

4 participants