Use uint32 instead of sdk.Dec

[robert-zaremba]

Summary

For % values, we should use uint32 rather than sdk.Dec:

• it's faster for serialization
• it's smaller
• it's faster to cmpute
• users tend to think 15% rather than 0.15
• Dec is not even a part of the new cosmos-sdk math module.

Ref: #1096 (comment)

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[toteki]

Seems any storage benefits we get from using uint32 per token will be too small to notice - tokens aren't that numerous.

Also we'd need to write uint32->sdk.Dec getter functions as well, i.e. if Token.CollateralWeight is uint32 then we need to write Token.GetCollateralWeight() sdk.Dec so we can multiply collateral prices in computations.

Overall, I don't think the storage changes are worth the extra boilerplate. Closing for now.

[robert-zaremba]

Let's don't close the issue before discussing it.

1. In places we use sdk.Int we don't need uint32->sdk.Dec (in fact that one is super simple)
2. there are computational benefits.

[robert-zaremba]

I've created #1149 to demonstrate how this can be done.
This doesn't add more complexity - it's hidden in helper math basis point math functions. For LiquidationIncentive we don't even need to convert to sdk.Dec and we can stay in sdk.Int - so operations will be faster (no need to multiply by big number: 1e18).

Exactly same thing can be applied for CollateralWeight.

Convertion to Dec is easy and fast (as fast as converting sdk.Int) - I've created a helper method to that.

[robert-zaremba]

Another motivation for this is to potentially switch sdk.Dec computation to General Decimal Arithmetic, which will provide more performance gains over sdk.Dec and configurable precision, without changing the state (so we can do updates in code, without a need to update the state).

[toteki]

Some things to watch out for if we go through with this:

• Some of our current sdk.Dec values do not follow Contract: b in [0; 10000] of FixedMul seen in the basis points PR. For example, MaxBorrowAPY will likely be above 100% in many cases.
• We should define the scope of which sdk.Dec values are being considered for change. For example, x/leverage params, Token params, x/leverage state (e.g. InterestScalar), x/oracle prices and params, and return values from queries (e.g. borrow_limit). For now, are we only looking at x/leverage and Token params which are interpreted as percents?

I think the long term benefit of being able to change our internal math types without modifying state is worth considering as well - but that benefit is only realized if we convert all our decimals in state, which I do not expect will happen. If there will inevitable be at least some sdk.Dec in state when we launch to mainnet, then I don't think this issue is worth rushing for v3 as it will not eliminate the need for a migration later.

[robert-zaremba]

MaxBorrowAPY will likely be above 100% in many cases.

We can lift that limitation, and change to 100'000'000% (100k times) which is more than enough. I created FixedBP type for 0-1 values. We can have another type: BP for 0-100k values.

[robert-zaremba]

We can easily convert all params and token params from x/leverage to uint. We will need to continue using sdk.Dec for some internal operations, before a bigger Dec refactoring, without changing the state.

I think the bigger question is if we want to use per-cent, per-mil or basis point.

[toteki]

How about for a sequence of events:

• First merge new params (proto) as sdk.Dec in feat(proto)!: Add MaxCollateralShare, MinCollateralLiquidity, MaxSupplyUtilization #1159
• Then a PR with all the relevant sdk.Dec -> uint32 on proto.

Notably, this line will be omitted - proto and the generated Token struct will know these as regular uint32

(gogoproto.casttype) = "github.com/umee-network/umee/v2/math/bpmath.BP"

And a single func fromBasisPoints(uint32) sdk.Dec function in x/leverage can handle everything internal.

This would eliminate the need for custom types and math functions while making the minimum future-minded changes to proto.

[robert-zaremba]

OK for #1159

For the other part:
It was my initial approach to use uint32. However working on it I found that a custom types will have more advantages:

• be more informative about the meaning of the number.
• have more type safety when creating function arguments
• has a small optimizations when doing simple multiplication over sdk.Dec

[toteki]

Disadvantages were

• Required 2 custom types: one for params capped at 100% and one for allowing >100%
• Locks in any custom type decision to proto (or at least proto tags)
• Started requiring a supporting math library - though this was more a result of aspiring to eliminate sdk.Dec from computations.

The uint32 approach gives us basis point representation and future flexibility, with minimal complexity cost.

[robert-zaremba]

Required 2 custom types: one for params capped at 100% and one for allowing >100%

I see this is an advantage. I added it to control the value range using the type system, which can be checked at compilation time. It's optional though.

Locks in any custom type decision to proto (or at least proto tags)

This is neutral - we are already doing it with Dec, but in less intuitive type because we use stringy as the carrier.

Started requiring a supporting math library

You mean the new package? The bpmath?

[toteki]

I'm going to try to keep threads in the Issue rather than the PR, since there's some bouncing back and forth.

The problem I had with #1149 is it was leaving the territory of minimum proto changes, and started creating the math for computations without Dec in general.

Things like

• having two types, and making them distinct in proto
• creating Mul with signature (BP, sdk.Int) -> sdk.Int)
• creating Quo with signature (sdk.Int, sdk.Int) -> BP

These changes would need design discussion instead of going straight to code.

For example, the current Mul and Quo are only usable if we decide to do some internal x/leverage arithmetic at 4-precision instead of 18, which does not work for most of our current sdk.Dec usage (due to chained operations and rounding error size).

Also we would need to consider potential consequences of setting a field as FixedBP which we later realize needs to be BP. It is state breaking, but do custom type tags also count as proto breaking? Will cw-umee-types have to replicate the type distinction? This was not discussed.

To contrast, a minimal string -> uint32 conversion could be merged immediately (for v3) and later supplemented with custom math types (in v3, v4, or never). That is my reasoning for doing it in separate steps. The other option is to not merge any code changes until the full design has been agreed upon.

[robert-zaremba]

We will need to decide if we are going that route or not.