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

Support for ERC1155 #190

Closed
7 tasks
remiroyc opened this issue Nov 24, 2023 · 6 comments
Closed
7 tasks

Support for ERC1155 #190

remiroyc opened this issue Nov 24, 2023 · 6 comments
Assignees
Labels
good first issue Good for newcomers lang:cairo Work on the Starknet part of the application; you need to know Cairo lang. ODHack8 Only dust 8 priority: high

Comments

@remiroyc
Copy link
Contributor

remiroyc commented Nov 24, 2023

Add ERC-1155 Support to NFT Orderbook Smart Contract

Description

We need to extend our NFT orderbook smart contract to support the ERC-1155 standard in addition to the currently supported ERC-721. The current structure of our smart contract is already well-suited for this addition, requiring minimal changes to implement ERC-1155 support.

Current Status

  • The smart contract currently supports ERC-721 tokens.
  • The existing structure is nearly ready to accommodate ERC-1155.

Tasks

  1. Deploy an ERC-1155 token contract on a devnet for testing purposes.
  2. Add support for the ERC-1155 standard to the orderbook smart contract:
    • Implement dedicated functions for ERC-1155 interactions.
    • Modify existing logic to handle token quantities (the main difference from ERC-721).

Implementation Details

  • The primary change from the current ERC-721 support will be handling token quantities.
  • Existing functions and data structures should be reviewed and updated where necessary to account for token quantities.
  • New functions specific to ERC-1155 operations may need to be added.

Acceptance Criteria

  • ERC-1155 token contract successfully deployed on devnet
  • Orderbook smart contract updated to support ERC-1155 tokens
  • All existing ERC-721 functionality remains intact
  • New ERC-1155 specific functions are implemented and tested
  • Quantity handling is correctly implemented for ERC-1155 tokens
  • All tests pass, including new tests for ERC-1155 functionality
  • PR must be made with the branch https://github.com/ArkProjectNFTs/ark-project/tree/feat/contract-v2 as a base

Additional Notes

  • Ensure backwards compatibility with ERC-721 tokens
  • Update documentation to reflect new ERC-1155 support
  • Consider gas optimization when implementing quantity handling
@remiroyc remiroyc added difficulty: medium To resolve the issue, it is necessary to gain an understanding of some aspects of the codebase difficulty: hard To resolve the issue, it is necessary to comprehend the entire codebase. priority: medium app:pontos ArkProject NFT indexer lib lang:rust and removed difficulty: medium To resolve the issue, it is necessary to gain an understanding of some aspects of the codebase difficulty: hard To resolve the issue, it is necessary to comprehend the entire codebase. priority: medium labels Nov 24, 2023
@remiroyc remiroyc added the status: help wanted Extra attention is needed label Nov 24, 2023
@kwiss kwiss added ODHack8 Only dust 8 good first issue Good for newcomers lang:cairo Work on the Starknet part of the application; you need to know Cairo lang. priority: high and removed difficulty: medium To resolve the issue, it is necessary to gain an understanding of some aspects of the codebase status: help wanted Extra attention is needed priority: medium app:pontos ArkProject NFT indexer lib lang:rust labels Sep 25, 2024
@manlikeHB
Copy link

manlikeHB commented Sep 26, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi, I am Cairo developer with lots of experience contributing to Cairo projects, my OD profile is a witness to this. I've had the experience of extending smart contracts to support and interact with erc20, erc721 or erc-1155, so I believe this is within my comfort zone.

How I plan on tackling this issue

  • I will deploy an erc1155 token on testnet
  • I will add the support for erc1155 token to the orderbook smart contract by embedding an OZ erc1155 component and ensure it's interface is compatible with the standard erc1155 token.
  • Modify existing logic to handle token quantities.
  • I will write test covering all edge cases and ensure the new functionality behaves as expected.

@aji70
Copy link

aji70 commented Sep 26, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

i'm a solidity and cairo smart contract developer with over 2 years experience and belive i have the skill set for the task

@saimeunt
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have some experience dealing with ERC721/1155 in Cairo by implementing NFT related ERCs in Cairo:
https://github.com/carbonable-labs/cairo-erc-7496
https://github.com/carbonable-labs/cairo-erc-7498

How I plan on tackling this issue

I will carefully implement the plan described in the issue as it is already very detailed concerning what needs to be done.
I will incrementally develop the feature to match acceptance criteria and make sure all tests pass along the way. I will account for token quantities regarding ERC1155 specificities and take care of gas optimization by trying to leverage batch functions from ERC1155.

@kwiss
Copy link
Contributor

kwiss commented Sep 26, 2024

@manlikeHB you are assigned to this issue

@kwiss
Copy link
Contributor

kwiss commented Oct 1, 2024

just as a reminder you need to use this branch as a base #440 @manlikeHB

@manlikeHB manlikeHB mentioned this issue Oct 1, 2024
13 tasks
kwiss pushed a commit that referenced this issue Oct 10, 2024
## Description
This PR extends the existing NFT orderbook smart contract to add support
for the ERC-1155 token standard

- [x] ERC-1155 token contract successfully deployed on devnet
- [x] Orderbook smart contract updated to support ERC-1155 tokens
- [x] All existing ERC-721 functionality remains intact
- [x] New ERC-1155 specific functions are implemented and tested
- [x] Quantity handling is correctly implemented for ERC-1155 tokens
- [x] All tests pass, including new tests for ERC-1155 functionality
- [x] PR must be made with the branch
https://github.com/ArkProjectNFTs/ark-project/tree/feat/contract-v2 as a
base

<!--
Please do not leave this blank.
Describe the changes in this PR. What does it [add/remove/fix/replace]?

For crafting a good description, consider using ChatGPT to help
articulate your changes.
-->

## What type of PR is this? (check all applicable)

- [x] 🍕 Feature (`feat:`)
- [x] 🧑‍💻 Code Refactor (`refactor:`)
- [x] ✅ Test (`test:`)
- [x] 🚀 Breaking Changes (`BREAKING CHANGE:`)

## Related Tickets & Documents

<!--
Please use this format to link related issues: Fixes #<issue_number>
More info:
https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

## Added tests?

- [x] 👍 yes

## Added to documentation?

- [x] 📜 README.md

## Closing Issues
Closes #190
<!--
Use keywords to close related issues. This ensures that the associated
issues will automatically close when the PR is merged.

- `Fixes #123` will close issue 123 when the PR is merged.
- `Closes #123` will also close issue 123 when the PR is merged.

You can also use multiple keywords in one comment:
- `Fixes #123, Resolves #456`

More info:
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
-->
@ptisserand
Copy link
Contributor

Closed by #473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers lang:cairo Work on the Starknet part of the application; you need to know Cairo lang. ODHack8 Only dust 8 priority: high
Projects
None yet
Development

No branches or pull requests

6 participants