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❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3456 +/- ##
==========================================
- Coverage 59.04% 59.02% -0.02%
==========================================
Files 2199 2188 -11
Lines 182096 181356 -740
==========================================
- Hits 107513 107049 -464
+ Misses 64933 64687 -246
+ Partials 9650 9620 -30
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryHigh Risk Overview Bank / EVM funds: spendable checks clamp Vesting tests are refactored (shared fixtures, table-driven marshal) and add 7h / 17h continuous-vesting cases. Reviewed by Cursor Bugbot for commit 3798ebb. Bugbot is set up for automated code reviews on this repo. Configure here. |
![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: