Giveth / giveth-dapps-v2

This project is the aggregation of GIVeconomy and Giveth.io DApps in a single repo
https://staging.giveth.io
GNU General Public License v3.0
55 stars 34 forks source link

Error on Staking with Permit xDAI/FOX farm - NOT_VALID_CALL_SIGNATURE #3420

Open divine-comedian opened 8 months ago

divine-comedian commented 8 months ago

Describe the bug When users try to stake an LP in the xDAI/FOX farm the transaction is failing - The error is coming from the UnipoolTokenDistributor contract and says UnipoolTokenDistributor: NOT_VALID_CALL_SIGNATURE

https://dashboard.tenderly.co/tx/gnosis-chain/0xb61995a18f956bf81c7e535c29d30463339bb667247f35aafeaaf0fe05336bd1

To Reproduce Steps to reproduce the behavior:

  1. Go to GIVfarm on xDAI
  2. Create xdai/fox lp on honeyswap
  3. Click stake
  4. toggle 'Permit Mode' on
  5. stake some tokens
  6. see error in metamask
  7. proceed anyway
  8. see failed tx in tenderly

Expected behavior The user should be able to successfuly stake their LP tokens with Permit mode enabled

Screenshots image

Desktop (please complete the following information):

Additional context Reported on production by Shapeshift users

divine-comedian commented 7 months ago

cc: @laurenluz

laurenluz commented 7 months ago

the rewards on the fox/xDAI farm I believe have ended as of yesterday from this PR they started on May 30.

would have been great to see this issue before! what's the story here?

divine-comedian commented 7 months ago

the rewards on the fox/xDAI farm I believe have ended as of yesterday from this PR they started on May 30.

would have been great to see this issue before! what's the story here?

Vlad DM'ed me to report the bug and so I made an issue for it

laurenluz commented 7 months ago

I mean that I see Krati was assigned, then Ramin, then after my comment Ramin set an estimate... so it looks to me like there is some conversation & coordination going out outside of this issue that I'm not in the loop on...

This farm needs to be archived. I will make a new issue for that... but we should disable the stake function anyway, so this issue is kind of irrelvant unless we deploy a new regen farm or reactive/extend this.

divine-comedian commented 7 months ago

I mean that I see Krati was assigned, then Ramin, then after my comment Ramin set an estimate... so it looks to me like there is some conversation & coordination going out outside of this issue that I'm not in the loop on...

This farm needs to be archived. I will make a new issue for that... but we should disable the stake function anyway, so this issue is kind of irrelvant unless we deploy a new regen farm or reactive/extend this.

I think it's handled on any unipool contract or any GIVeconomy contract that can call stakeWithPermit @aminlatifi would know more tbh. but actually this only affects the farms - not our GIVpower contract IIRC.

aminlatifi commented 7 months ago

It's a frontend issue, the permit value is not computed correctly! I think after refactorings, the code snippet was responsible for computing the permission is touched and it's generating a wrong value. Compared with the old implementation or old successful stakeWithPermit transactions, it will be totally clear.

aminlatifi commented 7 months ago

This may help if we cannot use our old code snippet for calculating the permit value https://github.com/vacekj/wagmi-permit?utm_source=substack&utm_medium=email#readme

MoeNick commented 6 months ago

What's the status of this issue? is it p1 or p2? or even p0 ? or is it resolved?

divine-comedian commented 6 months ago

AFAIK not fixed - xDAI/FOX farm has ended and there are currently no regenfarms using this.

If we make another regenfarm or LP farm we will need to fix this issue

JFYI @jainkrati

jainkrati commented 4 months ago

If there are no regen farms using this , making this a p2. @RamRamez pls work on this once p0,p1 issues are done.

jainkrati commented 2 months ago

@MohammadPCh pls take this up

jainkrati commented 1 month ago

@MohammadPCh pls guide @kkatusic to take this up

aminlatifi commented 1 month ago

@kkatusic The very problem is the stakeWithPermit method is now called with an invalid permit value. It used to be working in older versions like this https://github.com/Giveth/giveth-dapps-v2/blob/bf4e56c8312ad044636ccb2d7c83b04ed99eeea8/src/lib/stakingPool.ts The problem can be reproduced with any farming contract with stkeWithPermit method, including new unipool givpower contracts deployed on OP mainnet.

