Skip to content

[N-01] Allowance Audit - #424

Merged
kosedogus merged 1 commit into
mainfrom
fix/allowance-N-01
Jul 2, 2026
Merged

[N-01] Allowance Audit#424
kosedogus merged 1 commit into
mainfrom
fix/allowance-N-01

Conversation

@kosedogus

@kosedogus kosedogus commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Fixes N-01

Summary by CodeRabbit

  • Documentation
    • Clarified how withdrawal failures are determined at execution time based on the object’s live balance.
    • Updated guidance so integrators rely on dry runs to detect insufficient-funds cases instead of matching a specific error code.
    • Refined notes around withdraw behavior to better distinguish live balance from settled balance.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 95576dfe-2517-406c-b908-c15386b45d83

📥 Commits

Reviewing files that changed from the base of the PR and between 12086c2 and 71f9bc0.

📒 Files selected for processing (2)
  • contracts/allowance/README.md
  • contracts/allowance/sources/spend_vault.move

📝 Walkthrough

Walkthrough

This PR updates documentation comments in the allowance contracts module and README to clarify that InsufficientFundsForWithdraw failures relate to the object's live balance at execution time, rather than the settled balance. No code logic, error codes, or public API signatures are altered.

Changes

Documentation wording clarification

Layer / File(s) Summary
README pool-short explanation
contracts/allowance/README.md
Reworded explanation of pool-short failures to reference live balance at execution, anchored at redeem_funds, and reiterating that the failure surfaces via SDK/dry-run rather than module abort codes.
Module and function docstring updates
contracts/allowance/sources/spend_vault.move
Updated module-level "Mixed error model" commentary and docstrings for spend, withdraw, and withdraw_all to consistently describe InsufficientFundsForWithdraw as tied to live balance at execution instead of settled balance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A wee rabbit hops through docs so fine,
Swapping "settled" for "live" in every line.
No code was touched, no logic bent,
Just clearer words for what redeem_funds meant.
Thump thump! 🐇📜

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is far too sparse and omits the required issue reference, change summary, and checklist items. Add a Resolves # issue reference, a brief description of the fix, and complete the PR checklist for Tests, Documentation, and Changelog.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is related to the allowance audit work, though it is broader than the specific balance-check fix.
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-01

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

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

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.41%. Comparing base (12086c2) to head (71f9bc0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #424      +/-   ##
==========================================
+ Coverage   96.35%   96.41%   +0.06%     
==========================================
  Files          31       31              
  Lines        3014     3014              
  Branches      734      736       +2     
==========================================
+ Hits         2904     2906       +2     
  Misses         66       66              
+ Partials       44       42       -2     
Flag Coverage Δ
contracts/access 65.46% <ø> (ø)
contracts/allowance 52.00% <ø> (ø)
contracts/finance 25.57% <ø> (ø)
contracts/utils 44.09% <ø> (ø)
math/core 87.04% <ø> (+0.14%) ⬆️
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 ericnordelo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bidzyyys bidzyyys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@immrsd immrsd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kosedogus
kosedogus merged commit 0ffcbad into main Jul 2, 2026
29 checks passed
@kosedogus
kosedogus deleted the fix/allowance-N-01 branch July 2, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants