sygmaprotocol / sygma-widget

Transfer widget for the sygmaprotocol
5 stars 2 forks source link

Wrong Evm amount calculation #139

Closed mpetrunic closed 3 months ago

mpetrunic commented 3 months ago

Expected Behavior

To match substrate behavior as well as behavior of other token bridges. Bridge fee should be subtracted from user submitted amount to transfer.

Current Behavior

Bridge fee is added on top of user input amount. And validation doesn't take that into account

Possible Solution

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Version

Sygma commit (or docker tag): sygma-solidity version: sygma-substrate version: Go version:

LyonSsS commented 3 months ago

Percentage fee handler behavior applies % behavior for values that are limited to the minimum and maximum bounds. This is the definition of the Percentage Fee Handler for sygUSDC on spolia image PerceFeeHandler: https://sepolia.etherscan.io/address/0x2e77dEa116117eCF44a427064260D16D488ccff2#readContract For EX - inputting amount 8 to transfer of sygUSDC will span the correct allow spending value ( 1 sygUSDC ) image But incorrect for the actual transfer amount ( 7.111 instead of 7) image

mpetrunic commented 3 months ago

I wasn't even aware that percentage handler had min/max bounds 🤦🏻‍♂️

mpetrunic commented 3 months ago

Oh noes, I need to modify sdk for that...

LyonSsS commented 3 months ago

Also @mpetrunic , please integrate this fix https://github.com/sygmaprotocol/sygma-widget/issues/124 to test this behavior. As MAX value might behave strange. I want to cover it here for booth actual allowance behavior + transfer amount.

mpetrunic commented 3 months ago

Also @mpetrunic , please integrate this fix #124 to test this behavior. As MAX value might behave strange. I want to cover it here for booth actual allowance behavior + transfer amount.

done

LyonSsS commented 3 months ago

If I use a token that has a percentage fee approach, if I use the in between value ( min threshold < value < max threshold ), the transfer amount is not correct calculated. This is going to have issues when using the max value. Step 1 - use a middle value of which ( in this example it takes 0.1% fee) is calculated via percentage image Step 2 - allow spending for the transfer amount ( EXPECTED to work on this logic "calculate amount: amount=input/(1+feePercentage) ) ACTUAL - it calculates the entire value. image Step 2 applies to the transfer amount also.

NOTE: The same logic goes the the upper bound. The transferred amount is not equal to INPUT amount - Upper Bound value. This also affect the MAX value behavior to fail.

LyonSsS commented 2 months ago

Tested on latest Release version - it works fine image

LyonSsS commented 2 months ago

Retested on core PR main branch - everything works as expected image