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

Adding element nft #128

Merged
merged 6 commits into from
Feb 13, 2023
Merged

Conversation

rstevens2022
Copy link
Contributor

A few changes since the last time I opened this

Have table defs for all three of these contracts (ERC115, ERC721 old, ERC721 new) with the main Element exchange being the contract address in the files (all three are identified as proxies on Polygonscan)

@TimNooren
Copy link
Collaborator

@rstevens2022 LGTM, but just wanted to check whether Old and New is the right terminology. If that's what is used by the project itself then I guess that's fine, otherwise maybe something like V1/V2 would be more future-proof.

@rstevens2022
Copy link
Contributor Author

Hi Tim!

The reason I went with "old" vs "new" is because I don't see any references to versioning in the Element docs (I actually don't see a reference to 0x205 at all even though it does seem to be an old implementation) - I just wasn't sure if it was a V1 versus V2 thing or a V1.1 versus V1.2 thing and I didn't want to label incorrectly - but I can definitely just switch to V1 and V2 for sake of clarity and to leave room for the situation where a newer version is released

@TimNooren
Copy link
Collaborator

The reason I went with "old" vs "new" is because I don't see any references to versioning in the Element docs (I actually don't see a reference to 0x205 at all even though it does seem to be an old implementation) - I just wasn't sure if it was a V1 versus V2 thing or a V1.1 versus V1.2 thing and I didn't want to label incorrectly - but I can definitely just switch to V1 and V2 for sake of clarity and to leave room for the situation where a newer version is released

https://element.market/polygon seems to reference 2.0 quite prominently. Maybe this proxy upgrade is somehow unrelated, but otherwise V1/V2 seems like a safe choice.

@rstevens2022
Copy link
Contributor Author

got it! updated!

@TimNooren TimNooren merged commit 48ea0d5 into blockchain-etl:main Feb 13, 2023
@TimNooren TimNooren mentioned this pull request Feb 13, 2023
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