Removal of multi-denom vaults#243
Conversation
📝 WalkthroughWalkthroughVault functionality is converted from mixed-denom accounting to single-denom accounting based on each vault’s underlying asset. Legacy request fields and bootstrap NAV types are removed, existing state is flattened through v1→v2 migration, and tests, simulations, specifications, CLI metadata, and security guidance are updated. ChangesSingle-denom vault upgrade
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Filename | Overview |
|---|---|
| keeper/migrations.go | Adds the single-denom state migration, including fee normalization and pending swap-out rewrites. |
| keeper/migrations_test.go | Covers vault flattening, fee conversion, queue rewrites, migration registration, and repeated execution. |
| keeper/migrator.go | Routes the v1→v2 upgrade to the new flattening migration. |
| module.go | Removes mixed-denom options from the generated CLI surface. |
Reviews (6): Last reviewed commit: "add negative share failure to query" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
keeper/msg_server_test.go (1)
6244-6277: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftConsolidate duplicated AcceptAsset test setup into a shared helper.
The
TestMsgServer_AcceptAsset_Inbound/Outbound/ZeroPriceOutbound/ZeroPriceInbound/RestrictedMarker/Failures/InsufficientPrincipalDoesNotSettle/RejectAsset/RejectAsset_Failurestests all repeat the samesetupAssetSettlementVault+CreateAndFundAccount+FundAccount+createPaymentboilerplate (well beyond the 3-occurrence threshold), whileTestMsgServer_AcceptAsset_NAVGuardrail/SettlementNAV/SettlementNAV_MetadataDenom/Reconcilealready use a consolidatedsetupAcceptAssetScenariohelper for the same purpose. Since this PR is actively touching all of these tests for the underlying-asset migration, this is the right time to converge the remaining call sites ontosetupAcceptAssetScenario(or a suite_test.go equivalent) instead of leaving two parallel setup styles in the same file.As per path instructions,
**/*_test.go: "If setup appears in three or more tests, extract it into a helper insuite_test.go; proactively consolidate duplicated setup before adding or fixing tests."🤖 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 `@keeper/msg_server_test.go` around lines 6244 - 6277, Consolidate the repeated settlement setup in the listed AcceptAsset and RejectAsset tests by reusing the existing setupAcceptAssetScenario helper (or a shared suite_test.go equivalent). Update each test to obtain the vault, principal, source account, payment amounts, and external ID from that helper while preserving its individual assertions and scenario-specific behavior; remove the duplicated setupAssetSettlementVault, funding, and createPayment calls.Source: Path instructions
🧹 Nitpick comments (8)
keeper/msg_server.go (1)
939-942: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap the error with context.
The error returned by
settlementLegCoinsis not wrapped. As per coding guidelines, always wrap errors with contextual information using%w.♻️ Proposed refactor
assetCoin, paymentCoin, err := settlementLegCoins(payment, direction, vault.UnderlyingAsset) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to resolve settlement leg coins: %w", err) }🤖 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 `@keeper/msg_server.go` around lines 939 - 942, Update the error return in the settlementLegCoins call to wrap err with contextual information using %w, while preserving the existing nil result and error propagation behavior.Source: Coding guidelines
keeper/migrations_test.go (1)
12-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider converting to a table-driven test.
TestKeeper_MigrateFlattenMixedDenomVaultsuses 7 separates.Runblocks rather than the[]struct{...}table pattern used by sibling tests in this PR (e.g.TestNAVConversion_OversizedNAVReturnsErrorNotPanic,TestMsgServer_PauseVault_ForceVsStrict). Each case could be expressed as{name, setup func(*types.VaultAccount), assert func(*testing.T, *types.VaultAccount)}for consistency with the codebase's table-driven convention.As per coding guidelines, "Use table-driven tests for unit and integration tests to provide exhaustive edge-case coverage."
🤖 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 `@keeper/migrations_test.go` around lines 12 - 154, Refactor TestKeeper_MigrateFlattenMixedDenomVaults to use a table-driven test with cases containing a name, vault setup function, and post-migration assertion function. Preserve each existing scenario’s fixtures and assertions, including the separate non-vault, idempotency, NAV seeding, and pending swap-out checks; keep any scenario-specific setup and validation within its case.Source: Coding guidelines
keeper/valuation_engine_test.go (1)
792-832: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
IsAcceptedDenomassertion no longer reflects what's actually being filtered.
GetTVVInUnderlyingAssetnow excludesunacceptedDenombecause no internal NAV entry was seeded for it, not because ofIsAcceptedDenom. Thes.Require().False(vault.IsAcceptedDenom(unacceptedDenom), ...)check is orthogonal to the mechanism this test exercises and doesn't cover the case where a NAV entry exists for a denomIsAcceptedDenomwould call unaccepted.🤖 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 `@keeper/valuation_engine_test.go` around lines 792 - 832, The test TestGetTVVInUnderlyingAsset_AcceptedDenomFiltering should exercise filtering based on internal NAV entries rather than IsAcceptedDenom. Seed an internal NAV entry for unacceptedDenom that IsAcceptedDenom would reject, then assert GetTVVInUnderlyingAsset excludes its balance; remove the orthogonal vault.IsAcceptedDenom assertion and update expectedTVV to match the filtered result.keeper/query_server_test.go (1)
352-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
setupVaultclosure acrossEstimateSwapIn/EstimateSwapOuttests.Both closures are identical (
requireAddFinalizeAndActivateMarker+CreateVaultWithParams). Consider extracting to a sharedsuite_test.gohelper.♻️ Suggested extraction
// in suite_test.go func (s *TestSuite) setupSingleDenomVault(shareDenom, underlyingDenom string) { s.requireAddFinalizeAndActivateMarker(sdk.NewCoin(underlyingDenom, math.NewInt(1000)), s.adminAddr) s.CreateVaultWithParams(shareDenom, underlyingDenom) }- setupVault := func() { - s.requireAddFinalizeAndActivateMarker(sdk.NewCoin(underlyingDenom, math.NewInt(1000)), s.adminAddr) - s.CreateVaultWithParams(shareDenom, underlyingDenom) - } + setupVault := func() { s.setupSingleDenomVault(shareDenom, underlyingDenom) }As per path instructions,
**/*_test.go: "If setup appears in three or more tests, extract it into a helper insuite_test.go; proactively consolidate duplicated setup before adding or fixing tests."Also applies to: 453-456
🤖 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 `@keeper/query_server_test.go` around lines 352 - 355, Extract the identical vault setup closures used by the EstimateSwapIn and EstimateSwapOut tests into a shared TestSuite helper in suite_test.go, such as setupSingleDenomVault. Have the helper perform requireAddFinalizeAndActivateMarker and CreateVaultWithParams, then update both tests to call it with shareDenom and underlyingDenom.Source: Path instructions
proto/provlabs/vault/v1/vault.proto (1)
192-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
PendingSwapOut.redeem_denomshould be marked deprecated for consistency.This field is documented as always equal to
underlying_assetand "retained for wire compatibility" — the same rationale used forMsgSwapOutRequest.redeem_denomintx.proto, which is annotated[deprecated = true]. This field lacks that annotation, so codegen/API-doc tooling won't flag it as deprecated for consumers.♻️ Proposed fix
- string redeem_denom = 4; + string redeem_denom = 4 [deprecated = true];🤖 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 `@proto/provlabs/vault/v1/vault.proto` around lines 192 - 205, Mark the PendingSwapOut.redeem_denom field as deprecated in its protobuf declaration, matching the existing deprecated annotation on MsgSwapOutRequest.redeem_denom while preserving its field number and wire compatibility.Source: Coding guidelines
keeper/reconcile_test.go (2)
209-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInternal comments scattered throughout test logic.
This file has many inline/blockquote comments explaining setup rationale and math derivations inside test bodies (e.g. here, and at lines 244, 248, 276, 281-284, 414-419, 1461-1463, 1638-1641, 2020-2023, 2443-2446, 2486-2497, 2549-2552). As per path instructions for
**/*_test.go, prefer descriptive names/control flow over internal comments in test logic.🤖 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 `@keeper/reconcile_test.go` around lines 209 - 211, Remove the explanatory inline comments scattered through test bodies in keeper/reconcile_test.go, including the setup and math-rationale comments near the referenced sections. Replace only where needed with descriptive helper, variable, or test names and clearer control flow; preserve the existing test behavior and assertions.Source: Path instructions
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInternal comments in test logic across
keeper/reconcile_test.goandkeeper/vault_test.go. Both files add explanatory/internal comments inside test bodies rather than relying on descriptive names and clear control flow, per the**/*_test.gopath guideline ("Avoid internal comments in test logic; use descriptive names and clear control flow instead").
keeper/reconcile_test.go#L209-211: remove or replace the "Local helper for the composite setup..." comment block (and the similar ones at lines 244, 248, 276, 281-284, 414-419, 1461-1463, 1638-1641, 2020-2023, 2443-2446, 2486-2497, 2549-2552) with self-explanatory helper/variable names.keeper/vault_test.go#L293-296: drop the// Set to zero for instant processing in the same block's endblockercomment; the zero-value assignment is already self-explanatory in context.🤖 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 `@keeper/reconcile_test.go` at line 1, Remove internal explanatory comments from test logic in reconcile_test.go and vault_test.go, including the listed helper/setup comments and the comment above the zero-value assignment; preserve the test behavior and improve clarity through descriptive helper and variable names or straightforward control flow where needed.Source: Path instructions
keeper/vault_test.go (1)
293-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline comment in test body.
// Set to zero for instant processing in the same block's endblockeris an internal comment inside test logic. As per path instructions for**/*_test.go, prefer descriptive names/control flow over internal comments in test logic.🤖 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 `@keeper/vault_test.go` around lines 293 - 296, Remove the internal comment from the test setup and make the intent clear through the existing assignment and surrounding test structure; keep WithdrawalDelaySeconds set to zero so processing remains immediate within the same block’s endblocker.Source: Path instructions
🤖 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 @.changelog/unreleased/improvements/239-disallow-multi-denom-vault.md:
- Line 1: The changelog entry’s statement that existing mixed-denom vaults are
unaffected conflicts with the v1→v2 migration behavior. Update the entry to
state that new vaults must use an empty or matching payment denom, while
existing mixed-denom vaults are migrated or flattened so payment_denom equals
underlying_asset.
In `@keeper/migrations.go`:
- Around line 94-123: Update normalizeOutstandingAumFee to zero the outstanding
fee only when ToUnderlyingAssetAmount returns ErrInternalNAVNotFound, checking
wrapped errors with errors.Is. For other conversion errors, surface the failure
instead of mutating the vault or silently writing off the liability, while
preserving the existing successful conversion behavior.
---
Outside diff comments:
In `@keeper/msg_server_test.go`:
- Around line 6244-6277: Consolidate the repeated settlement setup in the listed
AcceptAsset and RejectAsset tests by reusing the existing
setupAcceptAssetScenario helper (or a shared suite_test.go equivalent). Update
each test to obtain the vault, principal, source account, payment amounts, and
external ID from that helper while preserving its individual assertions and
scenario-specific behavior; remove the duplicated setupAssetSettlementVault,
funding, and createPayment calls.
---
Nitpick comments:
In `@keeper/migrations_test.go`:
- Around line 12-154: Refactor TestKeeper_MigrateFlattenMixedDenomVaults to use
a table-driven test with cases containing a name, vault setup function, and
post-migration assertion function. Preserve each existing scenario’s fixtures
and assertions, including the separate non-vault, idempotency, NAV seeding, and
pending swap-out checks; keep any scenario-specific setup and validation within
its case.
In `@keeper/msg_server.go`:
- Around line 939-942: Update the error return in the settlementLegCoins call to
wrap err with contextual information using %w, while preserving the existing nil
result and error propagation behavior.
In `@keeper/query_server_test.go`:
- Around line 352-355: Extract the identical vault setup closures used by the
EstimateSwapIn and EstimateSwapOut tests into a shared TestSuite helper in
suite_test.go, such as setupSingleDenomVault. Have the helper perform
requireAddFinalizeAndActivateMarker and CreateVaultWithParams, then update both
tests to call it with shareDenom and underlyingDenom.
In `@keeper/reconcile_test.go`:
- Around line 209-211: Remove the explanatory inline comments scattered through
test bodies in keeper/reconcile_test.go, including the setup and math-rationale
comments near the referenced sections. Replace only where needed with
descriptive helper, variable, or test names and clearer control flow; preserve
the existing test behavior and assertions.
- Line 1: Remove internal explanatory comments from test logic in
reconcile_test.go and vault_test.go, including the listed helper/setup comments
and the comment above the zero-value assignment; preserve the test behavior and
improve clarity through descriptive helper and variable names or straightforward
control flow where needed.
In `@keeper/valuation_engine_test.go`:
- Around line 792-832: The test
TestGetTVVInUnderlyingAsset_AcceptedDenomFiltering should exercise filtering
based on internal NAV entries rather than IsAcceptedDenom. Seed an internal NAV
entry for unacceptedDenom that IsAcceptedDenom would reject, then assert
GetTVVInUnderlyingAsset excludes its balance; remove the orthogonal
vault.IsAcceptedDenom assertion and update expectedTVV to match the filtered
result.
In `@keeper/vault_test.go`:
- Around line 293-296: Remove the internal comment from the test setup and make
the intent clear through the existing assignment and surrounding test structure;
keep WithdrawalDelaySeconds set to zero so processing remains immediate within
the same block’s endblocker.
In `@proto/provlabs/vault/v1/vault.proto`:
- Around line 192-205: Mark the PendingSwapOut.redeem_denom field as deprecated
in its protobuf declaration, matching the existing deprecated annotation on
MsgSwapOutRequest.redeem_denom while preserving its field number and wire
compatibility.
🪄 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: f6f77abe-a757-4c56-b15b-2a89de198306
⛔ Files ignored due to path filters (5)
api/provlabs/vault/v1/tx_grpc.pb.gois excluded by!**/*.pb.gotypes/events.pb.gois excluded by!**/*.pb.gotypes/query.pb.gois excluded by!**/*.pb.gotypes/tx.pb.gois excluded by!**/*.pb.gotypes/vault.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (58)
.changelog/unreleased/deprecated/240-deprecation-sweep.md.changelog/unreleased/features/191-add-internal-nav-seed-migration.md.changelog/unreleased/improvements/239-disallow-multi-denom-vault.md.changelog/unreleased/state-machine-breaking/240-remove-mixed-denom.md.golangci.ymlGEMINI.mdREADME.mdSECURITY.mdapi/provlabs/vault/v1/events.pulsar.goapi/provlabs/vault/v1/query.pulsar.goapi/provlabs/vault/v1/tx.pulsar.goapi/provlabs/vault/v1/vault.pulsar.gokeeper/export_test.gokeeper/genesis_test.gokeeper/migrations.gokeeper/migrations_test.gokeeper/migrator.gokeeper/msg_server.gokeeper/msg_server_test.gokeeper/nav.gokeeper/nav_test.gokeeper/payout_restrictions_test.gokeeper/payout_test.gokeeper/query_server_test.gokeeper/queue_test.gokeeper/reconcile.gokeeper/reconcile_test.gokeeper/settlement.gokeeper/settlement_test.gokeeper/suite_test.gokeeper/valuation_engine.gokeeper/valuation_engine_test.gokeeper/vault.gokeeper/vault_test.gomodule.goproto/provlabs/vault/v1/events.protoproto/provlabs/vault/v1/query.protoproto/provlabs/vault/v1/tx.protoproto/provlabs/vault/v1/vault.protosimulation/estimate_accuracy_test.gosimulation/operations.gosimulation/operations_test.gosimulation/rand.gosimulation/setup.gosimulation/vault.gospec/01_concepts.mdspec/02_state.mdspec/03_messages.mdspec/04_events.mdspec/05_queries.mdspec/06_blocker.mdtypes/events.gotypes/genesis.gotypes/genesis_test.gotypes/msgs.gotypes/msgs_test.gotypes/vault.gotypes/vault_test.go
💤 Files with no reviewable changes (2)
- .changelog/unreleased/features/191-add-internal-nav-seed-migration.md
- keeper/payout_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/provlabs/vault/v1/tx.pulsar.go (1)
29733-29756: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFull removal breaks wire compatibility for these request fields.
payment_denom/initial_payment_navandredeem_denomare reserved out of the generated API, so existing clients will silently drop those values on decode instead of continuing to send deprecated-but-supported fields. If backward compatibility is still required, keep them intx.protowith[deprecated = true]and regenerate; otherwise update the PR text to call out the breaking change.🤖 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 `@api/provlabs/vault/v1/tx.pulsar.go` around lines 29733 - 29756, The generated request type is missing the deprecated payment_denom, initial_payment_nav, and redeem_denom fields, breaking wire compatibility. If backward compatibility is required, restore these fields in tx.proto with deprecated annotations and regenerate the API, preserving their existing field numbers; otherwise explicitly document the removal as a breaking change in the PR.
🧹 Nitpick comments (1)
keeper/valuation_engine_test.go (1)
367-368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider re-adding integration-level "missing NAV" error coverage for the held-denom model.
The payment-denom-era tests for missing-NAV error propagation in
GetTVVInUnderlyingAssetwere removed and no held-denom equivalent is visible in this file; the closest remaining coverage is the unit-level "missing internal NAV" case inTestUnitPriceFraction_Table. Not blocking, but an integration-level case (held asset present in principal balance, no NAV set) would close the gap.Also applies to: 428-429
🤖 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 `@keeper/valuation_engine_test.go` around lines 367 - 368, Restore an integration-level test for GetTVVInUnderlyingAsset covering a held asset present in the principal balance with no NAV configured. Assert that the missing-NAV error is propagated, using the existing TestGetTVVInUnderlyingAsset test setup and error expectations rather than relying only on TestUnitPriceFraction_Table.
🤖 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.
Outside diff comments:
In `@api/provlabs/vault/v1/tx.pulsar.go`:
- Around line 29733-29756: The generated request type is missing the deprecated
payment_denom, initial_payment_nav, and redeem_denom fields, breaking wire
compatibility. If backward compatibility is required, restore these fields in
tx.proto with deprecated annotations and regenerate the API, preserving their
existing field numbers; otherwise explicitly document the removal as a breaking
change in the PR.
---
Nitpick comments:
In `@keeper/valuation_engine_test.go`:
- Around line 367-368: Restore an integration-level test for
GetTVVInUnderlyingAsset covering a held asset present in the principal balance
with no NAV configured. Assert that the missing-NAV error is propagated, using
the existing TestGetTVVInUnderlyingAsset test setup and error expectations
rather than relying only on TestUnitPriceFraction_Table.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d8da9c7-8bb7-43d4-9a9d-01473d451996
⛔ Files ignored due to path filters (2)
types/query.pb.gois excluded by!**/*.pb.gotypes/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (35)
.changelog/unreleased/deprecated/240-deprecation-sweep.md.changelog/unreleased/state-machine-breaking/240-remove-mixed-denom.mdapi/provlabs/vault/v1/query.pulsar.goapi/provlabs/vault/v1/tx.pulsar.gokeeper/genesis_test.gokeeper/migrations.gokeeper/migrations_test.gokeeper/msg_server.gokeeper/msg_server_security_test.gokeeper/payout.gokeeper/payout_test.gokeeper/query_server.gokeeper/query_server_test.gokeeper/reconcile.gokeeper/reconcile_test.gokeeper/suite_test.gokeeper/valuation_engine.gokeeper/valuation_engine_test.gokeeper/vault.gokeeper/vault_test.goproto/provlabs/vault/v1/query.protoproto/provlabs/vault/v1/tx.protosimulation/estimate_accuracy_test.gosimulation/operations.gosimulation/operations_test.gosimulation/vault.gospec/01_concepts.mdspec/02_state.mdspec/03_messages.mdspec/05_queries.mdtypes/genesis_test.gotypes/msgs.gotypes/msgs_test.gotypes/vault.gotypes/vault_test.go
💤 Files with no reviewable changes (3)
- types/msgs.go
- simulation/operations.go
- keeper/query_server_test.go
🚧 Files skipped from review as they are similar to previous changes (13)
- keeper/msg_server.go
- .changelog/unreleased/deprecated/240-deprecation-sweep.md
- types/genesis_test.go
- keeper/migrations.go
- spec/02_state.md
- spec/05_queries.md
- simulation/operations_test.go
- spec/01_concepts.md
- types/vault.go
- keeper/migrations_test.go
- keeper/suite_test.go
- types/vault_test.go
- keeper/reconcile_test.go
nullpointer0x00
changed the title
1 participant
Summary by CodeRabbit
nav_authoritywhen unset.Upgrade and release coordination notes
Downstream: provenance dependency bump
This PR deletes the exported
keeper.MigrateVaultAccountPaymentDenomDefaults(shipped in v1.1.0).provenance-io/provenancestill calls it inapp/upgrades.go(thedaisyanddaisy-rc1handlers). Both upgrades have already executed on-chain (daisy applied at mainnet height 30,898,444; daisy-rc1 applied on testnet), so these are spent handlers: the provenance PR that bumps the vault dependency to v1.2 must delete those two call sites in the same change. The new v1->v2 flatten migration subsumes what that function did.Supported upgrade path
In-place upgrade via
RunMigrations(ConsensusVersion 1->2) is the only supported path. A genesis file exported from a v1.1 chain that contains mixed-denom vaults will fail v1.2InitGenesisvalidation (payment_denommust be empty or equal the underlying asset).Release-note items
redeem_denompositional argument is removed fromswap-out/estimate-swap-out, andcreateno longer takes--payment-denom. Wire/gRPC compatibility is preserved (fields deprecated, not removed).Live-state verification (2026-07-14)
outstanding_aum_feevalues are zero, there are no pending swap-outs, and all six vaults pass the newValidate(). Every vault is rewritten once by the migration (v1.1 state has nonav_authority).nuva.helocplusholds 1,074,397 uylds.fcc; testnetnuva.share.nvheloc.beta2holds 699,701,103 uylds.fcc. Post-migration these are unpriced (invisible to TVV); handling is an operational decision tracked separately.