core: avoid big.Int in modexp required gas computation #16396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yperbasis
requested changes
Aug 4, 2025
chfast
force-pushed
the
chfast/modexp_gas_opt
branch
from
0ff8a01 to
ded9d74
Compare
yperbasis
requested changes
Aug 13, 2025
| if !allZero(header[0:28]) || !allZero(header[32:32+28]) || !allZero(header[64:64+28]) { | ||
| // Except for the case where both base and mod are zeros. | ||
| if allZero(header[0:32]) && allZero(header[64:96]) { | ||
| return minGas |
Member
There was a problem hiding this comment.
This isn't right for Fusaka because MultComplexity won't be 0 even when both base and mod are zeros.
Member
Author
There was a problem hiding this comment.
The issue is hidden (i.e. not testable) because of the EIP-7823 (modexp length limits) applied later in the Run() function. In other words: we compute incorrect (by EIP-7883) gas cost here, but it does not matter because the execution ends with an error (by EIP-7823).
This is an argument to move EIP-7823 implementation from Run() to RequiredGas().
For now, I'm correcting the implementation.
Optimize the gas calculation for the modexp precompile by avoiding big.Int. The implementation is ported from evmone: https://github.com/ipsilon/evmone/blob/2cbfad3d5eda9bd920f1174088fe48d41f9edcd7/test/state/precompiles.cpp#L102 This has been additionally tested by executing all tests from the precompiles fuzzing corpus. However, this implementation has not been directly fuzzed.
chfast
force-pushed
the
chfast/modexp_gas_opt
branch
from
ded9d74 to
1131587
Compare
yperbasis
approved these changes
Aug 15, 2025
yperbasis
reviewed
Aug 15, 2025
| for i := 0; i < len(expHeadExplicitBytes); i++ { | ||
| expByte := expHeadExplicitBytes[i] | ||
| if expByte != 0 { | ||
| expTopByteBitWidth := 8 - uint32(bits.LeadingZeros8(expByte)) |
Member
There was a problem hiding this comment.
I'm wondering whether it is more performant to pad the input to 32 bytes and use uint256.BitLen instead of going byte by byte
Member
Author
There was a problem hiding this comment.
In this case this made it slower so I'm not going to change it now. However, I used uint256 for initial lengths and the performance has improved by up to 4% in the full benchmarks.
Member
Author
|
Final benchmarks: |
NazariiDenha
pushed a commit
that referenced
this pull request
Oct 24, 2025
Optimize the gas calculation for the modexp precompile by avoiding big.Int. The implementation is ported from evmone: https://github.com/ipsilon/evmone/blob/2cbfad3d5eda9bd920f1174088fe48d41f9edcd7/test/state/precompiles.cpp#L102 This has been additionally tested but executing all tests from the precompiles fuzzing corpus. However, this implementation has not been directly fuzzed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimize the gas calculation for the modexp precompile by avoiding big.Int. The implementation is ported from evmone: https://github.com/ipsilon/evmone/blob/2cbfad3d5eda9bd920f1174088fe48d41f9edcd7/test/state/precompiles.cpp#L102
This has been additionally tested but executing all tests from the precompiles fuzzing corpus. However, this implementation has not been directly fuzzed.