Skip to content

[Sonic] Deploy 031 - Disable minting on Sonic Vault#2947

Open
naddison36 wants to merge 3 commits into
masterfrom
nicka/sonic-mint
Open

[Sonic] Deploy 031 - Disable minting on Sonic Vault#2947
naddison36 wants to merge 3 commits into
masterfrom
nicka/sonic-mint

Conversation

@naddison36

@naddison36 naddison36 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Disables public minting on the Sonic Vault as part of the OS sunset.

The new OSVault implementation restricts asset-backed mints to the Strategist or Governor, following the approach previously used for the Plume Vault.

Deployment

Deployment 031_disable_mints upgrades OSonicVaultProxy to the new implementation through the Sonic Timelock.

Contract

Name Address
OSVault 0x41DF78939406BF3f189c304C72f01fAd7acAfce7

Deployment transaction: 0xcd82182cfedcbfd35b92b15c3da6fc789d3e6c4f552be8a2a7c1cb20e8983653

Constructor argument:

Name Value
wS 0x039e2fB66102314Ce7b64Ce5Ce3E5183bc94aD38

Governance actions

Name File
Schedule contracts/deployments/sonic/operations/031_disable_mints.schedule.json
Execute contracts/deployments/sonic/operations/031_disable_mints.execute.json

Storage-layout validation

The initial deployment transaction succeeded, but post-deployment metadata generation failed because the OpenZeppelin validations cache was no longer generated after @openzeppelin/hardhat-upgrades was removed.

The storage-layout helper now:

  • Detects a missing or stale validation cache.
  • Reads the available Hardhat build-info files.
  • Reconstructs OpenZeppelin validation runs using @openzeppelin/upgrades-core.
  • Combines and writes the rebuilt validation cache.
  • Continues with the existing storage upgrade-safety checks and layout generation.

The repaired validation path confirms:

[storage-slots] Contract OSVault is safe for upgrade

Code Change Checklist

To be completed before internal review begins:

  • The contract code is complete
  • Executable deployment file
  • Fork tests that test after the deployment file runs
  • Unit tests *if needed
  • The owner has done a full checklist review of the code + tests

Internal review:

  • Two approvals by internal reviewers

Deploy checklist

Two reviewers complete the following checklist:

- [ ] All deployed contracts are listed in the deploy PR's description
- [ ] Deployed contract's verified code (and all dependencies) match the code in master
- [ ] Contract constructors have correct arguments
- [ ] The transactions that interacted with the newly deployed contract match the deploy script.
- [ ] Governance proposal matches the deploy script
- [ ] Smoke tests pass after fork test execution of the governance proposal

Rebuild OpenZeppelin validation data from Hardhat build-info when the validation cache is missing or stale. This restores storage-layout checks after removing the hardhat-upgrades plugin.
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.01%. Comparing base (07a7fcb) to head (9144fdb).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2947      +/-   ##
==========================================
- Coverage   47.33%   46.01%   -1.32%     
==========================================
  Files         110      110              
  Lines        4920     4924       +4     
  Branches     1362     1364       +2     
==========================================
- Hits         2329     2266      -63     
- Misses       2587     2654      +67     
  Partials        4        4              

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@naddison36 naddison36 changed the title Deploy Sonic 031 - Disable minting on Sonic Vault [Sonic] Deploy 031 - Disable minting on Sonic Vault Jul 22, 2026
shahthepro
shahthepro previously approved these changes Jul 22, 2026
Comment on lines +81 to +85
await oSonicVault.connect(governor).setStrategistAddr(user.address);
await wS.connect(user).approve(oSonicVault.address, amount);
const tx = await oSonicVault.connect(user).mint(amount);
await oSonicVault.connect(governor).setStrategistAddr(strategist.address);
return tx;

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.

nit: alternative, strategist can mint and transfer to the user. But this works as well if we are tracking events

@sparrowDom

Copy link
Copy Markdown
Member

Deployment verification — 031_disable_mints

Verified against: nicka/sonic-mint @ f9c6ef78 (exact match to PR head) · Network: sonic
Script: contracts/deploy/sonic/031_disable_mints.js

