Skip to content

Improve docs - #763

Merged
andrew-fleming merged 8 commits into
OpenZeppelin:mainfrom
andrew-fleming:minor-doc-tweaks
Aug 1, 2026
Merged

Improve docs#763
andrew-fleming merged 8 commits into
OpenZeppelin:mainfrom
andrew-fleming:minor-doc-tweaks

Conversation

@andrew-fleming

@andrew-fleming andrew-fleming commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation
    • Clarified proposal visibility, recipient routing, validation limitations, and proposal ID behavior.
    • Documented treasury accounting, zero-value deposits, send-result disclosure, and underflow assumptions.
    • Added guidance for stateless shielded transfers, including off-chain ownership and double-spend handling.
    • Corrected shielded burn-address references.
  • Refactor
    • Updated circuit cost and row-count metadata for deposit and drain operations.

@andrew-fleming
andrew-fleming requested review from a team as code owners August 1, 2026 15:07
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 01d9f1cb-c1f2-44f8-9807-c70c2627c886

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

The pull request updates circuit metadata and expands documentation for multisig proposals, shielded treasury accounting, stateless coin handling, and shielded burn destinations. It does not change executable logic or public declarations.

Changes

Multisig metadata and documentation

Layer / File(s) Summary
Circuit metadata updates
contracts/src/multisig/ForwarderPrivate.compact, contracts/src/multisig/ForwarderShielded.compact, contracts/src/multisig/ForwarderUnshielded.compact
The documented circuit parameters now reflect updated k and row values for _drain and _deposit.
Proposal semantics documentation
contracts/src/multisig/ProposalManager.compact
The documentation describes public proposal data, recipient routing, inactive-state handling, deferred validation, unrecoverable destinations, and proposal ID allocation.
Treasury and burn documentation
contracts/src/multisig/ShieldedTreasury.compact, contracts/src/multisig/ShieldedTreasuryStateless.compact, contracts/src/multisig/presets/ShieldedMultiSigV3.compact
The documentation describes public treasury state, zero-value deposits, plaintext coin results, accounting assumptions, stateless coin handling, and shieldedBurnAddress().

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

Possibly related PRs

Suggested reviewers: 0xisk

Poem

A rabbit reads the circuits bright,
Rows and k now match just right.
Proposals tell their paths with care,
Treasury truths are written there.
Burn addresses point the way—
Hop, review, and ship today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title relates to documentation changes but is too vague to identify the primary documentation updates. Use a specific title that identifies the main documentation changes, such as clarifying treasury behavior and proposal limitations.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@contracts/src/multisig/ProposalManager.compact`:
- Around line 203-206: Update the documentation near _cancelProposal to remove
the claim that an incompatible proposal is permanently Active. State that it
remains Active and unexecutable on that execution rail until cancelled, while
preserving the existing explanation of the execution-time failure.

In `@contracts/src/multisig/ShieldedTreasury.compact`:
- Around line 58-62: Update the zero-value deposit documentation near `_coins`
to qualify that a zero-value coin is inserted only when that color has no
existing entry; when `_coins` already contains `coin.color`, `_deposit` uses
`mergeCoinImmediate` and replaces the existing entry rather than adding a
separate coin.
- Around line 110-114: Update the notice above ShieldedSendResult to remove the
claim that returning it leaks nothing new. State that _coins publishes only the
retained change coin, removes entries when there is no change, and does not
store result.sent; therefore returning the result can disclose the sent coin
beyond public _coins state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9cfd5709-e5e7-4887-8893-212886f43e1a

📥 Commits

Reviewing files that changed from the base of the PR and between c7f9ffc and f947457.

📒 Files selected for processing (7)
  • contracts/src/multisig/ForwarderPrivate.compact
  • contracts/src/multisig/ForwarderShielded.compact
  • contracts/src/multisig/ForwarderUnshielded.compact
  • contracts/src/multisig/ProposalManager.compact
  • contracts/src/multisig/ShieldedTreasury.compact
  • contracts/src/multisig/ShieldedTreasuryStateless.compact
  • contracts/src/multisig/presets/ShieldedMultiSigV3.compact

Comment thread contracts/src/multisig/ProposalManager.compact Outdated
Comment thread contracts/src/multisig/ShieldedTreasury.compact Outdated
Comment thread contracts/src/multisig/ShieldedTreasury.compact
@andrew-fleming
andrew-fleming merged commit c4242a9 into OpenZeppelin:main Aug 1, 2026
7 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.

2 participants