Repository navigation
Fungible: migration path for the total supply moved out of instance storage - #933
Conversation
…torage Up to v0.7.x, `Base` tracked the total supply in `instance` storage. Since #795 it lives in a `persistent` entry owned by the `TotalSupply` contract type, so a token upgraded in place reads its supply as 0 and burns panic. - Add `total_supply::migrate_total_supply`, which moves the legacy entry (same key encoding) into the persistent one, additively, and removes it. - Turn the `upgradeable` v1/v2 example into this upgrade: v1 is a real `stellar-tokens` 0.7.2 token, v2 the same token on the current library with a `migrate()` calling the helper. The upgrade tests live in v2, since the soroban-sdk 26 test host cannot run the v2 wasm. - Point the module docs and README at the helper and the example. Closes #889
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (3)
📒 Files selected for processing (14)
💤 Files with no reviewable changes (2)
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughThe pull request adds a helper to migrate fungible-token supply from instance storage to persistent storage. It updates the upgradeable v1 and v2 examples, adds migration tests, and documents the upgrade requirement. ChangesToken Supply Migration
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: High Sequence Diagram(s)sequenceDiagram
participant V2 as ExampleContract v2
participant Helper as migrate_total_supply
participant Instance as Instance storage
participant Persistent as Persistent supply storage
V2->>Helper: Call after role and schema checks
Helper->>Instance: Read legacy total supply
Helper->>Persistent: Add legacy amount to current supply
Helper->>Instance: Remove legacy entry
Helper-->>V2: Return migrated amount
V2->>V2: Set schema version to 2
Merge Risk: ⚪ Minimal · up to The migration matches the legacy supply layout, and no merge-blocking issue is established. Normal checks can proceed. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Most coding objectives in Full details: Docstring CoverageExplanation Docstring coverage is 48.28% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 9 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
I’m a rabbit with a ledger to keep, Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Fixes #889
Up to v0.7.x,
Basetracked the total supply ininstancestorage. Since #795 the supply lives in apersistententry owned by theTotalSupplycontract type. A token upgraded in place keeps its balances but reads its supply as0, and burns panic with#104until the supply is migrated (with the knock-on effects on Capped, RWA and Vault listed in #889). This was also raised by the auditors.Changes
total_supply::migrate_total_supply(e) -> i128, as proposed in 🐞 [Bug]: total supply moved to persistent storage without a migration path #889. It reads the legacyinstanceentry (the oldFungibleStorageKey::TotalSupplyencodes the same asTotalSupplyStorageKey::TotalSupply, since unit variants are stored by name), adds it to thepersistentcounter, and removes the old entry. It's additive, so mints betweenupgradeandmigratestay accounted for, and a second call is a no-op. No auth, so it carries a Security Warning. Unit tests cover the migrated value, the removed entry, the additive case, the no-op case, calling it twice, and the key-encoding equality against a copy of the old enum.examples/upgradeablev1/v2 now demonstrate this upgrade instead of the abstractConfig { rate }→Config { rate, active }change (already covered bylazy-v1/lazy-v2and the module docs):v1is a fungible token built with the publishedstellar-tokens = "=0.7.2"(soroban-sdk 26.1.0).v2is the same token on the current library (Compose<(TotalSupply,)>+FungibleTotalSupply), withmigrate(operator)behind#[only_role(operator, "migrator")], guarded by the schema version and callingmigrate_total_supply.total_supply() == 0and a burn fails. After migrating, the supply is correct and the old entry is gone. Later mints and burns update the supply, and a secondmigrateis rejected. A second test mints on v2 before migrating. Theupgradertest now uses the token.total_supply,upgradeable) andpackages/tokens/README.mdpoint at the helper and the example.Divergence from #889: v1 is a real 0.7.2 token
#889 suggested reproducing the old layout on current crates, on the assumption that an old-version
v1can't be a workspace member. It can:v1pins its own dependencies (=0.7.2, soroban-sdk=26.1.0) and builds withstellar contract build. So the migration is tested against the storage a deployed token really has, not a hand-written copy. I also diffed v0.7.2 againstmain: the total supply is the only stored data that changed (access-control, upgradeable and other fungible keys are unchanged).Costs, open for pushback:
Cargo.lockgrows by about 680 lines and CI compiles a second SDK tree.v1tov2. v2 registers the v1 wasm instead.v1no longer runs natively, so its lines count as uncovered inllvm-cov.The testdata wasms were rebuilt with stellar-cli 27.0.0. Release notes for v0.9.0 should get an "Upgrading from 0.7 or earlier" section pointing at the helper; that lives outside the repo.
PR Checklist
Summary by CodeRabbit
stellar-tokensv0.7.x or earlier. It moves the legacy supply to current storage and removes the old entry; repeat calls do not change supply.