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.

Sign up for GitHub

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

Jump to bottom

[Fix] Limit program size to 100KBs #2384

Merged
merged 5 commits into from
Mar 10, 2024
Merged

[Fix] Limit program size to 100KBs #2384

merged 5 commits into from
Mar 10, 2024

Conversation

vicsn
Copy link
Contributor

@vicsn vicsn commented Mar 5, 2024 •
edited
Loading

Motivation

Closes https://github.com/AleoHQ/snarkOS/issues/3141

Introduces:

  • MAX_PROGRAM_SIZE
  • MAX_STRUCTS
  • MAX_RECORDS
  • MAX_CLOSURES

Some notes:

  • MAX_STRUCTS, MAX_CLOSURES and MAX_RECORDS don't add security but don't hurt either and were requested in the issue above. Even existing parameters like MAX_INSTRUCTIONS can't be fully utilized as it will now quickly lead to programs which surpass the size limit.
  • I did not add custom error types to test on to keep it simple and because we don't use those anywhere yet.

Can't add you as reviewer do doing it manually: @miazn

Test Plan

Added extensive unit test

@vicsn vicsn requested review from ljedrz and d0cd March 5, 2024 23:35
howardwu
howardwu suggested changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@howardwu howardwu left a comment

Choose a reason for hiding this comment

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

We have N::MAX_IMPORTS. Can we also enforce that size here?

howardwu
howardwu reviewed Mar 5, 2024
View reviewed changes
console/network/src/lib.rs Outdated Show resolved Hide resolved
@vicsn vicsn mentioned this pull request Mar 6, 2024
Closed
@vicsn vicsn removed the request for review from ljedrz March 6, 2024 10:51
@vicsn vicsn requested a review from howardwu March 6, 2024 19:16
@howardwu
Copy link
Contributor

howardwu commented Mar 8, 2024

We have N::MAX_IMPORTS. Can we also enforce that size here?

This issue is still unaddressed @vicsn

@vicsn
Copy link
Contributor Author

vicsn commented Mar 9, 2024 •
edited
Loading

MAX_IMPORTS

The limit was already enforced. Test was added here: 81e51a8

Let me know if you have something else in mind.

@howardwu
Copy link
Contributor

I missed that commit, thanks @vicsn

@howardwu howardwu merged commit faffd15 into mainnet-staging Mar 10, 2024
77 of 78 checks passed
@howardwu howardwu deleted the limit_program_size branch March 10, 2024 20:55
@howardwu howardwu changed the title Limit program size and add unit test [Fix] Limit program size to 100KBs Mar 10, 2024
@howardwu howardwu mentioned this pull request Mar 10, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howardwu howardwu howardwu approved these changes

@d0cd d0cd Awaiting requested review from d0cd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add limits on program struct definitions
2 participants
@vicsn @howardwu