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

feat: added adr for burning or tterc721 token #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

justussoh
Copy link

This PR adds decision records of burning TradeTrustErc721 token to 0xdead instead if 0x0.

}
```

However, TradeTrust is still required to render and track burnt token. Since unminted tokens and destoryed token have the same owner address in the Erc721 Token Registry, we needed to find a way to differentiate a unminted and burnt token. This means that TradeTrust would have to modify the ERC721 implementation or modify its verifier method.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
However, TradeTrust is still required to render and track burnt token. Since unminted tokens and destoryed token have the same owner address in the Erc721 Token Registry, we needed to find a way to differentiate a unminted and burnt token. This means that TradeTrust would have to modify the ERC721 implementation or modify its verifier method.
In the TradeTrust application, tokens are still required to be traceable and viewable after destruction. Since unminted tokens and burnt token appear to have the same owner address (.ownerOf() throws error 'cannot query non-existent token') in the ERC721 Token Registry, it is necessary to find a way to differentiate an unminted and burnt token.
There exists two broad approaches to resolve this:
1. The ERC721 implementation is modified to support the additional state of 'destroyed'
1. The verification method in OpenAttestation or TradeTrust is modified to detect the history of the token, if any exists

- event emission might be lost in future
- state is not immediately available to be queried (as both unminted token and burnt tokens had `0x0` address, thus we need to query events)

#### using 0xknown as burn address (like [0x000000000000000000000000000000000000dead](https://etherscan.io/address/0x000000000000000000000000000000000000dead))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### using 0xknown as burn address (like [0x000000000000000000000000000000000000dead](https://etherscan.io/address/0x000000000000000000000000000000000000dead))
#### using a known address as a burn address (e.g [0x000000000000000000000000000000000000dead](https://etherscan.io/address/0x000000000000000000000000000000000000dead))


#### using 0xknown as burn address (like [0x000000000000000000000000000000000000dead](https://etherscan.io/address/0x000000000000000000000000000000000000dead))

This method instead would see that the token would be burn to `0xdead` address, a known burn address which no one has the keys for. Subsequently, in order to track the token, we will need to check the `ownerOf` token from the token registry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This method instead would see that the token would be burn to `0xdead` address, a known burn address which no one has the keys for. Subsequently, in order to track the token, we will need to check the `ownerOf` token from the token registry.
This method instead would see that the token would be burn to `0xdead` address, a known burn address that there is an extremely infinitesimal chance anyone has the keys to. Subsequently, in order to track the token, we will need to check the `ownerOf` token from the token registry.

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

Successfully merging this pull request may close these issues.

2 participants