Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to exit withdrawal of multiple ERC721 tokens on Mainnet. #340

Open
clumsier opened this issue Jun 7, 2022 · 6 comments
Open

Unable to exit withdrawal of multiple ERC721 tokens on Mainnet. #340

clumsier opened this issue Jun 7, 2022 · 6 comments

Comments

@clumsier
Copy link

clumsier commented Jun 7, 2022

Hey!

We are having our own UI for bridge, implemented using your SDK. Everything was going smoothly for over past 3-4 months, but some of our users reported that they can't exit their assets from Polygon to ETH and I've reproduced the issue.

For Polygon -> ETH we are using this method to start withdrawal - https://maticnetwork.github.io/matic.js/docs/pos/erc721/withdraw-start-many/

Then when withdraw tx is ready we call exit - https://maticnetwork.github.io/matic.js/docs/pos/erc721/withdraw-exit-many/

To reproduce the issue I've setup this sample repository with 2 sample txes to test with - https://github.com/clumsier/poly-bridge-debug

Just do yarn​ and yarn dev​ and you should be good to go.

When I try to exit with withdrawExitMany on Mainnet, I got issue as other users did, probably while generating proof

ERC721Predicate: INVALID_SIGNATURE
{
  "originalError": {
    "code": 3,
    "data": "0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000224552433732315072656469636174653a20494e56414c49445f5349474e4154555245000000000000000000000000000000000000000000000000000000000000",
    "message": "execution reverted: ERC721Predicate: INVALID_SIGNATURE"
  }
}

I can exit though relevant transaction on Testnet (Goerli).

Last change to codebase regarding Bridge was on 22th of April.
Last successful Exit for multiple ERC721 was this transaction on 25th of May - https://etherscan.io/tx/0xf6f89de1f8b16813ee61d03d11780e7a64fd9058e3375fdd4eab7ed6fde2cb68

@ujjwalgupta94
Copy link
Contributor

yeah we also have not changed anything in maticjs or contracts as far as i know, the last release was on may3. anyway @clumsier could you please share the txhash where user is not able to exit please ?

@clumsier
Copy link
Author

clumsier commented Jun 8, 2022

@ujjwalgupta94

Here is tx that user can't exit on Mainnet - https://github.com/clumsier/poly-bridge-debug/blob/main/pages/index.tsx#L67

Here is tx that user can exit on Testnet - https://github.com/clumsier/poly-bridge-debug/blob/main/pages/index.tsx#L74

I've suggested in several places that maybe something in proof generation API have changed.

@rahuldamodar94
Copy link
Member

Hey @clumsier , we have released a new version of matic.js which will unblock you for now - https://github.com/maticnetwork/matic.js/releases/tag/v3.4.0-beta.0

Have posted an example here - https://github.com/maticnetwork/matic.js/blob/master/examples/pos/erc721/withdraw_exit_on_index.js#L11 on how you can exit. We are updating the official matic.js documentation today and you will have better clarity on this.

So the reason why your exit was not going through was because we had to make changes to the ERC721 Predicate due to a vulnerability that existed on the contract. This change on the contract will not allow you to batchExit. You will have to exit each NFT one by one using the functionality added in the above release.

@rahuldamodar94
Copy link
Member

Also, I want to add on that we are exploring ways to implement the batchExit functionality on the smart contract level. Once it's done, you should be able to do the batchExit as you used to do it before. Until that you have to use matic.js to generate payload for individual NFT exits and user will have to sign each of these exits one by one. @py-zoid - please add on any other details if you feel is relevant and post the documentation link when you are done, Thanks,

@clumsier
Copy link
Author

clumsier commented Jun 20, 2022

Hi @rahuldamodar94 @py-zoid

thanks. I've played with that on testnet and it seems to work.

Do you have any ETA on bringing back batchExit functionality, is it days/weeks?

@rahuldamodar94
Copy link
Member

@clumsier The smart contract team is evaluating different solutions to implement it. Will keep you posted when have decided on how to proceed.

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

No branches or pull requests

4 participants
@rahuldamodar94 @ujjwalgupta94 @clumsier and others