feat: contract versioning, health check, session archival & batch archive#266
Merged
Luluameh merged 5 commits intoMay 31, 2026
Merged
Conversation
…#246) - Add DataKey::ContractVersion to instance storage - Add migrate(new_version) public function (admin-guarded, once-per-version) - Add migrations.rs with SessionV1 struct and migrate_v1_to_v2 logic - Add admin.rs with admin helper re-exports - Add unit tests: test_migrate_v1_to_v2 and test_migrate_same_version_is_rejected
…Hub#247) - Add ContractStatus struct { version, admin, is_paused, total_sessions, active_sessions } - Add health_check() read-only function (no auth required) - Add DataKey::ActiveSessionCount to instance storage - Increment on start_session, decrement on all terminal transitions - Add test_health_check_returns_correct_status unit test
- Add contracts/src/storage.rs with write_archive/read_archive helpers and ARCHIVE_TTL_LEDGERS (~90 days at 5s/ledger) - Add ARCHIVE_DELAY_SECS (90 days) constant to lib.rs - Add archive_session(session_id): admin-only, enforces 90-day delay, moves session from persistent to temporary storage, removes persistent entry - Add get_archived_session(session_id): reads from temporary storage - Emit (session, archived) event with session_id and archived_at timestamp - Add 3 unit tests covering happy path, early-call rejection, active-session rejection
- Add MAX_ARCHIVE_BATCH_SIZE = 50 constant
- Add ArchiveSummary { archived, skipped } contracttype struct
- Add batch_archive_sessions(session_ids): admin-only, enforces batch cap,
skips non-existent / active / too-recent sessions without panicking,
emits (session, archived) per archived session
- Add 3 unit tests: happy path, skip ineligible, oversized batch rejection
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds admin helpers, contract versioning and migration (v1→v2), temporary archival storage and TTL, active-session counting across lifecycle transitions, and public health/version/archive entrypoints with tests and batch archival controls. ChangesContract Versioning, Admin Access, and Session Archival
Sequence Diagram(s)sequenceDiagram
participant Client
participant Contract
participant PersistentStorage
participant TemporaryStorage
Client->>Contract: start_session()
Contract->>PersistentStorage: write Session
Contract->>PersistentStorage: increment ActiveSessionCount
Client->>Contract: settle_session() / complete
Contract->>PersistentStorage: update Session -> Completed
Contract->>PersistentStorage: decrement ActiveSessionCount
Client->>Contract: health_check()
Contract->>PersistentStorage: read ContractVersion & ActiveSessionCount
Contract->>Client: return ContractStatus
Client->>Contract: archive_session(session_id) [admin, after 90 days]
Contract->>PersistentStorage: read Session
Contract->>TemporaryStorage: write_archive(Session) with TTL
Contract->>PersistentStorage: delete Session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@contracts/src/lib.rs`:
- Around line 1127-1151: The migrate function currently calls
migrations::run(&env, current, new_version) but unconditionally writes
DataKey::ContractVersion to new_version even when no migration ran; change
migrations::run to return a Result or a boolean (e.g., Result<(), Error> or
Result<bool, Error>) indicating whether the migration was applied/supported,
then update migrate to call migrations::run and: if the call returns an Err or a
"not supported" result, return an appropriate Error (do not update
ContractVersion or emit the "migrated" event); only set DataKey::ContractVersion
and publish the migrated event after migrations::run indicates success. Ensure
references are to the migrate function, migrations::run, and
DataKey::ContractVersion so the check is implemented in the right place.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: acf3d943-0480-4500-a8fb-1fd189efc80d
📒 Files selected for processing (4)
contracts/src/admin.rscontracts/src/lib.rscontracts/src/migrations.rscontracts/src/storage.rs
Comment on lines
+1127
to
+1151
| /// Migrate storage schema to `new_version`. Admin-only, runs once per version bump. | ||
| pub fn migrate(env: Env, new_version: u32) -> Result<(), Error> { | ||
| Self::require_admin(&env)?; | ||
|
|
||
| let current: u32 = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::ContractVersion) | ||
| .unwrap_or(1u32); | ||
|
|
||
| if new_version <= current { | ||
| return Err(Error::InvalidSessionState); | ||
| } | ||
|
|
||
| migrations::run(&env, current, new_version); | ||
|
|
||
| env.storage() | ||
| .instance() | ||
| .set(&DataKey::ContractVersion, &new_version); | ||
|
|
||
| env.events() | ||
| .publish((symbol_short!("migrated"),), (current, new_version)); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Migration allows setting arbitrary version without actual migration logic.
The migrate function sets ContractVersion to new_version after calling migrations::run(), but run() silently does nothing for unsupported version pairs. For example, calling migrate(100) when current version is 1 will set version to 100 without any migration logic executing, potentially blocking future legitimate migrations.
Consider having migrations::run() return a success indicator or panic on unsupported version transitions to prevent accidental version corruption.
Proposed fix in migrations.rs
-pub fn run(env: &Env, from: u32, to: u32) {
+pub fn run(env: &Env, from: u32, to: u32) -> bool {
if from == 1 && to == 2 {
migrate_v1_to_v2(env);
+ return true;
}
// future: if from == 2 && to == 3 { migrate_v2_to_v3(env); }
+ false
}Then in migrate():
- migrations::run(&env, current, new_version);
+ if !migrations::run(&env, current, new_version) {
+ return Err(Error::InvalidSessionState); // or a new UnsupportedMigration error
+ }🤖 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 `@contracts/src/lib.rs` around lines 1127 - 1151, The migrate function
currently calls migrations::run(&env, current, new_version) but unconditionally
writes DataKey::ContractVersion to new_version even when no migration ran;
change migrations::run to return a Result or a boolean (e.g., Result<(), Error>
or Result<bool, Error>) indicating whether the migration was applied/supported,
then update migrate to call migrations::run and: if the call returns an Err or a
"not supported" result, return an appropriate Error (do not update
ContractVersion or emit the "migrated" event); only set DataKey::ContractVersion
and publish the migrated event after migrations::run indicates success. Ensure
references are to the migrate function, migrations::run, and
DataKey::ContractVersion so the check is implemented in the right place.
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.
Closes #246
Closes #247
Closes #248
Closes #249
#246 — Contract Versioning & Schema Migration Tooling
DataKey::ContractVersionto instance storagemigrate(new_version): admin-only, runs once per version bump, dispatches tomigrations::run()contracts/src/migrations.rs:SessionV1struct +migrate_v1_to_v2re-serialisation logiccontracts/src/admin.rs: admin helper re-exports#247 — Contract Health Check Endpoint
ContractStatus { version, admin, is_paused, total_sessions, active_sessions }health_check(): read-only, no auth requiredDataKey::ActiveSessionCount— incremented onstart_session, decremented on all terminal transitions#248 — Session Archival to Temporary Storage
contracts/src/storage.rs:write_archive/read_archivehelpers (~90-day TTL)archive_session(session_id): admin-only, 90-day delay enforced, moves persistent → temporary, emits eventget_archived_session(session_id): read accessor#249 — Bulk Session Archival (Admin Batch Operation)
MAX_ARCHIVE_BATCH_SIZE = 50ArchiveSummary { archived: u32, skipped: u32 }batch_archive_sessions(session_ids): admin-only, batch cap enforced, skips ineligible sessions silentlyTesting
All 82 contract tests pass (
cargo test).Summary by CodeRabbit
Release Notes