docs: add production update runbook to README#19
Open
hardyjosh wants to merge 8 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a new "Production Deployment (Base)" section to README.md describing two deployment flows (fresh redeploy and in-place production upgrade), step-by-step commands and verification, and a procedure for adding a new vault oracle; documentation-only, no code changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
README.md (1)
134-134: Split this step into two sentences for readability.Line 134 is dense and easier to misread during incident-style execution. Minor doc clarity tweak only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 134, The sentence beginning with "Update `LibProdDeploy.sol` — replace all address constants with the new deployments, with date + run ID comments" should be split into two sentences for clarity: keep the first sentence instructing to update LibProdDeploy.sol and replace address constants with new deployments, and make a second sentence that states to include date and run ID comments for each replacement; update the README line referencing `LibProdDeploy.sol` accordingly so the two shorter sentences read clearly and are easier to follow during incident execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 150: Replace the hardcoded multisig address in the runbook step that
instructs calling upgradeTo(newImplementation) on the beacon: remove the literal
0x8E4b...9f5b and instead instruct operators to retrieve the current beacon
owner from the canonical deployment/source-of-truth (e.g., deployment config or
on-chain source) and confirm it prior to calling upgradeTo; add an explicit
verification step to fetch and display the beacon owner and require operator
confirmation that it matches the expected admin before executing the upgrade.
- Around line 144-149: The README shows invoking an internal function
deployPythOracleAdapterBeaconSet(uint256) via forge --sig, but that function in
script/Deploy.sol is internal and not callable; update the docs to instruct
running the deployment suite via run() instead: describe setting DEPLOYMENT_KEY
and DEPLOYMENT_SUITE (e.g., pyth-oracle-adapter-beacon-set or
multi-pyth-oracle-adapter-beacon-set) and running forge script script/Deploy.sol
with --rpc-url/--broadcast/--verify so Forge executes the public run()
entrypoint that dispatches the correct Deploy.sol suite for
PythOracleAdapter/MultiPythOracleAdapter.
- Around line 127-132: The README's deploy example incorrectly calls --sig
"deployAll(uint256)" for script/Deploy.sol which only exposes run() and selects
behavior via DEPLOYMENT_SUITE; update the doc to call script/Deploy.sol without
--sig and show setting DEPLOYMENT_KEY and DEPLOYMENT_SUITE (e.g.,
DEPLOYMENT_SUITE=pyth-oracle-adapter-beacon-set) before running nix develop -c
forge script script/Deploy.sol, and list repeating suites
(morpho-protocol-adapter-beacon-set, passthrough-protocol-adapter-beacon-set,
oracle-unified-deployer, oracle-registry-beacon-set,
multi-pyth-oracle-adapter-beacon-set, multi-oracle-unified-deployer) so
operators run run() once per suite.
---
Nitpick comments:
In `@README.md`:
- Line 134: The sentence beginning with "Update `LibProdDeploy.sol` — replace
all address constants with the new deployments, with date + run ID comments"
should be split into two sentences for clarity: keep the first sentence
instructing to update LibProdDeploy.sol and replace address constants with new
deployments, and make a second sentence that states to include date and run ID
comments for each replacement; update the README line referencing
`LibProdDeploy.sol` accordingly so the two shorter sentences read clearly and
are easier to follow during incident execution.
| --sig "deployPythOracleAdapterBeaconSet(uint256)" $DEPLOYMENT_KEY \ | ||
| --rpc-url <base-rpc> --broadcast --verify | ||
| ``` | ||
| 2. **Upgrade the beacon** — call `upgradeTo(newImplementation)` on the existing beacon (owned by the multisig at `0x8E4b...9f5b`). All existing proxy instances upgrade in-place — no per-vault redeployment needed |
There was a problem hiding this comment.
Avoid hardcoded multisig address in runbook steps.
Line 150 embeds a specific owner address in operational docs. This can drift and cause upgrades to be sent to the wrong admin target. Prefer “current beacon owner from source-of-truth/deployment config” plus a verify step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 150, Replace the hardcoded multisig address in the runbook
step that instructs calling upgradeTo(newImplementation) on the beacon: remove
the literal 0x8E4b...9f5b and instead instruct operators to retrieve the current
beacon owner from the canonical deployment/source-of-truth (e.g., deployment
config or on-chain source) and confirm it prior to calling upgradeTo; add an
explicit verification step to fetch and display the beacon owner and require
operator confirmation that it matches the expected admin before executing the
upgrade.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
README.md (3)
150-150:⚠️ Potential issue | 🟠 MajorAvoid hardcoded multisig address in runbook steps.
The hardcoded owner address
0x8E4b...9f5bcan drift over time and cause upgrades to be sent to the wrong admin target. Instead, instruct operators to retrieve the current beacon owner from the canonical deployment source (e.g.,LibProdDeploy.solor on-chain query) and verify it matches the expected admin before executing the upgrade.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 150, Remove the hardcoded multisig address and update the runbook step around calling upgradeTo(newImplementation) on the beacon to instruct operators to fetch and verify the current beacon owner from the canonical deployment source (e.g., LibProdDeploy.sol) or by querying the beacon's owner() on-chain, confirm it matches the expected admin/multisig, and only then call upgradeTo; reference the beacon, upgradeTo(newImplementation), LibProdDeploy.sol and the beacon's owner() query in the step and replace the literal address with a placeholder and verification checklist.
144-149:⚠️ Potential issue | 🔴 CriticalPath B script invocation calls an internal function.
The command uses
--sig "deployPythOracleAdapterBeaconSet(uint256)", but this function isinternalinscript/Deploy.soland cannot be called by Forge as an entrypoint. Userun()withDEPLOYMENT_SUITEinstead (e.g.,DEPLOYMENT_SUITE=pyth-oracle-adapter-beacon-setormulti-pyth-oracle-adapter-beacon-setfor multi-feed adapters).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 144 - 149, The README's deploy example incorrectly calls an internal function deployPythOracleAdapterBeaconSet(uint256) via --sig; instead update the instructions to invoke the script's public entrypoint run() using the DEPLOYMENT_SUITE environment variable (e.g., DEPLOYMENT_SUITE=pyth-oracle-adapter-beacon-set or DEPLOYMENT_SUITE=multi-pyth-oracle-adapter-beacon-set) so Forge runs the appropriate deployment path in script/Deploy.sol rather than trying to call the internal function.
127-132:⚠️ Potential issue | 🔴 CriticalDeployment command is invalid and will not run.
This command references
deployAll(uint256)which is not exposed byscript/Deploy.sol. The script only exposesrun()and selects deployment behavior via theDEPLOYMENT_SUITEenvironment variable. Operators following this runbook will encounter a failure.Use
run()withDEPLOYMENT_SUITEset to the appropriate suite name (e.g.,pyth-oracle-adapter-beacon-set,morpho-protocol-adapter-beacon-set, etc.) and run once per suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 127 - 132, The README's deploy example incorrectly calls deployAll(uint256); update the instructions to call the script's exposed run() entrypoint and set the DEPLOYMENT_SUITE environment variable to the desired suite name (e.g., pyth-oracle-adapter-beacon-set, morpho-protocol-adapter-beacon-set) and run once per suite; specifically, replace the deployAll(uint256) reference with run() and add guidance to export or prefix DEPLOYMENT_SUITE before invoking the existing forge script so operators run the correct suite-specific deployment via run().
🧹 Nitpick comments (2)
README.md (2)
164-165: Provide concrete command templates for new vault oracle deployment.Steps 1-2 describe calling
OracleUnifiedDeployer.deploy()andOracleRegistry.setOracle()but don't provide specific commands or parameter examples. For a production runbook, concrete templates improve operator confidence and reduce errors.📋 Suggested improvement
1. Call `OracleUnifiedDeployer.deploy(...)` (or `MultiOracleUnifiedDeployer` for multi-feed) with the vault address, Pyth feed ID(s), and protocol adapter config + ```bash + cast send <unified-deployer-address> "deploy(address,bytes32,bool,bool,bool)" \ + <vault-address> <pyth-feed-id> true true true \ + --rpc-url <base-rpc> --private-key <key> + ``` + (Adjust boolean flags based on which protocol adapters are needed: Morpho, Aave, Compound) 2. Call `OracleRegistry.setOracle(vault, oracleAdapter)` from the registry admin to register the new oracle + ```bash + cast send <registry-address> "setOracle(address,address)" \ + <vault-address> <oracle-adapter-address> \ + --rpc-url <base-rpc> --private-key <admin-key> + ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 164 - 165, Add concrete CLI command templates and parameter examples to the README for deploying and registering a vault oracle: show a cast/send (or ethers-cli) example invoking OracleUnifiedDeployer.deploy(...) and MultiOracleUnifiedDeployer.deploy(...) with placeholders for <unified-deployer-address>, <vault-address>, <pyth-feed-id(s)>, and explicit boolean flags (e.g., true/false for Morpho/Aave/Compound) and RPC/private-key options, and likewise provide a cast/send template for OracleRegistry.setOracle(vault, oracleAdapter) with placeholders for <registry-address>, <vault-address>, <oracle-adapter-address>, --rpc-url and --private-key; ensure the templates clearly label which parameters to replace and add a short note to adjust booleans per protocol adapters.
138-138: Provide a concrete verification command.Path A's final verification step is vague ("check
latestRoundData()on a proxy") without specifying which proxy or providing a command, unlike Path B step 4 (lines 152-155) which includes a completecast callexample. This reduces the runbook's actionability for operators.📋 Suggested improvement
-7. **Verify** — check `latestRoundData()` on a proxy and confirm the subgraph is indexing +7. **Verify** — check `latestRoundData()` on a deployed oracle proxy and confirm the subgraph is indexing: + ```bash + cast call <oracle-proxy-address> "latestRoundData()(uint80,int256,uint256,uint256,uint80)" --rpc-url <base-rpc> + ``` + Confirm the subgraph is indexing by checking the Goldsky dashboard or querying the endpoint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 138, Replace the vague "check `latestRoundData()` on a proxy" step with a concrete verification command and clear post-check guidance: instruct operators to run a `cast call` against the oracle proxy address using the ABI signature `latestRoundData()(uint80,int256,uint256,uint256,uint80)` with a specified RPC URL (use placeholders like `<oracle-proxy-address>` and `<base-rpc>`), and then confirm indexing by checking the Goldsky dashboard or querying the subgraph endpoint; update the README step to include that exact command pattern and the follow-up verification action.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.md`:
- Line 150: Remove the hardcoded multisig address and update the runbook step
around calling upgradeTo(newImplementation) on the beacon to instruct operators
to fetch and verify the current beacon owner from the canonical deployment
source (e.g., LibProdDeploy.sol) or by querying the beacon's owner() on-chain,
confirm it matches the expected admin/multisig, and only then call upgradeTo;
reference the beacon, upgradeTo(newImplementation), LibProdDeploy.sol and the
beacon's owner() query in the step and replace the literal address with a
placeholder and verification checklist.
- Around line 144-149: The README's deploy example incorrectly calls an internal
function deployPythOracleAdapterBeaconSet(uint256) via --sig; instead update the
instructions to invoke the script's public entrypoint run() using the
DEPLOYMENT_SUITE environment variable (e.g.,
DEPLOYMENT_SUITE=pyth-oracle-adapter-beacon-set or
DEPLOYMENT_SUITE=multi-pyth-oracle-adapter-beacon-set) so Forge runs the
appropriate deployment path in script/Deploy.sol rather than trying to call the
internal function.
- Around line 127-132: The README's deploy example incorrectly calls
deployAll(uint256); update the instructions to call the script's exposed run()
entrypoint and set the DEPLOYMENT_SUITE environment variable to the desired
suite name (e.g., pyth-oracle-adapter-beacon-set,
morpho-protocol-adapter-beacon-set) and run once per suite; specifically,
replace the deployAll(uint256) reference with run() and add guidance to export
or prefix DEPLOYMENT_SUITE before invoking the existing forge script so
operators run the correct suite-specific deployment via run().
---
Nitpick comments:
In `@README.md`:
- Around line 164-165: Add concrete CLI command templates and parameter examples
to the README for deploying and registering a vault oracle: show a cast/send (or
ethers-cli) example invoking OracleUnifiedDeployer.deploy(...) and
MultiOracleUnifiedDeployer.deploy(...) with placeholders for
<unified-deployer-address>, <vault-address>, <pyth-feed-id(s)>, and explicit
boolean flags (e.g., true/false for Morpho/Aave/Compound) and RPC/private-key
options, and likewise provide a cast/send template for
OracleRegistry.setOracle(vault, oracleAdapter) with placeholders for
<registry-address>, <vault-address>, <oracle-adapter-address>, --rpc-url and
--private-key; ensure the templates clearly label which parameters to replace
and add a short note to adjust booleans per protocol adapters.
- Line 138: Replace the vague "check `latestRoundData()` on a proxy" step with a
concrete verification command and clear post-check guidance: instruct operators
to run a `cast call` against the oracle proxy address using the ABI signature
`latestRoundData()(uint80,int256,uint256,uint256,uint80)` with a specified RPC
URL (use placeholders like `<oracle-proxy-address>` and `<base-rpc>`), and then
confirm indexing by checking the Goldsky dashboard or querying the subgraph
endpoint; update the README step to include that exact command pattern and the
follow-up verification action.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
160-160:⚠️ Potential issue | 🟠 MajorRequire explicit on-chain beacon owner verification before
upgradeTo.Line 160 references
BEACON_INITIAL_OWNER, but that value can drift after ownership transfers. Add a mandatory pre-upgrade check (e.g.,cast call <beacon> "owner()(address)") and require operator confirmation against the expected admin source-of-truth before executingupgradeTo(newImplementation).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 160, The README's upgrade instructions assume BEACON_INITIAL_OWNER still controls the beacon; add an explicit on-chain owner check before calling upgradeTo by invoking the beacon's owner() (e.g., via cast call "<beacon> \"owner()(address)\"") and require the operator to confirm the returned owner matches the expected admin/source-of-truth (not BEACON_INITIAL_OWNER constant) before executing upgradeTo(newImplementation); update the step to require this verification and an operator confirmation step to proceed with LibProdDeploy's upgrade flow.
🧹 Nitpick comments (1)
README.md (1)
142-142: Split this runbook step into two sentences for scanability.Line 142 is dense and easy to misread during ops. Consider splitting the “update constants” instruction and the “date + run ID comments” instruction into separate sentences.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 142, The README step for updating LibProdDeploy.sol is too dense; split the current single sentence into two concise sentences: one instructing operators to replace all address constants in LibProdDeploy.sol with the new deployment addresses, and a separate sentence instructing them to append date and run ID comments to each replaced constant for auditability; ensure both sentences remain adjacent and preserve the original intent and wording tone for clarity during ops.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.md`:
- Line 160: The README's upgrade instructions assume BEACON_INITIAL_OWNER still
controls the beacon; add an explicit on-chain owner check before calling
upgradeTo by invoking the beacon's owner() (e.g., via cast call "<beacon>
\"owner()(address)\"") and require the operator to confirm the returned owner
matches the expected admin/source-of-truth (not BEACON_INITIAL_OWNER constant)
before executing upgradeTo(newImplementation); update the step to require this
verification and an operator confirmation step to proceed with LibProdDeploy's
upgrade flow.
---
Nitpick comments:
In `@README.md`:
- Line 142: The README step for updating LibProdDeploy.sol is too dense; split
the current single sentence into two concise sentences: one instructing
operators to replace all address constants in LibProdDeploy.sol with the new
deployment addresses, and a separate sentence instructing them to append date
and run ID comments to each replaced constant for auditability; ensure both
sentences remain adjacent and preserve the original intent and wording tone for
clarity during ops.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
No documented process for what needs to happen when oracle adapter logic changes. Easy to miss steps (beacon upgrade, subgraph redeploy, address updates).
Solution
Adds an "Update Runbook" section to the README covering:
LibProdDeploy.soladdressesAlso documents the process for adding a new vault oracle.
Checks
Summary by CodeRabbit