refactor: sd as 18 decimals

[andreivladbrg]

Closes #302

Changes

• store the snapshot debt as 18 decimals
• rename ongoingDebt to ongoingDebtScaled
  • use only uint256 type in this function
• rename snapshotDebt to snapshotDebtScaled
• update withdraw function accordingly
• reintroduce the scale/descale helpers functions
• update depletionTime function
  • the biggest change here is that we need to add to the stream balance with scaled 1 value (i.e. for USDC it would be1e12)
• use uint256 in scale/descale function from tests
  • the reason for this is that it safer to up cast from uint128 to uint256 compared to uint256 to uint128
• add ONE_MONTH_DEBT_18D constant for snapshotDebt tests

[andreivladbrg]

@smol-ninja nevermind for now the fail inv test, we will change the eq to ineq as we decided in #307

[smol-ninja]

Thank you for making all these changes. Clearly this is a big PR and has more impacts than I imagined. A few comments:

• I can see two variations of scaled naming: scaledABC and abcScaled. I suppose you are using abcScaled for function names (ongoingDebtScaledOf and getSnapshotDebtScaled) whereas scaledABC for variables names. If so, there are inconsistencies at a few places such as use of snapshotDebtScaled variable in getSnapshotDebtScaled function. Lets stick to one convention so that there is no confusion when deciding for a variable names that requires 18 decimal format.

My suggestion would be to either use scaledABC format or ABCScaled format for all variables and function naming and not both. Whats your preference?

• Would it be more consistent if we use the same naming convention for snapshot debt getter as well i.e. snapshotDebtOf similar to ongoingDebtOf? Its the only function that looks out of touch since it leans more towards getters defined in ISablierFlow than ISablierFlowBase. If you are aligned, we can consider refactoring it in the next release of Flow.

[smol-ninja]

One more comment: Technical doc is now outdated given that sd and od have been refactored to follow 18 decimal format. What do we do about it now? Here are some of the ideas:

• Lets remove numbers from it as it would require to be re-calculated.
• Only adds theory and formula as its easy to do.

Wdyt? if you think you want it to have numbers as well then we will have to update them as well as charts (I suppose charts are outdated too right?)

#314

[andreivladbrg]

• can see two variations of scaled naming: scaledABC and abcScaled

actually, my intention was to only have one, and that is abcScaled. the only exception, as you mentioned, is ongoingDebtOf, which would indeed end up in a situation where the function would have a sounded weird, "ongoingDebtOfScaled".

My suggestion would be to either use scaledABC format or ABCScaled format for all variables and function naming and not both. Whats your preference?

ofc, we should have just one, as said above. how would you keep consistency and still have a good function name? my preference is abcScaled

snapshotDebtOf similar to ongoingDebtOf

i think that would be misleading. snapshot debt is a state variable, while the other is a function that calculates an amount in real time. we should definitely keep the "get" keyword for all getters, as it only reads a state, and having "Of" points out that it’s different from the classic getters (dynamic function)

[smol-ninja]

actually, my intention was to only have one, and that is abcScaled.

Got it. Then lets use abcScaled everywhere, and for ongoing debt, we can use ongoingDebtScaledOf.

[smol-ninja]

Last few suggestions then looks good here.

[smol-ninja]

Fanstatic. Less goooo 🚀