-
Notifications
You must be signed in to change notification settings - Fork 8
fix(provider): calculate range from bounds #23
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
Conversation
When using boha provider without explicit --range, the solver was incorrectly using puzzle metadata (e.g., 30 bits for a 30-bit puzzle) instead of calculating the actual search space from [start, end] bounds. For puzzle 30, the range is [2^29, 2^30-1], which is 2^29 values (29 bits), not 30 bits. This caused validation to fail with "exceeds puzzle maximum". Now calculates actual bits needed using BigUint::bits() on the range size, and validates start is within [provider_start, provider_end] bounds. Co-Authored-By: Aei <aei@oad.earth>
Greptile OverviewGreptile SummaryThis PR refactors the range_bits calculation to compute from provider bounds instead of using puzzle metadata. When a user doesn't provide Key Changes:
Critical Issue Found: The fix is straightforward: remove the Confidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant resolve_params
participant calculate_range_bits
participant validate_search_bounds
participant Solver
User->>resolve_params: Args with target + optional range
alt User provides --range
resolve_params->>validate_search_bounds: Validate user range
validate_search_bounds->>validate_search_bounds: Check start >= provider_start
validate_search_bounds->>validate_search_bounds: Check start + 2^range_bits <= provider_end
validate_search_bounds-->>resolve_params: OK or Error
else No --range provided
resolve_params->>calculate_range_bits: Calculate from provider bounds
calculate_range_bits->>calculate_range_bits: Parse start, provider_start, provider_end
calculate_range_bits->>calculate_range_bits: Validate start within bounds
calculate_range_bits->>calculate_range_bits: Calculate: (end - start + 1).bits()
Note over calculate_range_bits: BUG: +1 causes off-by-one<br/>when end - start = 2^n - 1
calculate_range_bits-->>resolve_params: range_bits
end
resolve_params-->>User: ResolvedParams
User->>Solver: Start search with range_bits
|
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.
1 file reviewed, 1 comment
| let range_size = &provider_end_val - &start_val + BigUint::from(1u32); | ||
| let bits = range_size.bits() as u32; |
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.
[P0] Off-by-one error in range calculation
Adding 1 before calling bits() causes the function to return one bit too many when provider_end - start equals 2^n - 1.
Example bug scenario:
start = 0x20000000,provider_end = 0x3fffffff(which is2^30 - 1)- Current:
range_size = 0x1fffffff + 1 = 0x20000000,bits() = 30 - The solver would search
[0x20000000, 0x60000000](30 bits) - This exceeds
provider_end = 0x3fffffffand gets rejected by validation
Fix: Remove the + 1 since BigUint::bits() on the difference already gives the correct bit width:
| let range_size = &provider_end_val - &start_val + BigUint::from(1u32); | |
| let bits = range_size.bits() as u32; | |
| let range_size = &provider_end_val - &start_val; | |
| let bits = range_size.bits() as u32; |
The bits() method returns ceil(log2(value + 1)) for the maximum offset, which correctly calculates the minimum bits needed to represent the range [start, provider_end].
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib.rs
Line: 225:226
Comment:
**[P0] Off-by-one error in range calculation**
Adding 1 before calling `bits()` causes the function to return one bit too many when `provider_end - start` equals `2^n - 1`.
**Example bug scenario:**
- `start = 0x20000000`, `provider_end = 0x3fffffff` (which is `2^30 - 1`)
- Current: `range_size = 0x1fffffff + 1 = 0x20000000`, `bits() = 30`
- The solver would search `[0x20000000, 0x60000000]` (30 bits)
- This exceeds `provider_end = 0x3fffffff` and gets rejected by validation
**Fix:** Remove the `+ 1` since `BigUint::bits()` on the difference already gives the correct bit width:
```suggestion
let range_size = &provider_end_val - &start_val;
let bits = range_size.bits() as u32;
```
The `bits()` method returns `ceil(log2(value + 1))` for the maximum offset, which correctly calculates the minimum bits needed to represent the range `[start, provider_end]`.
How can I resolve this? If you propose a fix, please make it concise.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.
2 issues found across 1 file
Confidence score: 3/5
- Incorrect range_bits calculation in
src/lib.rscan double the search space for power-of-two ranges, so users may see slower or misleading results until it is fixed. - Auto-calculated range_bits isn’t validated against provider_end in
src/lib.rs, risking accept/reject mismatches when ranges overflow the provider’s bounds. - Pay close attention to
src/lib.rs– wrong range_bits math and missing range validation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/lib.rs">
<violation number="1" location="src/lib.rs:159">
P2: Missing validation when range is auto-calculated. When user doesn't provide --range, the calculated range_bits is not validated to ensure start + 2^range_bits doesn't exceed provider_end, creating inconsistent validation behavior.</violation>
<violation number="2" location="src/lib.rs:226">
P1: Incorrect calculation of range_bits from provider bounds. Using `range_size.bits()` instead of a proper ceil(log2) calculation causes off-by-one error for power-of-2 ranges, making the search space up to 2x larger than intended and potentially exceeding provider bounds.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| Ok(()) | ||
|
|
||
| let range_size = &provider_end_val - &start_val + BigUint::from(1u32); | ||
| let bits = range_size.bits() as u32; |
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.
P1: Incorrect calculation of range_bits from provider bounds. Using range_size.bits() instead of a proper ceil(log2) calculation causes off-by-one error for power-of-2 ranges, making the search space up to 2x larger than intended and potentially exceeding provider bounds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 226:
<comment>Incorrect calculation of range_bits from provider bounds. Using `range_size.bits()` instead of a proper ceil(log2) calculation causes off-by-one error for power-of-2 ranges, making the search space up to 2x larger than intended and potentially exceeding provider bounds.</comment>
<file context>
@@ -182,20 +182,63 @@ fn resolve_params(args: &Args) -> anyhow::Result<ResolvedParams> {
- Ok(())
+
+ let range_size = &provider_end_val - &start_val + BigUint::from(1u32);
+ let bits = range_size.bits() as u32;
+
+ Ok(bits)
</file context>
| let bits = range_size.bits() as u32; | |
| // For range_bits, we need: 2^range_bits >= range_size | |
| // This is ceil(log2(range_size)), which equals (range_size - 1).bits() for range_size > 0 | |
| let bits = if range_size == BigUint::from(0u32) { | |
| 0 | |
| } else { | |
| (range_size - BigUint::from(1u32)).bits() as u32 | |
| }; |
| None => { | ||
| // No user range - calculate from provider bounds if available | ||
| calculate_range_bits_from_provider(&start_str, pr)? | ||
| } |
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.
P2: Missing validation when range is auto-calculated. When user doesn't provide --range, the calculated range_bits is not validated to ensure start + 2^range_bits doesn't exceed provider_end, creating inconsistent validation behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 159:
<comment>Missing validation when range is auto-calculated. When user doesn't provide --range, the calculated range_bits is not validated to ensure start + 2^range_bits doesn't exceed provider_end, creating inconsistent validation behavior.</comment>
<file context>
@@ -150,18 +150,18 @@ fn resolve_params(args: &Args) -> anyhow::Result<ResolvedParams> {
- (Some(user_range), None) => user_range,
- (None, Some(provider_range)) => provider_range,
- (None, None) => 32,
+ None => {
+ // No user range - calculate from provider bounds if available
+ calculate_range_bits_from_provider(&start_str, pr)?
</file context>
| None => { | |
| // No user range - calculate from provider bounds if available | |
| calculate_range_bits_from_provider(&start_str, pr)? | |
| } | |
| None => { | |
| // No user range - calculate from provider bounds if available | |
| let calculated_range = calculate_range_bits_from_provider(&start_str, pr)?; | |
| validate_search_bounds(&start_str, calculated_range, pr)?; | |
| calculated_range | |
| } |
Summary
Fixes incorrect range_bits calculation when using boha provider without explicit --range flag. The solver now calculates actual search space from provider bounds instead of using puzzle metadata.
Closes #22
Changes
calculate_range_bits_from_provider()function that computes actual bits needed from(end - start + 1)usingBigUint::bits()startis within[provider_start, provider_end]boundsvalidate_range_override()functionresolve_params()to call calculation function when user doesn't provide--rangeTesting
Verified with multiple scenarios:
kangaroo --target boha:b1000/30- works, finds key0x3d94cd64kangaroo --target boha:b1000/30 --range 31- correctly rejects (exceeds maximum)kangaroo --target boha:b1000/30 --start 30000000- works, auto-calculates 29 bitskangaroo --target boha:b1000/30 --start 10000000- correctly rejects (below minimum)kangaroo --target boha:b1000/30 --start 50000000- correctly rejects (above maximum)Co-Authored-By: Aei aei@oad.earth