feat(proxy-oracle): standardize contract self-upgrade (ENG-481)#519
Merged
peer2f00l merged 11 commits intoJul 22, 2026
Merged
Conversation
Extend the self-upgrade path across contracts to accept a source enum —
raw wasm blob, global contract by code hash, or by account id — with an
atomic deploy-then-migrate. Add a governance SelfUpgrade operation and give
the proxy-oracle governance contract versioned state + a v0->v1 migration.
- UpgradeSource + deploy_and_migrate helper in templar-common. `Code` is
untagged in JSON (a bare base64 string) and the governance->oracle wire
key stays `code`, so raw-blob admin_upgrade is compatible across contract
versions and order-independent.
- proxy-oracle & pyth-lazer admin_upgrade take an UpgradeSource.
- Governance: new SelfUpgrade operation (own TTL, Admin role, empty-code
validation, local self-deploy dispatch); versioned_state + a v0->v1
migration that also converts pending proposal bodies to the new layout.
- Framework migrate() no-ops on a same-version refresh, decided by
needs_migration() (never the caller's args), so a required migration can
never be skipped.
- Governance lifecycle events now carry {id, kind} rather than the full
proposal, whose payload (an AdminUpgrade/SelfUpgrade wasm blob) otherwise
exceeds NEAR's 16KB per-log limit.
- pyth-lazer Upgraded event: structured UpgradeSummary + optional
migrate_args (v2.0.0).
- Manager: --code-file | --global-hash | --global-account-id, plus a
self-upgrade subcommand; typed proposal tests.
- Both-orders key-driven upgrade sandbox test against pinned mainnet blobs
(proxy-oracle 0.3.0 / proxy-governance 0.1.0).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…e-methods-support-upgrading-to-a-global-contract-by
Post-review cleanup from an adversarial correctness review and a /simplify pass: - Migration: panic on a >u32 proposal id (mirroring the contract's overflow behavior) instead of silently dropping the proposal; correct a stale test comment about sandbox migrate() coverage. - Manager: drop hand-rolled parse_global_hash; Base58CryptoHash's FromStr is clap's default value parser. - Sandbox test-common: reuse the shared test_signer (cached Arc<Signer>) and pin TEST_FINALITY_POLICY on reads/writes, matching the oracle test common; drop the dead `call` helper. - upgrade_ordering: collapse deploy_init/deploy_migrate into one helper and run the two independent setup deploys concurrently. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address review-bot findings on the admin_upgrade event: - The deploy+migrate runs in the returned Promise, a separate receipt from the one that emits the event, so the log is committed even if that receipt reverts. The prior "one atomic receipt" comment was wrong. Reframe the event (and its doc) as recording the *scheduled* upgrade, not completion. - migrate_args was logged in full (base64), an unbounded payload that could exceed NEAR's 16KB per-log limit and abort the receipt — the same class of bug fixed for the governance events. Replace it with a bounded `migrated` flag (also drops the migrate_args clone). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Ruthless /simplify pass over the change's comments: cut lines that restated the code, narrated a sequence the code already shows, or carried design-rationale that belongs in a commit/PR. Kept only invariants and non-obvious "why" (borsh-layout notes, the migrate decision guarantee, the untagged-JSON ordering, the drop-before-prefix-reuse footgun, the "event records scheduling not completion" semantics). Behavior and public API unchanged; comments only. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
`UpgradeSource::GlobalAccountId` was a live delegation to the publisher account: a later republish there would swap the contract's code with no proposal, timelock, role check, or `migrate` — bypassing governance and bricking versioned state. Remove it; keep only `Code` (raw blob) and `GlobalHash` (immutable, content-addressed). Pin the persisted borsh tags with explicit discriminants forwarded to the derive (`borsh(use_discriminant = true)` + `#[repr(u8)]`, `Code = 0`, `GlobalHash = 1`). `Code` stays declared last for serde's untagged rule, but its borsh tag no longer tracks declaration order — a future variant takes its own discriminant without disturbing stored proposals. A golden-byte test locks the format. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…igrate Address adversarial-review findings on the upgrade work: - versioned_state: after `Migrator::run`, re-assert `!needs_migration()` so a deploy more than one version ahead of stored reverts atomically instead of returning success with state stranded between versions. - upgrade_ordering: seed a pending `AdminUpgrade` proposal on the old gov before the v0→v1 migrate, then assert the migrated `Proposal<Operation>` reshaped its raw code into `UpgradeSource::Code` — so the on-chain proposal-body reshape is actually exercised, not an empty map. Probe each upgraded contract with a domain view (`own_get_owner`) to prove it still answers. Build the seed/expect values from the typed `Operation`, not `json!`. - Trim two comments that restated the docs they pointed at. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughSummary
Review focus
WalkthroughThis PR adds shared upgrade sources for raw WASM and global contract hashes, deploy-and-migrate execution, governance self-upgrades, versioned governance-state migration, compact event payloads, CLI support, and sandbox coverage. ChangesUpgrade and governance modernization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 4
🤖 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 `@contract/proxy-oracle/near/contract/tests/proxy_oracle.rs`:
- Around line 159-195: Strengthen the migrate action assertions in
admin_upgrade_from_global_hash_uses_global_contract_then_migrates by also
verifying that args equal the constructed migrate_args value and
attached_deposit is zero, matching
admin_upgrade_creates_one_self_receipt_with_deploy_then_migrate while preserving
the existing method and gas checks.
In `@contract/proxy-oracle/near/governance-contract/src/lib.rs`:
- Around line 293-296: Update the self-upgrade branch for Operation::SelfUpgrade
so the receipt returned by code.deploy_and_migrate is observed rather than
silently detached. Attach the contract’s established success/failure outcome
handling or alert mechanism, ensuring migration failures are surfaced while
preserving the existing deploy-and-migrate behavior.
In `@contract/pyth-lazer/contract/src/lib.rs`:
- Around line 500-510: Update admin_upgrade to call UpgradeSource::is_empty_code
before emitting PythLazerEvent::Upgraded or scheduling deploy_and_migrate, and
fail immediately when it reports an empty code blob. Preserve the existing
authorization, event, and deployment behavior for non-empty upgrade sources.
In `@contract/pyth-lazer/contract/tests/test.rs`:
- Around line 1042-1062: Extend admin_upgrade_emits_upgraded to parameterize the
UpgradeSource input and add an #[case] using UpgradeSource::GlobalHash(...).
Assert the emitted event’s event["data"]["code"]["GlobalHash"] summary for that
case, while preserving the existing Code hash assertion and migration-flag
coverage.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6069545f-0134-487b-a280-2b8a481982d2
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockgateway/testing/src/wasm/proxy_governance_0_1_0.wasmis excluded by!**/*.wasmgateway/testing/src/wasm/proxy_oracle_0_3_0.wasmis excluded by!**/*.wasm
📒 Files selected for processing (23)
.config/nextest.tomlcommon/src/lib.rscommon/src/upgrade.rscommon/src/versioned_state/macros.rscontract/proxy-oracle/near/common/src/governance.rscontract/proxy-oracle/near/contract/src/lib.rscontract/proxy-oracle/near/contract/tests/proxy_oracle.rscontract/proxy-oracle/near/governance-common/src/interface.rscontract/proxy-oracle/near/governance-common/src/lib.rscontract/proxy-oracle/near/governance-contract/Cargo.tomlcontract/proxy-oracle/near/governance-contract/src/lib.rscontract/proxy-oracle/near/governance-contract/src/state.rscontract/proxy-oracle/near/governance-contract/src/tests.rscontract/proxy-oracle/near/governance-contract/tests/common/mod.rscontract/proxy-oracle/near/governance-contract/tests/upgrade_ordering.rscontract/pyth-lazer/contract/src/events.rscontract/pyth-lazer/contract/src/lib.rscontract/pyth-lazer/contract/tests/test.rsgateway/testing/src/sandbox.rsgateway/testing/src/wasm.rstools/manager/src/commands/proxy_oracle/governance/create_proposal.rstools/manager/src/commands/proxy_oracle/governance/mod.rstools/manager/src/tests/proxy_oracle.rs
carrion256
reviewed
Jul 22, 2026
carrion256
left a comment
carrion256
left a comment
Collaborator
There was a problem hiding this comment.
Really cool actually. Couple Qs
| pub fn deploy_and_migrate( | ||
| self, | ||
| migrate_method: impl Into<String>, | ||
| migrate_args: Base64VecU8, |
Collaborator
There was a problem hiding this comment.
Is this type not limiting calldata by ~37% rather than raw byte array?
Collaborator
Author
There was a problem hiding this comment.
Depends. Vec<u8> is serialized as [1, 2, 3...]. However, we could use Borsh, though that hurts inspectability on block explorers. But the migration arguments are usually pretty simple (usually similar in size to the arguments sent to a new initialization constructor function call).
| GlobalHash(Base58CryptoHash) = 1, | ||
| /// A raw WASM blob deployed onto the account. | ||
| #[serde(untagged)] | ||
| Code(Base64VecU8) = 0, |
Collaborator
There was a problem hiding this comment.
Is this type not limiting calldata by ~37% rather than raw byte array? Ignoring *args, selecting base58 here reduces max contract blob.
Also, if i was being particularly anal about this I'd mention we dont even need to decode it at all and just take the first frame as the blob.
Collaborator
Author
There was a problem hiding this comment.
Yes, this is, but the only realistic way to reduce the size would be to start serializing everything with Borsh.
This should not really matter though, because we should be moving to using global contract hashes for the vast majority of contract deployments and upgrades moving forward.
Collaborator
There was a problem hiding this comment.
What is the issue with doing the proper way of just passing a Vec rather than intentionally limit every contract to ~63% size limit. I dont see every contract becoming a global contract just because, otherwise we shouldnt even support this type in the first place.
| )) | ||
| }) | ||
| .collect(); | ||
| drop(old_proposals); |
Collaborator
There was a problem hiding this comment.
Why are we smarter than the borrow checker here?
Collaborator
Author
There was a problem hiding this comment.
We want to run the Drop implementation here (we could equivalently call .flush(), I suppose) to actually clear the contract storage.
Collaborator
There was a problem hiding this comment.
The borrow checker calls drop when the variable goes out of scope. Why do we need to try to call it?
- pyth-lazer admin_upgrade: reject an empty `Code` blob up front (mirrors the governance path's EmptyUpgradeCode guard) so no misleading Upgraded event is logged and no gas is spent on a doomed deploy; add a should_panic test. - pyth-lazer test: cover the GlobalHash summary variant by cross-parameterizing admin_upgrade_emits_upgraded over source × migrate_args, asserting against the typed UpgradeSource::summary(). - proxy-oracle test: assert args and attached_deposit on the global-hash migrate receipt, matching the raw-blob sibling test. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 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 `@contract/pyth-lazer/contract/tests/test.rs`:
- Around line 1054-1075: Extend admin_upgrade_emits_upgraded to inspect the
created receipt in addition to the NEP-297 event. Assert the receipt contains
the expected Code or GlobalHash action matching code, the migrate arguments
matching migrate_args, zero deposit, and GAS_FOR_MIGRATE, while preserving the
existing assertions for both UpgradeSource variants.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4c1a62ac-228c-4dd9-9d3d-b22653787c1e
📒 Files selected for processing (3)
contract/proxy-oracle/near/contract/tests/proxy_oracle.rscontract/pyth-lazer/contract/src/lib.rscontract/pyth-lazer/contract/tests/test.rs
admin_upgrade_emits_upgraded now inspects the scheduled receipt for both UpgradeSource variants: DeployContract/UseGlobalContract as the first action, and a `migrate` call carrying the exact args, zero deposit, and GAS_FOR_MIGRATE — locking the adapter-specific deploy+migrate wiring, not just the NEP-297 event. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…e-methods-support-upgrading-to-a-global-contract-by # Conflicts: # .config/nextest.toml
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
justfile (1)
22-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep explicit package selection aligned with the sandbox filter.
When
--packageis supplied, the default-parguments are disabled, but the fixedsandbox_test_filterstill only admits packages insandbox_full_packages. Selecting another valid package can therefore run zero tests while appearing successful. Derive the filter from explicit packages, or reject packages outside the sandbox allowlist.Also applies to: 79-84, 106-115
🤖 Prompt for 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. In `@justfile` around lines 22 - 26, Align the explicit package-selection path with sandbox_test_filter so --package selections outside sandbox_full_packages are not silently filtered to zero tests. Update the relevant package argument/filter construction near sandbox_test_filter and its other usages to derive the sandbox filter from the explicitly selected packages, or reject selections outside the sandbox allowlist while preserving current defaults.
🤖 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 `@contract/pyth-lazer/contract/tests/test.rs`:
- Around line 1084-1088: Strengthen the assertions in the UpgradeSource match
test: for UpgradeSource::Code, destructure MockAction::DeployContract and
compare its payload with the source code bytes; for UpgradeSource::GlobalHash,
destructure MockAction::UseGlobalContract and compare its hash with the source
value. Preserve the existing action-variant checks while validating the
forwarded payloads.
---
Outside diff comments:
In `@justfile`:
- Around line 22-26: Align the explicit package-selection path with
sandbox_test_filter so --package selections outside sandbox_full_packages are
not silently filtered to zero tests. Update the relevant package argument/filter
construction near sandbox_test_filter and its other usages to derive the sandbox
filter from the explicitly selected packages, or reject selections outside the
sandbox allowlist while preserving current defaults.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d440936a-20b3-415a-8ddc-245f054fb0a3
📒 Files selected for processing (2)
contract/pyth-lazer/contract/tests/test.rsjustfile
…ction Destructure the deploy action and check the forwarded payload: for `Code`, compare `DeployContract`'s bytes to the source blob; for `GlobalHash`, compare `UseGlobalContract`'s `GlobalContractIdentifier::CodeHash` to the source hash. A regression forwarding a different source to `deploy_and_migrate` now fails. Adds near-primitives (already the workspace-pinned 0.34.7) as a dev-dep to name the identifier type. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25eeb21b85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
This follow up should be resolved before deploying new contract versions: https://linear.app/templar-protocol/issue/ENG-517/migrate-methods-support-running-n-consecutive-migrations-in-one |
peer2f00l
deleted the
feature/eng-481-upgrade-methods-support-upgrading-to-a-global-contract-by
branch
July 22, 2026 16:47
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
2 participants
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.
Standardizes NEAR contract self-upgrade across the proxy-oracle, its governance contract, and the pyth-lazer adapter.
What
UpgradeSource+deploy_and_migrateprimitive (common/src/upgrade.rs): one shared way to atomically deploy new code and runmigratein a single receipt (a failed migration reverts the deploy). Two sources —Code(raw blob) andGlobalHash(immutable, content-addressed global contract). Global-by-account-id is deliberately excluded: it is a live delegation to the publisher that would bypass governance and brick versioned state.Codeis untagged in JSON (bare base64, matching the pre-UpgradeSourcewire); borsh discriminants are pinned explicitly (use_discriminant) so persisted governance proposals stay stable across future variants.SelfUpgradeaction +self_upgradeTTL on the proxy-oracle governance contract, and a v0→v1 versioned-state migration that reshapes pendingAdminUpgradeproposal bodies into the newOperationlayout.impl_versioned_state!): stored-vs-target version — nevermigrate_args— decides whether to migrate, and a post-run re-assert makes a multi-version-skip deploy revert atomically instead of stranding state.{id, kind}(was the full proposal, incl. the wasm blob — over the 16KB log limit); pyth-lazer'sUpgradedemits a boundedUpgradeSummaryand is honestly framed as scheduled, not completed.admin-upgrade/self-upgradeproposal builders sharing a typedUpgradeArgs.Testing
upgrade_orderingtest: upgrades both contracts from their real pinned mainnet blobs to this branch, in both orders, seeding a real pending proposal so the on-chain reshape is exercised, and probing each upgraded contract with a domain view — proving no bricking.Adversarial review ran to zero findings;
/simplifyapplied.This change is