Problem: slow VerifyEthSig on large number of messages #518
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| go func(msg proto.Message) { | ||
| msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx) | ||
| if !ok { | ||
| errChan <- errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil)) | ||
| return | ||
| } | ||
|
|
||
| if err := msgEthTx.VerifySender(signer); err != nil { | ||
| return errorsmod.Wrapf(errortypes.ErrorInvalidSigner, "signature verification failed: %s", err.Error()) | ||
| err := msgEthTx.VerifySender(signer) | ||
| if err != nil { | ||
| err = errorsmod.Wrapf(errortypes.ErrorInvalidSigner, "signature verification failed: %s", err.Error()) | ||
| } | ||
| errChan <- err | ||
| }(msg) |
Check notice
Code scanning / CodeQL
Spawning a Go routine Note
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #518 +/- ##
===========================================
+ Coverage 61.57% 61.60% +0.03%
===========================================
Files 128 128
Lines 9512 9520 +8
===========================================
+ Hits 5857 5865 +8
Misses 3115 3115
Partials 540 540
|
| @@ -29,16 +30,26 @@ import ( | |||
| // Failure in RecheckTx will prevent tx to be included into block, especially when CheckTx succeed, in which case user | |||
| // won't see the error message. | |||
| func VerifyEthSig(tx sdk.Tx, signer ethtypes.Signer) error { | |||
| errChan := make(chan error, len(tx.GetMsgs())) | |||
| for _, msg := range tx.GetMsgs() { | |||
There was a problem hiding this comment.
@yihuang could reduce from 2s to 0.2s for 3k msgs when parallel
There was a problem hiding this comment.
Can you do this conditionally, so we can avoid extra overhead for normal cases where tx has single msg.
There was a problem hiding this comment.
should be same for small batch, but I only filter out when CheckTx > 1s
There was a problem hiding this comment.
I mean at least for tx that has a single msg, this change is pure overhead, no benifit.
| @@ -29,16 +30,26 @@ import ( | |||
| // Failure in RecheckTx will prevent tx to be included into block, especially when CheckTx succeed, in which case user | |||
| // won't see the error message. | |||
| func VerifyEthSig(tx sdk.Tx, signer ethtypes.Signer) error { | |||
| errChan := make(chan error, len(tx.GetMsgs())) | |||
Closes: #XXX
Description
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)