koinos / koinos-contract-standards

A repository to propose, track, and discuss smart contract standards for the Koinos Blockchain.
3 stars 2 forks source link

KCS-1, KCS-3, KCS-4, and KCS-5 #7

Closed joticajulian closed 3 weeks ago

joticajulian commented 3 months ago

Here is a summary of the different standards:

KCS-1: Token standard without allowances (current KOIN and VHP contracts).

KCS-3: Token standard with allowances but without using the koinos authority system (current contracts in KoinCity).

KCS-4: Token standard that use allowances and the koinos authority system. This is achieved thanks to the new get_contract_metadata system call introduced after the gov proposal.

KCS-5: NFT standard with extended features (like storing metadata onchain). This one also take advantage of the new system call to support allowances and smart wallets.

mvandeberg commented 2 months ago

I think it is worth discussing adding memos to KCS-4. Technically, there is nothing stopping memos from being added to KCS-1 tokens, except the memos will not be included in the transfer event, which would be useful.

The rest of the PR is blocked by https://github.com/koinos/koinos-sdk-as/pull/74 requiring testing.

joticajulian commented 1 month ago

I just added memos to KCS-1 and KCS-3

mvandeberg commented 1 month ago

I must have missed that memos were already in KCS-4. They should not be in KCS-1 or 3 as those standards are already being used.

We should also add the events to the standards. Along with that, memos should be added to the transfer event for KCS-4.

Should memos also be in the args and events for mint and burn?

We should also note the memo field as optional. All fields in proto3 are technically optional, but adding the optional keyword generates useful methods in target libraries.

joticajulian commented 1 month ago

hmm ok. From your comment I thought you were requesting to add memos in KCS-1 as well. Then I will remove them from KCS-1 and KCS-3. With respect to memos in mint and burn this feature is not popular or requested so I would not include it.

sgerbino commented 3 weeks ago

Usurped by #8, #9, #12, and #13 so they can be reviewed and merged individually.