-
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
Add parameter to track credit types and associated precision #424
Comments
This can replace #398 |
was the intention to use the proto map type? might be an issue: the safety of using protobuf maps isn't quite well defined yet see: cosmos/cosmos-sdk#9073 (comment) perhaps we do something like this using // Params defines the updatable global parameters of the ecocredit module for
// use with the x/params module.
message Params {
// -- snip -- //
// credit_types is a list of definitions for credit types
repeated CreditType credit_types = 2;
}
message CreditType {
string typeOf = 1;
string unit = 2;
uint32 precision = 3;
} |
Let's use repeated 👍 |
should we just rip out all |
I think just rip it out! This isn't currently deployed, so we're changing the design to just have precision at the credit type level. |
I would even goes as far as to say we might allow 6 as the only valid value for precision and then consider whether we loosen that in the future. What do you think @ruhatch? |
related q: how do we enforce credit types as a gov param while simultaneously not allowing the precision to change? the validation function is stateless so i can't grab the existing params and check for changes.. so is just enforcing a hard 6 the solution for now? |
I think so. x/params is sort of broken anyway, we should have rewriting it as something in our roadmap (cosmos/cosmos-sdk#6983). An alternative would be just setting parameters directly in x/ecocredit without x/params when cosmos/cosmos-sdk#9438 is ready. But for now, I think hard-coded 6 is probably the solution. |
Does that param issue mean that we can't validate param changes? In that case I guess hardcoding seems like the best option for now and we can migrate it to a gov param once we can do validation and gov/group/param stuff is all a bit more stable? |
+Adds credit types which includes type name, unit type (kg, tonne, etc), and precision (currently locked @ 6). +Adds query to get all credit types on CLI and grpc/http +Updates CreateClass tx command to include credit class arg +Adds tests for validating the credit type params -removes per-class precision setting functionality Closes: #424
As part of integration with
x/bank
, we decided to have a single consistent precision for each type of credit, rather than being able to set precision per credit class. This issue should:x/ecocredit
containing a map of credit types and precisionsThe text was updated successfully, but these errors were encountered: