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

Refactor/Optimize Contract for multi series minting #4

Open
caseywescott opened this issue Aug 26, 2024 · 8 comments
Open

Refactor/Optimize Contract for multi series minting #4

caseywescott opened this issue Aug 26, 2024 · 8 comments
Assignees
Labels
ODHack7 Only Dust Hackathon

Comments

@caseywescott
Copy link
Collaborator

caseywescott commented Aug 26, 2024

**THIS LINKS TO THE NEW UPDATED SPEC:

#4 (comment)**

This contract could be further optimized and better aligned with common practices for NFT series smart contracts.

Typically, NFT series contracts are structured in a way that encapsulates series and artist information in a more cohesive and organized manner. This often includes the use of a Series struct that not only contains metadata about the series and the artist but also manages the NFTs associated with that series.

A More Common Structure for NFT Series Contracts:

Series Struct:

A Series struct would encapsulate all data related to a series, including the series metadata, artist information, and a mapping of NFTs belonging to that series.

Example:

struct Series {
name: ByteArray,
description: ByteArray,
artist: ArtistMetadata,
base_uri: felt252,
token_ids: LegacyMap<u256, TokenData>, // Map of token IDs to their data -
}

Artist Struct:

The ArtistMetadata can be part of the Series struct or managed separately if there is a need for more complex relationships between artists and series. (I'm open to something different here if it makes more sense)

Example:

struct ArtistMetadata {
name: ByteArray,
bio: ByteArray,
wallet_address: ContractAddress,
}

Token Data:

Instead of just mapping a token ID to an owner, a TokenData struct could be used to encapsulate all information related to an individual SVG NFT, including its ownership, metadata, and any other relevant properties.

Example:
(Have a think about this as it might need more thought)
struct TokenData {
owner: ContractAddress,
token_uri: ByteArray, // URI or direct SVG data
}

Mapping and Management:

The contract would maintain a mapping of series IDs to Series structs, where each Series struct internally manages its associated NFTs.

#[storage]
struct Storage {
_name: ByteArray,
_symbol: ByteArray,
_owner: ContractAddress,
_series_counter: u256,
_series_data: LegacyMap<u256, Series>, // Series now contains all token data
}

Minting Process:

During minting, the contract would add the NFT to the appropriate series by updating the token_ids mapping within the relevant Series struct.

Rough Mint Function Idea:

fn mint(ref self: ContractState, to: ContractAddress, series_id: u256, token_uri: felt252) {
assert(!to.is_zero(), 'ERC721: invalid receiver');
let token_id: u256 = self._series_counter.read() + 1.into();
assert(!self._exists(token_id), 'ERC721: token already minted');

// Update token data
let new_token = TokenData { owner: to, token_uri };
self._token_data.write(token_id, new_token);

// Add token to the series
let mut series = self._series_data.read(series_id);
series.token_ids.write(token_id, new_token);
self._series_data.write(series_id, series);

// Emit event
self.emit(Event::Transfer(Transfer { from: Zeroable::zero(), to, token_id }));

}

This is rough so please reach out to me to discuss

@od-hunter
Copy link

Hi @caseywescott , can I work on this ASAP? I can’t see where to apply via Onlydust.

Copy link

onlydustapp bot commented Aug 26, 2024

Hey @od-hunter!
Thanks for showing interest.
We've created an application for you to contribute to Nicera Labs.
Go check it out on OnlyDust!

@ShantelPeters
Copy link

Hi @caseywescott can i please be assigned to this issue

Copy link

onlydustapp bot commented Aug 26, 2024

Hey @ShantelPeters!
Thanks for showing interest.
We've created an application for you to contribute to Nicera Labs.
Go check it out on OnlyDust!

@caseywescott
Copy link
Collaborator Author

Title: Refactor to Use ERC721Component for Series Management

Description: We need to refactor the current implementation of our NFT series smart contract to better align with common practices for ERC721 contracts.

Specifically, we want to transition to using an ERC721Component within the Series struct. This refactoring will make the contract more modular, maintainable, and scalable by reusing standard ERC721 functionality while incorporating additional series-specific metadata.

Current Implementation:

The current contract uses a custom Series struct that manages its own mapping of token IDs to TokenData.
The TokenData struct is custom and includes information like the token owner and URI.
Series and artist metadata are managed separately.

Refactored Design:

Series Struct:

Integrate the ERC721Component as a substorage within the Series struct.
This component will handle all standard ERC721 functionality, such as token ownership, transfers, and approvals.
Example:

mod Series {
    use openzeppelin::token::erc721::ERC721Component;

    #[storage]
    struct Storage {
        name: ByteArray,
        description: ByteArray,
        artist: ArtistMetadata,
        base_uri: felt252,
        #[substorage(v0)]
        erc721: ERC721Component::Storage,  // Substorage for ERC721 functionality
    }
}


struct ArtistMetadata {
    name: ByteArray,
    bio: ByteArray,
    wallet_address: ContractAddress,
}

Token Management:

The ERC721Component will manage token ownership and metadata through its standard methods.
Each Series will act as a mini-ERC721 contract, with its own set of NFTs managed through the ERC721Component.

Remove Custom TokenData Struct and Mapping:

The TokenData struct and the token_ids mapping are no longer needed because the ERC721Component will handle all token-related data.

Remove the following:

struct TokenData {
    owner: ContractAddress,
    token_uri: ByteArray,
}

Remove the following from Series Struct:

token_ids: LegacyMap<u256, TokenData>,
Minting Process:

Update the minting logic to leverage the ERC721Component for creating and managing new tokens within a series.
Example Mint Function:

impl Series {
    pub fn mint(&mut self, to: ContractAddress, token_uri: ByteArray) {
        let token_id: u256 = self.erc721._balances.size() + 1.into();
        self.erc721._owners.write(token_id, to);
        self.erc721._balances.write(to, self.erc721.balance_of(to) + 1.into());
        self.erc721._token_uri.write(token_id, token_uri);

        // Emit transfer event (omitted for brevity)
    }

Storage Struct Adjustments:

  • The Storage struct in the contract’s main module will remain mostly the same, but now the Series struct will encapsulate its own ERC721 logic.

Updated Storage Struct:

Tasks:

  • Refactor the Series struct to integrate the ERC721Component.
  • Remove the custom TokenData mapping and related logic from the current implementation.
  • Update the minting function to use the ERC721Component for token management.
  • Ensure that series-specific metadata, such as description and artist, remains accessible and well-integrated with the new design.
  • Update the create_series function to reflect the structural changes
  • Test the refactored contract to ensure that all ERC721 functionality works as expected within each series.

@od-hunter od-hunter removed their assignment Aug 31, 2024
@mubarak23
Copy link

Title: Refactor to Use ERC721Component for Series Management

Description: We need to refactor the current implementation of our NFT series smart contract to better align with common practices for ERC721 contracts.

Specifically, we want to transition to using an ERC721Component within the Series struct. This refactoring will make the contract more modular, maintainable, and scalable by reusing standard ERC721 functionality while incorporating additional series-specific metadata.

Current Implementation:

The current contract uses a custom Series struct that manages its own mapping of token IDs to TokenData. The TokenData struct is custom and includes information like the token owner and URI. Series and artist metadata are managed separately.

Refactored Design:

Series Struct:

Integrate the ERC721Component as a substorage within the Series struct. This component will handle all standard ERC721 functionality, such as token ownership, transfers, and approvals. Example:

mod Series {
    use openzeppelin::token::erc721::ERC721Component;

    #[storage]
    struct Storage {
        name: ByteArray,
        description: ByteArray,
        artist: ArtistMetadata,
        base_uri: felt252,
        #[substorage(v0)]
        erc721: ERC721Component::Storage,  // Substorage for ERC721 functionality
    }
}


struct ArtistMetadata {
    name: ByteArray,
    bio: ByteArray,
    wallet_address: ContractAddress,
}

Token Management:

The ERC721Component will manage token ownership and metadata through its standard methods. Each Series will act as a mini-ERC721 contract, with its own set of NFTs managed through the ERC721Component.

Remove Custom TokenData Struct and Mapping:

The TokenData struct and the token_ids mapping are no longer needed because the ERC721Component will handle all token-related data.

Remove the following:

struct TokenData {
    owner: ContractAddress,
    token_uri: ByteArray,
}

Remove the following from Series Struct:

token_ids: LegacyMap<u256, TokenData>, Minting Process:

Update the minting logic to leverage the ERC721Component for creating and managing new tokens within a series. Example Mint Function:

impl Series {
    pub fn mint(&mut self, to: ContractAddress, token_uri: ByteArray) {
        let token_id: u256 = self.erc721._balances.size() + 1.into();
        self.erc721._owners.write(token_id, to);
        self.erc721._balances.write(to, self.erc721.balance_of(to) + 1.into());
        self.erc721._token_uri.write(token_id, token_uri);

        // Emit transfer event (omitted for brevity)
    }

Storage Struct Adjustments:

  • The Storage struct in the contract’s main module will remain mostly the same, but now the Series struct will encapsulate its own ERC721 logic.

Updated Storage Struct:

Tasks:

  • Refactor the Series struct to integrate the ERC721Component.
  • Remove the custom TokenData mapping and related logic from the current implementation.
  • Update the minting function to use the ERC721Component for token management.
  • Ensure that series-specific metadata, such as description and artist, remains accessible and well-integrated with the new design.
  • Update the create_series function to reflect the structural changes
  • Test the refactored contract to ensure that all ERC721 functionality works as expected within each series.

I understand what we are trying to achieved here,
Kindly assign @caseywescott

@caseywescott caseywescott added the ODHack7 Only Dust Hackathon label Aug 31, 2024
@mubarak23
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a Cairo smart contract developer with experience working on projects such as Just Art Peace, Dojo, Kart, TBA, and Shinigami. Before transitioning to Cairo development, I was a backend developer specializing in Rust.

My recent work with cairo starknet

How I plan on tackling this issue

implement the series management using openzeppelin ERC721Component

Copy link

onlydustapp bot commented Aug 31, 2024

The maintainer caseywescott has assigned mubarak23 to this issue via OnlyDust Platform.
Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ODHack7 Only Dust Hackathon
Projects
None yet
Development

No branches or pull requests

5 participants
@mubarak23 @caseywescott @od-hunter @ShantelPeters and others