feat(poker-table): canary release process for contract upgrades (#348) - #477
Merged
Marvy247 merged 2 commits intoAug 29, 2026
Merged
Conversation
…mPoka#348) Deploy upgraded contract to a single test table first, monitor for errors, gradually roll out, roll back automatically if error rate exceeds threshold — as the issue asks. PokerTableContract already had a timelocked propose_upgrade/execute_upgrade/cancel_upgrade mechanism, but it's table-scoped only in its API: create_table returns a table_id that's a counter inside one shared contract instance, and execute_upgrade -> env.deployer().update_current_contract_wasm(hash) upgrades the entire instance's WASM at once. There is no per-table contract deployment to canary independently at the WASM level — a hard Soroban constraint, not something fixable in this issue's scope. docs/adr/ADR-006-canary-contract-upgrades.md documents the two-phase process this constraint leads to (full rationale, and the alternatives considered/rejected, in the ADR): 1. Off-chain canary phase: deploy the candidate WASM as a genuinely separate contract instance, create a small number of real test tables on it, route controlled traffic, monitor its event stream (upgrade_proposed/upgrade_executed/upgrade_reverted plus this contract's existing gameplay events) against the production instance's baseline. This is where "single test table first, gradually more" is actually true, since each canary deployment is its own instance. 2. Production rollout via the existing propose_upgrade/execute_upgrade timelock, once the canary has proven stable — this is the single point where all tables on the instance move to the new code together. 3. Automated rollback via the new revert_last_upgrade entrypoint below, which an off-chain monitor calls automatically when post-upgrade error rate exceeds a threshold. New in this PR: - UpgradeRecord (types.rs): previous_wasm_hash (chained from the last tracked upgrade, None if this is the first this mechanism has ever recorded), new_wasm_hash, executed_at. execute_upgrade now writes one of these under DataKey::LastUpgrade(table_id) before swapping the WASM. - revert_last_upgrade(table_id) (lib.rs): admin-authorized, no timelock — a rollback needs to be fast, not deliberated over like a forward upgrade — but only available for ROLLBACK_WINDOW_SECONDS (6 hours) after the upgrade it would revert, and only reverts the single most recent one. The record is consumed on use: no redo, no reverting further back than one step, and going forward again after a revert needs a fresh propose/execute cycle with the normal timelock. - get_last_upgrade(table_id) view function. - New PokerTableError variants NoUpgradeToRevert = 70 and RollbackWindowExpired = 71 — checked against the existing enum for discriminant collisions before picking these (the enum unfortunately already has several *pre-existing* duplicate discriminants — 62-66 each double-assigned — and stale test expectations elsewhere referencing discriminants that don't match current values; flagged separately since it's unrelated to this change and deserves its own fix and a decision on whether renumbering is safe for an already-deployed contract). Tests (upgrade_test.rs): revert rejects with nothing ever executed; revert rejects after only the first-ever executed upgrade (no prior hash recorded); a second upgrade's revert correctly restores the first upgrade's hash; the record is consumed so reverting twice in a row fails the second time; revert succeeds exactly at the ROLLBACK_WINDOW_SECONDS boundary and rejects one second past it; get_last_upgrade returns None before any upgrade has executed. Verification: no local Rust toolchain available in this environment (link.exe fails compiling proc-macro2/quote build scripts — the same limitation hit on other Rust work this session), so `cargo test -p poker-table` could not be run here. Verified by close manual review: confirmed 70/71 don't collide with any existing discriminant (explicit grep across the whole enum, including the pre-existing duplicates noted above), confirmed Option<T> contracttype fields are already an established pattern in this file (street_time_limit, rit_state) before adding previous_wasm_hash the same way, and traced the timestamp/saturating_sub boundary math by hand against the existing delay-boundary test's style (execute_upgrade_rejects_right_up_until_the_delay_boundary) already in this file.
|
@Davidemulo Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
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
Deploy upgraded contract to a single test table first, monitor for errors, gradually roll out, roll back automatically if error rate exceeds threshold — as the issue asks.
PokerTableContractalready had a timelockedpropose_upgrade/execute_upgrade/cancel_upgrademechanism, but it's table-scoped only in its API:create_tablereturns atable_idthat's a counter inside one shared contract instance, andexecute_upgrade→env.deployer().update_current_contract_wasm(hash)upgrades the entire instance's WASM at once. There is no per-table contract deployment to canary independently at the WASM level — a hard Soroban constraint, not something fixable in this issue's scope.docs/adr/ADR-006-canary-contract-upgrades.mddocuments the two-phase process this constraint leads to (full rationale and alternatives considered/rejected in the ADR):propose_upgrade/execute_upgradetimelock, once the canary has proven stable — the single point where all tables on the instance move to the new code together.revert_last_upgradeentrypoint, which an off-chain monitor calls automatically when post-upgrade error rate exceeds a threshold.Changes
UpgradeRecord(types.rs):previous_wasm_hash(chained from the last tracked upgrade,Noneif first-ever),new_wasm_hash,executed_at.execute_upgradenow writes one underDataKey::LastUpgrade(table_id)before swapping the WASM.revert_last_upgrade(table_id): admin-authorized, no timelock — a rollback needs to be fast, not deliberated over — but only available forROLLBACK_WINDOW_SECONDS(6 hours) after the upgrade it would revert, and only the single most recent one. Consumed on use: no redo, no reverting further back than one step.get_last_upgrade(table_id)view function.NoUpgradeToRevert = 70,RollbackWindowExpired = 71— checked against the existing enum for collisions first (see note below).Note: pre-existing issues found, not fixed here
While picking discriminants I found the
PokerTableErrorenum already has several duplicate discriminants (62-66 each double-assigned to two different variants) andupgrade_test.rshas someshould_panicexpectations referencing codes that don't match the enum's current values. Flagged separately as its own task — unrelated to this change, and fixing it needs a decision on whether renumbering is safe for an already-deployed contract.Acceptance Criteria
revert_last_upgrade, callable by an automated monitor with no timelockTests
upgrade_test.rs: revert rejects with nothing ever executed; revert rejects after only the first-ever executed upgrade (no prior hash recorded); a second upgrade's revert correctly restores the first upgrade's hash; the record is consumed so reverting twice in a row fails the second time; revert succeeds exactly at theROLLBACK_WINDOW_SECONDSboundary and rejects one second past it;get_last_upgradereturnsNonebefore any upgrade has executed.Verification
No local Rust toolchain is available in this environment (
link.exefails compilingproc-macro2/quotebuild scripts).cargo test -p poker-tablecould not be run here. Verified by close manual review: confirmed 70/71 don't collide with any existing discriminant, confirmedOption<T>contracttypefields are already an established pattern in this file before addingprevious_wasm_hashthe same way, and traced the timestamp/saturating_subboundary math by hand against the existing delay-boundary test's style already in this file. Would appreciate CI/a reviewer confirming locally.Closes #348