Optimise vested amounts computation using math/big#3456
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3456 +/- ##
==========================================
- Coverage 59.11% 59.11% -0.01%
==========================================
Files 2187 2187
Lines 182236 182173 -63
==========================================
- Hits 107732 107684 -48
+ Misses 64852 64839 -13
+ Partials 9652 9650 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryMedium Risk Overview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
masih
force-pushed
the
masih/math-perf-optimisation
branch
2 times, most recently
from
dedf4c6 to
2ba4418
Compare
Continuous-vesting proration is amount * elapsed / duration, which is an integer computation. Going through sdk.Dec allocates a scaled big.Int per coin only to truncate the extra precision back away. Use math/big.Int directly and skip the round trip. While at it: * rename the loop-local "x" in PeriodicVestingAccount.GetVestedCoins to "elapsed" so it matches the continuous-vesting variable naming * correct the Validate() error string on continuous and periodic vesting accounts: the predicate fails when start-time >= end-time, so "cannot be before end-time" was reversed * cover two non-aligned timestamps in TestGetVestedCoinsContVestingAcc
masih
force-pushed
the
masih/math-perf-optimisation
branch
from
ed60a56 to
c530ac3
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c530ac3. Configure here.
| newUsei, newWei := SplitUseiWeiAmount(postTotal) | ||
| lockedUsei := k.LockedCoins(ctx, addr).AmountOf(denom) | ||
| if newUsei.LT(lockedUsei) { | ||
| return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%suei is smaller than locked %suei", newUsei, lockedUsei) |
There was a problem hiding this comment.
Error message misspells denomination as "uei" instead of "usei"
Low Severity
The format string "%suei is smaller than locked %suei" produces output like 100uei instead of 100usei. The %s format verb consumes the numeric argument, and uei is the literal suffix — but the denomination is usei, not uei. The nearby wei error correctly uses "%swei". The fix is "%susei is smaller than locked %susei".
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c530ac3. Configure here.
2 participants
Continuous-vesting proration is amount * elapsed / duration, which is an integer computation. Going through sdk.Dec allocates a scaled big.Int per coin only to truncate the extra precision back away. Use math/big.Int directly and skip the round trip.
While at it: