Repository navigation
feat(kora-deploy): add resume and buffer cleanup support to upgrade() - #775
raushan728 wants to merge 6 commits into
Conversation
Adds `UpgradeState`, the persisted state for resuming an upgrade, with its own state file so it never collides with the deploy state file. It implements `ChunkWriterState` so `write_chunks` can be shared between deploy and upgrade. `DeployState` and `UpgradeState` now share `load_state_file` and `save_state_file` helpers. This changes deploy-side behavior slightly: - load returns `None` only on `NotFound`; other `IO` errors are now propagated instead of being treated as a missing file. - save removes the temp file if writing, syncing or renaming fails, so raw keypair bytes are not left behind.
…e engine Add state tracking and resume support for program upgrades via `UpgradeState`, matching the deploy workflow. Refactor chunk writing, state invariant validation, and buffer cleanup into unified helpers shared across deploy and upgrade. Expose `--resume`, `--no-cleanup-on-failure`, and `--state-path` flags for upgrade commands. Introduce preflight checks to verify on-chain state and avoid redundant writes if an upgrade already finalized. Expand the RPC mock harness and add comprehensive tests for upgrade resume, crash recovery, and state invariants.
 [Medium impact] The changes affect the operator workflow for deploying and upgrading programs through the paymaster. Fix the buffer-creation recovery gap and satisfy the required error-log sanitizing before merging. Findings
SummaryAdds saved upgrade progress, resume support, buffer cleanup, and state-file locks.
Reviews (1) · Last reviewed commit: "docs(kora-deploy): document upgrade resu..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let buffer = Keypair::new(); | ||
| create_buffer_account(ctx, &kora_pubkey, &buffer, bytes.len()).await?; | ||
|
|
||
| let new_state = UpgradeState { |
There was a problem hiding this comment.
If buffer creation lands but its HTTP response or confirmation times out, create_buffer_account(...).await? returns before UpgradeState is saved. No cleanup runs on that error. The next run cannot resume or recover the buffer from a state file, leaving the paymaster's rent locked.
Save the buffer identity before sending the create transaction, and handle an uncertain creation result during resume.
| match sig { | ||
| Some(s) => println!(" upgraded program as registered owner (sig {s})"), | ||
| None => println!(" program was already upgraded"), | ||
| } |
There was a problem hiding this comment.
This test deploys and upgrades the same program_so, so the new hash check returns None before uploading or sending an upgrade transaction. Accepting that result lets the smoke suite pass even when real upgrades are broken.
Use a different valid binary for the upgrade and require Some(signature) so the test still checks an on-chain upgrade.
| Either restore the original `.so` file, or delete the state file | ||
| and start a fresh deploy. | ||
| and start over. |
There was a problem hiding this comment.
The guide still tells users to delete the state file and start over after a hash mismatch. This PR now preserves the buffer on a mismatch, even with cleanup enabled. Following these instructions discards the recovery file while leaving the paymaster's rent locked.
Tell users to close the buffer before deleting the state file, matching the new error message.
| Either restore the original `.so` file, or delete the state file | |
| and start a fresh deploy. | |
| and start over. | |
| Either restore the original `.so` file, or close the buffer account shown | |
| in the error message before deleting the state file and starting over. | |
| Deleting the state file without closing the buffer leaves its rent locked. |
| Ok(resp) if resp.value => return Ok(()), | ||
| Ok(_) => {} | ||
| Err(err) => { | ||
| log::debug!("transient error confirming {sig}: {err}; retrying..."); |
There was a problem hiding this comment.
The new retry logs write external RPC errors directly through {err} here and in wait_for_next_slot(). The repository guide requires errors from external services to be wrapped in sanitize_error! before logging.
Sanitize both errors before logging them. This repository requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
Closes #701
Adds
--resumeand buffer cleanup support toupgrade(), bringing it to parity with what was added fordeploy()in #614, and reuses the mockito + Solana RPC mock test harness established in #710.Previously, if an upgrade failed midway (due to RPC timeouts, network drops, etc.), no local state file was saved. The buffer account ended up orphaned on-chain, keeping the paymaster's rent locked with no way for the user to resume where they left off.
What was done in this:
Added
UpgradeState(persisted to.kora-upgrade-state.json) and introduced aChunkWriterStatetrait sodeploy()feat(kora-deploy): add resume capability and buffer cleanup on transient failures #614 andupgrade()share the exact same chunk-writing, invariant check, and buffer cleanup logic without duplicating code.Added preflight checks if the on-chain program already matches the local binary hash, we exit early without creating a buffer. When resuming, if the buffer is already gone, it checks whether the upgrade transaction had already landed on-chain so we don't error out on network drops during finalization.
Added
StateLockGuard(<state_path>.lock) to prevent concurrent processes from colliding on the same state file.Handled backward compatibility for older
DeployStatefiles wherewallet_pubkeywas missing, and updated hash-mismatch error messages to include the buffer pubkey so users know to close the buffer before deleting the state file.Unlocked
--resume,--no-cleanup-on-failure, and--state-pathflags on the upgrade CLI, and updated the README with recovery instructions.Expanded the mock orchestration suite introduced in test(kora-deploy): add
mockitobased orchestration test coverage fordeploy()#710 to test all upgrade flows end-to-end.AI tooling was used
No AI tooling was used
Tool and extent: OpenCode was used with free-tier models (e.g.,
GLM-5.3-Flash,DeepSeek V4.1 Flash) for coding tasks. The exact model is not fixed and varies based on availability and successful responses. All changes were reviewed and verified by me before committing.