Neutral Trade

The `refund_deposit()` function calculates `shares_to_revert` as `user_share_of_pending_deposits * last_total_shares_minted / PRECISION`, yet `bundle_temp_data.last_total_shares_minted` is **never reduced** after each refund. When multiple deposits are refunded in succession, earlier refunds shrink `cumulative_pending_deposits` but leave `last_total_shares_minted` unchanged; later iterations therefore cancel too many shares. The bundle ends with the correct underlying balance but an under‑reported `total_shares`, distorting share price and subsequent accounting. The same stale value is reused in `process_deposits.rs`, compounding the mismatch whenever refunds and new deposits occur in the same cycle.

The current implementation for calculating management fees is susceptible to a loss of precision due to its reliance on sequential integer divisions. This will cause the protocol to undercharge users for management fees over the long term systematically. ```rust let time_fraction = seconds_elapsed .checked_mul(bundle.asset_precision as u128) .ok_or(ErrorCode::MathError)? .checked_div(SECONDS_IN_YEAR) .ok_or(ErrorCode::MathError)?; // calculate fee percentage: (time_fraction * annual_fee_rate) / 10000 let mgmt_fee_pct = time_fraction .checked_mul(annual_fee_rate) .ok_or(ErrorCode::MathError)? .checked_div(10000) // Convert from bps to decimal .ok_or(ErrorCode::MathError)?; ``` The calculation is performed in three steps: A `time_fraction` is calculated, representing the portion of a year that has passed since the last fee collection. This calculation involves a division that truncates any remainder. The `time_fraction` is then used to calculate a `mgmt_fee_pct`, which involves another division and potential truncation. Finally, this percentage is used to determine the `management_fee_shares`. Because integer division in computing discards the remainder, any fractional component is lost at each step. This error, while potentially small for any single calculation, compounds and guarantees that the final fee charged will be less than or equal to the correct amount, but never more. Over thousands of transactions and users, this results in a slow but steady leak of value from the protocol's treasury. This is also the case for the P&L for shares: ```rust let pnl_usd = pnl_per_share .checked_mul(user_bundle_account.shares) .ok_or(ErrorCode::MathError)? .checked_div(bundle.asset_precision as u128) .ok_or(ErrorCode::MathError)?; // Step 2: Second division let perf_fee_usd = pnl_usd .checked_mul(bundle.performance_fee as u128) .ok_or(ErrorCode::MathError)? .checked_div(10000) .ok_or(ErrorCode::MathError)?; // Step 3: Third division performance_fee_shares = perf_fee_usd .checked_mul(bundle.asset_precision as u128) .ok_or(ErrorCode::MathError)? .checked_div(share_price) .ok_or(ErrorCode::MathError)?; ```

The code can benefit from the following validations and improvements: 1. Add `withdrawal_fee` to `ChangedCoreParams` and include `cooldown_period` plus `estimated_pending_withdrawal_value` in `WithdrawalRequested`. 2. `new_manager ` instruction is not needed in `change_manager()`, unless `new_manager` is required as additional signer. 3. Drop the unused `oracle_data` account from `set_max_deposit_amount()` or reference it explicitly. 4. Remove the redundant pause check in `end_distribution()`. 5. In `request_withdrawal ()`, since there is an earlier check that estimated pending withdrawls value is zero, it is possible to replace `checked_add()` for `user_bundle_account.estimated_pending_withdrawal_value` with `estimated_withdrawal_amount` with direct assignment. 6. Initialize `user_bundle_account.owner` in `initialize_bundle_depositor()` instead of `request_deposit()`. 7. Delete or annotate unused helpers like `fetch_account_balance()` in `bundle.rs`. 8. Reject `Pubkey::default()` in `change_bundle_master_admin()` to prevent null assignments. 9. Require the treasury to be a valid `TokenAccount` with `mint == asset_address` in all relevant instructions. 10. Declare `treasury_account` as `Account<'info, TokenAccount>` in `request_deposit()`. 11. The `receiver_address` in `add_strategy()` should be checked against the `Pubkey::default()`to prevent null assignments. 12. The `newkeeper` in `set_keeper()` should be checked against the `Pubkey::default()` to prevent null assignments.

The protocol assumes every strategy follows a strict life‑cycle: it is first added, later removed, and only then re‑enabled. The enable_strategy instruction, however, does not verify that `strategy_account.allowed` is currently false. If a bundle manager submits an enable_strategy call while the strategy is still marked as allowed—whether due to a failed or out‑of‑order `remove_strategy` transaction—the function blindly adds `strategy_account.allocation_bps` to `bundle_account.current_allocation_bps` a second time. This double‑counting breaks the bundle’s internal accounting, and may prevent subsequent strategy operations or open unintended allocation imbalances. Although unlikely in normal use, the absence of the guard leaves room for human error or automated scripts that batch transactions without checking intermediate results.

The `perform_refill()` function recomputes equity by adding the refill amount to `oracle_data.average_external_equity`. Yet it never verifies that the oracle price is fresh; if `last_update_time` is well in the past, the function records equity with outdated market data, skewing bundle accounting, share‑price calculations.

The `update_allocation()` function is designed to adjust the position size of an active strategy. Yet the instruction never checks that `strategy_account.allowed` is `true` before proceeding. If a bundle manager calls `update_allocation` on a strategy that has already been removed, the logic becomes harmful: 1. It subtracts the strategy’s previous `allocation_bps` from `bundle_account.current_allocation_bps`, even though that subtraction already occurred during `remove_strategy`. 2. It then adds the new allocation value and writes it to the strategy account. The modification in the bundle's allocation causes it to drift from the correct allocation, potentially turning negative or falling below the real sum of active strategies. Situations that can trigger the bug include: * Managers misunderstanding the life‑cycle and adjusting a removed strategy. * Transaction ordering issues where `remove_strategy` fails but a queued `update_allocation` succeeds.