# feat: add contract versioning, migration framework, and frontend compatibility warnings - #199
Conversation
…banner Smart contracts: - Add Migratable trait and migration helpers in smartcontract/migration/ crate - Add VERSION const + get_version() to all 6 contracts (rotational, target, flexible, factory, reputation, yield-strategy) - Add migrate() with admin auth and version-step validation to pool + factory contracts - Add migrated_from lineage tracking to pool and factory contracts - Add register_migration() to Factory contract - Add cargo fmt formatting fixes across all contracts Frontend: - Add KNOWN_CONTRACT_VERSIONS constant and isContractVersionUnknown() helper - Add contractVersion field to pool state types and fetch it on-chain - Add version warning banners in pool cards and group details pages - Add contract-version.test.ts unit tests - Add contract-version.test.ts to test:unit script Documentation: - Add MIGRATIONS.md with full migration guide, testnet deployment steps, rollback notes - Add sample migration script (migrate_rotational_v1_to_v2.sh)
Sendi0011
left a comment
There was a problem hiding this comment.
Thanks for the comprehensive PR! The migration framework is well-designed. A few issues to address before merge:
Critical:
-
migrate()idempotency claim is incorrect — The doc says runningmigrate(2)twice is safe, but the code doesassert!(to_version == current + 1). If current is already2, the second call fails with2 == 3 → false. Either update the docs to say "not idempotent, will reject re-runs" or change the logic to treatto_version == currentas a no-op success. -
package.jsonreferenceslib/pending-transactions.test.tsin thetest:unitscript but that file doesn't exist in this PR — it's from another branch. This will break CI.
Improvements:
-
fetchContractVersionadds an extra RPC call per pool load — On the dashboard with many pool cards, this means N additional calls. Consider batching or caching to avoid performance regression. -
group-details.tsxuses an inline IIFE in JSX — The(() => { ... })()pattern is hard to read and debug. Extract to a named component or helper function. -
KNOWN_CONTRACT_VERSIONSincludesreputationandyieldStrategybut the frontend never fetches their versions. Remove them or add a comment explaining they're reserved for future use. -
Test duplicates
isContractVersionUnknown— The test file re-implements the function to avoid React imports. Extract the logic to a pure util file (no React deps) so both can import from the same source.
|
I will implement the necessary fix
Thanks for the guidance.
…On Sat, Jul 25, 2026, 6:20 PM Sendi John ***@***.***> wrote:
***@***.**** commented on this pull request.
Thanks for the comprehensive PR! The migration framework is well-designed.
A few issues to address before merge:
*Critical:*
1.
*migrate() idempotency claim is incorrect* — The doc says running
migrate(2) twice is safe, but the code does assert!(to_version ==
current + 1). If current is already 2, the second call fails with 2 ==
3 → false. Either update the docs to say "not idempotent, will reject
re-runs" or change the logic to treat to_version == current as a no-op
success.
2.
*package.json references lib/pending-transactions.test.ts* in the
test:unit script but that file doesn't exist in this PR — it's from
another branch. This will break CI.
*Improvements:*
3.
*fetchContractVersion adds an extra RPC call per pool load* — On the
dashboard with many pool cards, this means N additional calls. Consider
batching or caching to avoid performance regression.
4.
*group-details.tsx uses an inline IIFE in JSX* — The (() => { ... })()
pattern is hard to read and debug. Extract to a named component or helper
function.
5.
*KNOWN_CONTRACT_VERSIONS includes reputation and yieldStrategy* but
the frontend never fetches their versions. Remove them or add a comment
explaining they're reserved for future use.
6.
*Test duplicates isContractVersionUnknown* — The test file
re-implements the function to avoid React imports. Extract the logic to a
pure util file (no React deps) so both can import from the same source.
—
Reply to this email directly, view it on GitHub
<#199?email_source=notifications&email_token=BLUYM7VCGFRPH2AHZQL63KL5GTT6RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZXHE3TGNRQHAYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4779736080>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BLUYM7QUJXOM5OWFBUX5QDL5GTT6RAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMRTHE2DAMBXG45US43TOVSTWNBZG42TANZTGAZTNILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/BLUYM7V7TS66URUTKQ4TNHD5GTT6RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZXHE3TGNRQHAYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/BLUYM7RI5HMWJ7IABDH65QL5GTT6RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZXHE3TGNRQHAYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
- Add migration idempotency (early return when to_version == current_version) for all 4 contracts - Cache fetchContractVersion RPC results to avoid redundant fetches for same contract - Extract isContractVersionUnknown pure utility to frontend/lib/contract-version.ts - Extract VersionWarning component in group-details.tsx (replace IIFE pattern) - Remove reputation and yieldStrategy from KNOWN_CONTRACT_VERSIONS (no migrate() functions) - Add 12 migration idempotency Rust tests (3 per contract)
- Merge main to restore CHAT_MESSAGE_MAX_LENGTH and CHAT_RATE_LIMIT_MS constants - Fix Prettier formatting in group-details.tsx, pool-card.tsx, useJointSaveContracts.ts - Update test:unit script to include new test files from main
…om/JemimahEkong/Joint_Save into feat/contract-versioning-migration # Conflicts: # frontend/lib/constants.ts # frontend/package.json
Trailing ─── line after closing brace caused 62 compilation errors in jointsave-flexible during cargo test
Sendi0011
left a comment
There was a problem hiding this comment.
Great job 👍 - Approving
Overview
This PR introduces a complete contract versioning and migration foundation for JointSave's Soroban contracts, enabling safer future upgrades without requiring full contract replacement or manual state migrations.
The implementation adds version tracking across contracts, migration helpers, frontend compatibility checks, and documentation for future migration workflows.
Closes #184
Smart Contract Changes
Contract Versioning
Added version support to all six contracts:
Changes include:
VERSION: u32 = 1constantget_version()view functionMigration Framework
Added new migration crate:
smartcontract/migration/Features:
Migratabletrait definitionMigration rules:
Pool Contract Migration Support
Added
migrate(to_version)support for:Includes:
Migration Lineage Tracking
Added
migrated_fromstorage tracking for pool and factory contracts.This allows deployments to preserve migration history and trace contract lineage.
Factory Migration Registry
Added:
register_migration(admin, old_factory, new_factory)This records relationships between factory versions while maintaining admin-only control.
Reputation & YieldStrategy
Added version tracking only:
VERSIONget_version()These contracts remain standalone and do not require migration execution.
Frontend Changes
Contract Version Fetching
Added contract version awareness to:
New helpers:
fetchContractVersion()isContractVersionUnknown()Pool state objects now include:
contractVersionKnown Version Registry
Added:
KNOWN_CONTRACT_VERSIONSThis keeps frontend compatibility expectations centralized.
Unknown Version Warning UI
Added user-facing warnings when a pool contract version is newer than the frontend supports.
Warnings added to:
Users will see:
Documentation
Added:
smartcontract/MIGRATIONS.mdDocumentation covers:
Added example migration script:
migrations/migrate_rotational_v1_to_v2.shDemonstrates:
Testing
Added contract version frontend tests covering:
Results:
npm run test:unit
105/105 tests passing
Additional validation:
cargo fmt --check ✅
TypeScript validation confirmed no new errors were introduced in modified files.
Validation Notes
cargo buildandcargo testcould not be executed locally due to missing MSVC Build Tools environment dependency, not related to code changes.Impact
This change provides:
Existing contracts remain fully backward compatible at version
1, and existing functionality is preserved.