Skip to content

fix(pool): round pool repayment interest half-up#579

Merged
sanmipaul merged 2 commits into
astera-hq:mainfrom
Dannyswiss1:feature/smart-contract-interest-accrual
Jun 19, 2026
Merged

fix(pool): round pool repayment interest half-up#579
sanmipaul merged 2 commits into
astera-hq:mainfrom
Dannyswiss1:feature/smart-contract-interest-accrual

Conversation

@Dannyswiss1

Copy link
Copy Markdown
Contributor

Summary

  • Apply checked round-half-up division to pool interest calculations instead of truncating fractional yield
  • Document the repay_invoice interest rounding policy
  • Add a regression test for the 12,499 principal / 800 bps case, asserting 1,000 interest is charged

Tests

  • git diff --check
  • cargo test -p share
  • cargo test -p pool (blocked by existing Soroban macro compile failures in the pool crate)

Closes #564

@sanmipaul

Copy link
Copy Markdown
Contributor

Thanks for picking this up, @Dannyswiss1, the core idea is right and the simple interest fix is clean. A few things need to work on to ensure PR is ready.

1. Tests can't run

Your own PR description says cargo test -p pool is blocked by compile failures. That means the regression test you added for the exact bug being fixed has never executed. Please resolve the compile issue and confirm the suite passes.

2. The factoring-fee test is now a tautology

You updated test_factoring_fee_is_charged_and_tracked_separately to compute expected_interest by calling div_round_half_up(...) directly but estimate_repayment() calls the same function with the same inputs internally. The assertion reduces to:

assert_eq!(div_round_half_up(A, B), div_round_half_up(A, B))

3. Compound interest rounds twice

When elapsed_secs spans both full days and leftover seconds, both the elapsed_days > 0 block and the remaining_secs > 0 block call div_round_half_up independently. Each adds up to 1 stroop, so borrowers can be over-charged by up to 2 stroops instead of the intended 0–1.

The correct approach: accumulate exact intermediate values, then round once on the final result:

let interest = total_amount - principal;
div_round_half_up(interest_numerator, denominator)

4. No test for compound interest rounding

The default config has compound_interest: false, so the new test never exercises the compound path at all, the three new div_round_half_up call sites in the full-days and remaining-seconds branches are completely untested. Please add a test that:

  • calls client.set_compound_interest(&admin, &true)
  • advances time by e.g. 90_000 seconds (1 day + 1 hour, hitting both branches)
  • asserts against a hardcoded expected value

5. div_round_half_up panics on denominator == 0

The .map(|r| r / denominator) closure will panic at runtime if denominator is ever zero. Not reachable with current callers, but worth a one-liner guard so this utility stays safe for any future use:

if denominator == 0 {
    return Err(PoolError::AmountOverflow);
}

6. Ensure CI/CD passes

@Dannyswiss1 Dannyswiss1 changed the title Round pool repayment interest half-up fix(pool): round pool repayment interest half-up Jun 18, 2026
@Dannyswiss1

Copy link
Copy Markdown
Contributor Author

@sanmipaul its all good now, please merge.

@sanmipaul

Copy link
Copy Markdown
Contributor

@sanmipaul its all good now, please merge.

Good work resolving all five issues. LGTM.

@sanmipaul sanmipaul merged commit c658b7d into astera-hq:main Jun 19, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: yield calculation uses integer division — cumulative investor underpayment across repayments

2 participants