Skip to content

fix: align repay function name between XDR builder and transaction checker#90

Open
khaylebfortune wants to merge 3 commits into
StepFi-app:mainfrom
khaylebfortune:fix/align-repay-function-name
Open

fix: align repay function name between XDR builder and transaction checker#90
khaylebfortune wants to merge 3 commits into
StepFi-app:mainfrom
khaylebfortune:fix/align-repay-function-name

Conversation

@khaylebfortune

Copy link
Copy Markdown
Contributor

Fix repayment function name mismatch

Problem

The XDR builder (creditline.client.ts) builds transactions using repay_installment but the transaction status checker was looking for repay_loan. This mismatch meant the follow-up loan balance deduction never triggered after successful on-chain repayments — loans appeared unpaid indefinitely.

Root Cause

The function name was changed in one place but not the other during development, as documented in ISSUES.md.

Changes

File Change
src/jobs/transaction-status-checker/transaction-status-checker.service.ts Check for repay_installment instead of repay_loan
src/stellar/contracts/clients/creditline.client.ts Fix error log to match actual function name
src/modules/loans/loans.controller.ts Update Swagger description
src/modules/loans/dto/loan-payment-response.dto.ts Update Swagger description
test/unit/stellar/contract-function-name.spec.ts New: verifies XDR builder and checker use the same function name

Verification

  • npm run build passes
  • npm test — 25 suites, 290 tests pass
  • New alignment test passes (2/2)

Closes #62

…ecker

The deployed creditline contract function is 'repay_installment' but the
transaction status checker was looking for 'repay_loan'. This mismatch
meant successful on-chain repayments never triggered the follow-up
loan balance deduction — loans appeared unpaid indefinitely.

Changes:
- transaction-status-checker.service.ts: check for 'repay_installment'
- creditline.client.ts: fix error log to match actual function name
- loans.controller.ts, loan-payment-response.dto.ts: update Swagger docs
- Add alignment test to prevent future drift

Closes StepFi-app#62

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: ✅ APPROVE with minor corrections

Good catch and a real fix. The status-checker matched functionName === 'repay_loan' while the XDR builder emits repay_installment, so the checker never recognized any repayment — all repayment follow-up/status tracking was silently broken. Realigning to repay_installment is correct, and the checker's nativeArgs[1]=loanId, nativeArgs[2]=amount parsing matches what the builder actually emits (contract.call('repay_installment', userArg, loanIdArg, amountArg), 3 args). Doc strings + Swagger updated consistently; no any added.

Minor corrections:

  1. The new test reads the two source files as text and regex-matches function names. That's brittle — a formatting change breaks it. Prefer asserting on the actual built XDR / decoded operation args.
  2. buildRepayLoanTx is now a misnomer (it builds repay_installment). Consider renaming for clarity.

⚠️ Separate, more serious issue surfaced while reviewing this (not caused by this PR): the API builds repay_installment with 3 args (borrower, loanId, amount), but the creditline contract source defines repay_installment(borrower, loan_id, installment_index, amount)4 args. If the deployed contract matches the repo, every repayment transaction the API builds would fail on-chain (arg-count/type mismatch). Please verify the API's builder against the deployed contract signature and open a follow-up if they disagree — this is a money-path blocker independent of this PR.

@khaylebfortune

Copy link
Copy Markdown
Contributor Author

@EmeditWeb thanks for the review. I've addressed both issues.

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-audit for merge: ⚠️ Request changes — I WILL NOT merge yet (money-critical arg mismatch) @khaylebfortune

The rename repay_loan → repay_installment is correct and the XDR-decode test rewrite is good. But re-auditing this against the current contract source surfaced a blocking mismatch on the repayment path:

buildRepayInstallmentTx sends 3 args; the contract takes 4.

// API (creditline.client.ts) — 3 args
contract.call('repay_installment', userArg, loanIdArg, amountArg)
//                                  borrower  loan_id    amount
// creditline-contract/src/lib.rs:751 — 4 caller args
pub fn repay_installment(env, borrower: Address, loan_id: u64,
                         installment_index: u32, amount: i128)

Two problems:

  1. Missing installment_index: u32 — the call omits it entirely.
  2. loan_id type: sent as nativeToScVal(loanId, { type: 'string' }), but the contract expects u64.

Against the current contract, server.prepareTransaction (simulation) will reject this invocation, so repayments can't be built at all. Against an older deployed build it might pass now but will break the moment the contract is upgraded to the current source (which the merged waterfall PR #60 requires via upgrade()).

Before this can merge:

  1. Add installment_index — the API needs to determine which installment is being paid from the loan's schedule and pass it as u32.
  2. Encode loan_id as u64 (nativeToScVal(BigInt(loanId), { type: 'u64' }) or equivalent), not string.
  3. Verify the deployed testnet repay_installment signature and confirm the API matches it (there's a known repo-vs-deployment gap on the contracts).

The naming half of this PR is right; the on-chain call shape is not. Happy to re-review once the arg/type are aligned.

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.

critical: align repayment function name between XDR builder and status checker

2 participants