Skip to content

feat(dao): add MANAGER changeUserTranche correction (pre-finalize) - #121

Merged
qingyang-lista merged 1 commit into
masterfrom
fix/pre-ipo-audit
Jul 28, 2026
Merged

feat(dao): add MANAGER changeUserTranche correction (pre-finalize)#121
qingyang-lista merged 1 commit into
masterfrom
fix/pre-ipo-audit

Conversation

@razww

@razww razww commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a MANAGER-only correction method, changeUserTranche, to PreIPODistributor. It lets operations fix an account's locked delivery tranche after deposit (the tranche is otherwise locked at an account's first deposit), up until the settlement root is finalized. Stacks on the merged audit-fix work (#120); a single commit on top of master.

What it does

function changeUserTranche(uint64 _saleId, address _account, uint8 _tranche) external onlyRole(MANAGER)
  • Overrides userTranche[saleId][account] and emits ChangeUserTranche(saleId, account, oldTranche, newTranche).
  • Does not touch deposit amounts (deposits / pubDeposits) or the sale aggregates — the tranche never affects on-chain accounting, only off-chain delivery routing.

Guards

  • onlyRole(MANAGER).
  • Valid sale (whitelistRoot != 0).
  • settlements[saleId].root == 0 — allowed until the root is finalized. A pending (not-yet-finalized) root does not block the change; the operator must then revoke the pending root and rebuild the settlement tree off-chain so it reflects the corrected tranche.
  • New tranche must be valid (TRANCHE_UNLOCKED / TRANCHE_LOCKED).
  • Account must already have a tranche (oldTranche != 0, i.e. has deposited).
  • New tranche must differ from the current one ("Same tranche").

Storage / upgrade safety

Adds no storage (one external function + one event). The storage layout is byte-identical to the current production contract, and remains an append-only, upgrade-safe superset of the live #118 subscription impl (salesnextSaleId at slots 301–305 unchanged; settlements / claimed / waitingPeriod at 306–308). The same upgradeToAndCall(newImpl, initializeV2()) upgrade carries this change.

Testing

forge test --match-contract PreIPODistributorTest64 passing, including 8 new cases:

  • ok — manager flips XKLSH → PKLSH; deposits untouched, event emitted
  • acl — non-MANAGER reverts
  • invalidSale / invalidTranche / noDeposit / sameTranche reverts
  • whileSettlementPending_ok — allowed while a root is pending
  • afterFinalize_reverts — reverts once finalized ("Settlement finalized")

Operational note

Because a correction is permitted while a root is pending, finalizeSettlement does not detect a tranche changed underneath a pending root — operations must revoke-and-rebuild before finalizing. Same class of operational discipline as L03.

Adds changeUserTranche(saleId, account, tranche): a MANAGER-only override
of an account's locked tranche, allowed until the settlement root is
finalized (settlements.root == 0). A pending (not-yet-finalized) root does
not block it — the pending root must then be revoked and the tree rebuilt
off-chain to reflect the change. The new tranche must be valid, differ from
the current one, and the account must already have a tranche (i.e. have
deposited). Deposit amounts and aggregates are untouched — the tranche
never affects on-chain accounting. Adds no storage (append-only layout
unchanged; upgrade-safe).

8 tests: ok/acl/invalid-sale/invalid-tranche/no-deposit/same-tranche,
allowed while a root is pending, and reverts after finalize.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hashdit-bot

hashdit-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Pull Request Review

This Solidity PR adds a MANAGER-only changeUserTranche function and event to correct a depositor’s delivery tranche before settlement finalization without changing deposit accounting. It also adds Foundry tests covering authorization, input validation, pending-settlement behavior, and rejection after finalization.

Sensitive Content

No sensitive content detected.

Security Issues

🟠 [HIGH] Tranche changes can invalidate a pending settlement without restarting review

File: contracts/dao/PreIPODistributor.sol
changeUserTranche permits changes while pendingRoot is set but neither revokes that root nor prevents finalizeSettlement from immediately finalizing it after the existing waiting period. Because tranche selection controls whether shares are transferred on-chain or delivered off-chain, a root built and reviewed using the previous tranche can be finalized after the routing state changes, potentially causing incorrect, omitted, or duplicate delivery. Relying on operators to revoke and rebuild does not enforce this settlement invariant and creates a race between correction and finalization.
Recommendation: Reject tranche changes whenever pendingRoot != 0, or atomically clear the pending root, refund total, and timestamp when a tranche changes so a rebuilt root must undergo a fresh review period. Add a test proving that an old pending root cannot be finalized after a correction.


Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits.

@qingyang-lista qingyang-lista 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.

LGTM

@qingyang-lista
qingyang-lista merged commit c32347a into master Jul 28, 2026
1 check passed
@qingyang-lista
qingyang-lista deleted the fix/pre-ipo-audit branch July 28, 2026 10:01
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