feat: implement token swap functionality by parsaaba · Pull Request #175 · EigenExplorer/liquid-avs-token

parsaaba · 2025-06-03T12:22:56Z

No description provided.

gowthamsundaresan · 2025-06-16T07:21:56Z

src/FinalAutoRouting.sol

+    // ============================================================================
+
+    modifier onlyAuthorizedCaller() {
+        if (!authorizedCallers[msg.sender] && msg.sender != owner()) {


Can we restrict this to admin and LTM? So our role permissions setup is similar to other LAT contracts. Do you foresee that that we would have other authorized callers?

We can introudcue kinda role management style

it's really depends on how we gonna use that far contract.
Do you see any other use case beside that ltm usage?

I guess the original comment is stale -- I do think it would be good to keep the option open for other contracts to call. WM may use it when settling user withdrawals (undecided as of now)

The role management style makes sense, as its in-line with the pattern of other contracts

But if we do the lib + storage method, we wouldn't need any auth on the swap functions (in lib) right? Only the management functions that control the FAR state (in storage contract) would need something like DEFAULT_ADMIN_ROLE?

…overage

gowthamsundaresan · 2025-07-10T21:41:37Z

Hey, looks great overall!!

The one consideration that I have structurally is that future contracts like WM and potentially others would have to maintain separate (but synced) state of FAR. So I wonder if it's a good idea to split the FAR into:

AutoRoutingLib: All swap functions
AutoRoutingStorage : All state (including custom DEX Registry) and all their management functions. We add this to deploy process

The implications of this change would then be --

S1: I believe that none of the swap functions change any state right? So we can forgo all auth-related state variables, functions and modifers and related to authorizedCallers and routeManager in FAR. We can just keep DEFAULT_ADMIN_ROLE in AutoRoutingStorage taken during init (similar to other contracts) to control all management and emergency functions
S2: We can remove all changes to LTM init and in the future, any other contract can easily integrate the token swap functionality with
using AutoRoutingLib for AutoRoutingStorage
S3: We can remove farSupportedTokens and instead,
- just keep tokenDecimals mapping (if 0, token not valid)
- impose the check of token support using LTM’s supportedTokens mapping

Overall, looking to reduce complexity + improve deployment/dev experience. Lemme know your thoughts!

- Improve ETH transfer safety in emergencyWithdraw with proper error handling - Clean up comments and imports in FinalAutoRouting - Update LiquidTokenManager with latest implementation - Remove deprecated TokenSwap test directory and related test files

…payable address type for LiquidTokenManager

gowthamsundaresan · 2025-06-16T07:22:14Z

src/FinalAutoRouting.sol

+        if (poolAddresses.length != curveInterfaces.length)
+            revert InvalidParameter("interfaceArrays");
+
+        // Configure supported tokens


Can we use the same supported tokens from LTM? Since there can only be swaps between those tokens (we remain agnostic to intermediate tokens).

Because maintaining another registry here can create a new point of human error

gowthamsundaresan · 2025-06-16T07:23:24Z

src/FinalAutoRouting.sol

+        require(msg.sender == address(this), "Internal only");
+
+        // Check quote freshness
+        if (block.timestamp - quote.timestamp > QUOTE_MAX_AGE)


Does the manager need to specifically watch out for this revert and adjust its calls accordingly (like splitting a heavy call into multiple smaller attempts, etc.)?

gowthamsundaresan · 2025-06-16T07:25:04Z

src/FinalAutoRouting.sol

+        uint256 minAmountOut
+    ) private returns (uint256 amountOut) {
+        if (params.tokenIn == address(WETH) && params.tokenOut == OSETH) {
+            return _performWETHToOsETHSwap(params, minAmountOut);


Is there a reason to specifically handle WETH -> osETH here instead of having the user do a multi-step swap? In case we don’t handle WETH -> osETH here, then can we remove the entire category of Protocol.MultiHop right? Since any other multi-hop is handled with (Protocol.Uniswap && route.isMultiHop)

Perhaps we can instead recommend WETH -> rETH -> osETH to the manager in its config for when it sets up RouteConfigs for AutoSwaps and put responsibility on the caller if they choose to use swapAssets for WETH <> osETH

gowthamsundaresan · 2025-06-16T07:27:16Z

src/FinalAutoRouting.sol

+     * @param params Swap parameters
+     * @return amountOut Amount of output tokens received by caller
+     */
+    function swapAssets(


I think the params.minAmountOut provided by the user is not actually used right?

Quoter: checks quote -> adds buffer and calculates new minAmountOut from quote -> attempts swap -> success if outputAmount >= new minAmountOut

Fallback: Directly calculates minAmountOut from SlippageConfig

If so, then I feel that we can give the caller the autonomy to choose their slippage without configs/fallbacks:

Quoter: checks quote -> if quote >= params.minAmountOut -> adds buffer and calculates new minAmountOut from quote -> attempts swap -> success if outputAmount >= new minAmountOut

Fallback: None, revert because caller has been too strict with their slippage expectations

Note 1: If this suggestion is taken, we can consider removing SlippageConfig altogether and instead add slippage uint256 slippageBps to RouteConfig.

Note 2: If both suggestions from L530 and Note 1 are taken, we can consider removing AssetType altogether. The bridgeAsset can be taken in the initialization params directly OR we can add unitOfAccount as an immutable constant on the LiquidToken contract and use that (this actually helps in a few ways for the backend/manager too)

gowthamsundaresan · 2025-06-16T07:28:16Z

src/FinalAutoRouting.sol

+            );
+        }
+
+        if (amountOut < params.minAmountOut) revert InsufficientOutput();


What are your thoughts on not using params.minAmountOut at all and relying on the pre-existing config. So this function can v simple for the manager to call and is truly “auto”

gowthamsundaresan · 2025-06-16T07:29:22Z

src/FinalAutoRouting.sol

+            return amountOut;
+        }
+
+        // 3) Bridge route


Nice, this is smart!

gowthamsundaresan · 2025-06-16T07:29:51Z

src/FinalAutoRouting.sol

+     * @param password password for safety
+     * @return amountOut Amount of tokens received
+     */
+    function executeBackendSwap(


Can we add minAmountOut here and check amountOut >= minAmountOut?

gowthamsundaresan · 2025-06-16T07:30:22Z

src/FinalAutoRouting.sol

+            IERC20(tokenIn).safeApprove(dex, 0);
+        }
+
+        emit BackendSwapExecuted(


Can we include gasUsed like we did in AssetSwapped? Would be nice for backend to evaluate a given DEX over time and judge when to use it

gowthamsundaresan · 2025-07-11T06:58:41Z

src/FinalAutoRouting.sol

+    // ============================================================================
+
+    modifier onlyAuthorizedCaller() {
+        if (!authorizedCallers[msg.sender] && msg.sender != owner()) {


I guess the original comment is stale -- I do think it would be good to keep the option open for other contracts to call. WM may use it when settling user withdrawals (undecided as of now)

The role management style makes sense, as its in-line with the pattern of other contracts

But if we do the lib + storage method, we wouldn't need any auth on the swap functions (in lib) right? Only the management functions that control the FAR state (in storage contract) would need something like DEFAULT_ADMIN_ROLE?

gowthamsundaresan · 2025-07-26T18:15:40Z

src/core/LiquidTokenManager.sol

+
+        // Swap using FAR - for every tokenIn swap to corresponding tokenOut
+        for (uint256 i = 0; i < assetsLength; i++) {
+            address tokenIn = address(assetsToSwap[i]);


Note that tokenIn must be a supported token and have a corresponding supported strategy

gowthamsundaresan · 2025-07-28T14:54:03Z

Hey putting this PR into draft as we are splitting it up into LTM integration and a new repo. Will keep it open for reference and close it entirely after merge of v2-core to dev

parsaaba force-pushed the feat/tokenswap branch from 6578720 to 30e7715 Compare June 11, 2025 11:12

gowthamsundaresan reviewed Jun 16, 2025

View reviewed changes

Add FinalAutoRoutingLib functionality

0a4a665

parsaaba force-pushed the feat/tokenswap branch from 00b240b to 0a4a665 Compare July 1, 2025 06:09

parsaaba added 2 commits July 7, 2025 14:42

feat: implement token swap functionality and add comprehensive test c…

53c0e1e

…overage

feat: add FinalAutoRouting and interface contracts

59328ea

parsaaba added 4 commits July 11, 2025 13:27

4th approach implementation

ea53d48

Custody and ETH/Weth conversion resolved

39ce0ec

Superior Version of FAR with Instruction generation only

3305927

Revised FAR

fb0a483

parsaaba force-pushed the feat/tokenswap branch from 70148df to fb0a483 Compare July 18, 2025 16:20

parsaaba and others added 9 commits July 22, 2025 17:12

Update FinalAutoRouting.sol

ffe45a1

Path Generator version of FAR added

ebfb4d6

optimized and inner audit version added

f7d46a8

Resolve audit issues

ae9651b

optimized and inner audit version added2

912d791

initial version of LTM

69dce16

feat: implement token swap functionality and refactor tests

c88d0d9

refactor: improve code formatting and Update LTM script files to use …

61306b6

…payable address type for LiquidTokenManager

gowthamsundaresan reviewed Jul 28, 2025

View reviewed changes

gowthamsundaresan marked this pull request as draft July 28, 2025 14:52

Comments

Conversation

parsaaba commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gowthamsundaresan commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gowthamsundaresan commented Jul 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gowthamsundaresan commented Jul 10, 2025 •

edited

Loading