Skip to content

Add NAV-based balance using convertToAssets instead of AMM quote#189

Open
loic1 wants to merge 3 commits intomainfrom
loic/pkm-385-add-nav-based-balance-to-yield-vault-strategies
Open

Add NAV-based balance using convertToAssets instead of AMM quote#189
loic1 wants to merge 3 commits intomainfrom
loic/pkm-385-add-nav-based-balance-to-yield-vault-strategies

Conversation

@loic1
Copy link
Collaborator

@loic1 loic1 commented Feb 27, 2026

Deposits into ERC-4626 vaults enter at NAV rate, but balance was measured via UniswapV3 AMM quote which undervalues positions due to simulated swap price impact, causing enter strategy post-condition failures.

  • navBalance() on Strategy interface (defaults to availableBalance()), overridden in all three V1 strategies to convert AutoBalancer shares via ERC-4626 convertToAssets dry call
  • YieldVault.getNAVBalance() exposes NAV measurement to transaction callers

⚠️ Requires onflow/FlowActions#142 to be merged and submodule updated

@loic1 loic1 marked this pull request as ready for review February 27, 2026 16:37
@loic1 loic1 requested a review from a team as a code owner February 27, 2026 16:37
@nialexsan
Copy link
Collaborator

tests are blocked by this PR:
onflow/FlowALP#188

access(all) fun availableBalance(ofToken: Type): UFix64
/// Returns the NAV-based balance of the given token. Defaults to availableBalance(); strategies backed by
/// ERC-4626 vaults should override to return convertToAssets(shares) instead of an AMM quote.
access(all) fun navBalance(ofToken: Type): UFix64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navBalance should mean mark-to-market position value, not “safe-to-withdraw now”.

Right now the base Strategy.navBalance() default delegates to availableBalance(). For strategies that don’t override it, this makes navBalance health/withdrawability-constrained (as it depends on FlowALP health considerations), which is a different concept from NAV and can understate value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liobrasil Good catch. My intention was that this comment would clarify what navBalance represents: Returns the NAV-based balance of the given token. Defaults to availableBalance(); strategies backed by ERC-4626 vaults should override to return convertToAssets(shares) instead of an AMM quote.

Does updating the method definition to access(all) fun navBalance(ofToken: Type): UFix64? { return nil } address your concern? If not, do you have any suggestions? The contract was already updated to mainnet to unblock enter strategy transaction failures in peak money.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @nialexsan for vis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants