Skip to content

Fixed unfinished TODOs#130

Open
mts1715 wants to merge 12 commits intomainfrom
taras/110-fix-unfinished-todos
Open

Fixed unfinished TODOs#130
mts1715 wants to merge 12 commits intomainfrom
taras/110-fix-unfinished-todos

Conversation

@mts1715
Copy link
Contributor

@mts1715 mts1715 commented Feb 3, 2026

Closes #110

Summary

  • Implement graceful failure recovery in ERC4626SinkConnectors.AssetSink when EVM approve/deposit calls fail
  • Add tokenSource for bridging assets back from EVM on recovery
  • Add proper documentation for swap() and swapBack() methods in ERC4626SwapConnectors
  • Add validation for feeSource (must provide FlowToken) and underlying asset matching

Changes

ERC4626SinkConnectors.cdc

  • Added tokenSource: EVMTokenConnectors.Source for failure recovery
  • Updated COA capability to include EVM.Bridge entitlement
  • Implemented recovery on approve() failure (bridge tokens back)
  • Implemented recovery on deposit() failure (revoke approval, bridge tokens back)
  • Replaced TODO comments with proper implementation

ERC4626SwapConnectors.cdc

  • Added @param and @return documentation for swap() and swapBack()

Tests

  • New test testDepositToPausedVaultRecoversGracefully verifying recovery behavior
  • New transaction deposit_to_paused_vault.cdc for testing paused vault scenarios
  • Updated test helpers with access control facet deployment

@mts1715 mts1715 self-assigned this Feb 3, 2026
@mts1715 mts1715 requested review from jribbink and nialexsan February 3, 2026 15:56
@mts1715 mts1715 requested a review from jribbink February 4, 2026 15:24
@mts1715 mts1715 requested a review from a team February 10, 2026 17:08
receiver.deposit(from: <-recovered)
return true
}
Burner.burn(<-recovered)
Copy link
Member

@zhangchiqing zhangchiqing Feb 23, 2026

Choose a reason for hiding this comment

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

   /// If unsuccessful (no tokens recovered), destroys the empty vault and returns false.

Are we sure the recovered is empty here? I think withdrawAvailable will return a partial balance like (50 tokens when asked for 100), if it fails the check above, and then we will burn these 50 tokens. Do we want to deposit back anyway, and return true or false depending on whether the amount is full or partial ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: withdraw to receiver available up to amount.
Returns true if the full amount was recovered (within rounding tolerance), false if only partial or zero.

1f4cb11

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.

FLOW-15 Unfinished TODOs

3 participants