Skip to content

feat(kora-deploy): add resume capability and buffer cleanup on transient failures - #614

Open
raushan728 wants to merge 10 commits into
solana-foundation:mainfrom
raushan728:fix/deploy-transient-failures
Open

feat(kora-deploy): add resume capability and buffer cleanup on transient failures#614
raushan728 wants to merge 10 commits into
solana-foundation:mainfrom
raushan728:fix/deploy-transient-failures

Conversation

@raushan728

@raushan728 raushan728 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Adds resume capability and automatic buffer cleanup to kora-deploy, persisting local state so deployments can recover from transient failures instead of leaving orphaned on-chain buffers.

  • Resume interrupted deployments with --resume, guarded by SHA-256 hash verification and chunk bounds checking
  • Buffers are closed automatically on failure by default; pass --no-cleanup-on-failure to leave the buffer open for a later --resume
  • Detect already-succeeded deploys after a timeout by checking ProgramData before resubmitting, skipping this check entirely on fresh (non-resume) deploys
  • Force buffer cleanup when the initial state save fails, since the buffer can't be recovered any other way in that case
  • Refactor deploy() into smaller functions with a shared, parametrized cleanup_buffer! macro
  • Add .kora-deploy-state.json to .gitignore since it contains raw keypair bytes
  • Add share_mode(0) on the non-Unix save path as a partial mitigation (concurrency guard only, not an ACL restriction full Windows permission support is out of scope here)
  • Add unit tests for the state-invariant checks (hash mismatch, chunk-count overflow) via a standalone check_state_invariants() helper
  • Document the resume/recovery flow and new flags in the README

@raushan728
raushan728 requested a review from dev-jodee as a code owner July 20, 2026 04:36
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds resume capability and automatic buffer cleanup to kora-deploy, persisting deployment state in .kora-deploy-state.json so interrupted deploys can recover without permanently orphaning on-chain buffers. The refactoring of deploy() into load_or_init_state / validate_state / write_chunks / finalize_deploy significantly improves correctness: hash verification catches silent binary changes, cleanup_buffer! is called consistently on every failure path, and the is_already_live programdata check handles the timeout-but-success scenario on resume.

  • The state.rs file correctly uses mode(0o600) and an atomic rename for safe writes, adding SHA-256 identity tracking to the persisted state.
  • main.rs uses clap's overrides_with mutual exclusion to wire --no-cleanup-on-failure cleanly, with cleanup_on_failure: !args.no_cleanup_on_failure correctly defaulting to true.

Confidence Score: 5/5

  • This PR is safe to merge. The deploy/resume logic is substantially more robust than the prior single-function implementation, and the remaining observations are cosmetic or edge-case quality improvements.
  • The refactoring addresses a comprehensive set of failure modes: every error path that can leave an orphaned on-chain buffer now calls cleanup_buffer!, the is_already_live programdata check handles the timeout-but-success scenario, and the state file is guarded against accidental overwrites. The two remaining issues are minor: an orphaned .tmp file (protected by 0o600 on Unix) and a misleading "Deployed via..." stdout message when a resume finds the program already live.
  • No files require special attention; crates/kora-deploy/src/state.rs and crates/kora-deploy/src/lib.rs are the most complex changed files and are worth a second read, but no blocking issues were found.

Important Files Changed

Filename Overview
crates/kora-deploy/src/lib.rs Core deploy logic refactored into sub-functions with a shared cleanup_buffer! macro; addresses most previously flagged state-management issues; two minor UX concerns remain (misleading output on already-live resume, orphaned temp file).
crates/kora-deploy/src/state.rs New file handling atomic state persistence with proper Unix permissions (0o600) and acknowledged Windows limitation; temp file not cleaned up on rename failure leaves keypair bytes on disk.
crates/kora-deploy/src/main.rs Adds --resume and --no-cleanup-on-failure flags using clap overrides_with mutual exclusion; default cleanup_on_failure=true is correctly derived from !args.no_cleanup_on_failure.
crates/kora-deploy/README.md Documents the resume/recovery flow, new flags, and manual recovery steps clearly.
examples/devnet-deploy-paymaster/src/suite/happy.rs Updated to pass the three new DeployConfig fields with sensible defaults for an automated test.
.gitignore Correctly excludes the state file from version control with an explanatory comment.

Reviews (29): Last reviewed commit: "fix(deploy): update devnet-deploy-paymas..." | Re-trigger Greptile

Comment thread crates/kora-deploy/src/lib.rs
Comment thread crates/kora-deploy/src/lib.rs Outdated
Comment thread crates/kora-deploy/src/state.rs
Comment thread crates/kora-deploy/src/main.rs
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from f7f20bc to 409473b Compare July 20, 2026 08:21
Comment thread crates/kora-deploy/src/lib.rs Outdated
Comment thread crates/kora-deploy/src/lib.rs Outdated
Comment thread crates/kora-deploy/src/lib.rs Outdated
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from de0ed37 to 946793a Compare July 20, 2026 09:48
Comment thread crates/kora-deploy/src/lib.rs
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch 2 times, most recently from 6ef22fe to affdf0f Compare July 20, 2026 10:31
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from affdf0f to 4205050 Compare July 20, 2026 12:13
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from 4205050 to 4c2465b Compare July 20, 2026 12:27
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch 3 times, most recently from 04f8086 to ccc6d96 Compare July 20, 2026 13:24
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from ccc6d96 to be3fdc6 Compare July 20, 2026 13:34
@raushan728
raushan728 marked this pull request as draft August 16, 2026 13:11
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch 2 times, most recently from 0562074 to 83dc669 Compare August 16, 2026 16:26
Comment thread crates/kora-deploy/src/lib.rs Outdated
@raushan728
raushan728 marked this pull request as ready for review August 18, 2026 10:09
Comment thread crates/kora-deploy/src/lib.rs

@dev-jodee dev-jodee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • should .gitignore .kora-deploy-state.json
  • should prob have tests for hash-mismatch detection, written_chunks > chunk_count corruption handling, skip-already-written-chunks, is_already_live short-circuit, etc.

Comment thread crates/kora-deploy/src/state.rs Outdated
Comment thread crates/kora-deploy/src/state.rs Outdated
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from ff0dad3 to 87743c2 Compare August 29, 2026 09:37
@raushan728

Copy link
Copy Markdown
Contributor Author
  • should .gitignore .kora-deploy-state.json

Added

  • should prob have tests for hash-mismatch detection, written_chunks > chunk_count corruption handling, skip-already-written-chunks, is_already_live short-circuit, etc.

Added pure-logic unit tests for the two state-invariant checks

For the remaining cases you mentioned (skip-already-written-chunks, is_already_live short-circuit) agreed these need coverage too, but they're integration-style paths that depend on RPC/network calls (Kora + Solana RPC), which this crate doesn't currently have mocking infra for.

Rather than bolting on ad-hoc mocks in an already-large PR, we can file a follow-up issue to properly set up mock-based integration testing for kora-deploy. I'll scope that issue to cover both the integration tests AND adding a Windows CI target, since the non-Unix file-permission gap (raised above) would benefit from actually being exercised in CI rather than just reviewed by eye. That way both gaps get closed together with proper CI coverage backing them, instead of being two separate half-measures.

@raushan728
raushan728 requested a review from dev-jodee August 29, 2026 10:35
@raushan728

Copy link
Copy Markdown
Contributor Author

Addresses in 87743c2

@dev-jodee dev-jodee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all looks good except the 1 nit

Comment thread crates/kora-deploy/src/state.rs Outdated
@dev-jodee

Copy link
Copy Markdown
Collaborator

@raushan728 some CI failures

…r cleanup

Add local state persistence (.kora-deploy-state.json) so deployments
can resume after transient RPC failures. Resume is guarded by SHA-256
binary hash verification and chunk bounds checking, with the hash
check evaluated first so a rebuilt binary always surfaces a clear
hash-mismatch error.

On unrecoverable failure, a cleanup macro closes the orphaned on-chain
buffer to reclaim rent. Query ProgramData before the final transaction
to detect if a prior attempt already succeeded, avoiding a timeout
deadlock.

Add --resume and --no-cleanup-on-failure CLI flags.
Add --resume and --no-cleanup-on-failure to the flags table. Explain
the .kora-deploy-state.json lifecycle: normal resume, hash mismatch
handling, and manual recovery for the deploy-timeout edge case.
Use share_mode(0) for Windows state file creation to prevent concurrent access.Add .kora-deploy-state.json to .gitignore Extract check_state_invariants from validate_state and add unit tests
@raushan728
raushan728 force-pushed the fix/deploy-transient-failures branch from e400dd7 to eb5cc90 Compare September 2, 2026 13:09
@raushan728

Copy link
Copy Markdown
Contributor Author

@raushan728 some CI failures

Fixed in eb5cc90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants