`toAmount` in `AssetSwapped` event during `_executeSwaps` could be misleading

[sujithsomraaj]

Context: GenericSwapFacetV3.sol#L392

Description: The GenericSwapFacetV3 contract emits an AssetSwapped event after each swap in a multi-swap transaction. This event includes fields for fromAssetId, toAssetId, fromAmount, and toAmount.

However, the toAmount reported in this event can be misleading. The problem arises because after the swap, the amount is determined by checking the contract's balance of the` receiving token.

In a multi-swap transaction, it's possible that the user initially deposited some amount of the receiving token into the contract before the swaps began. If this is the case, then the contract's balance of receivingAssetId after a swap will include both the tokens received from that specific swap and the previously deposited tokens.

This means the toAmount in the AssetSwapped event will be inflated by the previously deposited amount and will not accurately reflect the amount received from just that single swap. This can be very misleading when trying to reconcile the individual swaps based on the AssetSwapped events.

Recommendation: Consider adding a snapshot of balance before the swap and after the swap to accurately determine the swap output amount.

LI.FI:

Researcher:

[0xDEnYO]

I would tend to leave it as-is and consider that a known issue. Otherwise we would have to add gas-intense balance checks for a use case that we are not even using yet.

Waiting for the other team members to share their opinion.

[maxklenk]

Valid point and storing the balance before would help. We don't really swap multiple assets into one so far, but plan to do it more in the future. It will for sure mess up our reporting if we try to analyse swaps and how good DEXs perform. For most calls it will not matter. How much would the gas would it cost to retrieve the balance before and calculate the difference?

[ezynda3]

I would lean in favor of accuracy otherwise why have events?

[maxklenk]

The gas overhead is quite high. We can ignore that special case here and instead use a more expensive function dedicated for that in the future instead of having higher gas costs for normal swap.

[0xDEnYO]

Agreed with Max. I will add a comment for now so that we dont forget about it and if the use case become more popular we can update the facet and add a function that is more accurate and less gas efficient (or just use the old version...)