All protocol buffer method arguments should be _arguments and all results should be _result. Also, we should add each of these standards to https://github.com/koinos/koinos-proto. I propose we put them in contracts/standard/kcs-*.proto. I did not completely review all of these refactors, simply normalize them so they can be reviewed individually.

KCS-2 has events listed in a different way. We should probably indicate if standards are supersets of other standards as well.

Perhaps we can add fields in the markdown header?

@joticajulian @jredbeard The events are not clearly defined on KCS-2, we should have them clearly defined on both KCS-2 and this standard (KCS-5) as well.

Can we use KCS-1 and KCS-3 as a standard way to define events and add them on both of these?

are you referring to the name of the event? or the protobuffer structure?

as far as I understand kollection.app expects to find collections.* in the name of the event to be able to track NFT changes. Maybe we should continue using them to not introduce breaking changes.

With regarding to protobuffers they use the same structure as the arguments.

are you referring to the name of the event? or the protobuffer structure?
as far as I understand kollection.app expects to find collections.* in the name of the event to be able to track NFT changes. Maybe we should continue using them to not introduce breaking changes.

@sgerbino @joticajulian can confirm yes, Kollection looks through receipts for these specific event names on collection contracts:

  collections_royalties: "collections.royalties_event",
  collection_owner: "collections.owner_event",
  token_mint: "collections.mint_event",
  token_transfer: "collections.transfer_event",

I'm fine with adding this to the KCS-2 standard - that seems to make sense. Collections should emit these events. I think it makes sense to have events on all of the standards actually... it makes it easy to track things.

In general, it's great to have things like KCS-5 has where you can get all NFT's minted in a collection using an endpoint - that's useful, however, I wouldn't want to forego having events where I can follow the chain for changes. I'd much rather be able to make an API call every 3 seconds to get a new block rather than spamming the endpoint of a contract every time someone requests data about a collection. It's much fewer requests and much more efficient this way. It's not a big deal for small size websites, but, for large ones, they will want to store information in a database and serve it from there rather than relying on calls directly to nodes.

I added these extra functions to simplify the work of the developers. In this way, they will not have to setup an API to track changes in the ownership of tokens.
At the same time as I discussed with you last year, I would like to introduce the possibility of storing the metadata onchain. The purpose of this feature is the same one: Improve the development experience as much as possible. In this way, the developer will not have to setup an API to resolve the metadata of the tokens. Of course this is optional, they can continue providing an API and storing the metadata offchain.
It would be good if you can adapt Kollection.app for this. The process is the following: If the collection has an uri defined then the metadata is stored offchain. If the uri is undefined then the metadata is stored onchain. You can use the event of the set_metadata function to track changes there.

I added these extra functions to simplify the work of the developers. In this way, they will not have to setup an API to track changes in the ownership of tokens. At the same time as I discussed with you last year, I would like to introduce the possibility of storing the metadata onchain. The purpose of this feature is the same one: Improve the development experience as much as possible. In this way, the developer will not have to setup an API to resolve the metadata of the tokens.

Yes being able to get all NFT's from a collection is very useful, but, if it's required and you cannot get updates via events it is a non-starter. Calling a contract each and every time someone visits a page does not scale, and no large scale businesses will take us seriously if we don't have events for tracking changes. It's a vast difference in being able to simply get every block and follow the chain vs making a call to a node for every page visit.

I like the idea of optionally storing metadata on-chain for NFT's - this is totally fine and I don't have a problem eventually updating Kollection to support this. It's probably worth noting that developers don't need an API to serve metadata, most just upload the metadata json files to IPFS - but it is useful to only have to worry about image hosting rather than storing metadata in IPFS - if people want to spend their mana on that, that's totally fine. It's also more immutable than IPFS which is kinda cool - the metadata would live on forever no matter what which to me makes an NFT more valuable.

Sure, I'm not trying to get rid off the events by adding these functions. The events still exist and anyone can track changes with them.
This is good for small and large businesses: Small businesses can just query the contract to get the list of tokens, they don't have to worry about extra APIs. And large businesses can optimize this by having their own API that tracks the events.

Sure, I'm not trying to get rid off the events by adding these functions. The events still exist and anyone can track changes with them. This is good for small and large businesses: Small businesses can just query the contract to get the list of tokens, they don't have to worry about extra APIs. And large businesses can optimize this by having their own API that tracks the events.

Ok sounds good then. Same page.

Please add protobuf definitions and events on both KCS-2/KCS-5 that match the current standard when you can, you can merge into this PR branch.

Blocked on events in the standard from @joticajulian and/or @jredbeard

done. I added events for KCS-5

done. I added events for KCS-5

@joticajulian is your intention to make the events incompatible with KCS-2? I'm seeing some discrepancies in both the owner_event and burn_event.

For reference I'm looking at the collection base implementation here:
https://github.com/kollection-nft/collection-base

no. It's not my intention. Could you elaborate more in the discrenpancies? to update them.

no. It's not my intention. Could you elaborate more in the discrenpancies? to update them.

Owner event

Your owner event:

// Event
message owner_event {
   bytes value = 1 [(koinos.btype) = ADDRESS];
}

His owner event:

message owner_event {
   bytes from = 1 [(koinos.btype) = ADDRESS];
   bytes to = 2 [(koinos.btype) = ADDRESS];
}

Burn event

Your burn event:

// Event
message burn_event {
   bytes token_id = 1 [(koinos.btype) = HEX];
}

His burn event:

message burn_event {
   bytes from = 1 [(koinos.btype) = ADDRESS];
   bytes token_id = 2 [(koinos.btype) = HEX];
}

These are not backwards compatible.

thanks. Updated.

I have noticed that we are a little loose with some of our wording. For example, when emitting events we say the event should be emitted. It should have certain impacted accounts. Is, "should" the correct word here. Many standards add the following pre-amble. Should we add this pre-amble and update the wording of the standard accordingly?

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED",  "MAY", and  "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).

@joticajulian Did you see Michael's last comment? Can we address this so this issue can be merged?

I'm not expert with the wording, but if you consider we should update it for me it's ok.
What should be the change? use "must" instead of "should"?