Skip to content

Block update_split while a mutable split holds escrow balance - #1

Merged
Fury03 merged 1 commit into
mainfrom
guard-escrow-redirect-on-update-split
Jul 20, 2026
Merged

Block update_split while a mutable split holds escrow balance#1
Fury03 merged 1 commit into
mainfrom
guard-escrow-redirect-on-update-split

Conversation

@Fury03

@Fury03 Fury03 commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

Closes tributary-protocol#253
A split's controller could call update_split between deposit and distribute, replacing recipients and shares and redirecting an already-escrowed deposit to arbitrary recipients. Immutable splits (controller: None) were never affected since their routing table can't change.

Fix: update_split now rejects (Error::SplitHasBalance) while held_tokens(id) is non-empty for the split, forcing the controller to distribute every held token first before changing recipients or shares.

  • contracts/splitter/src/lib.rs: guard added to update_split, doc comments on Error::SplitHasBalance, update_split, and deposit clarifying the trust model
  • contracts/splitter/src/test.rs: update_split_rejects_while_balance_outstanding — deposits, attempts an update (rejected), distributes, then confirms the update succeeds afterwards and the new recipient never sees funds that were already paid out
  • SECURITY.md: new "Trust model: escrow and mutable splits" section documenting the guarantee and its limits
  • docs/api-reference.md, docs/architecture.md, CHANGELOG.md: updated to reflect the new error case and behavior

Test plan

  • cargo test in contracts/splitter — 42 passed, including the new regression test
  • cargo clippy --all-targets — clean

Summary by CodeRabbit

  • New Features

    • Mutable splits can no longer change recipients or share allocations while holding escrowed funds.
    • Added the SplitHasBalance error for blocked updates.
  • Bug Fixes

    • Prevented deposited funds from being redirected through later routing-table changes.
    • Split updates are allowed again after held funds are distributed.
  • Documentation

    • Clarified escrow, mutable split behavior, deposit routing guarantees, and the new error in the API and architecture documentation.

A controller could accept a deposit under one routing table and pay
it out under another by calling update_split between deposit and
distribute, redirecting escrowed funds to arbitrary recipients.

update_split now refuses to run while held_tokens(id) is non-empty,
forcing distribute first. Immutable splits (controller: None) were
never affected since their routing table can't change.

Documents the trust model in SECURITY.md and clarifies deposit()/
update_split() docs on whether escrow can be redirected.
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Mutable split routing

Layer / File(s) Summary
Balance guard and validation
contracts/splitter/src/lib.rs, contracts/splitter/src/test.rs
update_split returns SplitHasBalance while escrow balances are held, and the integration test verifies updates succeed after distribution.
Public routing contract documentation
CHANGELOG.md, SECURITY.md, docs/api-reference.md, docs/architecture.md
Documentation describes error code 12, distribution-time routing, escrow restrictions, and immutable splits.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: spagero763, esc1200

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: rejecting update_split on mutable splits with escrow balance.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch guard-escrow-redirect-on-update-split

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.

@github-actions

Copy link
Copy Markdown

Contract coverage: tributary-splitter

Metric Coverage
Lines 92.74%
Functions 81.63%
Regions 95.41%

@Fury03
Fury03 merged commit e1ef10e into main Jul 20, 2026
5 checks passed
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.

Controller can redirect already-escrowed funds by calling update_split before distribute

2 participants