feat(versioned-state): run a chained list of migrations in one upgrade (ENG-517)#522
Open
peer2f00l wants to merge 6 commits into
Open
Conversation
…e (ENG-481) Generalize the macro-generated `migrate()` from a single state transform to an ordered list `[A, B, C]` run consecutively in one atomic receipt. The chain is fully validated before any transform runs — first input == stored version, each output == the next input, last output == target — so an invalid chain reverts without writing state. Each step's `StateTransformer::run` still re-asserts `stored == input_version()` as it goes. Backwards compatible: `migrate_args` still accepts the legacy single-object wire shape (parsed as a one-element chain) as well as a JSON array. Production-deployed migrations (incl. universal-account `UnbrickV1`) are untouched — the `Migrator` trait gains only additive `input_version`/`output_version` accessors that delegate to the underlying transformer. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Drop the per-iteration `index == 0` branch and `.enumerate()` from `run_migration_chain`: split the first migration off with `split_first` (which also yields the empty-chain check), validate its start against stored version once, then loop the rest for link mismatches. Same behavior and same distinct Start/Link/End errors, less branching. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 selected for processing (1)
📝 WalkthroughSummary
Human review focus
WalkthroughThe versioned-state migration framework now accepts legacy or ordered migration input, validates complete version chains before execution, and runs multiple migrations sequentially. Migration implementations report input and output versions, with tests covering successful chains and rejected invalid or deserialization-failing migrations. ChangesVersioned migration chains
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeneratedMigrate
participant parse_one_or_many
participant run_migration_chain
participant MigrationSteps
Caller->>GeneratedMigrate: Submit legacy object or migration array
GeneratedMigrate->>parse_one_or_many: Deserialize migration input
parse_one_or_many-->>GeneratedMigrate: Return ordered migrations
GeneratedMigrate->>run_migration_chain: Validate chain against target version
run_migration_chain->>MigrationSteps: Run validated steps sequentially
MigrationSteps-->>GeneratedMigrate: Advance stored state version
GeneratedMigrate-->>Caller: Return migration result
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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: 3
🤖 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 `@common/src/versioned_state/core.rs`:
- Around line 264-411: Parameterize the four rejection tests around
run_migration_chain with rstest, consolidating their shared context setup,
invocation, expected MigrationChainError, and unchanged-state assertion into one
readable test. Supply cases for empty, wrong-start, broken-link, and
end-mismatch scenarios while preserving each case’s inputs and exact error
expectations.
- Around line 171-215: Verify the runtime’s panic-rollback semantics support the
atomicity assumed by run_migration_chain when multiple migration.run() calls
execute in one receipt. Also confirm realistic migration chain lengths fit
available gas, especially alongside migrate() authorization and the updated
required Migrator methods; update implementation or validation only if these
checks expose an issue.
In `@common/src/versioned_state/macros.rs`:
- Around line 48-68: Document and enforce a bounded migration-chain policy
around the macro’s migrations parsing and run_migration_chain call: validate
that the number of steps does not exceed a defined sane maximum before executing
any transform, and panic with a clear error when exceeded. Expose or reuse a
named limit so callers can account for the expected gas per step while
preserving atomic validation and execution.
🪄 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: 7540f220-8ca8-4802-80e9-dafb0d143f0e
📒 Files selected for processing (6)
common/src/versioned_state/core.rscommon/src/versioned_state/macros.rscontract/proxy-oracle/near/common/src/state/migration/mod.rscontract/proxy-oracle/near/governance-contract/src/state.rscontract/pyth-lazer/contract/src/state/migration.rsuniversal-account/src/state/migration.rs
peer2f00l
added a commit
that referenced
this pull request
Jul 22, 2026
Consolidate the four chain_rejects_* tests into a single #[rstest] case table (empty, wrong-start, broken-link, not-landing-on-target), each pinning the exact MigrationChainError variant and asserting no state was written. Addresses PR #522 review feedback; aligns with the repo's rstest guideline. 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: 1b6df7b6e9
ℹ️ 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".
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: 2
🤖 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/universal-account/tests/migration.rs`:
- Around line 450-454: Extend the failed V0→V1 migration assertions around the
existing outcome checks to verify stored_state_version(...) remains 0 after
execution. Keep the current failure and deserialization-error assertions, and
use the existing stored-state lookup to confirm the failed receipt does not
persist a version update.
In `@script/sandbox-up.sh`:
- Line 27: Restore the prebuild step in sandbox startup by re-enabling the
prebuild-test-contracts.sh invocation in script/sandbox-up.sh, ensuring it
builds the test Wasms and exports TEST_CONTRACTS_PREBUILT=1 before node-backed
tests run. If startup logic was moved elsewhere, place both the prebuild
invocation and environment export in that replacement path instead.
🪄 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: c60e1203-26b9-4a08-8d6c-460ce10234a3
📒 Files selected for processing (3)
common/src/versioned_state/core.rscontract/universal-account/tests/migration.rsscript/sandbox-up.sh
Consolidate the four chain_rejects_* tests into a single #[rstest] case table (empty, wrong-start, broken-link, not-landing-on-target), each pinning the exact MigrationChainError variant and asserting no state was written. Addresses PR #522 review feedback; aligns with the repo's rstest guideline. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…-517)
Fixes a red-on-dev regression: ENG-481's all-or-nothing migrate guard forbids
leaving state stranded between versions, so the universal-account integration
tests that migrated a two-versions-behind contract one step at a time began
reverting (masked in CI because these sandbox tests are gated out of the default
run). Convert them to the chained API this branch introduces:
- from_0_2_0: migrate [V0, V1] in one call to reach v2.
- migrate_accepts_legacy_direct_payload: send a bare `{"from_version":"unbrick_v1"}`
object against 0.4.0 (one step to target), still exercising the legacy
non-array wire shape.
- from_0_4_0_v0_then_v1_fails_to_deserialize: single chained [V0, V1] call that
passes version validation but fails to borsh-read 0.4.0's V1-shaped state.
Test-only change; the V0/V1/UnbrickV1 state transformers are untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
peer2f00l
force-pushed
the
feature/eng-517-migrate-methods-support-running-n-consecutive-migrations-in
branch
from
July 22, 2026 20:55
1b6df7b to
530b04a
Compare
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/universal-account/tests/migration.rs`:
- Around line 438-458: Verify the migration integration test covering the
legacy-object V0→V1 failure and atomic rollback using the narrowest documented
Rust sandbox test command. Confirm asynchronous receipt execution and sandbox
deployment are exercised, and explicitly document the verification step if it
cannot be run in the current environment.
🪄 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: 7113dd1d-3190-451c-8422-d8a27eae8ad8
📒 Files selected for processing (2)
common/src/versioned_state/core.rscontract/universal-account/tests/migration.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 530b04ab27
ℹ️ 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".
Post-review cleanup (no behavior change): - Drop the inline comment in `run_migration_chain` that restated the function's doc comment verbatim. - Fix the pyth-lazer empty-`Migration` doc: a new variant now needs arms in the whole `Migrator` impl (input_version/output_version/run), not just `run`; and with the contract at its launch version `migrate` is a no-op rather than "fails to deserialize". Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
migrate now validates and runs the whole migration chain in one call, landing on the target state version. The stepwise recipe (migrate V0, then migrate V1) no longer works — it fails on the first legacy object with EndMismatch. Document the JSON-array chain form, the end-to-end validation rules, the still-supported bare-object form, and that unbrick_v1 lands directly on V2. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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.
Summary
Follow-up to ENG-481. Generalizes the macro-generated
migrate()from running a single state transform per upgrade to an ordered list[A, B, C]run consecutively in one atomic receipt. This lets a contract that has fallen 2+ state versions behind catch up in a single deploy+migrate, instead of needing multiple round-trips or a bespoke "skip N versions" transformer.The chain is fully validated before any transform runs, so an invalid chain reverts without writing state:
<CurrentState>::VERSION).Each step's
StateTransformer::runstill independently re-assertsstored == input_version()and bumps the stored version as it goes; the trailingrequire!(!needs_migration())post-condition is kept as defense-in-depth.Backwards compatibility
migrate_argsstill accepts the legacy single-object wire shape ({"from_version":"v0"}, parsed as a one-element chain) as well as a JSON array. No pending governance proposals or existing callers change. The opaque governance/manager--migrate-args-filepassthrough already forwards raw bytes, so an operator can supply an array today with no code change.Production-migration immutability
No already-deployed migration is modified. The
Migratortrait gains only additiveinput_version/output_versionaccessors that delegate to the underlyingStateTransformer— so universal-account'sUnbrickV1(which overridesinput_version()to 0 while reading v1 state) flows through unchanged.Changes
common/src/versioned_state/core.rs— extendtrait Migrator; addparse_one_or_many(single-object-or-array),run_migration_chain(validate-then-run),MigrationChainError.common/src/versioned_state/macros.rs—migrate()parses one-or-many and delegates torun_migration_chain, keeping the ENG-481 needs/empty-input gate and post-condition.Migratorimpls (universal-account, proxy-oracle common, governance-contract, pyth-lazer empty-enum) gain the two accessors.Tests
templar-common: chain runner (multi-step, single-step, empty, wrong-start, broken-link, not-landing-on-target) +parse_one_or_many(object, array, leading-whitespace).templar-universal-account: genuine v0→v1→v2 two-step catch-up in one call over real state structs; invalid-chain rejection before running; all pre-existingUnbrickV1/rejection tests stay green.Verification
cargo fmt --all --checkclean; clippy-D warningsclean on touched crates.Closes ENG-517.
🤖 Generated with Claude Code
This change is