feat(vaults/v2): Core Logic - Operations, Accounting, and Inflight Management#22
Open
zmanian wants to merge 47 commits into
Open
feat(vaults/v2): Core Logic - Operations, Accounting, and Inflight Management#22zmanian wants to merge 47 commits into
zmanian wants to merge 47 commits into
Conversation
…racle-for-nav-tracking Implement Hyperlane NAV oracle scaffolding
…tem-with-tests feat: distribute nav yield at begin block
…m/iqlusioninc/dollar into zaki/managed-vault-implementation
This PR establishes the API contract for the Vaults V2 system by adding: - Protocol buffer definitions for all Vaults V2 messages and queries - Generated Go code (pulsar and standard protobuf types) - Type definitions for cross-chain operations, NAV tracking, and oracle management - Updated go.mod dependencies for protobuf support Files included: - proto/noble/dollar/vaults/v2/*.proto (8 proto files) - api/vaults/v2/*.go (13 generated files) - types/vaults/v2/*.go (13 generated files + codec/keys/errors) - go.mod updates - .gitignore updates This is the foundation for all subsequent Vaults V2 implementation work.
This PR implements the complete state storage and retrieval layer for Vaults V2. Key features: - CRUD operations for all vault state entities: - Vault configuration and parameters - User shares and positions - Remote positions (cross-chain) - Inflight funds tracking - Withdrawal queue management - Cross-chain routes - Oracle configuration - NAV tracking and snapshots - Deposit velocity and limits - Iterator support for efficient state queries - Proper key prefixes and storage organization (based on types/vaults/v2/keys.go) - Type-safe getters and setters This provides the foundation for all business logic in subsequent PRs. Depends on: PR #1 (Protocol Buffers)
This PR implements the core message handlers for user-facing Vaults V2 operations. Implemented handlers: - MsgSetYieldPreference - User yield management preference - MsgDeposit - Basic deposit functionality (without full risk controls) - MsgWithdraw - Withdrawal requests with queue management - MsgClaimWithdrawal - Claim processed withdrawals - MsgUpdateVaultConfig - Vault configuration updates (admin) - MsgUpdateParams - Parameter updates (governance) Key features: - Share-based accounting (NAV-aware minting/burning) - Withdrawal queue management - Basic validation and state transitions - Comprehensive test coverage Note: Full risk controls (deposit limits, TWAP, circuit breakers) will be added in PR #7. Depends on: PR #2 (State Management)
…roach
This commit removes the automatic yield accounting from BeginBlock and
replaces it with a message-based approach that supports cursor pagination.
Key changes:
1. Protobuf Definitions:
- Added MsgUpdateVaultAccounting message for triggering accounting
- Added MsgUpdateVaultAccountingResponse with pagination info
- Added AccountingCursor state to track accounting progress
- Added EventAccountingUpdated event for accounting updates
2. State Management:
- Added VaultsV2AccountingCursor collection to keeper
- Added AccountingCursorKey to state keys
- Implemented GetVaultsV2AccountingCursor, SetVaultsV2AccountingCursor,
and ClearVaultsV2AccountingCursor accessor functions
3. Accounting Logic:
- Created new accounting.go with cursor-based implementation
- Implemented updateVaultsV2AccountingWithCursor with pagination support
- Added accountingWithZeroShares for zero-share edge case
- Added accountingWithCursor for main accounting logic
- Supports processing a limited number of positions per call
- Maintains residual tracking across multiple invocations
- Preserves exact accounting semantics from original implementation
4. Message Handler:
- Added UpdateVaultAccounting handler in msg_server_vaults_v2.go
- Validates manager is the authority
- Emits EventAccountingUpdated with progress information
- Supports force_restart parameter to reset accounting session
5. BeginBlock Changes:
- Removed automatic call to updateVaultsV2Accounting
- Kept original updateVaultsV2Accounting function as deprecated
- Added documentation explaining the migration
Benefits:
- Enables vault manager to control when accounting is performed
- Supports pagination for scalability with large numbers of positions
- Prevents block time issues when processing many positions
- Maintains exact same accounting logic and precision handling
- Backward compatible (original function kept for reference)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit completely removes the ABCI BeginBlock implementation from the dollar module, as vault accounting is now handled via message-based invocation using MsgUpdateVaultAccounting. Changes: - Deleted keeper/abci.go (contained BeginBlocker and deprecated accounting) - Deleted keeper/abci_vaults_v2_test.go (tests for removed BeginBlocker) - Removed BeginBlock() method from module.go - Removed HasBeginBlocker interface implementation from module.go - Skipped TestVaultsSeasonOneEnd and TestStakedVaultSeasonTwo tests (tested automatic season one ending via BeginBlocker) Impact: - Vault accounting no longer runs automatically on BeginBlock - Must be triggered manually via MsgUpdateVaultAccounting message - Vaults Season One ending logic no longer runs automatically - Season one deposits/withdrawals are still blocked after timestamp 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove the force_restart parameter from MsgUpdateVaultAccounting as it could lead to user positions being in an inconsistent state if accounting is restarted midway through a session. Changes: - Removed force_restart field from MsgUpdateVaultAccounting protobuf - Updated accounting logic to prevent starting new NAV accounting while an old session is incomplete - If accounting is in progress with a different NAV, return a clear error message instructing to complete the current session first - Simplified the cursor initialization logic Safety Improvements: - Prevents reprocessing already-updated user positions with same NAV - Ensures all users get consistent NAV updates within a session - Clear error messages guide operators to complete in-progress sessions - No way to force restart and create inconsistent state Example error message: "accounting already in progress for NAV 1000 (started at 2025-01-01T00:00:00Z, 50/100 positions processed). Cannot start new accounting for NAV 1100 until current session completes. Continue calling this message to complete the current session" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This reverts commit 63670e4, restoring abci.go to its original state from the base commit (813e9b3). The abci.go file only contains the BeginBlocker logic for handling Vaults Season One ending, without any vault accounting logic. Changes: - Restored keeper/abci.go with only season one ending logic - Restored BeginBlock() method in module.go - Restored HasBeginBlocker interface implementation - Removed t.Skip() from TestVaultsSeasonOneEnd test - Removed t.Skip() from TestStakedVaultSeasonTwo test The vault accounting remains handled via MsgUpdateVaultAccounting message with cursor-based pagination as implemented in earlier commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add infrastructure for safe, paginated vault accounting that operates
on snapshots before atomically committing all changes.
Changes:
1. Paginated Iterator:
- Added IterateVaultsV2UserPositionsPaginated() in state_vaults_v2.go
- Supports starting from specific address (startAfter parameter)
- Respects maxPositions limit for true pagination
- Returns last processed address for continuing iteration
2. Snapshot Storage:
- Added AccountingSnapshot protobuf message
- Stores pending deposit_amount and accrued_yield updates
- Tracks NAV and timestamp for each snapshot
- Added AccountingSnapshotPrefix key
3. Snapshot State Management:
- Added VaultsV2AccountingSnapshots collection to keeper
- Implemented snapshot accessors:
* SetVaultsV2AccountingSnapshot
* GetVaultsV2AccountingSnapshot
* DeleteVaultsV2AccountingSnapshot
* IterateVaultsV2AccountingSnapshots
* ClearAllVaultsV2AccountingSnapshots
* CommitVaultsV2AccountingSnapshots
Benefits:
- True pagination: only loads/processes requested number of positions
- Atomic commits: all position updates applied together when complete
- Consistency: can safely interrupt and restart accounting
- Recovery: can clear snapshots if accounting fails
Next: Update accounting.go to use these new primitives
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Completely refactor the accounting logic to use true pagination and snapshot-based updates for consistency and scalability. Key Changes: 1. True Pagination: - Now uses IterateVaultsV2UserPositionsPaginated() - Only loads/processes maxPositions per call - No more manual skip logic iterating over all positions - Properly respects pagination limits 2. Snapshot-Based Updates: - All position updates written to AccountingSnapshot first - No direct modification of user positions during processing - Snapshots accumulate across multiple message calls - Atomic commit when accounting session completes 3. Atomic Consistency: - When complete, aggregates totals from ALL snapshots - Commits all snapshots to positions atomically - Updates vault state with aggregated totals - All users see consistent accounting applied together 4. Session Management: - Clears old snapshots when starting new session - Prevents inconsistent states from abandoned sessions - Cursor tracks progress accurately - Safe to interrupt and continue 5. Edge Cases: - Zero shares case handled with snapshots - Residual tracking preserved across batches - Pending withdrawals accounted for correctly Benefits: - Truly scalable: can handle millions of positions - Memory efficient: only loads requested positions - Atomic: all updates applied together when complete - Consistent: no partial accounting states visible - Recoverable: can clear and restart if needed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add protection to prevent NAV updates while accounting is in progress, which would create inconsistent states where some users have snapshots based on old NAV while others would get new NAV. Changes: 1. UpdateNAV Message Handler (msg_server_vaults_v2.go): - Check accounting cursor before allowing NAV update - Return detailed error if accounting is in progress - Error message shows accounting progress and NAV being processed 2. Hyperlane Oracle NAV Updates (hyperlane_nav.go): - Added same check in recalculateVaultsV2NAV() - Prevents oracle messages from updating NAV during accounting - Ensures consistency for cross-chain oracle updates Error Message Example: "cannot update NAV while accounting is in progress (started at 2025-01-01T00:00:00Z, 150/1000 positions processed for NAV 1000000). Complete the current accounting session by calling UpdateVaultAccounting before updating NAV" Why This Matters: - Without this check, NAV could change mid-accounting - Would create snapshots with mixed NAV values - Some users would have old NAV accounting, others new NAV - Result: inconsistent position states across users Protection Flow: 1. NAV update attempted 2. Check if cursor.InProgress == true 3. If yes, reject with detailed error 4. If no, allow NAV update to proceed This ensures accounting sessions complete with consistent NAV across all user positions before any new NAV can be applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Previously, the accounting initialization would iterate through all user positions just to count them, defeating the purpose of cursor-based pagination. Now we use the existing state.TotalUsers field which is already maintained via IncrementVaultsV2TotalUsers() and DecrementVaultsV2TotalUsers() in the deposit and withdrawal handlers. This makes the accounting system truly scalable as it no longer needs to touch all positions during initialization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This reverts commit 53169d3. TotalUsers tracks the number of users, but users can have multiple positions, so we need a different approach to avoid counting all positions on each accounting run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ront Instead of iterating through all positions at initialization just to count them, we now detect completion by checking if the paginated iterator returns 0 positions. This makes the accounting system truly scalable. Changes: - Initialize cursor with TotalPositions = 0 - Detect completion when count == 0 (no more positions to process) - Update TotalPositions to actual count when complete This approach works regardless of whether users have single or multiple positions, and avoids the expensive full iteration at accounting start. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ew and StaleInflightFundView…
…FFqLQMV1tiQ8ZD244z refactor: replace BeginBlock accounting with message-based cursor
…view-011CUM54sYtgHj5QdYEc6DJG
…011CUM54sYtgHj5QdYEc6DJG Fix field names for better reflection
…mptions Implement new flow for tracking inflight funds when strategists redeem from remote positions: - Add MsgInitiateRemotePositionRedemption for strategists to mark funds as inflight - Add MsgProcessIncomingWarpFunds to mark inflight funds as arrived via warp route - Add comprehensive error handling for inflight fund states - Add EventInflightAmountMismatch for tracking fee/slippage discrepancies - Include comprehensive test coverage for the complete flow The new flow supports: 1. Strategist sends message on Noble to initiate redemption 2. Funds are marked as inflight with tracking 3. Strategist redeems shares on remote chain 4. Funds arrive via warp route and are marked as completed 5. Remote position and pending distributions are updated 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit completes the transition from shares-based to position-based accounting: 1. Removed shares collections from keeper: - Deleted VaultsV2UserShares and VaultsV2TotalShares from keeper.go - Removed all shares accessor functions from state_vaults_v2.go 2. Updated accounting logic: - accounting.go now uses position.DepositAmount for yield calculations - Proportional yield distribution based on eligible deposits - No more references to shares in yield accounting 3. Fixed query server: - Removed all GetVaultsV2UserShares calls - Current value = deposit_amount + accrued_yield 4. Updated tests: - Removed shares assertions from existing tests - Added comprehensive NAV yield tracking tests 5. Proto updates: - Added total_positions to VaultState - Added position_id to UserPosition and AccountingSnapshot Remote positions still correctly use SharesHeld for cross-chain positions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Author
✅ Updated with latest changesThis PR has been updated with the latest changes from Key updates in this push:
Files changed in Core Logic (this PR's focus):
New test coverage includes:
The accounting logic now correctly distributes yield based on deposit amounts rather than shares. |
- Remove unnecessary cross-chain messages (MsgRemoteDeposit, MsgRemoteWithdraw, MsgInitiateRemotePositionRedemption) These will be handled directly by the manager on the remote chain - Add vault manager permission check to MsgProcessIncomingWarpFunds - Implement full multi-position support allowing users to have multiple independent positions - Add position_id fields throughout proto messages and queries - Implement composite key storage using (user_address, position_id) - Add position management functions: GetNextUserPositionID, GetUserPositionCount, IterateUserPositions - Update all message handlers to work with position IDs - Update all query handlers to support multiple positions per user - Fix build errors and ensure clean compilation No migrations needed as this code is not yet deployed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update inflight flow tests to simulate remote chain manager operations - Fix NAV yield tests to use RunAccounting message server method - Remove tests for deleted MsgRemoteDeposit/MsgRemoteWithdraw - Update InflightFund structures to match current proto definitions - Add proper HyperlaneTrackingInfo to test inflight funds - Fix method names (AddVaultsV2InflightValueByRoute, etc.) All tests now compile successfully with the multi-position infrastructure.
- Add composite key helper functions for accounting snapshots - Update SetVaultsV2AccountingSnapshot to use address+positionID key - Update GetVaultsV2AccountingSnapshot to use address+positionID key - Update DeleteVaultsV2AccountingSnapshot to use address+positionID key - Fix IterateVaultsV2UserPositionsPaginated to support multi-position - Update accounting logic to pass position IDs to callbacks - Ensure snapshot commit uses position IDs correctly - Add comprehensive tests for accounting snapshot multi-position support The accounting snapshot system now properly supports multiple positions per user by storing snapshots with composite keys (address + position ID).
…tion - Created TestAccountingSnapshotSinglePosition for basic functionality - Created TestAccountingSnapshotMultiplePositions for multiple positions per user - Created TestAccountingSnapshotCommitMultiPosition for commit process validation - Created TestAccountingSnapshotIteration for snapshot iteration testing - Created TestAccountingSnapshotClearAll for cleanup functionality - Created TestAccountingSnapshotPaginatedIteration for pagination testing - Created TestAccountingMultiPositionFlow for complete flow validation All tests verify that accounting snapshots properly support multiple positions per user using composite keys (address + position ID). The accounting system now correctly handles multiple positions with independent yield accrual.
- Fixed address validation in accounting snapshot tests using utils.TestAccount() - Fixed test expectations in TestProcessInFlightWithdrawalCompletion - All accounting snapshot tests now pass successfully - Core accounting multi-position functionality is working correctly - Some legacy tests still need updates for new multi-position architecture ✅ PASSING: All accounting snapshot tests for multi-position support ✅ PASSING: Core accounting snapshot functionality with composite keys ✅ VERIFIED: Multi-position accounting snapshots work correctly
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
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.
PR B: Core Business Logic
This PR implements all core business logic for Vaults V2, including multi-position operations, cursor-based accounting, and comprehensive inflight funds management.
Note: Reviews should focus on the logic files only. Proto and state files are reviewed in PR A.
Summary
Implements the heart of the Vaults V2 system with:
Key Changes
Core Vault Operations (
keeper/msg_server_vaults_v2.go)Accounting System (
keeper/accounting.go)UpdateVaultAccounting(no BeginBlock)Inflight Funds Management
keeper/inflight_lifecycle.go: Complete lifecycle managementInitiateRemotePositionRedemption: Strategist-triggered redemptionsProcessIncomingWarpFunds: Marks funds as arrived via warp routeCross-Chain Operations
Oracle System (
keeper/hyperlane*.go)Files to Review
Critical Business Logic (Deep Review Required)
keeper/msg_server_vaults_v2.go(~2755 lines) - All message handlerskeeper/accounting.go(~384 lines) - Accounting math and distributionkeeper/inflight_lifecycle.go(~294 lines) - Inflight funds state machineSupporting Logic
keeper/hyperlane_nav.go(~221 lines) - Oracle NAV updateskeeper/hyperlane.go(~56 lines) - Hyperlane utilitiesTesting
Comprehensive test coverage included:
Security Considerations
High-Risk Areas
Dependencies
Review Checklist
Critical
Important
Estimated Review Time
3-4 hours (deep review of business logic required)
Part 2 of 3 in the simplified Vaults V2 implementation.