Paradaux

PAR-325

0

Government salary double-notifies/double-counts on concurrent collapsed payout

Pending ReleaseUnassignedTreasury

Found in code review of PR #2 (develop→main).

LOW→ correctness. SalaryServiceImpl.payout decided whether a payout actually happened from a racy pre-check (findTxnIdByDedupKey before the transfer). When a manual /salary run overlaps the scheduled SalaryTask, both runs see the pre-check as absent; one inserts, the other collapses on the uq_ledger_dedup UNIQUE inside the transfer — but with the pre-check saying "new", the collapsing run still sent the player a second "you were paid" notification and inflated the returned paid count. No double-credit (the ledger dedup prevents that), but a spurious notify + wrong count.

Fix: the ledger engine now reports outcome via LedgerService.TransferResult{txnId, created}; transferInternal returns created=false at both collapse points (pre-check and the concurrent-insert race) and created=true only on a fresh insert. Salary notifies/counts only when created. Removed the now-redundant, race-prone findTxnIdByDedupKey probe. Public transfer/adminTransfer signatures unchanged (delegate to .txnId()).

Also added a cross-engine parity comment: the REST engine reads overdraft flags unlocked by design (it has no writer for those flags), which is safe and must not be "fixed" as a deadlock issue.

Salary collapse test inverted to assert the corrected behaviour; adversarially reviewed before/after; build green, :treasury:test passes.

Resources

Comments

No comments yet.

Activity

  • ParadauxIO linked a commit — Commit 8286ff9 — Fix PR #2 review findings across business, treasury, rest-api, flyway
  • tesks created the issue