194 lines
13 KiB
Markdown
194 lines
13 KiB
Markdown
# Learning Guide: Custody Accounting Behind a UUPS Proxy
|
|
|
|
> Educational demo — mock token — never use real funds.
|
|
|
|
## The two addresses that make upgrades possible
|
|
|
|
Users and scripts call the **proxy**, whose address remains stable and whose storage contains the asset address, customer ledger, liabilities, owner, and pause state. The **implementation** contains executable logic. The proxy forwards each application call with `delegatecall`: implementation code runs in the proxy's context, so `address(this)` is the proxy and all application reads and writes affect proxy storage. Calling the implementation directly is not an application call.
|
|
|
|
UUPS places upgrade authorization in the implementation. OpenZeppelin's proxy-context checks ensure upgrade entry points run only through a compatible proxy; `BankV1._authorizeUpgrade` then restricts authorization to the owner. This keeps the proxy small, but makes implementation correctness, storage compatibility, and owner security critical.
|
|
|
|
## Initializers replace constructor state
|
|
|
|
A normal implementation constructor changes only the implementation's own storage. It cannot initialize the proxy storage used by delegated calls. `initialize(asset, initialOwner)` therefore performs the one-time proxy setup and initializes ownership and pausing. Its public visibility lets the pinned OpenZeppelin upgrades-core tooling recognize the initializer as inherited by `BankV2`; the `initializer` modifier still permits exactly one proxy initialization. The encoded initializer runs atomically when the proxy is created, avoiding an uninitialized-proxy takeover window.
|
|
|
|
The `BankV1` constructor calls `_disableInitializers()`. That locks the standalone implementation so an outsider cannot initialize it and create a misleading or dangerous separately owned instance. The narrow constructor annotation tells the OpenZeppelin validator why this constructor is intentional; it does not bypass storage-layout, missing-initializer, or UUPS compatibility checks. Double initialization of the proxy and direct initialization of either implementation revert with `InvalidInitialization`.
|
|
|
|
`BankV2` needs no initializer or reinitializer because the V1 proxy is already initialized and V2 adds behavior only. It inherits the existing asset, owner, pause state, customer balances, liabilities, and transient reentrancy guard. Adding an initializer when there is no new state would create an unnecessary privileged transition and another state to reason about.
|
|
|
|
## Storage layout is a permanent API
|
|
|
|
Delegate calls interpret storage according to the active implementation's layout. Alongside OpenZeppelin's namespaced base-contract state, the actual V1 application fields occupy these slots in source order:
|
|
|
|
```text
|
|
BankV1 proxy application storage
|
|
├─ slot 0: _asset: IERC20
|
|
├─ slot 1: _balances: mapping(address => uint256)
|
|
├─ slot 2: _totalLiabilities: uint256
|
|
└─ slots 3…49: __gap: uint256[47]
|
|
```
|
|
|
|
OpenZeppelin upgradeable ownership and pausing use their own namespaced storage. `ReentrancyGuardTransient` uses transient storage rather than adding an initialized persistent field. Those implementation details do not make the application's declaration order optional.
|
|
|
|
V2 is safe because inheritance preserves that layout byte-for-byte and V2 declares no state:
|
|
|
|
```text
|
|
BankV2 is BankV1
|
|
├─ slot 0: _asset: IERC20 unchanged
|
|
├─ slot 1: _balances: mapping(address => uint256) unchanged
|
|
├─ slot 2: _totalLiabilities: uint256 unchanged
|
|
├─ slots 3…49: __gap: uint256[47] unchanged
|
|
└─ no V2 storage variables
|
|
```
|
|
|
|
A conceptual future revision could consume one reserved word only by carefully changing the layout in the base contract that declares the gap and validating every descendant against the prior version:
|
|
|
|
```text
|
|
Conceptual validated revision of the declaring base layout
|
|
├─ slot 0: _asset: IERC20
|
|
├─ slot 1: _balances: mapping(address => uint256)
|
|
├─ slot 2: _totalLiabilities: uint256
|
|
├─ slot 3: _newValue: uint256
|
|
└─ slots 4…49: __gap: uint256[46]
|
|
```
|
|
|
|
Simply declaring a new `BankV2` field would append it after the already inherited `__gap`; it would not consume that gap. This demo avoids that ambiguity entirely because the actual `BankV2` declares no state. Reordering existing declarations is unsafe:
|
|
|
|
```text
|
|
Unsafe layout
|
|
├─ slot 0: _totalLiabilities: uint256 moved into _asset's old slot
|
|
├─ slot 1: _asset: IERC20 moved into the mapping's old slot
|
|
├─ slot 2: _balances: mapping(address => uint256) moved into liabilities' old slot
|
|
└─ slots 3…49: __gap: uint256[47]
|
|
```
|
|
|
|
Changing order, type, inheritance order, or removing state can make balances appear as addresses, overwrite accounting, or damage control state. A bad implementation can also remove working upgrade machinery and brick future upgrades.
|
|
|
|
`BankV2` declares `@custom:oz-upgrades-from src/BankV1.sol:BankV1`. The upgrade script sets `BankV1` as the reference contract and calls `Upgrades.validateUpgrade` before broadcasting. Foundry emits AST, build information, and storage layouts; the OpenZeppelin validator compares the inheritance and storage layouts and checks UUPS compatibility. The validation uses no storage or UUPS bypass. It is a compatibility gate, not a business-logic audit: it cannot prove that a future owner-authorized implementation is honest, solvent, or correctly governed.
|
|
|
|
## Reserves, liabilities, and surplus
|
|
|
|
**Reserves** are `MockUSDC.balanceOf(proxy)`: tokens actually held by the proxy. **Liabilities** are `totalLiabilities()`: the aggregate amount the ledger owes customers. **Surplus** is reserves minus liabilities. The solvency rule is:
|
|
|
|
```text
|
|
reserves >= total liabilities
|
|
```
|
|
|
|
Equality holds in the prepared local states. Anyone can transfer mock tokens directly to the proxy without receiving ledger credit, so a surplus is possible and the invariant deliberately uses `>=`.
|
|
|
|
For `deposit(amount)`, the bank rejects zero, paused, or reentrant calls; reads reserves; uses `SafeERC20.safeTransferFrom`; measures the exact received delta; and only then credits the sender's internal balance and total liabilities. A fee-on-transfer or otherwise unexpected asset delta reverts the entire transaction.
|
|
|
|
For `withdraw(amount)`, the bank rejects zero, paused, reentrant, or underfunded ledger calls. It debits the customer's internal balance and total liabilities before `SafeERC20.safeTransfer` sends tokens. That ordering is checks-effects-interactions: validate first, commit internal effects second, interact externally last. `nonReentrant` adds a second boundary against a malicious token callback. A token revert rolls the whole transaction back.
|
|
|
|
`SafeERC20` handles ERC-20 implementations that return `false`, omit return values, or revert in different ways. Pausing gives the owner an emergency stop for deposits, withdrawals, and V2 internal transfers while views remain available. There is intentionally no owner reserve sweep.
|
|
|
|
## V2 transfer conservation proof
|
|
|
|
`transferBalance(recipient, amount)` validates the active pause state, a nonzero amount, a nonzero recipient distinct from the sender, and sufficient sender balance. It then debits the sender, credits the recipient, and emits `BalanceTransferred(from, to, amount)`. It makes no external call, so it needs no reentrancy guard and cannot move ERC-20 tokens.
|
|
|
|
The prepared Act 3 transfers `250e6` base units (`250.000000 mUSDC`) from Alice to Bob:
|
|
|
|
| Quantity | Before | Delta | After |
|
|
|---|---:|---:|---:|
|
|
| Alice ledger balance | `900e6` | `-250e6` | `650e6` |
|
|
| Bob ledger balance | `500e6` | `+250e6` | `750e6` |
|
|
| Tracked balance sum | `1_400e6` | `0` | `1_400e6` |
|
|
| Total liabilities | `1_400e6` | `0` | `1_400e6` |
|
|
| Proxy token reserves | `1_400e6` | `0` | `1_400e6` |
|
|
| Surplus | `0` | `0` | `0` |
|
|
|
|
Conservation follows directly from subtracting and adding the same `amount`: `(Alice - amount) + (Bob + amount) = Alice + Bob`. The `BalanceTransferred` log proves the ledger operation. The absence of a MockUSDC `Transfer` log in that transaction, together with equal before/after proxy reserves, proves that no reserve token moved.
|
|
|
|
## What the upgrade snapshot proves
|
|
|
|
Before broadcasting, `UpgradeV2` records the proxy, old implementation, owner, asset, pause state, every manifest actor balance, liabilities, reserves, surplus, deployment block, and version. After the OpenZeppelin-validated owner upgrade, it takes the same snapshot and requires:
|
|
|
|
- the proxy, owner, asset, pause state, actor count and balances, liabilities, reserves, surplus, and deployment block to be identical;
|
|
- the implementation address to differ;
|
|
- the implementation slot and manifest to identify the new implementation; and
|
|
- `contractVersion()` to change from `1` to `2`.
|
|
|
|
The finalizer independently matches the successful broadcast and `Upgraded(newImplementation)` log, reads live proxy and token state, and only then atomically updates the confirmed manifests. Thus the upgrade proof covers both storage continuity and the identity of the logic now serving the stable proxy.
|
|
|
|
## The owner is a central threat
|
|
|
|
The owner can pause every customer mutation and authorize an implementation containing arbitrary future logic. A malicious or compromised owner could install code that changes balances, transfers reserves, removes checks, breaks storage, or prevents later upgrades. Tests proving today's V1 and V2 behavior cannot constrain tomorrow's authorized implementation.
|
|
|
|
The demo deliberately uses one owner and has no multisig, timelock, role separation, upgrade delay, monitoring service, emergency governance process, or audited deployment procedure. OpenZeppelin's `onlyOwner` check proves that the configured owner authorized an upgrade; it does not prove the owner made a safe decision. A production threat model must protect the key, constrain and review upgrade proposals, make changes observable, plan incident response, and address legal and regulatory obligations.
|
|
|
|
> **Trust boundary:** MockUSDC has no value. These contracts are educational and unaudited; real deposits must never be sent here. The owner can pause customer actions and install arbitrary future logic. UUPS mistakes can corrupt state or permanently brick upgradeability. A real custody product requires professional audits, operational key controls, multisig or timelocked governance, incident procedures, legal advice, and jurisdiction-specific compliance work.
|
|
|
|
## Local exercises: observe the named failures
|
|
|
|
These commands run deterministic tests and need no wallet or public RPC. Add `-vvvv` to inspect a revert trace.
|
|
|
|
### Every V2 transfer failure
|
|
|
|
```bash
|
|
# Inherited BankV1.ZeroAmount
|
|
forge test --match-test testTransferBalanceRejectsZeroAmount -vv
|
|
|
|
# BankV2.InvalidRecipient(address)
|
|
forge test --match-test testTransferBalanceRejectsZeroRecipient -vv
|
|
|
|
# BankV2.SelfTransfer()
|
|
forge test --match-test testTransferBalanceRejectsSenderAsRecipient -vv
|
|
|
|
# Inherited BankV1.InsufficientBalance(account, available, requested)
|
|
forge test --match-test testTransferBalanceReportsAvailableAndRequestedWhenBalanceIsInsufficient -vv
|
|
|
|
# OpenZeppelin EnforcedPause()
|
|
forge test --match-test testTransferBalanceRejectsCallsWhilePaused -vv
|
|
```
|
|
|
|
### V1, initialization, authorization, and script failures
|
|
|
|
```bash
|
|
# BankV1.InvalidAsset
|
|
forge test --match-test testInitializationChecksZeroAssetBeforeZeroOwner -vv
|
|
|
|
# BankV1.ZeroAmount on both V1 customer paths
|
|
forge test --match-test 'test(Deposit|Withdraw)RejectsZeroAmount' -vv
|
|
|
|
# BankV1.InsufficientBalance with available/requested values
|
|
forge test --match-test testWithdrawReportsAvailableAndRequestedOnInsufficientInternalBalance -vv
|
|
|
|
# BankV1.UnexpectedAssetDelta with a fee-taking token
|
|
forge test --match-test testFeeOnTransferDepositRevertsAndRollsBackTokenAndAccounting -vv
|
|
|
|
# OpenZeppelin EnforcedPause
|
|
forge test --match-test 'test(Deposit|Withdraw)RejectsCallsWhilePaused' -vv
|
|
|
|
# OpenZeppelin OwnableUnauthorizedAccount
|
|
forge test --match-test 'testNonOwnerCannot(Pause|Unpause|AuthorizeUpgrade)' -vv
|
|
|
|
# OpenZeppelin InvalidInitialization on the proxy and both implementations
|
|
forge test --match-test 'test(ProxyCannotBeInitializedTwice|ImplementationCannotBeInitialized|NewImplementationCannotBeInitializedDirectly)' -vv
|
|
|
|
# OpenZeppelin ReentrancyGuardReentrantCall
|
|
forge test --match-test testDepositPropagatesNestedRevertAtomicallyWhenConfigured -vv
|
|
|
|
# DemoScript.UnsupportedChain
|
|
forge test --match-test testUnsupportedChainsAreRejectedBeforeBroadcast -vv
|
|
|
|
# DemoScript.ManifestChainMismatch and DemoScript.MissingCode
|
|
forge test --match-test 'test(WrongManifestChain|AddressWithoutCode)IsRejected' -vv
|
|
|
|
# DemoScript.InvalidDeploymentBlock
|
|
forge test --match-test testActiveManifestRejectsDeploymentBlockZero -vv
|
|
|
|
# DemoScript.InvalidManifestSchema
|
|
forge test --match-test testLegacyParallelActorManifestIsRejected -vv
|
|
|
|
# DemoScript.InvalidActorConfiguration, including strict local and Base actor schemas
|
|
forge test --match-test 'test(AnvilManifestRequiresOwnerAtActorZero|BaseManifestRequiresExactPresenterRecipientActors)' -vv
|
|
|
|
# DemoScript.UnexpectedState and DemoScript.UnexpectedAddress
|
|
forge test --match-test 'test(UnexpectedStateExerciseRevertsThroughAssertionBranch|CheckStateRejectsManifestImplementationMismatch)' -vv
|
|
|
|
# CheckState.Insolvent and CheckState.UnknownStage
|
|
forge test --match-test testCheckStateExercisesRevertThroughInsolventAndUnknownStageBranches -vv
|
|
```
|
|
|
|
Finish with `make verify`; it combines formatting, build, storage/upgrade validation, unit, fuzz, invariant, script, process-safety, scanner, and web gates.
|