Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

perf: make use of sender cache to reduce hotspot #1705

Closed
wants to merge 1 commit into from

Conversation

mmsqe
Copy link
Contributor

@mmsqe mmsqe commented Mar 8, 2023

Closes: #1703

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #1705 (1dcfce9) into main (fcdc625) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1705   +/-   ##
=======================================
  Coverage   68.20%   68.20%           
=======================================
  Files         112      112           
  Lines       10167    10168    +1     
=======================================
+ Hits         6934     6935    +1     
  Misses       2827     2827           
  Partials      406      406           
Impacted Files Coverage Δ
app/ante/sigverify.go 100.00% <100.00%> (ø)
x/evm/types/msg.go 80.97% <100.00%> (+0.08%) ⬆️

@@ -331,7 +331,8 @@ func (msg MsgEthereumTx) AsMessage(signer ethtypes.Signer, baseFee *big.Int) (co
// GetSender extracts the sender address from the signature values using the latest signer for the given chainID.
func (msg *MsgEthereumTx) GetSender(chainID *big.Int) (common.Address, error) {
signer := ethtypes.LatestSignerForChainID(chainID)
from, err := signer.Sender(msg.AsTransaction())
tx := msg.AsTransaction()
Copy link
Contributor

Choose a reason for hiding this comment

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

we better to fill the tx.from field using msg.From here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes? ethtyes.Sender set in tx.from.Store

Copy link
Contributor

@yihuang yihuang Mar 14, 2023

Choose a reason for hiding this comment

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

but the tx is a temporary object, cache sender inside is not useful, we might need to find a way to cache the eth tx itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

crypto-org-chain#227
I did a simple fix here.

@mmsqe mmsqe closed this Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove unnecessary sender address recovery
2 participants