PAR-324
Admin proprietor-override diverges firm money from ownership on DB failure
Found in code review of PR #2 (develop→main).
HIGH — FirmServiceImpl.adminSetProprietor. The method ran the cross-plugin Treasury account reassignment (IPC, cannot enrol in the JDBC @Transactional) before its final DB write (recordAdminOverride). If that insert threw, the DB rolled back the proprietor change while the Treasury owner reassignment stayed committed — Treasury accounts owned by the new proprietor while the firm row named the old one (the ADT-11 divergence class). The sibling consent-transfer path was already deliberately ordered IPC-last; the admin path violated it.
Fix: reordered so the IPC is the last statement (updateProprietor → cancelActiveTransfers → recordAdminOverride → reassignAccountsToNewProprietor). Added a regression test (recordAdminOverride throws → reassignment never runs) that fails against the old ordering.
Also (LOW, same change):
- New
FirmRequestMapper.cancelActiveTransfers(firmId)— the override now cancels any in-flight (PENDING/CONFIRMED) player transfer so it can't later be completed and re-hand the firm. FirmAccountServiceImpl.createAccountrejects archived firms (was archived-inclusive → could mint an orphan account against a disbanded firm).
Adversarially reviewed before and after implementation; mutation-tested regression guards. Build green, unit tests pass.
Resources
- commit8286ff9 Fix PR #2 review findings across business, treasury, rest-api, flywayParadauxIO/hibernia-economy
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