aminlatifi commented 1 month ago

Sample of working stake with permit https://gnosisscan.io/tx/0x801ecb80e130765822518416633174b5e42a1a7ad1d0e75ab4c003a05f3506e4

MohammadPCh commented 1 month ago

@kkatusic I fixed the Permit functionality, can you check it, and if it is ok continue this task and add the toggle button and handle to the StakeGIV modal and change the states?

kkatusic commented 1 month ago

@kkatusic The very problem is the stakeWithPermit method is now called with an invalid permit value. It used to be working in older versions like this https://github.com/Giveth/giveth-dapps-v2/blob/bf4e56c8312ad044636ccb2d7c83b04ed99eeea8/src/lib/stakingPool.ts The problem can be reproduced with any farming contract with stkeWithPermit method, including new unipool givpower contracts deployed on OP mainnet.

@aminlatifi just to notice here it wasn't error on optimism network it was problem on gnosis network.

kkatusic commented 1 month ago

@kkatusic I fixed the Permit functionality, can you check it, and if it is ok continue this task and add the toggle button and handle to the StakeGIV modal and change the states?

Excellent @MohammadPCh , it is working, @divine-comedian you can also check, image is bellow:

Screenshot 2024-06-12 at 21 46 07

kkatusic commented 1 month ago

And @divine-comedian missing image from your example:

Screenshot 2024-06-12 at 21 52 22

MohammadPCh commented 1 month ago

@divine-comedian @aminlatifi Can we show the permit toggle on all the stake cards, or are there some that do not support this feature?

aminlatifi commented 1 month ago

@divine-comedian @aminlatifi Can we show the permit toggle on all the stake cards, or are there some that do not support this feature?

It depends on the farm, most of them support it except the Gnosis GIVPower. A rule of thumb is to look at the contract and check it has stakeWithPermit method or no!

maryjaf commented 3 weeks ago

Is it ready for testing @MohammadPCh @kkatusic

kkatusic commented 3 weeks ago

Is it ready for testing @MohammadPCh @kkatusic

@maryjaf It it ready ;) , thx

maryjaf commented 3 weeks ago

Describe the bug When users try to stake an LP in the xDAI/FOX farm the transaction is failing - The error is coming from the UnipoolTokenDistributor contract and says UnipoolTokenDistributor: NOT_VALID_CALL_SIGNATURE

https://dashboard.tenderly.co/tx/gnosis-chain/0xb61995a18f956bf81c7e535c29d30463339bb667247f35aafeaaf0fe05336bd1

To Reproduce Steps to reproduce the behavior:

  1. Go to GIVfarm on xDAI
  2. Create xdai/fox lp on honeyswap
  3. Click stake
  4. toggle 'Permit Mode' on
  5. stake some tokens
  6. see error in metamask
  7. proceed anyway
  8. see failed tx in tenderly

Expected behavior The user should be able to successfuly stake their LP tokens with Permit mode enabled

Screenshots image

Desktop (please complete the following information):

  • OS: Ubunutu 22.04
  • Browser: Brave
  • Wallet: MetaMask

Additional context Reported on production by Shapeshift users

How can I test and reproduce theses steps ? I didn't see this lp on stg xdai/fox

kkatusic commented 3 weeks ago

Describe the bug When users try to stake an LP in the xDAI/FOX farm the transaction is failing - The error is coming from the UnipoolTokenDistributor contract and says UnipoolTokenDistributor: NOT_VALID_CALL_SIGNATURE https://dashboard.tenderly.co/tx/gnosis-chain/0xb61995a18f956bf81c7e535c29d30463339bb667247f35aafeaaf0fe05336bd1 To Reproduce Steps to reproduce the behavior:

  1. Go to GIVfarm on xDAI
  2. Create xdai/fox lp on honeyswap
  3. Click stake
  4. toggle 'Permit Mode' on
  5. stake some tokens
  6. see error in metamask
  7. proceed anyway
  8. see failed tx in tenderly

