-
Notifications
You must be signed in to change notification settings - Fork 274
Add a modexp example guest program + benchmark #1067 #1119
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
base: main
Are you sure you want to change the base?
Conversation
examples/modexp/guest/src/lib.rs
Outdated
| let mut out: u128 = 0; | ||
| for _ in 0..num_iters { | ||
| out = mod_pow(base, exp, modulus); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation of modexp_chain doesn't create a chain of operations - it repeatedly calculates the same modular exponentiation num_iters times without using the result of the previous calculation as input to the next one.
To create a proper chain where each result feeds into the next calculation, consider modifying the implementation to:
let mut out: u128 = base;
for _ in 0..num_iters {
out = mod_pow(out, exp, modulus);
}This way, each iteration will use the previous result as the base for the next modular exponentiation, creating a true chain of operations.
| let mut out: u128 = 0; | |
| for _ in 0..num_iters { | |
| out = mod_pow(base, exp, modulus); | |
| } | |
| let mut out: u128 = base; | |
| for _ in 0..num_iters { | |
| out = mod_pow(out, exp, modulus); | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| fn mul_mod(mut a: u128, mut b: u128, m: u128) -> u128 { | ||
| let mut res: u128 = 0; | ||
| a %= m; | ||
| while b > 0 { | ||
| if (b & 1) == 1 { | ||
| res = (res + a) % m; | ||
| } | ||
| a = (a << 1) % m; | ||
| b >>= 1; | ||
| } | ||
| res | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical overflow bug in mul_mod: When modulus > u128::MAX / 2 + 1, the operations res + a (line 11) and a << 1 (line 13) can overflow before the modulo is applied, causing either:
- Panic in debug builds
- Silent incorrect results in release builds due to wrapping
Example: If modulus = (u128::MAX / 2) + 2, then res and a can each be up to modulus - 1, making res + a > u128::MAX.
Fix by using checked arithmetic:
fn mul_mod(mut a: u128, mut b: u128, m: u128) -> u128 {
if m == 0 {
panic!("modulus cannot be zero");
}
let mut res: u128 = 0;
a %= m;
while b > 0 {
if (b & 1) == 1 {
res = (res + a) % m;
}
a = (a + a) % m; // Use addition instead of shift to avoid overflow
b >>= 1;
}
res
}Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| fn mod_pow(mut base: u128, mut exp: u128, m: u128) -> u128 { | ||
| let mut result: u128 = 1 % m; | ||
| base %= m; | ||
| while exp > 0 { | ||
| if (exp & 1) == 1 { | ||
| result = mul_mod(result, base, m); | ||
| } | ||
| base = mul_mod(base, base, m); | ||
| exp >>= 1; | ||
| } | ||
| result | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulus validation missing: The mod_pow function doesn't validate that modulus != 0. Division by zero will occur on line 20 (1 % m) and line 21 (base %= m) if modulus is 0, causing a panic.
Add validation:
fn mod_pow(mut base: u128, mut exp: u128, m: u128) -> u128 {
if m == 0 {
panic!("modulus cannot be zero");
}
// rest of implementation
}| fn mod_pow(mut base: u128, mut exp: u128, m: u128) -> u128 { | |
| let mut result: u128 = 1 % m; | |
| base %= m; | |
| while exp > 0 { | |
| if (exp & 1) == 1 { | |
| result = mul_mod(result, base, m); | |
| } | |
| base = mul_mod(base, base, m); | |
| exp >>= 1; | |
| } | |
| result | |
| } | |
| fn mod_pow(mut base: u128, mut exp: u128, m: u128) -> u128 { | |
| if m == 0 { | |
| panic!("modulus cannot be zero"); | |
| } | |
| let mut result: u128 = 1 % m; | |
| base %= m; | |
| while exp > 0 { | |
| if (exp & 1) == 1 { | |
| result = mul_mod(result, base, m); | |
| } | |
| base = mul_mod(base, base, m); | |
| exp >>= 1; | |
| } | |
| result | |
| } | |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
- Use previous modexp result as base for next iteration - Matches sha2-chain pattern where each result feeds into next computation - Fixes issue where iterations were not chained together
This PR adds a new guest program
modexp-chainthat performs repeated modularexponentiation operations similar to EVM
modexp.✔ Configurable bitlengths for base, modulus, exponent
✔ Configurable number of iterations (similar to sha2-chain)
✔ Added corresponding benchmark entry in
e2e_profiling.rs✔ Uses fast modular exponentiation
✔ Randomized inputs for benchmarking
This closes issue #1067.