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.

Sign up for GitHub

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

Jump to bottom

Marketplace contract implementation #4

Merged
merged 33 commits into from Jan 12, 2023
Merged

Marketplace contract implementation #4

merged 33 commits into from Jan 12, 2023

Conversation

bobo-k2
Copy link
Collaborator

@bobo-k2 bobo-k2 commented Jan 2, 2023

This is PR for initial implementation of the NFT marketplace contract with the following features:

  • factory - creates a new instance of Shiden34 contract and registers the instance to the Marketplace. Caller needs to provide link to IPFS metadata file for NFT and some NFT details. Prerequisite for the factory is that Shiden34 contract is deployed to a network and the contract hash is provided to the Marketplace via set_nft_contract_hash call.
  • register - registers an existing contract to the Marketplace. This operation can be performed by contract or marketplace owner.
  • list - Lists a NFT for sale on the marketplace. This operation can be performed by a token owner.
  • unlist - Un-lists a NFT from the marketplace. This operation can be performed by a token owner.
  • buy - Transfers a token ownership to the caller. Also pays royalties and marketplace fees predefined receivers.

bobo-k2 and others added 25 commits December 14, 2022 11:47
@bobo-k2
Set caller
7d678eb
@bobo-k2
Set contract metadata method
3274e1b
@bobo-k2
Merge branch 'main' into feat/contract-implementation
e783918
@bobo-k2
Set marketplace fee test
b6d1875
@bobo-k2
List method
3f0ddd4
@bobo-k2
List method implementation
0826a02
@Maar-io
added test psp34 contract
77b567b
@bobo-k2
Get price method
5fc3880
@bobo-k2
Get and set marketplace fee recepient
64534eb
@bobo-k2
Register
d1e1fbd
@bobo-k2
Buy implemented
24292df
@bobo-k2
Transfer token and set token marketplace ipfs
0a4c229
@bobo-k2
Inegration test fix to handle OpenBrush String
92c76a4
@bobo-k2
Reentrancy guard
7bb75f9
@bobo-k2
Set NFT contract hash implementation
b136509
@bobo-k2
Factory method parameters
f220d86
@bobo-k2
Try to reference shiden34
7f8c0cc
@bobo-k2
Fixed ref to Shiden34 contract
d0810ca
@bobo-k2
Fixed compilatio error by adding default-fatures = false
72a3948
@bobo-k2
Factory implementation
aba96b0
@bobo-k2
Check if buyer is already owner
aab3433
@bobo-k2
Ensure macro
9efd7fb
@bobo-k2
Factory test
12fe9c3
@bobo-k2
Added missing parameters to factory method
9f1c1ca
@bobo-k2
Tests for set_nft_contract_hash
a972e04
@bobo-k2 bobo-k2 marked this pull request as ready for review January 2, 2023 11:22
PierreOssun
PierreOssun reviewed Jan 3, 2023
View reviewed changes
contracts/marketplace/Cargo.toml Outdated Show resolved Hide resolved
contracts/psp34/lib.rs Outdated Show resolved Hide resolved
contracts/psp34/lib.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/types.rs Show resolved Hide resolved
contracts/psp34/lib.rs Outdated Show resolved Hide resolved
PierreOssun
PierreOssun reviewed Jan 3, 2023
View reviewed changes
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
@bobo-k2 bobo-k2 mentioned this pull request Jan 3, 2023
Closed
PierreOssun
PierreOssun reviewed Jan 4, 2023
View reviewed changes
Copy link

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

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

You should run cargo clippy and checks the hints to make your code mode clean/idiomatic

logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
PierreOssun
PierreOssun approved these changes Jan 4, 2023
View reviewed changes
Copy link

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

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

LGTM

@ganesh1997oli
Copy link

Is contract_hash in factory same as contract_address in register's Marketplace contract implementation? I mean are they both Shiden34 contract hash?

@ganesh1997oli
Copy link

ganesh1997oli commented Jan 5, 2023 •
edited

while fee calculating why divide by 10_000?

@bobo-k2
Copy link
Collaborator Author

bobo-k2 commented Jan 5, 2023

Is contract_hash in factory same as contract_address in register's Marketplace contract implementation? I mean are they both Shiden34 contract hash?

Those two are not the same, the first one is contact hash, the second one is the deployed contract instance address.
Factory method will create a new instance of Shiden34 contract based on a hash of a contract already deployed on a network. Which means you can create your own NFT just by providing metadata.
Via register method you can register already instantiated Shiden34 contract by providing the contract address. Let's say you already deployed your contract and created NFTs and just want to register it to the marketplace.

@bobo-k2
Copy link
Collaborator Author

bobo-k2 commented Jan 5, 2023

while fee calculating why divide by 10_000?

fee is percentage represented by integer value 100 is 1%, while 10_000 is 100%. If we want to calculate 10% fee we need to do the following 1_000 / 10_000

@Maar-io
Copy link
Member

Maar-io commented Jan 5, 2023

Add comment for each function in traits and impl

@bobo-k2
Copy link
Collaborator Author

bobo-k2 commented Jan 5, 2023

Add comment for each function in traits and impl

Why to have the same comments on both places? This will be hard to maintain.

Maar-io
Maar-io requested changes Jan 9, 2023
View reviewed changes
logics/impls/marketplace/marketplace_sale.rs Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
logics/impls/marketplace/marketplace_sale.rs Outdated Show resolved Hide resolved
Maar-io
Maar-io reviewed Jan 9, 2023
View reviewed changes
logics/traits/marketplace.rs Outdated
@@ -45,7 +45,7 @@ pub trait MarketplaceSale {
price: Balance,
) -> Result<(), MarketplaceError>;

/// Removes NFT from the marketplace sale.
/// Removes aa NFT from the marketplace sale.
Copy link
Member

Choose a reason for hiding this comment

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

aa

Maar-io
Maar-io approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@Maar-io Maar-io left a comment

Choose a reason for hiding this comment

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

LGTM

@bobo-k2 bobo-k2 merged commit 921f6ed into main Jan 12, 2023
@bobo-k2 bobo-k2 deleted the feat/contract-implementation branch January 25, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Maar-io Maar-io Maar-io approved these changes

@PierreOssun PierreOssun Awaiting requested review from PierreOssun

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bobo-k2 @ganesh1997oli @Maar-io @PierreOssun