Skip to content

Conversation

@chfast
Copy link
Member

@chfast chfast commented Mar 11, 2025

Before looking up an answer for the expmod precompile in stub,
check if the input has at least the bytes encoding the value lengths.
If not or if the modulus length is 0 then the answer is trivially
the sequence of zero bytes of the length of the modulus.

@chfast chfast added the precompiles Related to EVM precompiles label Mar 11, 2025
@chfast chfast requested review from pdobacz and rodiazet March 11, 2025 21:45
@codecov
Copy link

codecov bot commented Mar 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.54%. Comparing base (d84b471) to head (209afb2).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1163      +/-   ##
==========================================
- Coverage   94.54%   94.54%   -0.01%     
==========================================
  Files         168      168              
  Lines       18254    18245       -9     
==========================================
- Hits        17259    17250       -9     
  Misses        995      995              
Flag Coverage Δ
eof_execution_spec_tests 20.55% <80.00%> (-1.92%) ⬇️
ethereum_tests 28.25% <100.00%> (-0.04%) ⬇️
ethereum_tests_silkpre 20.99% <ø> (+0.01%) ⬆️
execution_spec_tests 20.92% <100.00%> (-0.04%) ⬇️
unittests 89.86% <0.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
test/state/precompiles.cpp 98.98% <100.00%> (+0.01%) ⬆️
test/state/precompiles_stubs.cpp 99.41% <ø> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chfast chfast force-pushed the precompiles/expmod_stub branch from ba69dd1 to ffa5d86 Compare March 12, 2025 09:54
Copy link
Member

@rodiazet rodiazet left a comment

Choose a reason for hiding this comment

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

Looks good. As a comment I would also add less trivial check. In case when input size is less or equal than 96+base_len+exp_len it such a case modulus is also 0. So the result is 0 too. This can be added in a separated PR.

@chfast
Copy link
Member Author

chfast commented Mar 12, 2025

Looks good. As a comment I would also add less trivial check. In case when input size is less or equal than 96+base_len+exp_len it such a case modulus is also 0. So the result is 0 too. This can be added in a separated PR.

This is correct, but I'm not sure we want to go this path. I.e. we don't want do handle degenerated cases separately unless necessary.

Before looking up an answer for the expmod precompile in stub,
check if the input has at least the bytes encoding the value lengths.
If not or if the modulus length is 0 then the answer is trivially
the sequence of zero bytes of the length of the modulus.
@chfast chfast force-pushed the precompiles/expmod_stub branch from ffa5d86 to 209afb2 Compare March 12, 2025 11:10
@chfast chfast enabled auto-merge (squash) March 12, 2025 11:10
@chfast chfast merged commit c2b94bc into master Mar 12, 2025
23 checks passed
@chfast chfast deleted the precompiles/expmod_stub branch March 12, 2025 11:31
gumb0 pushed a commit that referenced this pull request Mar 20, 2025
Before looking up an answer for the expmod precompile in stub,
check if the input has at least the bytes encoding the value lengths.
If not or if the modulus length is 0 then the answer is trivially
the sequence of zero bytes of the length of the modulus.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

precompiles Related to EVM precompiles

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants