refactor(ecocredit): remove unnecessary fields in events

[technicallyty]

Description

Ref: #1035

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[codecov]

Codecov Report

Merging #1044 (224240b) into master (c160e04) will decrease coverage by 6.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1044      +/-   ##
==========================================
- Coverage   68.91%   62.88%   -6.03%     
==========================================
  Files         218      213       -5     
  Lines       21350    19426    -1924     
==========================================
- Hits        14713    12217    -2496     
- Misses       5288     5929     +641     
+ Partials     1349     1280      -69     

Flag	Coverage Δ
experimental-codecov	?
stable-codecov	62.88% <100.00%> (+0.01%)	⬆️

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

[ryanchristo]

Looking good although we might need to make adjustments for the basket changes because we are currently not bumping the basket proto version in this next release.

[ryanchristo]

We shouldn't be marking the event messages as deprecated, only the credits field within the events that have it as deprecated. We should also avoid DEPRECATED in all caps and follow the same format as the sdk. We don't need to return the credits in the events if they are deprecated and we can specify that in the deprecated comment and revert 4e607ee.

[ryanchristo]

This is a step in the right direction but the current changes only partially solve the issue. We are still returning a lot of unnecessary fields for events and we should further refine. I would be happy to do a pair-programming session on this to provide clarity on the proposed changes.

I also think it might be helpful to take a look at the group module and how minimal those events are. I do think it would be nice to include the address such as the admin, issuer, etc, which is not included in the group events, but not necessary because that information can be queried using the main identifier included in the event.

https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/group/v1/events.proto

[aleem1314]

lgtm

[ryanchristo]

I also think it might be helpful to take a look at the group module and how minimal those events are. I do think it would be nice to include the address such as the admin, issuer, etc, which is not included in the group events, but not necessary because that information can be queried using the main identifier included in the event.

https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/group/v1/events.proto

For the update events in core...

• EventClassAdminUpdated
• EventClassIssuersUpdated
• EventClassMetadataUpdated
• EventProjectAdminUpdated
• EventProjectMetadataUpdated

...we could reduce the fields to just the identifier that was updated, which is how it is done in the group module. To avoid dragging this out, I can approve and we can get these changes merged and then address my comments in a followup. I think this is a step in the right direction and would be happy to do pair-programming on the proposed changes if you would prefer to handle them in this pull request. Otherwise I think we should keep the issue open after merging these changes.

[technicallyty]

I also think it might be helpful to take a look at the group module and how minimal those events are. I do think it would be nice to include the address such as the admin, issuer, etc, which is not included in the group events, but not necessary because that information can be queried using the main identifier included in the event.
https://github.com/cosmos/cosmos-sdk/blob/main/proto/cosmos/group/v1/events.proto

For the update events in core...

• EventClassAdminUpdated
• EventClassIssuersUpdated
• EventClassMetadataUpdated
• EventProjectAdminUpdated
• EventProjectMetadataUpdated

...we could reduce the fields to just the identifier that was updated, which is how it is done in the group module. To avoid dragging this out, I can approve and we can get these changes merged and then address my comments in a followup. I think this is a step in the right direction and would be happy to do pair-programming on the proposed changes if you would prefer to handle them in this pull request. Otherwise I think we should keep the issue open after merging these changes.

i'm ok with either merging or you can just take over this PR 👍🏻

[ryanchristo]

This is a step in the right direction. Approving and merging but keeping the issue open and will add a followup pull request. I started making adjustments but stumbled into a bit of a rabbit hole with how we've been writing events and how I think we can improve and I think a separate pull request would make more sense with the updates I would like to make.