refactor(remittance_nft): consolidate repayment floor validations and remove dead code

[K1NGD4VID]

Pull Request: Consolidate Repayment Floor Validation and Clean Dead Code

Target Issue: Resolves #13
Target Repository: LabsCrypt/remitlend-contracts
Branch: RemittanceNFT-update_score

Overview

This PR refactors and consolidates the overlapping validation checks in the update_score function inside remittance_nft/src/lib.rs. It removes redundant logic, eliminates an unreachable points_i128 == 0 early return, improves documentation clarity, and adds comprehensive unit tests verifying all boundary conditions.

---

Detailed Changes

1. Error Enum Cleanup (NftError)

• Removed the redundant InvalidRepaymentAmount variant.
• Kept BelowMinimum as the sole surviving error variant to describe the consolidated floor-check semantics.
• Updated all doc comments and references (e.g., in MIN_SCORE_UPDATE_REPAYMENT documentation) to point to BelowMinimum (error code 17).

2. Validation Floor Consolidation in update_score

The function previously had three redundant repayment-floor checks:

1. if repayment_amount < min_repayment { return BelowMinimum }
2. if repayment_amount < Self::MIN_SCORE_UPDATE_REPAYMENT (100) { return InvalidRepaymentAmount }
3. let points_i128 = repayment_amount / 100; if points_i128 == 0 { return Ok(()) }

These have been replaced with a single consolidated check:

• The effective floor is determined as the larger of the configurable min_repayment and the fixed constant Self::MIN_SCORE_UPDATE_REPAYMENT (100):

  let effective_floor = min_repayment.max(Self::MIN_SCORE_UPDATE_REPAYMENT);

• If the repayment amount is below this effective floor, the function returns a single error:

  if repayment_amount < effective_floor {
      return Err(NftError::BelowMinimum);
  }

• This check naturally handles zero and negative values as the effective floor is always >= 100.

3. Removal of Unreachable points_i128 == 0 early-return

• Because the floor check guarantees that repayment_amount >= effective_floor where effective_floor >= 100, the division repayment_amount / 100 (integer division) will always result in a value >= 1.
• Therefore, points_i128 can never be 0. The redundant early-return check was removed, and a comment justifying its removal was added.

4. Improved Documentation

• Updated the doc comment on update_score to clearly describe the consolidated validation rules, the parameters, the effective floor calculation, and the exact condition under which BelowMinimum is returned.

5. Boundary Tests Added

Appended five new unit tests to remittance_nft/src/test.rs covering all boundary conditions:

• test_update_score_floor_boundary_99: Asserts that repayment_amount == 99 fails with BelowMinimum when min_repayment = 0.
• test_update_score_floor_boundary_100: Asserts that repayment_amount == 100 succeeds and updates the score when min_repayment = 0.
• test_update_score_floor_configured_min_150_repay_120: Asserts that repayment_amount == 120 fails with BelowMinimum when min_repayment = 150.
• test_update_score_floor_exact_effective: Asserts that a repayment exactly at the effective floor (150) succeeds and updates the score.
• test_update_score_floor_above_effective: Asserts that a repayment one unit above the effective floor (151) succeeds and updates the score.

---

Verification Results

All tests (including the 5 new boundary tests) run and pass successfully on the host system:

• Command executed: cargo test -p remittance_nft --lib
• Test results: ok. 70 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.70s

[ogazboiz]

ci is red on cargo fmt: remittance_nft/src/lib.rs:605 (trailing whitespace on the blank line after points_i128) and test.rs:1901 (an extra blank line). run cargo fmt --all from contract/ and push.

once fmt is green, ci will actually run clippy and your new floor tests, and i'll verify the dead-code removal then, the claim that the points_i128 == 0 early-return is unreachable hinges on effective_floor always being >= 100, so i want to see the floor_boundary_99/100 tests go green in ci before merging. quick fmt and we're most of the way there.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

[ogazboiz]

the prior items are fixed, merging once ci is green. fmt is clean now (the trailing whitespace + EOF blank line are gone) and the floor boundary tests pass. the consolidation is semantically equivalent: the three guards collapse to if repayment_amount < effective_floor { BelowMinimum } with effective_floor = min_repayment.max(100), which equals the old max(1, min_repayment, 100) since 100 >= 1, and because set_min_repayment_amount rejects negatives the removed points==0 early return is genuinely unreachable. removed InvalidRepaymentAmount has zero remaining refs. clean.

note: your other PR #41 carries an older copy of this same remittance_nft change, once this lands #41 should rebase and drop it.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0