Verdict: 🔴 Blocked on deployment bookkeeping

The deployed bytecode is correct — I'd sign off on the contract itself. What blocks verification is that the deployment artifact and the governance operation files were never committed, so the governance step can't be verified and the repo currently resolves OSVault to the old implementation.


Deploy checklist

  • All deployed contracts are listed in the deploy PR's descriptionOSVault 0x41DF…Fce7 is listed and is genuinely the contract created by tx 0xcd82…3653.
  • Deployed contract's verified code (and all dependencies) match the code in mastersol2uml diff clean, all 19 files match.
  • Contract constructors have correct arguments — trailing ctor word decodes exactly to wS.
  • The transactions that interacted with the newly deployed contract match the deploy script — N/A, no initialize/interaction call; the upgrade is delegated to governance.
  • Governance proposal matches the deploy script — ❌ cannot verify, see B2.
  • Smoke tests pass after fork test execution of the governance proposal — skipped per project decision.

🔴 B1 — deployments/sonic/OSVault.json is stale and was not updated by this PR

The committed artifact still records:

Field Committed artifact This PR's actual deploy
address 0xF66886e242e20cAb2496AF1d411eBcFb73440270 0x41DF78939406BF3f189c304C72f01fAd7acAfce7
tx 0x085fd708…f45d83a2 0xcd82182c…e8983653

