fix: make government account resolution symmetric across transfers and balance checks

buildSrc/src/main/kotlin/realty-conventions.gradle.kts

@@ -3,7 +3,7 @@ plugins {
 }
 
 group = "io.github.md5sha256"
-version = "1.4.1"
+version = "1.4.2"
 
 val targetJavaVersion = 25
 

realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java

@@ -9,20 +9,22 @@
 import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.List;
+import java.util.Optional;
 import java.util.UUID;
 
 /**
  * Economy provider backed by Treasury. Provides full ledger support:
  * each transfer is recorded with a human-readable message that appears
  * in the player's Treasury transaction history.
  * <p>
- * Account resolution: the payer is always resolved as a personal account
- * (created with starting balance if missing). The recipient is resolved by
- * preferring its GOVERNMENT account, then PERSONAL, then BUSINESS — so
- * government landlords (legacy DCGovernment-style real UUIDs that own both a
- * personal and a government account) route income to their government
- * treasury, while ordinary landlords still get their personal balance rather
- * than a firm BUSINESS account they happen to own.
+ * Account resolution is the same on both sides of a transfer: prefer the
+ * party's GOVERNMENT account, then PERSONAL, then BUSINESS. Government
+ * entities (legacy DCGovernment-style real UUIDs that own both a personal and
+ * a government account) therefore both receive income into and pay refunds out
+ * of their government treasury, while ordinary players resolve to their
+ * personal balance rather than a firm BUSINESS account they happen to own.
+ * Balance reads follow the same preference, so an affordability check always
+ * inspects the account the subsequent transfer would actually touch.
  */
 public final class TreasuryEconomyProvider implements EconomyProvider {
 
@@ -36,19 +38,27 @@ public TreasuryEconomyProvider(@NotNull TreasuryApi treasuryApi) {
 
     @Override
     public double getBalance(@NotNull UUID playerId) {
-        if (!treasuryApi.hasAccountByOwnerUuid(playerId)) {
+        // Read the same account transfer() would debit, not whichever one
+        // getBalanceByOwnerUuid happens to pick -- otherwise a government
+        // entity is checked for affordability against its personal balance.
+        // A read must not open an account, so there is no create-if-missing
+        // fallback here: no accounts means no funds.
+        Account account = preferredAccount(treasuryApi.getAccountsByOwner(playerId)).orElse(null);
+        if (account == null) {
             return 0.0;
         }
-        BigDecimal balance = treasuryApi.getBalanceByOwnerUuid(playerId);
+        BigDecimal balance = treasuryApi.getBalanceByAccountId(account.getAccountId());
         return balance != null ? balance.doubleValue() : 0.0;
     }
 
     @Override
     public @NotNull PaymentResult transfer(@NotNull UUID fromId, @NotNull UUID toId,
                                             double amount, @NotNull String ledgerMessage) {
         try {
-            Account payer = treasuryApi.resolveOrCreatePersonal(fromId);
-            Account recipient = resolveRecipientAccount(toId);
+            // Both sides resolve identically: a refund from a government landlord
+            // must leave the same account the rent was paid into.
+            Account payer = resolveAccount(fromId);
+            Account recipient = resolveAccount(toId);
             // Treasury rejects amounts with more than 2 decimal places. Amounts
             // derived from arithmetic (e.g. pro-rata refunds: price * remaining /
             // total) can carry extra precision, so normalise to 2 decimals here.
@@ -80,37 +90,50 @@ public boolean hasLedgerSupport() {
     }
 
     /**
-     * Resolves the recipient's Treasury account, preferring
+     * Resolves a party's Treasury account, preferring
      * GOVERNMENT &gt; PERSONAL &gt; BUSINESS &gt; first-available.
      * <p>
      * GOVERNMENT wins first because legacy government entities (e.g.
      * DCGovernment) are real Minecraft accounts whose UUID owns <em>both</em> a
      * personal and a government account; their leasehold income must land in
-     * the government treasury, not the player's personal balance.
+     * the government treasury, not the player's personal balance — and, on the
+     * paying side, a lease-termination refund must be debited from that same
+     * treasury rather than the entity's personal balance.
      * <p>
-     * Ordinary landlords have no government account, so PERSONAL is chosen next:
+     * Ordinary players have no government account, so PERSONAL is chosen next:
      * rental/sale income belongs to them personally, never a firm BUSINESS
      * account they happen to own (firm accounts are owned by the proprietor's
      * own UUID, which is how such funds previously leaked into business
      * accounts).
      * <p>
-     * When the recipient has no account at all, resolve-or-create their personal
+     * When the party has no account at all, resolve-or-create their personal
      * account.
      */
-    private @NotNull Account resolveRecipientAccount(@NotNull UUID ownerUuid) {
-        List<Account> accounts = treasuryApi.getAccountsByOwner(ownerUuid);
-        if (!accounts.isEmpty()) {
-            return accounts.stream()
-                    .filter(a -> a.getAccountType() == AccountType.GOVERNMENT)
-                    .findFirst()
-                    .or(() -> accounts.stream()
-                            .filter(a -> a.getAccountType() == AccountType.PERSONAL)
-                            .findFirst())
-                    .or(() -> accounts.stream()
-                            .filter(a -> a.getAccountType() == AccountType.BUSINESS)
-                            .findFirst())
-                    .orElse(accounts.get(0));
+    private @NotNull Account resolveAccount(@NotNull UUID ownerUuid) {
+        return preferredAccount(treasuryApi.getAccountsByOwner(ownerUuid))
+                .orElseGet(() -> treasuryApi.resolveOrCreatePersonal(ownerUuid));
+    }
+
+    /**
+     * Applies the GOVERNMENT &gt; PERSONAL &gt; BUSINESS &gt; first-available
+     * preference to an already-fetched account list, or empty when the party
+     * holds no accounts at all. Shared by {@link #resolveAccount(UUID)} and
+     * {@link #getBalance(UUID)} so a balance check and the transfer it gates
+     * can never disagree about which account is in play.
+     */
+    private @NotNull Optional<Account> preferredAccount(@NotNull List<Account> accounts) {
+        if (accounts.isEmpty()) {
+            return Optional.empty();
         }
-        return treasuryApi.resolveOrCreatePersonal(ownerUuid);
+        return Optional.of(accounts.stream()
+                .filter(a -> a.getAccountType() == AccountType.GOVERNMENT)
+                .findFirst()
+                .or(() -> accounts.stream()
+                        .filter(a -> a.getAccountType() == AccountType.PERSONAL)
+                        .findFirst())
+                .or(() -> accounts.stream()
+                        .filter(a -> a.getAccountType() == AccountType.BUSINESS)
+                        .findFirst())
+                .orElse(accounts.get(0)));
     }
 }

realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java

@@ -18,6 +18,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -30,6 +31,7 @@ class TreasuryEconomyProviderTest {
     private TreasuryEconomyProvider provider;
 
     private final UUID payer = UUID.randomUUID();
+    private final UUID recipient = UUID.randomUUID();
 
     @BeforeEach
     void setUp() {
@@ -46,7 +48,7 @@ private Account account(int id, AccountType type, UUID owner) {
 
     private int capturedDestination(UUID recipient) {
         Account payerPersonal = account(1, AccountType.PERSONAL, payer);
-        when(treasuryApi.resolveOrCreatePersonal(payer)).thenReturn(payerPersonal);
+        when(treasuryApi.getAccountsByOwner(payer)).thenReturn(List.of(payerPersonal));
         when(treasuryApi.transfer(any())).thenReturn(99L);
 
         PaymentResult result = provider.transfer(payer, recipient, 50.0, "Rental Payment: REGION");
@@ -107,4 +109,96 @@ void recipientWithNoAccounts_resolvesOrCreatesPersonal() {
 
         assertEquals(88, capturedDestination(newOwner));
     }
+
+    private int capturedSource(UUID payerUuid) {
+        Account recipientPersonal = account(2, AccountType.PERSONAL, recipient);
+        when(treasuryApi.getAccountsByOwner(recipient)).thenReturn(List.of(recipientPersonal));
+        when(treasuryApi.transfer(any())).thenReturn(99L);
+
+        PaymentResult result = provider.transfer(payerUuid, recipient, 50.0, "Lease Termination Refund: REGION");
+        assertInstanceOf(PaymentResult.Success.class, result);
+
+        ArgumentCaptor<TransferRequest> req = ArgumentCaptor.forClass(TransferRequest.class);
+        verify(treasuryApi).transfer(req.capture());
+        assertEquals(recipientPersonal.getAccountId(), req.getValue().toAccountId());
+        return req.getValue().fromAccountId();
+    }
+
+    @Test
+    void governmentPayer_refundIsDebitedFromGovernmentNotPersonal() {
+        UUID government = UUID.randomUUID();
+        // The mirror of legacyGovernment_withPersonalAndGovernmentAccount_routesToGovernment:
+        // a refund from a government landlord must leave the same account the rent
+        // was paid into, not the entity's personal balance.
+        when(treasuryApi.getAccountsByOwner(government)).thenReturn(List.of(
+                account(13, AccountType.PERSONAL, government),
+                account(9, AccountType.GOVERNMENT, government)));
+
+        assertEquals(9, capturedSource(government),
+                "a government landlord's refund must be debited from the government account");
+    }
+
+    @Test
+    void firmProprietorPayer_paysFromPersonalNotBusiness() {
+        UUID proprietor = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(proprietor)).thenReturn(List.of(
+                account(500, AccountType.BUSINESS, proprietor),
+                account(42, AccountType.PERSONAL, proprietor)));
+
+        assertEquals(42, capturedSource(proprietor),
+                "an ordinary payer must pay from their personal account, not a firm they own");
+    }
+
+    @Test
+    void payerWithNoAccounts_resolvesOrCreatesPersonal() {
+        UUID newPayer = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(newPayer)).thenReturn(List.of());
+        when(treasuryApi.resolveOrCreatePersonal(newPayer))
+                .thenReturn(account(88, AccountType.PERSONAL, newPayer));
+
+        assertEquals(88, capturedSource(newPayer));
+    }
+
+    @Test
+    void governmentBalance_readsTheGovernmentAccountNotPersonal() {
+        UUID government = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(government)).thenReturn(List.of(
+                account(13, AccountType.PERSONAL, government),
+                account(9, AccountType.GOVERNMENT, government)));
+        when(treasuryApi.getBalanceByAccountId(9)).thenReturn(new BigDecimal("250.00"));
+
+        assertEquals(250.0, provider.getBalance(government),
+                "a government entity's balance must be read from the account it transacts with");
+    }
+
+    @Test
+    void firmProprietorBalance_readsPersonalNotBusiness() {
+        UUID proprietor = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(proprietor)).thenReturn(List.of(
+                account(500, AccountType.BUSINESS, proprietor),
+                account(42, AccountType.PERSONAL, proprietor)));
+        when(treasuryApi.getBalanceByAccountId(42)).thenReturn(new BigDecimal("10.50"));
+
+        assertEquals(10.50, provider.getBalance(proprietor));
+    }
+
+    @Test
+    void balanceWithNoAccounts_isZeroAndCreatesNothing() {
+        UUID stranger = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(stranger)).thenReturn(List.of());
+
+        assertEquals(0.0, provider.getBalance(stranger));
+        // A balance read must never have the side effect of opening an account.
+        verify(treasuryApi, never()).resolveOrCreatePersonal(stranger);
+    }
+
+    @Test
+    void balanceOfNull_isZero() {
+        UUID owner = UUID.randomUUID();
+        when(treasuryApi.getAccountsByOwner(owner)).thenReturn(List.of(
+                account(42, AccountType.PERSONAL, owner)));
+        when(treasuryApi.getBalanceByAccountId(42)).thenReturn(null);
+
+        assertEquals(0.0, provider.getBalance(owner));
+    }
 }