Expected behavior The user should be able to successfuly stake their LP tokens with Permit mode enabled Screenshots image Desktop (please complete the following information):

  • OS: Ubunutu 22.04
  • Browser: Brave
  • Wallet: MetaMask

Additional context Reported on production by Shapeshift users

How can I test and reproduce theses steps ? I didn't see this lp on stg xdai/fox

@maryjaf go to this file find xdai/fox, change starting date and ending date, if you need help contact me.

maryjaf commented 3 weeks ago

It seems that this issue cannot be tested on the stage environment and needs to be tested on production

MohammadPCh commented 2 weeks ago

@maryjaf you can test it with GIV on Optimism and OP sepolia.

divine-comedian commented 1 week ago

@kkatusic - can we set this up to be able to be tested on staging properly? let's try setting up a test regenfarm for the next 2 months to give sufficient testing window

kkatusic commented 1 week ago

@divine-comedian let's try, Cherik isn't available but I think who can help, us, Amin.

@aminlatifi can you help. us with this? Can you check this setup for staging server are contracts address correct: link to config file if everything ok I can provide new timelines for that xdai/fox staking. Also you will need to provide some test tokens to Maryjaf :)

Thx in advance

aminlatifi commented 1 week ago

@divine-comedian let's try, Cherik isn't available but I think who can help, us, Amin.

@aminlatifi can you help. us with this? Can you check this setup for staging server are contracts address correct: link to config file if everything ok I can provide new timelines for that xdai/fox staking. Also you will need to provide some test tokens to Maryjaf :)

Thx in advance

@kkatusic Extended/activated fox/dai farm on staging for 14 days. https://github.com/Giveth/giveth-dapps-v2/pull/4395 cc @divine-comedian @maryjaf

kkatusic commented 1 week ago

@divine-comedian let's try, Cherik isn't available but I think who can help, us, Amin. @aminlatifi can you help. us with this? Can you check this setup for staging server are contracts address correct: link to config file if everything ok I can provide new timelines for that xdai/fox staking. Also you will need to provide some test tokens to Maryjaf :) Thx in advance

@kkatusic Extended/activated fox/dai farm on staging for 14 days. #4395 cc @divine-comedian @maryjaf

Thx @aminlatifi , great, it is working now on staging, @maryjaf you can test it.

maryjaf commented 1 week ago

In my tests the sign request is shown twice in permit mode and after that I could stake, please check below screen record

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/c9c4f58a-adbe-46c9-9782-d01bd996ce9c

divine-comedian commented 1 week ago

@kkatusic - looks like there is also an issue with the "permit mode" text colour

kkatusic commented 1 week ago

In my tests the sign request is shown twice in permit mode and after that I could stake, please check below screen record

Screen.Recording.2024-07-09.at.6.18.33.PM.mov

If I understand right Amin and Cherik you for permit staking needs two confirmation.

divine-comedian commented 1 week ago

In my tests the sign request is shown twice in permit mode and after that I could stake, please check below screen record Screen.Recording.2024-07-09.at.6.18.33.PM.mov

If I understand right Amin and Cherik you for permit staking needs two confirmation.

No, there is an error here, I tested it as well - it is asking for the signature twice - you sign the first signature request, then it bugs out. You need to click STAKE again then it asks for the same signature another time, once you sign the 2nd time around then it generates the actual stake tx and the transaction proceeds as normal

kkatusic commented 1 week ago

@divine-comedian, yes you are right it is asking you signature two times. I have been started working on this issue, but than Cherik needed to complete it because a lot of stack-related code had to be replaced.

But I will take a look and see is there any fast fix.

kkatusic commented 1 week ago

@divine-comedian, yes you are right it is asking you signature two times. I have been started working on this issue, but than Cherik needed to complete it because a lot of stack-related code had to be replaced.

But I will take a look and see is there any fast fix.

@divine-comedian I made fix, Ramin will check code

kkatusic commented 1 week ago

@maryjaf please test it when you have time

maryjaf commented 6 days ago

Thanks @kkatusic I've tested "permit mode" and also"approve mode", the problem has been fixed.

kkatusic commented 6 days ago

Thanks @kkatusic I've tested "permit mode" and also"approve mode", the problem has been fixed.

Excellent, thx @maryjaf