git log shows the artifact was last touched by 69923754b (#2894), and 0xF66886…0270 is the current, pre-upgrade implementation — op 029_vault_permissioned_rebase upgraded to it, and the proxy's EIP-1967 slot still reads it today. The new impl 0x41DF…Fce7 appears nowhere under deployments/.

Impact: anything resolving by name — ethers.getContract("OSVault"), later deploy scripts, Talos actions — gets the old implementation. The PR body already notes "post-deployment metadata generation failed"; the artifact was never regenerated and committed.

🔴 B2 — Governance operation files are missing

The description references deployments/sonic/operations/031_disable_mints.schedule.json and .execute.json. Neither exists in the repo — this PR changes only three files:

contracts/contracts/vault/OSVault.sol
contracts/deploy/sonic/031_disable_mints.js
contracts/test/vault/os-vault.sonic.js

With no committed payload, the script's single action — OSonicVaultProxy(0xa3c0…0186).upgradeTo(0x41DF…Fce7) — cannot be diffed against any governance artifact.

🟡 B3 — The description documents changes that aren't in the diff

The entire Storage-layout validation section (stale-cache detection, reconstructing OZ validation runs from build-info, rewriting the cache) corresponds to no file in this PR. Please confirm where that landed before relying on the quoted [storage-slots] Contract OSVault is safe for upgrade result — the validating tooling was itself reportedly patched.


✅ What checks out

Code match — sol2uml diff against the real deployed impl (clean)
sol2uml diff 0x41DF78939406BF3f189c304C72f01fAd7acAfce7 .,node_modules --network sonic

All files reported match, including contracts/vault/OSVault.sol, VaultCore.sol, VaultAdmin.sol, VaultStorage.sol, VaultInitializer.sol, token/OUSD.sol, governance/Governable.sol and the OpenZeppelin dependencies. No differences.

Constructor argument

Trailing 32-byte word of the creation input:

000000000000000000000000039e2fb66102314ce7b64ce5ce3e5183bc94ad38
-> 0x039e2fB66102314Ce7b64Ce5Ce3E5183bc94aD38  == addresses.sonic.wS  ✅
Intent — the mint restriction is actually effective

Inheritance resolves as OSVault → VaultAdmin → VaultCore, so the override binds. _mint(uint256) (VaultCore.sol:74, internal virtual) is the single chokepoint both public entry points route through:

  • VaultCore.sol:53 mint(...):58 _mint(_amount)
  • VaultCore.sol:65 mint(uint256):66 _mint(_amount)

Both are therefore gated behind msg.sender == strategistAddr || isGovernor(). ✅

ℹ️ Not a defect (expected pre-execution)

The upgrade has not executed — the proxy implementation slot still reads 0xF66886…0270. I also scanned the Sonic Timelock (0x31a91336414d3B955E494E7d485a6B06b55FC8fB) from the deploy block 76,311,221 → 76,321,409 and found no event referencing the new impl, i.e. no scheduled upgrade yet. Caveat: that window is only ~10k Sonic blocks, so it rules out a schedule shortly after deploy, not one made later.


Before merge

  1. Regenerate and commit deployments/sonic/OSVault.json pointing at 0x41DF…Fce7, plus the two 031_disable_mints ops JSON files.
  2. Once scheduled, confirm the Timelock payload is target 0xa3c0eCA00D2B76b4d1F170b0AB3FdeA16C180186, calldata 0x3659cfe6 + padded 41df78939406bf3f189c304c72f01fad7acafce7.
  3. Resolve B3 and independently confirm storage-layout upgrade safety.
  4. Off-chain multisig follow-up to schedule + execute the upgrade.

Read-only audit: block explorer + read-only RPC. No transactions sent, no files modified.

@naddison36

Copy link
Copy Markdown
Collaborator Author

@sparrowDom sorry, I missed pushing the deployment artifacts. Can you check again.
Thanks

@sparrowDom

Copy link
Copy Markdown
Member

Deployment verification — 031_disable_mints

Verified against nicka/sonic-mint @ 5acca972f · network sonic.

Re-reviewed after 5acca972f — the three gaps from my earlier pass (stale OSVault.json, missing governance ops files, storage-layout tooling absent from the diff) are all resolved. Code, constructor arg, deployment artifact and governance payload now agree with each other and with on-chain state.

Checklist

  • All deployed contracts listed — OSVault 0x41DF…Fce7; description, deployments/sonic/OSVault.json and creation tx 0xcd82…3653 all agree
  • Verified code + dependencies match master — sol2uml diff clean, 19/19 match
  • Constructor args correct — wS 0x039e2fB6…aD38, confirmed via deploy script, artifact .args, and the creation-input tail
  • Interaction txs match deploy script — N/A: implementation-only deploy, no initialize; the sole interaction is the governance upgrade below
  • Governance proposal matches deploy script
  • Smoke tests — skipped per project decision

Governance payload — both ops files target the Sonic Timelock 0x31a91336414d3B955E494E7d485a6B06b55FC8fB:

targets ["0xa3c0eCA00D2B76b4d1F170b0AB3FdeA16C180186"] (OSonicVaultProxy) · values ["0"] · payload 0x3659cfe6 + …41df78939406bf3f189c304c72f01fad7acafce7 (upgradeTo(address)) · predecessor 0x00…00 · delay 172800 = timelock getMinDelay().

Exactly one action, matching the script's cOSonicVaultProxy.upgradeTo(dOSonicVault.address). Schedule and execute share an identical salt 0x18621dd4…3321fe — worth noting, since a salt mismatch computes a different operation id and would revert executeBatch only after the 48h wait.

Storage layout — since tasks/storageSlots.js was itself patched in this PR, I diffed storageLayout/sonic/OSVault.json against master rather than trusting the tool's own verdict. Only solc AST type-ids moved (Strategy)45786→62668, OUSD)41354→56564); every slot (53/60/75/77), offset and label is unchanged. No slot shifts — consistent with OSVault adding only a function override and no new state.

Pending execution (not a blocker) — the proxy implementation slot still reads 0xF66886…0270, and no Timelock event references the new impl. Remaining: multisig scheduleBatch → 48h → executeBatch → confirm the slot then reads 0x41DF…Fce7.

Nit5acca972f is titled "New OSVault deployment", but nothing was redeployed: same address, regenerated metadata.

Read-only audit — block explorer + read-only RPC. No transactions sent.

sparrowDom
sparrowDom previously approved these changes Jul 22, 2026

@shahthepro shahthepro 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.

  • All deployed contracts are listed in the deploy PR's description
  • Deployed contract's verified code (and all dependencies) match the code in master
  • Contract constructors have correct arguments
  • The transactions that interacted with the newly deployed contract match the deploy script.
  • Governance proposal matches the deploy script
  • Smoke tests pass after fork test execution of the governance proposal

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.

3 participants