Add NFT launchpad template (contract only) #26

0xaptosj · 2024-06-03T14:21:36Z

A launchpad for contract deployer to create collections and anyone to mint NFTs.

frontend is a dummy repo, will create separate PR for frontend.

Only file needs review is the contract /templates/nft-template/move/sources/nft_launchpad.move

NOTE
Unlike ez-launch where nft is pre minted, NFTs in this contract are created on the fly when users call mint_nft function. The main reason is on the collection creation UI, we want to support creator only sign 1 tx (excludes the tx that pays arweave), ez-launch expects creator to call pre_mint_nft multiple times after creating the collection, that's something we want to avoid.

THIS IS A TEMPLATE, we should not treat it as a PRODUCT.

0xaptosj · 2024-06-03T14:29:59Z

will remove sh_scripts later after i can use cli in ts

0xaptosj · 2024-06-03T14:30:13Z

Will add more unit test later

0xaptosj · 2024-06-04T12:16:32Z

templates/nft-template/move/sources/nft_launchpad.move

+        }
+    }
+
+    fun configure_collection_and_token_properties(


We can probably remove configure_collection_and_token_properties as it's not commonly used, if developer needs it tey can add it back themselves. Thoughts?

BriungRi

Left some comments. Only looked at nft_launchpad.

templates/nft-template/move/sources/nft_launchpad.move

BriungRi · 2024-06-04T14:29:29Z

templates/nft-template/move/sources/nft_launchpad.move

+    /// No active mint stages
+    const E_NO_ACTIVE_STAGES: u64 = 7;
+
+    const ALLOWLIST_MINT_STAGE_CATEGORY: vector<u8> = b"Allowlist mint stage";


Is it worth generalizing this into a vector?

When length(&stages) == 1, there is only one stage
When length(&stages) == 2, there are two stages
etc.

Then, you can also do something like mint_fees that can be "zipped" with stages to represent the mint_fee at each stage.

Can even have a stage with MAX_U64 (or option::none?) mint_price to signify that minting is not allowed at this stage

I'm also worried that if you change the literal here, it will break existing NFT launches that are trying to look up the stage in the simple map

I'm also worried that if you change the literal here, it will break existing NFT launches that are trying to look up the stage in the simple map

This contract is meant to be instantiated for each launchpad, not to be used as an sdk, so I don't think updating it will break existing collections.

For deployed contract, changing value of constants is incompatible. (this is wrong)

For deployed contract, changing value of constants is incompatible.

Is this true? I thought you can

we can change constants during upgrade

ah i probably changed something else that triggered the incompatible error

Good to loop in @JohnChangUK here since this is more likely feedback for better devex with token-minter

Ideally enums would be good to use here. I don't have a strong preference here between using Strings vs numbers for differentiation, and think devex is somewhat similar. If you think using numbers vs Strings is better devex, let me know!

BriungRi · 2024-06-04T14:32:18Z

templates/nft-template/move/sources/nft_launchpad.move

+        mint_fee_per_nft_by_stages: simple_map::SimpleMap<string::String, u64>,
+        collection_owner_obj: object::Object<CollectionOwnerObjConfig>,
+        /// Monotonic increasing ID, Starts from 1
+        next_nft_id: u64,


can't you make use of token::mint_numbered_token?

didn't find this function, but i used collection::count(collection_obj) instead.

https://github.com/aptos-labs/aptos-core/blob/beab1a543f86d0196208a59dd58832f1b040523d/aptos-move/framework/aptos-token-objects/sources/token.move#L265

or create_numbered_token

BriungRi · 2024-06-04T14:35:47Z

templates/nft-template/move/sources/nft_launchpad.move

+
+    // If you deploy the module under an object, sender is the object's signer
+    // If you deploy the moduelr under your own account, sender is your account's signer
+    fun init_module(sender: &signer) {


nit: I like calling sender -> deployer since it will always be the deployer here.

templates/nft-template/move/sources/nft_launchpad.move

BriungRi · 2024-06-04T14:45:23Z

templates/nft-template/move/sources/nft_launchpad.move

+        public_mint_start_time: option::Option<u64>,
+        public_mint_end_time: option::Option<u64>,
+        public_mint_limit_per_addr: option::Option<u64>,
+        public_mint_fee_per_nft: option::Option<u64>,


prefer to import Option so we don't need to option::Option

i want to keep import consistent, so either import everything to lowest level or everything at high level 🫠

IMO I think better to optimize for code readability in these sections over import consistency :)

BriungRi · 2024-06-04T14:52:08Z

templates/nft-template/move/sources/nft_launchpad.move

+            );
+
+            for (i in 0..vector::length(&allowlist)) {
+                mint_stage::add_to_allowlist(


@JohnChangUK I feel iffy about this because this is encouraging vector allowlists which could grow long (and cause a tx to run out of gas). Also, would it be more performant to allow passing in a vector (e.g. pass in a copy of the entire vector and store it)?

Ideally, we support a Merkle version of this at some point.

support a Merkle version of this at some point.

I expect token-minter to support that so i can just call it in the template.

allowlist is implemented as a smart table in token_minter. (https://github.com/aptos-labs/token-minter/blob/main/token-minter/sources/modules/mint_stage.move#L56)

From an API design standpoint I don't think that coupling collection creation and mint stage is a good idea. Why not create the collection first and separately manage stages / allowlists? You can use the sdk to chain APIs as necessary but the move side should be as modular as possible. The current design doesn't allow creators to add people to allowlist or modify the stages.

+1, I think separating the transactions into:

Create the collection itself

Create each stage

is a good idea because:

It makes each function easier to read

Less likely to run out of gas

Each function has fewer arguments

Plus, someone can always write a higher-level function that combines it

Yes agreed, collection creation and adding mint stages, coin payments are all done separately as shown here.

We can look into supporting Merkle white lists too, but for now using vectors I think is fine

BriungRi · 2024-06-04T14:52:49Z

templates/nft-template/move/sources/nft_launchpad.move

+                    collection_obj,
+                    stage,
+                    *vector::borrow(&allowlist, i),
+                    *option::borrow(&allowlist_mint_limit_per_addr)


what if there is a different mint_limit for each address? Should we support this? (I give 5 to Alice, 3 to Bob etc.)

again for simplicity we don't support it

templates/nft-template/move/sources/nft_launchpad.move

0xaptosj · 2024-06-04T16:09:18Z

templates/nft-template/move/sources/nft_launchpad.move

+        let collection_owner_obj_signer = &object::generate_signer_for_extending(&collection_owner_config.extend_ref);
+
+        let next_nft_id = *option::borrow(&collection::count(collection_obj));
+        let nft_obj_constructor_ref = &token::create(


@BriungRi how do you feel about this part? I want to avoid creator pre mint (because we want to make creator only need to send 1 tx) and forbid users pass in random name.

Should premint just be another stage that only allowlists the creator?

Looping in @JohnChangUK because I want to discuss whether we should have a mint function that is imported from token-minter. That way, we can potentially have all the mint checks (stages, coin_payment) before the actual mint

I think the current approach is fine. Later we will add more modules which abstract the minting process, i.e. verify mint stages, payments are made and then mint the relevant token/NFT.

bowenyang007

Is there an overall spec sheet? One gap I see is that not all configs are mutable after creation. For example, mint stages, fee per stage, allowlist, etc. Usually creators are modifying these along the way. Also more often than not they're first managing the stages before even creating the collection (so that the collection data doesn't leak).

templates/nft-template/move/sources/nft_launchpad.move

bowenyang007 · 2024-06-04T16:43:40Z

templates/nft-template/move/sources/nft_launchpad.move

+    #[event]
+    struct CreateCollectionEvent has store, drop {


Do we not have a standard in TokenMinter for this @JohnChangUK? Custom event at this level means that the user has to have indexing to get the full benefit of the example contract. That's exactly what we're trying to avoid with the token standards.

I think there is a standard one in token minter. No strong opinion, but i usually create custom event just so it's more flexible. I can remove it if you feel strongly against it.

No event for collection creation, as collection creation is initially done via the framework. TokenMinter creates the peripherals for the collection only so far.

bowenyang007 · 2024-06-04T16:44:12Z

templates/nft-template/move/sources/nft_launchpad.move

+    }
+
+    #[event]
+    struct MintNftEvent has store, drop {


Ditto here. Seems like we're creating custom events that should be standard in TokenMinter.

bowenyang007 · 2024-06-04T16:49:35Z

templates/nft-template/move/sources/nft_launchpad.move

+    /// Unique per collection
+    struct CollectionConfig has key {
+        /// Key is stage, value is mint fee denomination
+        mint_fee_per_nft_by_stages: simple_map::SimpleMap<string::String, u64>,


We should be using the coin_payment module here:
https://github.com/aptos-labs/token-minter/blob/main/token-minter/sources/modules/coin_payment.move

I used that initially, but think it's an overkill here

@0xaptosj possible to give more specific feedback on why it's overkill? Could be useful feedback for how we can make coin_payment easier to use

bowenyang007 · 2024-06-04T16:52:52Z

templates/nft-template/move/sources/nft_launchpad.move

+        name: string::String,
+        uri: string::String,
+        pre_mint_amount: u64,
+        allowlist: option::Option<vector<address>>,


Probably not a good idea to output the allowlist here. It could be partial cuz it's only a snapshot at collection creation and it could be way too long.

bowenyang007 · 2024-06-04T16:57:47Z

templates/nft-template/move/sources/nft_launchpad.move

+            );
+
+            for (i in 0..vector::length(&allowlist)) {
+                mint_stage::add_to_allowlist(


allowlist is implemented as a smart table in token_minter. (https://github.com/aptos-labs/token-minter/blob/main/token-minter/sources/modules/mint_stage.move#L56)

From an API design standpoint I don't think that coupling collection creation and mint stage is a good idea. Why not create the collection first and separately manage stages / allowlists? You can use the sdk to chain APIs as necessary but the move side should be as modular as possible. The current design doesn't allow creators to add people to allowlist or modify the stages.

0xaptosj · 2024-06-04T17:03:44Z

@bowenyang007 this template is meant to be a boilerplate to learn move or modify for their own purpose instead of take it and launch it thing for developers.

With simplicty in mind, I put everything (create collection, set mint stage, set allowlist) in 1 function because it's easy to build UI (user only needs to send 1 tx).

I expect features like modifying allowlist to be added by developers themselves.

JohnChangUK · 2024-06-04T19:20:23Z

templates/nft-template/move/sources/nft_launchpad.move

+        allowlist_start_time: option::Option<u64>,
+        allowlist_end_time: option::Option<u64>,
+        allowlist_mint_limit_per_addr: option::Option<u64>,
+        allowlist_mint_fee_per_nft: option::Option<u64>,
+        public_mint_start_time: option::Option<u64>,
+        public_mint_end_time: option::Option<u64>,
+        public_mint_limit_per_addr: option::Option<u64>,
+        public_mint_fee_per_nft: option::Option<u64>,


Would be nice to have this in a separate function. Too many args being passed in this function I think

bowenyang007

The feedback holds but aligned that we don't need to worry about this for the hackathon. Accept to unblock.

0xaptosj · 2024-06-05T00:57:33Z

The feedback holds but aligned that we don't need to worry about this for the hackathon. Accept to unblock.

Thanks, I started this contract from the idea of simplifying ez_launch_with_stages.move, if we want to have separate endpoints we would go with original ez_launch_with_stages.

0xaptosj added 17 commits May 28, 2024 21:46

wip

80f4846

nit

1b2d40c

wip

6321ca7

finish initial version of nft template

6ba44aa

nit

4a0a104

store fa obj in registry

be41f19

fix naming

fbf8c06

fixed contract

d0adcdd

redeploy

b9a3ae9

ui working now with hardcode collection params

1079d22

wip

0ec7afe

rename

4ee579d

remove vite frontend for now

1e53696

remove next app

b538102

add back vite

e7852a0

fix workflow

863ac4a

nit

1cc04cc

0xaptosj requested review from 0xmaayan, GotenJBZ, JohnChangUK, BriungRi and jillxuu June 3, 2024 14:32

0xaptosj added 3 commits June 3, 2024 22:34

update github workflow

05fdf18

nit

bb93d68

fix

2780119

0xaptosj changed the title ~~Add NFT launchpad template~~ Add NFT launchpad template (contract only) Jun 3, 2024

0xaptosj added 2 commits June 4, 2024 00:53

add pending admin and use simple map for small map

6e50de2

redeploy

b86e628

0xaptosj commented Jun 4, 2024

View reviewed changes

BriungRi reviewed Jun 4, 2024

View reviewed changes

0xaptosj added 4 commits June 4, 2024 23:34

fix typo, no logic changed

e3e9463

use collection::count to get next_nft_id

68b0987

fix upgrade script

03ef900

remove configure_collection_and_token_properties for simplicity

5a50cd0

GotenJBZ reviewed Jun 4, 2024

View reviewed changes

templates/nft-template/move/sources/nft_launchpad.move Outdated Show resolved Hide resolved

0xaptosj added 2 commits June 4, 2024 23:53

fix typo

f97df1d

removed unused import

3eeeab0

0xaptosj commented Jun 4, 2024

View reviewed changes

bowenyang007 requested changes Jun 4, 2024

View reviewed changes

update error code naming convention and remove unused

f9783be

JohnChangUK reviewed Jun 4, 2024

View reviewed changes

bowenyang007 approved these changes Jun 4, 2024

View reviewed changes

update import convention

b1fdb01

0xaptosj merged commit fdb366c into main Jun 5, 2024
3 checks passed

0xaptosj deleted the j/nft-template branch June 5, 2024 15:55

Add NFT launchpad template (contract only) #26

Add NFT launchpad template (contract only) #26

Conversation

0xaptosj commented Jun 3, 2024 • edited Loading

0xaptosj commented Jun 3, 2024

0xaptosj commented Jun 3, 2024

Choose a reason for hiding this comment

BriungRi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xaptosj Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GotenJBZ Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BriungRi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

JohnChangUK Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xaptosj Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bowenyang007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xaptosj commented Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

bowenyang007 left a comment

Choose a reason for hiding this comment

0xaptosj commented Jun 5, 2024 • edited Loading

0xaptosj commented Jun 3, 2024 •

edited

Loading

0xaptosj Jun 4, 2024 •

edited

Loading

GotenJBZ Jun 4, 2024 •

edited

Loading

BriungRi Jun 4, 2024 •

edited

Loading

JohnChangUK Jun 4, 2024 •

edited

Loading

0xaptosj Jun 4, 2024 •

edited

Loading

0xaptosj commented Jun 4, 2024 •

edited

Loading

0xaptosj commented Jun 5, 2024 •

edited

Loading