fix(managed-wallet): block lease creation on absurdly-overpriced bids#3341
Draft
baktun14 wants to merge 3 commits into
Draft
fix(managed-wallet): block lease creation on absurdly-overpriced bids#3341baktun14 wants to merge 3 commits into
baktun14 wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesManaged-Wallet Lease Bid Price Guard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3341 +/- ##
==========================================
- Coverage 69.60% 68.28% -1.32%
==========================================
Files 1087 1000 -87
Lines 26669 24415 -2254
Branches 6415 5971 -444
==========================================
- Hits 18562 16671 -1891
+ Misses 7110 6779 -331
+ Partials 997 965 -32
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.spec.ts (1)
20-145: ⚡ Quick winAdd a precision-boundary test case for very large amounts.
Given this guard’s security role, add at least one test around
Number.MAX_SAFE_INTEGER-scale amounts to ensure warn/block decisions remain correct at high values and prevent regressions in threshold math.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.spec.ts` around lines 20 - 145, Add a new test case within the "relative limit" describe block that validates warn/block behavior with amounts near Number.MAX_SAFE_INTEGER scale. The test should use pricedBid to create bids with very large numbers (similar to the existing 9_519_658 patterns but significantly larger, approaching MAX_SAFE_INTEGER thresholds) and verify that the service correctly applies the warn and block multiplier thresholds at these precision boundaries. This ensures threshold math calculations remain accurate with extreme values and prevents future regressions in the bid price comparison logic, particularly for the ratio calculations used in the relative limit checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/billing/config/env.config.ts`:
- Around line 59-60: The schema currently validates
MANAGED_WALLET_BID_PRICE_WARN_MULTIPLIER and
MANAGED_WALLET_BID_PRICE_BLOCK_MULTIPLIER independently as positive numbers,
which allows the warn multiplier to exceed the block multiplier and break the
intended warn-before-block behavior. Add a cross-field validation constraint
using Zod's refine method on the schema object to ensure that
MANAGED_WALLET_BID_PRICE_WARN_MULTIPLIER is always less than or equal to
MANAGED_WALLET_BID_PRICE_BLOCK_MULTIPLIER, with an appropriate error message
describing the constraint violation.
In `@apps/api/src/billing/lib/lease-messages/lease-messages.ts`:
- Around line 52-54: The find predicate used to locate the accepted bid in the
orderBids array is missing the owner field from the identity comparison. The
predicate currently checks gseq, oseq, provider, and bseq but omits bidId.owner,
which is part of the bid identity key. Add b.bid.id.owner === bidId.owner to the
find predicate alongside the existing identity field comparisons to ensure the
accepted bid is correctly matched using the complete identity.
In
`@apps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.ts`:
- Around line 52-55: Replace the `Number()` conversion of
`accepted.bid.price.amount` with `BigInt()` to preserve precision for large
blockchain integers that exceed JavaScript's safe integer range. Update the
validation check for `acceptedPrice` to work with BigInt comparisons instead of
Number comparisons. Additionally, ensure that the peer prices and all threshold
ratio checks at lines 72-77 also use BigInt arithmetic instead of numeric
arithmetic to maintain consistency and prevent precision loss in these
security-critical bid validation comparisons.
---
Nitpick comments:
In
`@apps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.spec.ts`:
- Around line 20-145: Add a new test case within the "relative limit" describe
block that validates warn/block behavior with amounts near
Number.MAX_SAFE_INTEGER scale. The test should use pricedBid to create bids with
very large numbers (similar to the existing 9_519_658 patterns but significantly
larger, approaching MAX_SAFE_INTEGER thresholds) and verify that the service
correctly applies the warn and block multiplier thresholds at these precision
boundaries. This ensures threshold math calculations remain accurate with
extreme values and prevents future regressions in the bid price comparison
logic, particularly for the ratio calculations used in the relative limit
checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06b24255-fe6d-46c0-9887-8346368de292
📒 Files selected for processing (7)
apps/api/src/billing/config/env.config.tsapps/api/src/billing/lib/lease-messages/lease-messages.tsapps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.spec.tsapps/api/src/billing/services/lease-bid-price-guard/lease-bid-price-guard.service.tsapps/api/src/billing/services/managed-signer/managed-signer.service.spec.tsapps/api/src/billing/services/managed-signer/managed-signer.service.tsapps/api/src/billing/services/trial-validation/trial-validation.service.ts
Managed-wallet lease creation accepted the winning bid without validating its price-per-block. Add a server-side guardrail (LeaseBidPriceGuardService) in the managed-signer validation block: it refetches the order's bids and refuses leases whose price-per-block is far above the cheapest competing bid for the same order, with an absolute per-denom ceiling for the sole-bidder case. Moderately-high prices are allowed but logged at warn; blocks emit a structured error log. All thresholds are configurable and the guard can be disabled. Extracts shared bid resolution into lease-messages.ts, reused by trial validation.
Address review: validate WARN_MULTIPLIER <= BLOCK_MULTIPLIER in the config schema so a misconfiguration can't silently disable the warn tier, and include the deployment owner in the accepted-bid match so a lease referencing a foreign owner is rejected.
baktun14
force-pushed
the
fix/billing-block-overpriced-lease-bids
branch
from
28bcf57 to
fc31e75
Compare
Deployments are uact-denominated, so the per-denom absolute ceiling only needs uact. Remove the unused UAKT config key and the uakt branch; the relative check is denom-agnostic and unaffected. A non-uact bid simply has no absolute cap and relies on the relative check.
stalniy
approved these changes
Jun 22, 2026
| const { id } = accepted.bid; | ||
|
|
||
| // Cheapest bid for the same order (gseq + oseq). Needs at least one competing bid to be meaningful. | ||
| const peerPrices = orderBids |
Contributor
There was a problem hiding this comment.
nitpick: can be achieved with Iterator.from(orderBids) more efficiently
| * Two checks: | ||
| * - Relative: block when the accepted bid is far above the cheapest competing bid for the same | ||
| * order (apples-to-apples — same resources, denom and moment). Only applied when peers exist. | ||
| * - Absolute: block when the per-block price exceeds a configured per-denom ceiling. Covers the |
Contributor
There was a problem hiding this comment.
thought: overloaded term "block" - block when per-block hard to reason about. probably you wanted to say reject when the per-block...
| ); | ||
| } | ||
|
|
||
| #block(reason: "relative" | "absolute", accepted: Bid, userWallet: UserWalletOutput, details: Record<string, unknown>, message: string): never { |
Contributor
There was a problem hiding this comment.
**suggestion: ** rename to forbid or reject
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why
Managed-wallet lease creation accepted the winning bid without validating its price-per-block. This
adds a guardrail so an abnormally-priced bid is refused before the lease is created, with alerting
for visibility.
Fixes CON-554
What
Server-side price guardrail in the managed-wallet lease-creation path, evaluated before the
transaction is broadcast:
same order; warn-only (logged, not blocked) in a configurable band below the block threshold.
compare against.
MANAGED_WALLET_BID_PRICE_*, enabled by default with a kill-switch.Tests
Unit coverage for the guard (relative / absolute / warn / allow, sole-bidder, disabled, multi-order,
unresolved bid) plus a managed-signer wiring test ensuring a refused bid never reaches broadcast.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests