Conversation
| // If there is no percent, then you gain nothing from deposits. | ||
| // Withdrawals can only be made against the reserve, over time. | ||
| (0, 1) | ||
| } else if retry_percent <= 1.0 { |
There was a problem hiding this comment.
I think most often the percent will be 10, 20, around there. So, to not need to deal with large numbers in the most common case, this could probably be edit to just <= 0.5. What do you think?
There was a problem hiding this comment.
Makes sense, done. Anything at or below 0.5 keeps the old (1, 1/percent) pair, and only above that does it scale by 1000, since that's where the reciprocal truncates to 1. Worth noting the small values still truncate (0.3 gives 3, so effectively 33%), but that's the existing behavior and the numbers stay small.
56de74d to
d769498
Compare
|
Good call, switched to scaling only when retry_percent <= 0.5 so the common integer-ish percents keep the original arithmetic. Updated the test name to match. |
Collapses the fractional-percent and > 1 branches into one: scale deposits by 1000 and use (1000 / retry_percent) as the withdraw cost for every non-zero percent. This gives exact precision across the full [0, 1000] range without a special-case threshold. Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
Updated: applied the 1000x scaling uniformly for all non-zero retry_percent values (not just <= 0.5). This means the test case at 0.6 now actually verifies the fix: (1000, 1666) gives exactly 6 retries per 10 deposits. The > 1 case also simplifies to the same formula. |
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
TpsBudget::newused(1, (1.0 / retry_percent) as isize)forretry_percent <= 1.0, which truncates the reciprocal so e.g.0.6authorizes a full retry per deposit instead of 0.6. This reuses the existing 1000x deposit scaling for all positive percentages and adds a regression test. Closes #857.