From 24427231a5edf3c24a17ac514ce41d8a8f8c2b97 Mon Sep 17 00:00:00 2001 From: Rian Errity Date: Fri, 29 May 2026 20:44:00 +0000 Subject: [PATCH] Fix rental/sale income leaking into business accounts TreasuryEconomyProvider.resolveRecipientAccount resolved every payment recipient by preferring GOVERNMENT > BUSINESS > PERSONAL among the accounts that UUID owns. Because a firm's BUSINESS account is owned by the proprietor's own player UUID, any landlord who also runs a firm had their rent (and sale/auction/refund proceeds) silently routed into the business account instead of their personal balance. Prefer the recipient's PERSONAL account first, falling back to the prior GOVERNMENT > BUSINESS > first-available ordering only when no personal account exists. This is safe given Treasury's invariants: PERSONAL accounts are one-per-real-player, and GOVERNMENT/SYSTEM accounts are owned by the virtual UUID (0,0), never a player. So player landlords are paid personally while synthetic authority UUIDs (no personal account) still route to their government treasury account. Also: - Add TreasuryEconomyProviderTest covering the three routing cases. - Add treasury-api to the test classpath (was compileOnly, so tests couldn't reference Treasury types). - Fix RealtyPaperApiImplTest, which no longer compiled: ExecutorState gained a third component (networkExec) but the test passed only two. Co-Authored-By: Claude Opus 4.8 (1M context) --- realty-paper/build.gradle.kts | 1 + .../economy/TreasuryEconomyProvider.java | 31 ++++-- .../realty/api/RealtyPaperApiImplTest.java | 2 +- .../economy/TreasuryEconomyProviderTest.java | 95 +++++++++++++++++++ 4 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java diff --git a/realty-paper/build.gradle.kts b/realty-paper/build.gradle.kts index 265c648..c2f5f11 100644 --- a/realty-paper/build.gradle.kts +++ b/realty-paper/build.gradle.kts @@ -30,6 +30,7 @@ dependencies { implementation("org.spongepowered:configurate-yaml:4.2.0") testImplementation("io.papermc.paper:paper-api:1.21.8-R0.1-SNAPSHOT") + testImplementation("net.democracycraft:treasury-api:2.0.0") testImplementation("org.mockito:mockito-core:5.15.2") testImplementation("org.mockito:mockito-junit-jupiter:5.15.2") testImplementation("com.github.MilkBowl:VaultAPI:1.7") { diff --git a/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java b/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java index 970796b..1294cd9 100644 --- a/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java +++ b/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java @@ -16,10 +16,11 @@ * in the player's Treasury transaction history. *

* Account resolution: the payer is always resolved as a personal account - * (created with starting balance if missing). The recipient is resolved - * by preferring any non-personal account (government/business) owned by - * that UUID, falling back to a personal account. This correctly handles - * authority UUIDs that correspond to government Treasury accounts. + * (created with starting balance if missing). The recipient is resolved by + * preferring its PERSONAL account — rental/sale income belongs to the + * landlord personally, even if they happen to own a firm. Only when the + * recipient has no personal account (a synthetic authority UUID backing a + * government entity) do we fall back to a government/business account. */ public final class TreasuryEconomyProvider implements EconomyProvider { @@ -73,18 +74,28 @@ public boolean hasLedgerSupport() { } /** - * Resolves the recipient's Treasury account. Prefers a government or business - * account when one exists (e.g. for authority/landlord UUIDs tied to a town - * or government entity), falling back to a personal account. + * Resolves the recipient's Treasury account. + *

+ * A real player landlord always owns a PERSONAL account (Treasury enforces + * one per player), so we prefer it: their rental/sale income must land in + * their personal balance, never in 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). + *

+ * Only when the recipient has no personal account — i.e. a synthetic + * authority UUID that backs a government entity — do we fall back to the + * prior government > business > first-available ordering so authority + * payments still route to the configured government treasury account. */ private @NotNull Account resolveRecipientAccount(@NotNull UUID ownerUuid) { List accounts = treasuryApi.getAccountsByOwner(ownerUuid); if (!accounts.isEmpty()) { - // Prefer government > business > personal so that authority accounts - // correctly route funds to the configured government treasury account. return accounts.stream() - .filter(a -> a.getAccountType() == AccountType.GOVERNMENT) + .filter(a -> a.getAccountType() == AccountType.PERSONAL) .findFirst() + .or(() -> accounts.stream() + .filter(a -> a.getAccountType() == AccountType.GOVERNMENT) + .findFirst()) .or(() -> accounts.stream() .filter(a -> a.getAccountType() == AccountType.BUSINESS) .findFirst()) diff --git a/realty-paper/src/test/java/io/github/md5sha256/realty/api/RealtyPaperApiImplTest.java b/realty-paper/src/test/java/io/github/md5sha256/realty/api/RealtyPaperApiImplTest.java index b04308b..c9bd704 100644 --- a/realty-paper/src/test/java/io/github/md5sha256/realty/api/RealtyPaperApiImplTest.java +++ b/realty-paper/src/test/java/io/github/md5sha256/realty/api/RealtyPaperApiImplTest.java @@ -75,7 +75,7 @@ class RealtyPaperApiImplTest { @BeforeEach void setUp() { signCache = new SignCache(); - ExecutorState executorState = new ExecutorState(Runnable::run, sameThreadExecutorService()); + ExecutorState executorState = new ExecutorState(Runnable::run, sameThreadExecutorService(), sameThreadExecutorService()); api = new RealtyPaperApiImpl(realtyApi, economyProvider, executorState, database, regionProfileService, signTextApplicator, signCache); diff --git a/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java b/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java new file mode 100644 index 0000000..28412d7 --- /dev/null +++ b/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java @@ -0,0 +1,95 @@ +package io.github.md5sha256.realty.economy; + +import net.democracycraft.treasury.api.TreasuryApi; +import net.democracycraft.treasury.model.economy.Account; +import net.democracycraft.treasury.model.economy.AccountType; +import net.democracycraft.treasury.model.economy.TransferRequest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.math.BigDecimal; +import java.util.List; +import java.util.UUID; + +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.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class TreasuryEconomyProviderTest { + + @Mock + private TreasuryApi treasuryApi; + + private TreasuryEconomyProvider provider; + + private final UUID payer = UUID.randomUUID(); + + @BeforeEach + void setUp() { + provider = new TreasuryEconomyProvider(treasuryApi); + } + + private Account account(int id, AccountType type, UUID owner) { + Account a = new Account(); + a.setAccountId(id); + a.setAccountType(type); + a.setOwnerUuid(owner); + return a; + } + + private int capturedDestination(UUID recipient) { + Account payerPersonal = account(1, AccountType.PERSONAL, payer); + when(treasuryApi.resolveOrCreatePersonal(payer)).thenReturn(payerPersonal); + when(treasuryApi.transfer(any())).thenReturn(99L); + + PaymentResult result = provider.transfer(payer, recipient, 50.0, "Rental Payment: REGION"); + assertInstanceOf(PaymentResult.Success.class, result); + + ArgumentCaptor req = ArgumentCaptor.forClass(TransferRequest.class); + verify(treasuryApi).transfer(req.capture()); + assertEquals(payerPersonal.getAccountId(), req.getValue().fromAccountId()); + assertEquals(BigDecimal.valueOf(50.0), req.getValue().amount()); + return req.getValue().toAccountId(); + } + + @Test + void landlordWithFirm_routesToPersonalNotBusiness() { + UUID landlord = UUID.randomUUID(); + // Landlord is a firm proprietor: owns both their PERSONAL account and a + // firm BUSINESS account (which is owned by their own UUID). + when(treasuryApi.getAccountsByOwner(landlord)).thenReturn(List.of( + account(500, AccountType.BUSINESS, landlord), + account(42, AccountType.PERSONAL, landlord))); + + assertEquals(42, capturedDestination(landlord), + "rent must land in the landlord's personal account, not their firm"); + } + + @Test + void authorityUuid_withOnlyGovernmentAccount_routesToGovernment() { + UUID authority = UUID.randomUUID(); + // Synthetic authority/government entity: no personal account exists. + when(treasuryApi.getAccountsByOwner(authority)).thenReturn(List.of( + account(7, AccountType.GOVERNMENT, authority))); + + assertEquals(7, capturedDestination(authority), + "authority payments must still route to the government account"); + } + + @Test + void recipientWithNoAccounts_resolvesOrCreatesPersonal() { + UUID newOwner = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(newOwner)).thenReturn(List.of()); + when(treasuryApi.resolveOrCreatePersonal(newOwner)) + .thenReturn(account(88, AccountType.PERSONAL, newOwner)); + + assertEquals(88, capturedDestination(newOwner)); + } +}