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

big_mod_exp syscall: exclude inputs of numbers larger than 4096-bits #31859

Closed
wants to merge 3 commits into from

Conversation

valiksinev
Copy link
Contributor

Problem

The changes related to Big integer modular exponentiation (EIP-198). The calculation is implemented as syscall.

The formula that was used to approximate the compute budgets for large number exponentiation seems to under-approximate the costs for exponentiation of very large numbers (i.e. >>1000 byte numbers). This could lead to a negative impact on the network if the syscall was to be invoked with very large numbers.
In practice, it seems like any normal/non-adversarial use of the syscall will be on numbers of size less than or equal to 4096-bits (i.e. for RSA-4096)

Summary of Changes

Inside syscall all inputs are limited by value 4069 bits (512 bytes).
If the length is exceeded, an error SyscallError::InvalidLength returned.

@mergify mergify bot added community Community contribution need:merge-assist labels May 29, 2023
@mergify mergify bot requested a review from a team May 29, 2023 18:58
@t-nelson t-nelson requested a review from samkim-crypto May 31, 2023 18:08
@t-nelson
Copy link
Contributor

thanks! mind adding some test cases to cover this new logic?

@mvines mvines added the CI Pull Request is ready to enter CI label Jun 5, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Jun 5, 2023
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Merging #31859 (a2b73c2) into master (db8f118) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #31859     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         752      752             
  Lines      206751   206753      +2     
=========================================
- Hits       169427   169401     -26     
- Misses      37324    37352     +28     

@mvines mvines added the v1.16 PRs that should be backported to v1.16 label Jun 9, 2023
@mvines
Copy link
Member

mvines commented Jun 12, 2023

I've moved #29882 off the activation schedule, and put it into https://github.com/solana-labs/solana/wiki/Feature-Gate-Activation-Schedule#features-are-blocked as any activation of that feature cannot happen until this PR is merged and backported to v1.16

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 27, 2023
@derrekr
Copy link
Contributor

derrekr commented Jul 2, 2023

Ping @valiksinev do you have a test case yet?

@github-actions github-actions bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 3, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 17, 2023
@mvines
Copy link
Member

mvines commented Jul 24, 2023

The changes in this PR were merged via #32520

@mvines mvines closed this Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution need:merge-assist stale [bot only] Added to stale content; results in auto-close after a week. v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants