-
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
feat(x/ecocredit): add BurnRegen method #2107
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2107 +/- ##
==========================================
- Coverage 73.26% 73.21% -0.06%
==========================================
Files 232 234 +2
Lines 13748 13801 +53
==========================================
+ Hits 10073 10104 +31
- Misses 2941 2958 +17
- Partials 734 739 +5
|
It looks good; I was just wondering. Does it make sense that it is its own cosmos SDK x module? that could eventually evolve into a fee management module? However, if so, it makes sense to do it after and migrate it to it then. |
Thanks for the review @JeancarloBarrios. I agree that a standalone module would make more sense. But also, there's a fair amount of overhead in creating a new module, so I think we will get this live on chain sooner by putting it in x/ecocredit. Also, the other burning functionality we've discussed would be native to x/ecocredit so it's not totally out of place. |
It would be great to get one more review here if anyone has time. |
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
@@ -5,7 +5,6 @@ linters: | |||
disable-all: true | |||
enable: | |||
- bodyclose | |||
- depguard |
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.
This linter was causing CI errors but we never configured it and probably it just got enabled with a default blocking rule in #2148. cc @robert-zaremba
Description
This PR adds a
BurnRegen
method to the ecocredit module which allows a user to burn REGEN from their balance with an optional burn reason which can be used by clients to associate the burn with specific activities.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