chore(p/grc721): Distinct Event Types for GRC721 Functions

[r3v4s]

Description

Current event(emit) code in p/grc721 really doesn't emits event.

Therefore, modified code to emit the events.

And similar to #2749, made event type for each function to be unique.

Contributors' checklist...

• Added new tests, or not needed, or not feasible
• Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
• Updated the official documentation or not needed
• No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
• Added references to related issues and PRs
• Provided any useful hints for running manual tests

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.77%. Comparing base (4f27a57) to head (36b767a).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3102      +/-   ##
==========================================
- Coverage   63.77%   63.77%   -0.01%     
==========================================
  Files         548      548              
  Lines       78681    78681              
==========================================
- Hits        50180    50177       -3     
- Misses      25117    25122       +5     
+ Partials     3384     3382       -2     

Flag	Coverage Δ
contribs/gnodev	61.16% <ø> (+0.62%)	⬆️
contribs/gnofaucet	14.82% <ø> (ø)
gno.land	73.62% <ø> (ø)
gnovm	67.92% <ø> (ø)
misc/genstd	79.72% <ø> (ø)
tm2	62.34% <ø> (-0.13%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zivkovicmilos]

Thank you for adding event emits to the contract. Looks good 💯

Can you add a test, even just a txtar so we can catch them being emitted? 🙏

[r3v4s]

txtar for all 5 events
36b767a

[leohhhn]

@r3v4s

The EIP states 3 event types, none of which are Mint/Burn events; these are all transfer events. Why did you choose this approach?

I have a vague memory we talked about this in the GRC20 package as well; Transfer should be able to cover all needed actions. What's the benefit of having distinct events for Mint/Burn?

[r3v4s]

@leohhhn I'm aware of EIP 3 event types, and do remember discussion we talked about GRC20.

And Yes, as you said Transfer does cover all (mint, burn, transfer) but current GRC20 impls unique type, I just followed it.

IMHO, distinct event name sounds more straightforward.

[dongwon8247]

@leohhhn As @notJoon described in #2749, we can reduce the number of RPC calls by retrieving only the specific event data we need (e.g., Mint or Burn). Otherwise, applications have to retrieve all Transfer events and filter out the necessary data.

[leohhhn]

@r3v4s @dongwon8247

Sounds good! I remember.

Do you think it would be good to emit the Transfer event as well, to follow the standard? Or should we deviate?

I'm okay doing either if this makes it easier for off-chain services.

[r3v4s]

@r3v4s @dongwon8247

Sounds good! I remember.

Do you think it would be good to emit the Transfer event as well, to follow the standard? Or should we deviate?

I'm okay doing either if this makes it easier for off-chain services.

For now, I prefer not to(for 2 reasons).

1. (AFAIK) std.Emit spends gas
2. If we're willing to add standard event too, it should be handle in another pr (that also includes grc20 to emit standard event)

[thehowl]

Note; we'll need to refactor these tests eventually so we test for the events emitted inside a _test.gno, but it kind of depends on the discussions in #1475