Represent present value with 256 bits instead of 104

[jflatow]

This internally uses 256 bits for present values instead of artificially constraining them to 104 bits. This avoids an unsafe cast we were doing previously to save gas, and also opens up some other gas optimizations / contract size reductions. However, this also removes some previously unchecked math, which makes it a slight tradeoff in terms of gas.

[jflatow]

Just measured, and it seems to save ~100 bytes contract size and a few hundred gas on supply, transfer, and withdraw, in addition to avoiding the unsafe cast.

[kevincheng96]

It's nice that this shaves off some gas and contract size. However, other than the unchecked change, I don't think these changes are actually adding extra safety right?

I've always been supportive of using uint256/int256 for intermediate variables, but this is a pretty large change late in the cycle so I'll want to spend some more time to review this. But I definitely think this is the right change to make. 👍

[jflatow]

It's nice that this shaves off some gas and contract size. However, other than the unchecked change, I don't think these changes are actually adding extra safety right?

I've always been supportive of using uint256/int256 for intermediate variables, but this is a pretty large change late in the cycle so I'll want to spend some more time to review this. But I definitely think this is the right change to make. 👍

Well it gets rid of the unsafe cast in presentValue which was the original reason I was looking at it again, is that what you mean by the unchecked change? There's also the mulPrice / signedMulPrice change which I wasn't wanting or planning on making, but kind of fell out of it

[kevincheng96]

Well it gets rid of the unsafe cast in presentValue which was the original reason I was looking at it again, is that what you mean by the unchecked change? There's also the mulPrice / signedMulPrice change which I wasn't wanting or planning on making, but kind of fell out of it

Yeah, that's what I meant by "unchecked change". Just to make sure I'm not missing anything, I want to double check with you that that's the only change here that changes the safety of our code. The other changes are more gas savings/optimizations (e.g. less explicit casting) right?

[jflatow]

Well it gets rid of the unsafe cast in presentValue which was the original reason I was looking at it again, is that what you mean by the unchecked change? There's also the mulPrice / signedMulPrice change which I wasn't wanting or planning on making, but kind of fell out of it

Yeah, that's what I meant by "unchecked change". Just to make sure I'm not missing anything, I want to double check with you that that's the only change here that changes the safety of our code. The other changes are more gas savings/optimizations (e.g. less explicit casting) right?

Yes, those 2 changes should be the difference

[kevincheng96]

Nice, looks good!