Closed aaronc closed 3 years ago
/cc @clevinson ?
This seems like a pretty big refactor. Would it involve passing in the bank Keeper or rather using ADR33... wouldn't the latter also require the minting functionality to be built out in an internal msg server for bank?
I see the direction / reasoning here, but it feels pretty tight if we're trying to have this all done for v1.1 release.
Would you consider an intermediate step where we drop decimal support but keep tracking it in the ecocredit module for now? That seems like the slimmest refactor IMO, and we could then later migrate to the bank module later when ADR33 is ready in bank and release it alongside any AMM/liquidity module at the same time?
We would just need to pass in the bank keeper. We could store balances in both bank and ecocredit, but in bank we would need to issue in kg or grams which seems like a weird mental model...
It's possible we could issue in tons in ecocredit and grams in bank if we used the precision of the issuance to determine the multiplier. But again that seems like it could be a strange UX and likely we should just issue in grams everywhere...
In general I'm in support in integers overall. Number formatting shouldn't be a responsibility of a node. All blockchains are using integers. We can go for grams. No need to limit ourselves to kg.
OK, I'm in favor of moving forward with this approach, but this probably requires a more detailed estimation of how this will affect ecocredit readiness timeline for v1.1.
OK, I'm in favor of moving forward with this approach, but this probably requires a more detailed estimation of how this will affect ecocredit readiness timeline for v1.1.
My gut feeling on this is that it's a similar level of complexity as #242, which means it wouldn't really alter the timeline. We would drop #242 and replace it with this. Now, if we also want to add a layer of decimal support as kind of a UX enhancement (where 1.000000 in ecocredits gets converted to 1000000 in bank) then it's a bit more complex - I would say the size of this issue + #242.
The current thinking is to pause this work and maybe not do it because:
Update: As discussed on July 20th 2021, we decided to keep Decimal support inside the
x/ecocredit
module. To integrate withx/bank
we'll have a precision per credit type (#424) and we'll multiply Decimal values by the precision to create integers to store inx/bank
.In order for credits to be transferable using IBC and/or an existing AMM/liquidity module, they will need to be expressed in integer amounts. Even if we were to add decimals to x/bank, it will be a while before IBC and AMM/liquidity modules support decimals.
So likely the best path forward is to drop decimal support for credits and issue credit batches in either kilograms or grams so there is good fractionalization for buying and selling.This will mean we don't do regen-network/regen-ledger#242 and instead postpone that work till its integration in the SDK.