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

Created ERC1155 ownable example/template #2807

Merged
merged 31 commits into from May 8, 2022
Merged

Conversation

Doc-Pixel
Copy link
Contributor

@Doc-Pixel Doc-Pixel commented Apr 17, 2022

What I did

I created the ERC1155 ownable template.
This template implements ownable, name / symbol / uri to be compatible with NFT exchanges like opensea.io
Full documentation in the vyper file.

How I did it

Used the latest Vyper 0.3.2 [edit: the master-0.3.1 to be precise] and gratefully made use of the new bytes4 and dynarray functionality, kept the ERC1155 documentation and the opensea 1155 standard handy

How to verify it

Full brownie test is included with this PR. test coverage 100%.
Please run and verify

Commit message

Please review, test and hopefully accept the ERC1155 ownable contract

Description for the changelog

New example ERC1155 ownable contract + test.

Cute Animal Picture

Happy Easter :-)

image

examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
tests/tokens/test_ERC1155ownable.py Outdated Show resolved Hide resolved
tests/tokens/test_ERC1155ownable.py Outdated Show resolved Hide resolved
tests/tokens/test_ERC1155ownable.py Outdated Show resolved Hide resolved
Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

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

Some updates to the interface

vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
vyper/builtin_interfaces/ERC1155.py Outdated Show resolved Hide resolved
@charles-cooper
Copy link
Member

i think until #2814 is figured out, we should remove the ERC1155.py interface file.

@fubuloubu
Copy link
Member

i think until #2814 is figured out, we should remove the ERC1155.py interface file.

@Doc-Pixel sorry for making this suggestion originally, we'll add it later.

examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
@Doc-Pixel
Copy link
Contributor Author

i think until #2814 is figured out, we should remove the ERC1155.py interface file.

@Doc-Pixel sorry for making this suggestion originally, we'll add it later.

No worries. I removed it from the PR as well. Part of the journey :-)

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2022

Codecov Report

Merging #2807 (4d97475) into master (37fd1e0) will decrease coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2807      +/-   ##
==========================================
- Coverage   87.52%   87.39%   -0.14%     
==========================================
  Files          94       94              
  Lines       10007    10080      +73     
  Branches     2480     2492      +12     
==========================================
+ Hits         8759     8809      +50     
- Misses        782      803      +21     
- Partials      466      468       +2     
Impacted Files Coverage Δ
vyper/builtin_functions/functions.py 89.28% <0.00%> (-1.48%) ⬇️
vyper/semantics/validation/module.py 85.71% <0.00%> (-0.89%) ⬇️
vyper/cli/vyper_json.py 79.02% <0.00%> (-0.77%) ⬇️
vyper/utils.py 86.82% <0.00%> (-0.45%) ⬇️
vyper/ir/compile_ir.py 94.60% <0.00%> (-0.11%) ⬇️
vyper/semantics/types/user/struct.py 83.11% <0.00%> (ø)
vyper/semantics/types/indexable/sequence.py 88.18% <0.00%> (ø)
vyper/ast/nodes.py 94.22% <0.00%> (+0.02%) ⬆️
vyper/semantics/validation/annotation.py 92.20% <0.00%> (+0.05%) ⬆️
vyper/semantics/validation/local.py 90.85% <0.00%> (+0.08%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37fd1e0...4d97475. Read the comment docs.

examples/tokens/ERC1155ownable.vy Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
tests/examples/tokens/test_ERC1155ownable.py.brownie Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155ownable_ETHtester.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155ownable_ETHtester.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155ownable_ETHtester.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155ownable_ETHtester.py Outdated Show resolved Hide resolved
@Doc-Pixel
Copy link
Contributor Author

Updated version to 0.3.3 , no issues

examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
Doc-Pixel and others added 4 commits May 1, 2022 16:56
Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
tests/examples/tokens/test_erc1155.py Outdated Show resolved Hide resolved
@fubuloubu fubuloubu enabled auto-merge (squash) May 2, 2022 02:58
auto-merge was automatically disabled May 3, 2022 21:47

Head branch was pushed to by a user without write access

@fubuloubu fubuloubu enabled auto-merge (squash) May 4, 2022 05:10
auto-merge was automatically disabled May 4, 2022 15:48

Head branch was pushed to by a user without write access

examples/tokens/ERC1155ownable.vy Show resolved Hide resolved
examples/tokens/ERC1155ownable.vy Outdated Show resolved Hide resolved
@Doc-Pixel
Copy link
Contributor Author

Think we're at the end of the road now?

I'm going to deploy this to polygon testnet and furthee bud my dApp, to testdrive it

@fubuloubu
Copy link
Member

Think we're at the end of the road now?

I'm going to deploy this to polygon testnet and furthee bud my dApp, to testdrive it

I'm going to merge it, but feel free to update it later with any lessons learned you've gotten from deploying it

@fubuloubu fubuloubu merged commit 18b9083 into vyperlang:master May 8, 2022
tserg added a commit to tserg/vyper that referenced this pull request May 13, 2022
* Created ERC1155 ownable example/template

* Applied most of proposed changes

* Applied more of the feedback.

* ERC165 fix

* Applied @view/@pure, updated test to accomodate

* Moved tests into the correct folder

* added interface ERC1155.py

* refactor: clean up built-in interface

* rewrote tests to use eth-tester

* cleaned up brownie tests in favor of eth-tester

* Update examples/tokens/ERC1155ownable.vy

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

* applied proposed changes in contract and tests

* interface fix / callback bytes constant

* Ownership test assertion to ZERO_ADDRESS

* reworked uri part, simplified tests

* Fixed all testscript issues

* refactor: apply suggestions from code review

* Update examples/tokens/ERC1155ownable.vy

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

* Update examples/tokens/ERC1155ownable.vy

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

* applied codebase shrink tips, adjusted tests

* cleaning up commented out code

* refactor: flip orientation of `balanceOf`

* test: update tests for switch of args to `balanceOf`

* pull, isort, black, push

* Resolved linter issues

* Linter fixes for ERC1155 test script

* Fixed ownership check in safeTransferFrom

* Fixed setURI permissions, updated to docstrings

* Minor test update

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
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.

None yet

4 participants