[N-02] Allowance Audit

[kosedogus]

Fixes N-02

Summary by CodeRabbit

• Documentation
  • Clarified the abort behavior documented for spending and withdrawal operations.
  • Updated error details to show the specific framework Move error code and where it is propagated from.
  • No functional behavior, signatures, or control flow were changed.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad9d5fb7-c35a-49c3-bd8c-71fdccbbf2de

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR updates documentation comments only, within three public functions (spend, withdraw, withdraw_all) in spend_vault.move. The Aborts sections now describe EObjectFundsWithdrawNotEnabled as a framework Move #[error] abort (code 3 in sui::funds_accumulator) instead of generic execution-status wording. No code logic changes.

Changes

Abort doc updates

Layer / File(s)	Summary
Clarify EObjectFundsWithdrawNotEnabled abort docs contracts/allowance/sources/spend_vault.move	Aborts documentation for spend, withdraw, and withdraw_all is updated to describe EObjectFundsWithdrawNotEnabled as a framework Move #[error] abort (code 3 in sui::funds_accumulator), with withdraw_all additionally noting reachability only on a non-empty settled pool.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A rabbit reads the abort code lore,
Code "3" now clearer than before 🐇
No logic hopped, no functions changed,
Just comments tidied, neatly arranged,
Thump thump — documentation done once more!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description only links the issue and omits the change summary and PR checklist required by the template.	Add a short summary of the doc-only abort-behavior update and fill in the Tests, Documentation, and Changelog checklist items.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is related to the allowance audit fix, though it is broader than the specific documentation-only change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/allowance-N-02

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

contracts/allowance/sources/spend_vault.move (1)

1149-1151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Name the propagation source explicitly for consistency with spend.

spend documents this same abort as "propagated from withdraw_funds_from_object" (line 809). Consider aligning withdraw to name the primitive explicitly — the call is at line 1167 and integrators benefit from knowing the exact propagation path.

 /// - `EObjectFundsWithdrawNotEnabled` if the `enable_object_funds_withdraw`
 ///   protocol feature is off: a framework Move `#[error]` abort (code 3 in
-///   `sui::funds_accumulator`).
+///   `sui::funds_accumulator`) propagated from `withdraw_funds_from_object`.

🤖 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 `@contracts/allowance/sources/spend_vault.move` around lines 1149 - 1151,
Update the `withdraw` documentation in `spend_vault.move` to explicitly state
that `EObjectFundsWithdrawNotEnabled` is propagated from
`withdraw_funds_from_object`, matching the wording used by `spend` for
consistency. Keep the existing abort condition, but revise the comment near
`withdraw` to name the underlying primitive clearly so integrators can trace the
propagation path; use the `withdraw` and `withdraw_funds_from_object` symbols to
locate the relevant doc text.

Source: Path instructions

🤖 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.

Nitpick comments:
In `@contracts/allowance/sources/spend_vault.move`:
- Around line 1149-1151: Update the `withdraw` documentation in
`spend_vault.move` to explicitly state that `EObjectFundsWithdrawNotEnabled` is
propagated from `withdraw_funds_from_object`, matching the wording used by
`spend` for consistency. Keep the existing abort condition, but revise the
comment near `withdraw` to name the underlying primitive clearly so integrators
can trace the propagation path; use the `withdraw` and
`withdraw_funds_from_object` symbols to locate the relevant doc text.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e86fdb0c-7d5a-48b1-9c12-c95ed842e000

📥 Commits

Reviewing files that changed from the base of the PR and between 12086c2 and 7462bb1.

📒 Files selected for processing (1)

• contracts/allowance/sources/spend_vault.move

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.51%. Comparing base (0ffcbad) to head (aa0d957).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
- Coverage   96.54%   96.51%   -0.04%     
==========================================
  Files          32       32              
  Lines        3324     3324              
  Branches      784      783       -1     
==========================================
- Hits         3209     3208       -1     
  Misses         70       70              
- Partials       45       46       +1     

Flag	Coverage Δ
contracts/access	65.46% <ø> (ø)
contracts/allowance	52.00% <ø> (ø)
contracts/finance	26.07% <ø> (ø)
contracts/timelock	54.57% <ø> (ø)
contracts/utils	44.09% <ø> (ø)
math/core	86.89% <ø> (-0.08%)	⬇️
math/fixed_point	61.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ericnordelo]

LGTM!

[immrsd]

LGTM