-
Notifications
You must be signed in to change notification settings - Fork 8
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
#606 | Edit and revoke allowances #717
#606 | Edit and revoke allowances #717
Conversation
…t into 606-revoke-allowances-api-integration
…06-revoke-allowances-api-integration
…t into 606-revoke-allowances-api-integration
…06-revoke-allowances-api-integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on this!!! Minor modal copy suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Me saco el sombrero" 🎩
The only detail I noticed is that if you have a batch of revokes to process, cancelling the operation causes you to lose the selection you had. That is a colateral effect of reseting all checkboxes when the data changes. To fix this problem you would probably need to allow initial selection from the outside of the component which is an overkill solution. So let it as it is...
Good Job !!
Fixes #606
Description
This PR adds the ability to edit and batch revoke allowances. It also updates the contract store to prevent non-contract addresses from being saved as contracts incorrectly.
note: the toggle to show cancelled approvals has been temporarily removed from the page, as the indexer does not yet support showing cancelled approvals. there is a separate issue to fix that, #719
note: there are 3 different kinds of approvals to be aware of (this is also commented in the code for future reference):
approve
method0x0000...0000
, as there is no actual 'revoke' method to call. This is done using theapprove
methodsetApprovalForAll
method, which exists for both ERC721 and ERC1155 contracts. When callingsetApprovalForAll
, you provide an operator address and a boolean. If the boolean istrue
, the operator is allowed to send all token IDs in that collection which are owned by you, on your behalfTest scenarios
Setup
git fetch --all && git checkout 606-revoke-allowances-api-integration && echo 'NETWORK="mainnet"' > .env && yarn dev
0x3783...8949
approve
dropdown, enter an address for the spender (I suggest you choose an address owned by you, or a known contract like Staked TLOS which will not abuse the approval) and an amount, then click Write and approve the tx.approve
dropdown, enter an address like with ERC20, enter a token ID (which can be gotten from the NFT detail page on the Wallet app), and click Write and approve the TX.setApprovalForAll
dropdown, fill an address and set approved = true, then click Write and approve the txEdit allowances (note: complete these steps on desktop and mobile walletconnect with both MetaMask & SafePal)
Custom
radio should be selected with that number filled. If the allowance isUnlimited
, that radio should be selected, and theCustom
input should be disabled and showing 09,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999
Allowed
andNot Allowed
Allowed
) should be selected alreadyCollection Name #{tokenId}
as well as some text about single ERC721 approvalsNot Allowed
Collection Name #{tokenId}
and the operator addressTelosians
Allowed
andNot Allowed
Allowed
) should be selected already{collection name} (Entire Collection)
Not Allowed
{Collection Name}
and the operator addressAllowed
andNot Allowed
Allowed
) should be selected already{collection name} (Entire Collection)
Not Allowed
{Collection Name}
and the operator addressBatch Revoke
Checklist: