-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(x/ecocredit): reduce event fields and duplication #1086
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1086 +/- ##
==========================================
- Coverage 67.98% 67.89% -0.10%
==========================================
Files 210 209 -1
Lines 20949 20929 -20
==========================================
- Hits 14242 14209 -33
- Misses 5387 5398 +11
- Partials 1320 1322 +2
Flags with carried forward coverage won't be shown. Click here to find out more. |
OriginTx origin_tx = 2; | ||
// issuer is the account address of the issuer of the credit batch that has | ||
// minted credits to the credit batch. | ||
string issuer = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MintBatchCredits
asserts that req.Issuer == batch.Issuer. if this is the intended functionality, i think we can remove issuer
from this event. otherwise if accounts that != original issuer address should be able to mint, we should open a new issue to resolve that discrepancy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the original issuer should be able to mint and it looks like it is implemented correctly. Removed here and also removed in CreateBatch
. Was thinking something different at the time but not needed. 👍 67b3232
Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
@@ -326,7 +326,7 @@ devdoc-update: | |||
### Protobuf ### | |||
############################################################################### | |||
|
|||
containerProtoVer=v0.6 | |||
containerProtoVer=v0.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v0.7
required for go 1.18.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just one sneaky json file leftover 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
Closes: #1035
Removes unnecessary and redundant event fields. Also applies consistent use of present tense event names.
For basket
EventCreate
For basket
EventPut
Deprecating credits because core
EventTransfer
(previouslyEventReceive
) is emitted providing information about the sender, receiver, and credits transferred.Deprecating amount because sdk
coinbase
is triggered providing information about the denom and amount minted and sdktransfer
event provides information about sender, receiver, and coins transferred.For basket
EventTake
Deprecating credits because core
EventTransfer
(previouslyEventReceive
) is emitted providing information about the sender, receiver, and credits transferred.Deprecating amount because sdk
burn
event is emitted providing information about the denom and amount burned and sdktransfer
event provides information about sender, receiver, and coins transferred.For core
EventReceive
EventTransfer
basket_denom
because thesender
will be the module address in this caseFor update events in general
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...
!
to the type prefix if API or client breaking changeCHANGELOG.md
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...
!
in the type prefix if API or client breaking change