search

divergencetech / ethier

Golang and Solidity SDK to make Ethereum development ethier
MIT License
217 stars 23 forks source link

Revisit contract wide Wyvern proxy pre-approval. #26

Closed cxkoda closed 2 years ago

cxkoda commented 2 years ago

Motivation

Wyvern proxies are currently pre-approved by hard-coding them into the ERC721 base contract, which means that their access cannot be revoked by users. Since the corresponding ApprovalForAll events are not emitted, most external indexing tools will not be able to correctly recognized the approval state for a given user. This can lead to situations where users have wrong information on the security of their tokens and cannot change it if desired.

Implementation

Introducing a new wrapper class for pre-approvals that

cxkoda commented 2 years ago

Thank you for the review! I tried to realize your points about the testing but I'm not quite sure if I was able to do so as intended. Could you maybe have another look? Also testing for State.Active == 0 was not possible because the struct is never exported, so abigen won't pick it up. I am a bit torn if I should refactor this into a boolean mapping and drop the enum completely, since we no only need a binary state. Any opinions?

cxkoda commented 2 years ago

Thanks for the review! Implemented most of the comments. See a few remaining open ones above.