Repository navigation
docs(skills): correct outdated MASM guidance #3849
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,37 @@ | ||
| --- | ||
| name: felt-construction | ||
| description: Use when constructing a `Felt` from a numeric value in Rust — avoid silently truncating values that may exceed the field modulus. | ||
| description: Use when constructing a `Felt` from a numeric value in Rust — use checked construction unless the canonical bound is already proved. | ||
| --- | ||
|
|
||
| # Felt Construction From Untrusted Numeric Inputs | ||
|
|
||
| ## Rule | ||
|
|
||
| Do not call `Felt::new(x)` when `x` could exceed the field modulus. `Felt::new` silently truncates oversized values, which produces a valid-looking `Felt` that no longer equals the original input — a classic source of hard-to-attribute bugs. | ||
| `Felt::new(x)` is checked and returns `Result`, rejecting values greater than or equal to `Felt::ORDER`. Use it when a `u64` input may exceed the field modulus. | ||
|
|
||
| Use one of: | ||
|
|
||
| - `Felt::from(x)` where `x` is a `u32` or smaller (infallible). | ||
| - `Felt::try_from(x)` for `u64`-and-larger inputs, returning `Result`. | ||
| - An explicit `assert!(x < Felt::MODULUS)` before `Felt::new(x)` if you have already proven the bound. | ||
| - `Felt::new(x)` or `Felt::try_from(x)` for `u64` inputs; both return `Result` and check the bound. | ||
| - `Felt::new_unchecked(x)` only when `x < Felt::ORDER` has already been proved. | ||
|
|
||
| ## Why | ||
|
|
||
| The field modulus sits just below `2^64`, so `Felt::new` truncates only for a narrow band of large values — most tests pass and production hits the bad input as a value mismatch far from the call. `Felt::from(u32)` cannot truncate and `Felt::try_from` forces the bound check. | ||
| The field modulus sits just below `2^64`, so out-of-range inputs occupy a narrow band that tests can miss. Checked construction makes those inputs explicit errors; `new_unchecked` skips that protection. | ||
|
|
||
| ## Examples | ||
|
|
||
| ```rust | ||
| // Good: u32 input, infallible conversion | ||
| let f = Felt::from(slot_index as u32); | ||
|
|
||
| // Good: untrusted u64 input, checked conversion | ||
| let f = Felt::try_from(user_value).map_err(|_| Error::FeltOverflow)?; | ||
| // Good: untrusted u64 input, checked conversion returning Result | ||
| let f = Felt::new(user_value).map_err(|_| Error::FeltOverflow)?; | ||
|
|
||
| // Bad: silent truncation on any value >= MODULUS | ||
| let f = Felt::new(user_value); | ||
| // Good: unchecked construction only after proving the canonical bound | ||
| assert!(bounded_value < Felt::ORDER); | ||
| let f = Felt::new_unchecked(bounded_value); | ||
|
|
||
| // Bad: unchecked construction on an untrusted value | ||
| let f = Felt::new_unchecked(user_value); | ||
| ``` | ||
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,39 +1,41 @@ | ||
| --- | ||
| name: u32-assert-before-u32-ops | ||
| description: Use when writing MASM `u32*` instructions on values from user input or untrusted sources — ensure the operands are valid u32s first. | ||
| description: Use when writing MASM `u32*` instructions whose operands must already fit in 32 bits, especially for user input or untrusted values. | ||
| --- | ||
|
|
||
| # Validate u32 Operands Before u32 Instructions | ||
| # Validate Required u32 Operands | ||
|
|
||
| ## Rule | ||
|
|
||
| MASM's `u32*` instructions assume their operands are valid `u32` values (i.e. fit in 32 bits). Operating on a non-u32 value silently produces garbage or traps with a generic message. | ||
| Most MASM `u32*` arithmetic instructions require operands that fit in 32 bits. Their behavior on an out-of-range operand is undefined, so the executor may trap and the resulting proof is not valid. | ||
|
|
||
| Before applying any `u32*` instruction to a value that is not already known to be a valid u32 (e.g. it came from the stack as input, was read from memory, or arose from a non-u32 arithmetic op), assert the bound: | ||
| Before applying a `u32*` instruction whose documented precondition requires valid u32 operands, assert the bound of any operand that is not already known-valid (e.g. it came from the stack as input, was read from memory, or arose from a non-u32 arithmetic op): | ||
|
|
||
| ```masm | ||
| u32assert # one value | ||
| u32assert2 # two top values | ||
| u32assert4 # four top values | ||
| u32assertw # one word (four values) | ||
| ``` | ||
|
|
||
| If the operand is already known-valid (just produced by another `u32*` op, or a value loaded from a slot whose layout is u32 by construction), skip the assert. | ||
| If the operand is already known-valid (just produced as a valid-u32 output of another operation, or loaded from a slot whose layout is u32 by construction), skip the assert. | ||
|
|
||
| `u32test`, `u32testw`, `u32cast`, and `u32split` accept arbitrary field values, so they do not require a prior u32 assertion. | ||
|
|
||
| ## Why | ||
|
|
||
| `u32*` instructions are tuned for the precondition that operands fit in 32 bits, and the VM does not check it for you. Skipping `u32assert*` lets a non-u32 input silently produce a wrong result or trap uninformatively; the assert gives the bug a named failure mode. | ||
| Arithmetic instructions do not check the u32 precondition for you. An explicit assertion prevents undefined behavior and gives an out-of-range input a clear failure mode. | ||
|
|
||
| ## Examples | ||
|
|
||
| ```masm | ||
| # Good: assert u32 before the u32 op | ||
| u32assert.err=ERR_VALUE_NOT_U32 | ||
| u32add | ||
| # Good: assert both operands before producing one wrapping sum | ||
| u32assert2.err=ERR_VALUES_NOT_U32 | ||
| u32wrapping_add | ||
|
|
||
| # Good: both operands at once | ||
| u32assert2.err=ERR_VALUES_NOT_U32 | ||
| u32lt | ||
|
|
||
| # Bad: u32 op on untrusted input | ||
| u32add # one operand could be >2^32; silently wraps or traps | ||
| u32wrapping_add # either operand could be greater than or equal to 2^32 | ||
| ``` |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.