Nnennaokoye/acbu smart contract#455
Conversation
…value size limit overflow
…t spam and ledger load
|
@nnennaokoye Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
More reviews will be available in 37 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a shared ChangesContract helpers, rate-limiting, and shared utilities
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@acbu_reserve_tracker/src/lib.rs`:
- Around line 137-148: The cooldown in the reserve verification path is
contract-wide and permissionless, which lets any caller consume the shared slot
and block everyone else. Update the verify flow in
acbu_reserve_tracker/src/lib.rs around the last_verify_call check so the
cooldown is enforced per caller or only for authorized callers, and use a
distinct rate-limit response instead of returning false for both cooldown and
insufficient reserves. Use the existing verify entrypoint and
DATA_KEY.last_verify_call logic to locate the fix.
In `@acbu_savings_vault/src/lib.rs`:
- Around line 495-505: The `limit` contract in the deposit-lots pagination API
is inconsistent: the doc comment says `0` means no limit, but the implementation
coerces `0` to `100`. Update the `get_deposit_lots` docs and any related
pagination docs/comments to match the actual behavior, or change the `limit`
handling so `0` באמת means unbounded; keep the wording aligned wherever this
behavior is described.
- Around line 516-547: The pagination logic in the getter still deserializes the
entire temporary Vec<DepositLot> for a user/term bucket before slicing, so large
buckets can still hit Soroban resource limits. Update the storage/access pattern
used by deposit() and this retrieval path so lots are stored and fetched in
page- or lot-granularity keys, and only the requested window is loaded in the
pagination function instead of calling temporary().get on the full bucket.
In `@acbu_savings_vault/tests/test.rs`:
- Around line 515-538: The pagination test only checks counts because all lots
created in the deposit loop are identical, so it can miss overlap or
wrong-window bugs in get_user_lots. Update the test setup to make each deposited
lot uniquely distinguishable (for example by varying a field per iteration) and
add assertions that verify the actual page boundaries for the first and second
pages, not just lengths. Also add a case in the get_user_lots coverage for a
limit greater than 100 to confirm the clamp behavior in the pagination logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4006718-304e-4025-b4be-d7574d76a1ec
📒 Files selected for processing (6)
acbu_minting/src/lib.rsacbu_reserve_tracker/src/lib.rsacbu_savings_vault/src/lib.rsacbu_savings_vault/tests/test.rsshared/src/lib.rsshared/tests/test_utilities.rs
Closes #336
Closes #337
Closes #354
Closes #358
Summary by CodeRabbit
New Features
Bug Fixes
Tests