# DiamondBank-OG defect chart

| Severity | Meaning |
|---|---|
| 🔴 | Critical. Freezes the economy or server, or loses, duplicates or misdirects value or items, and any player can trigger it in normal play (API consumers included) |
| 🟠 | Serious. Wrong money or state that needs an admin action, another plugin, a reload or a rare state to arm, or a feature any player hits that is broken or refuses valid input |
| 🟡 | Minor. Bounded slowdown, wrong message or audit value, API declaration mismatch, edge-input refusal |

## 1. Public API, offline-player compatibility

## 2. Public API, general correctness

| Entry point | Severity | Note |
|---|---|---|
| `J.addToPlayerBankShards`, `J.subtractFromPlayerBankShards`, `J.getBankShards`, `J.getInventoryShards`, `J.getEnderChestShards`, `J.getTotalShards`, `J.getAllShards`, `J.consumeFromPlayer`, `J.playerPayPlayer` | 🔴 server freeze | **J on the main thread deadlocks against any lock holder that is waiting for the next tick.** Every J method does `runBlocking { transactionLock.withLockSuspend(uuid) { ... } }` (`DiamondBankAPIJava.kt:40`, `76`, `155`, `169`, `226`, `285`), which parks the main thread in `Semaphore.acquire()` (`TransactionLock.kt:28`). Every holder that touches an inventory releases only after `runOnMainThread` (`MainThreadBlock.kt:14-26`), which needs the next scheduler heartbeat: `/deposit`, `/withdraw`, `/pay`, `/compress` (`Deposit.kt:82`, `Withdraw.kt:85`, `Pay.kt:95`, `Compress.kt:54`), `AutoCompress` (`AutoCompress.kt:39`), K and J `consumeFromPlayer` and `playerPayPlayer` called off the main thread, and the recounts in `onPlayerJoin`, `onPlayerQuit`, `onPlayerRespawn`, `onEntityPickupItem`, `onPlayerDropItem`, `onInventoryClose` (`Events.kt:80`, `105`, `133`, `190`, `233`, `383`). The pickup and drop recounts run for every item a player picks up or drops (`Events.kt:184-197`, `231-239`), so the window opens constantly. Repro: a plugin's command executor calls `J.getBankShards` for a player in the tick that player drops any item: the main thread parks, the recount never gets its tick, and the watchdog kills the server. The KDoc documents blocking, synchronous use (`:25-26`). *Fix: when `Bukkit.isPrimaryThread()`, acquire the lock in a polling loop that drains queued `runOnMainThread` blocks from a plugin-owned queue instead of `acquire()`.* |
| `K.addToPlayerBankShards`, `K.subtractFromPlayerBankShards`, `K.consumeFromPlayer`, `K.playerPayPlayer`, `K.getBankShards`, `K.getInventoryShards`, `K.getEnderChestShards`, `K.getTotalShards`, `K.getAllShards`, `K.getBaltop` under caller cancellation, and the same J methods when the calling thread is interrupted | 🟠 economy disabled without a database error | **Caller cancellation at a database await is classified as a database error and disables the economy.** The `PostgreSQL` methods catch `Exception`, including the `CancellationException` thrown by a cancelled await (`PostgreSQL.kt:97`, `119`, `201`, `231`, `266`, `279`, `305`, `334`, `425`, `444`; kotlinx-coroutines 1.10.2 `future/Future.kt:148-154`, `168-173`), and return `DatabaseException`. K then calls `handleError` (`DiamondBankAPIKotlin.kt:42`, `47`, `77`, `82`, `111`, `129`, `142`, `176`, `184`, `241`, `251`) and so does J (`DiamondBankAPIJava.kt:46`, `51`, `83`, `88`, `158`, `180`, `201`, `237`, `245`, `299`, `309`), although no database error occurred. Interrupting J's `runBlocking` caller cancels its coroutine (`Builders.kt:93-98`). Repro: `withTimeout(1) { api.getBankShards(uuid) }` while an uncached UUID's database read is pending disables the economy for everyone. *Fix: rethrow `CancellationException` before every generic catch in `PostgreSQL`.* |
| K argument contracts and K/J exception documentation: `K.addToPlayerBankShards`, `K.subtractFromPlayerBankShards`, `K.consumeFromPlayer`, `K.playerPayPlayer`, `K.getBaltop`, `J.consumeFromPlayer`, `J.playerPayPlayer`, `J.diamondsToShards` | 🟡 API contract mismatch | **K argument validation escapes its Result contract, J omits checked exceptions, and one KDoc names a nonexistent failure.** (a) `require(shards <= Long.MAX_VALUE.toULong())` (`DiamondBankAPIKotlin.kt:33`, `67`, `161`, `221`) and `require(offset >= 0)` (`CachingBalanceManager.kt:180`, reached from `DiamondBankAPIKotlin.kt:141`) throw `IllegalArgumentException` instead of returning K's promised `Result` (`api/README.md:29`). (b) J rethrows `InsufficientBalanceException` (`DiamondBankAPIJava.kt:244`, `308`, and `240`, `302`, which rethrow every non-database failure of `consume`) that its `@Throws` (`220`, `273-278`) and the README omit; reachable via a negative receiver bank (`PostgreSQL.kt:143-144`), via the excess-change credit on a negative bank (`InventorySnapshotUtils.kt:41-43`, `CachingBalanceManager.kt:67-68`), or split caches after reload. (c) `MoreThanOneDecimalDigitRuntimeException` extends the checked `DiamondBankException` (`DiamondBankException.kt:20`, base `Exception` at `:3`) and `J.diamondsToShards` has no `@Throws` (`DiamondBankAPIJava.kt:335`, `343`), so `catch (MoreThanOneDecimalDigitRuntimeException e)` does not compile in Java and `catch (RuntimeException e)` misses it. (d) `K.playerPayPlayer` KDoc (`DiamondBankAPIKotlin.kt:206-208`) names `CouldNotRemoveEnoughException.notRemoved` and tells callers to continue with a partial amount; the real failure is `InsufficientFundsException.short` and nothing moved. Repro: `K.getBaltop(-1)` throws `IllegalArgumentException` instead of returning a `Result`. *Fix: return `Result.failure(InvalidArgumentException())` for K's four shard validations and validate K's offset into a failure before the manager call at `DiamondBankAPIKotlin.kt:141`; keep these caller errors out of `handleError`; add `InsufficientBalanceException` to both J `@Throws` lists and the README; make `MoreThanOneDecimalDigitRuntimeException` extend `DiamondBankRuntimeException`; correct the KDoc.* |

## 3. Commands

| Entry point | Severity | Note |
|---|---|---|
| `/setbankbalance`, `/setbankbal` | 🟠 negative balance accepted | **Any negative shard count is stored, and nothing downstream expects one.** `SetBankBalance.kt:68` parses with `toLong()` and `74` stores it. With `-900`: `/withdraw all` takes the negative balance as the amount (`Withdraw.kt:99-104`), `subtractFromBankShards` negates it into a credit (`Withdraw.kt:144`, `CachingBalanceManager.kt:67`, `85-86`), the inventory change is skipped because `addItem` ignores non-positive stacks (`Withdraw.kt:123-131`), the bank returns to 0 and the player reads "Successfully withdrew -100.0 Diamonds" (`Withdraw.kt:161-169`); `/pay` and `playerPayPlayer` demand `shards + 900` from the inventory (`CommonOperations.kt:29-41`), and if it covers that, the negative amount hits `require(shardsToSubtractFromSender >= 0)` (`CachingBalanceManager.kt:94`) with any excess-change credit (`InventorySnapshotUtils.kt:41`, `67`, `74`) already committed. A negative receiver also arms the half-transfer in `PostgreSQL.internalAddToPlayerShards`. Repro: `/setbankbal Alice -900`, then Alice runs `/withdraw all`. *Fix: reject `balance < 0` after the parse at `SetBankBalance.kt:68`.* |
| `/withdraw`, `/pay` | 🟠 economy disabled on a refusal | **A balance refusal from the database pre-check is handled as a database error, so `/withdraw` and `/pay` disable the economy whenever the cached balance is higher than the stored one.** `Withdraw.kt:144-145` and `Pay.kt:141-142` pass every failure of `subtractFromBankShards` and `transferBankShards` to `handleError`, including the `InsufficientBalanceException` that `CachingBalanceManager.kt:67-68` and `104-105` return when the stored balance does not cover the amount; K and J treat that exception as a caller error (`DiamondBankAPIKotlin.kt:76`, `183`, `250`, `DiamondBankAPIJava.kt:82`, `244`, `308`). The commands' own check reads the cached balance (`Withdraw.kt:89-90`, `CommonOperations.kt:25`), which is higher than the stored one after a reload (the commands keep the old manager), after a failed transfer leaves the sender's cache stale, or when `/setbankbal`, which takes no transaction lock (`SetBankBalance.kt:74`), lowers the balance during the command. Repro: after `/diamondbankreload`, another plugin debits 90 of a player's 100 bank shards through `J.subtractFromPlayerBankShards`; the player runs `/withdraw 5`: the economy is disabled. *Fix: at `Withdraw.kt:144` and `Pay.kt:141`, answer `InsufficientBalanceException` with the insufficient-funds reply and refresh the cached balance instead of calling `handleError`.* |
| `/compress`, `AutoCompress.compress` (from `onEntityPickupItem`, `onInventoryClose`) | 🟡 wrong refusal | **The space check runs before the shards it will convert are removed, so it ignores the slots they free.** `Compress.kt:131-147` (diamonds) and `149-165` (blocks) check before the removals at `169-183` and `198-212`; `AutoCompress.kt:65-99` repeats the same check before `101-157`. Repro: 35 slots of anything, 18 Diamond Shards in one slot, occupied offhand: "You do not have enough space" although the freed slot fits 2 diamonds; with auto-compress on, the message repeats on every item pickup and container close. *Fix: in both places compute free space after applying the pending removals to a copy, or run the `addItem` calls first and abort on leftover; share the routine.* |
| `/deposit all`, `/withdraw all` | 🟡 wrong audit value | **Both log `-1`, the `all` sentinel, as the transferred amount.** `Deposit.kt:148` passes `shards` (`50`); `Withdraw.kt:171` passes `shards` (`53`). Repro: `/deposit all` with 5 diamonds: the audit row reads `-1`. *Fix: pass `removedInShards` at `Deposit.kt:148` and `shardsToWithdraw` at `Withdraw.kt:171`.* |
| `/balancetop <page>`, `/baltop <page>` | 🟡 no reply or wrong page | **Unchecked page bounds and Int offset arithmetic cause exceptions or nonsensical negative-page replies.** `Balancetop.kt:58-59` parses the page and computes `offset = 9 * (index - 1)` in `Int`. Page 0, ordinary negative pages, and positive pages whose offsets wrap negative, such as 238609296, reach `require(offset >= 0)` (`CachingBalanceManager.kt:180`) through `Balancetop.kt:113-114`, throw `IllegalArgumentException` and kill the coroutine without a reply. Other negative pages wrap to positive offsets: -238609295 produces 2147483632, passes the offset check and the upper-page check (`Balancetop.kt:133`), and displays a negative page number. Repro: `/baltop 0`: no reply, a stack trace in the console. *Fix: at `Balancetop.kt:58-59` reject `index < 1` with the existing page message, compute the offset as `Long`, and refuse pages beyond the last before querying.* |
| `/pay` | 🟡 documented `all` refused | **`README.md:19` promises `/pay <player> all`; `Pay.kt:69` parses with `toFloat()` and answers "Invalid argument".** Repro: `/pay Bob all`. *Fix: accept `all` at `69` (pay the full bank balance) or drop it from the README.* |

## 4. Event listeners and automatic features

| Entry point | Severity | Note |
|---|---|---|
| `AutoDeposit.deposit` via `onEntityPickupItem` | 🟠 item loss on database error | **The picked-up stack is deleted before the credit, so a failed credit loses it.** `AutoDeposit.kt:39-40` empties the item entity on the main thread (the entity is discarded on its next tick, `ItemEntity.java:130-131`), and `42-53` credit the bank later on a coroutine; when `addToBankShards` fails (`AutoDeposit.kt:44`), `handleError` disables the economy and the player is told to notify staff, but the stack is never given back. Repro: auto-deposit on, stop PostgreSQL, walk over 64 diamonds: the economy disables and the 64 diamonds are gone. *Fix: keep a copy of the stack before `39` and, in the `getOrElse` at `44-52`, drop it at the player through `runOnMainThread`.* |
| `onEntityPickupItem`, `onPlayerDropItem`, `onInventoryClose` | 🟠 interactions cancelled, commands refused, per-pickup load | **The currency test now guards only the disabled-economy branch, so every pickup or drop of any item runs the auto features, a main-thread Redis read and a locked recount, and with auto-compress on every pickup locks the whole inventory.** `Events.kt:154-173` and `205-224` test the item type only while `economyDisabled` is set; otherwise `Events.kt:180-197` and `231-239` run for cobblestone as for diamonds, including the synchronous Redis read at `Events.kt:180` (a new connection per call, see the Redis row). Each recount holds the transaction lock across a main-thread hop and a database write (`Events.kt:184-197`, `231-239`, `381-400`), and `/deposit`, `/withdraw`, `/pay` and `/compress` take that lock with `tryWithLockSuspend` (`Deposit.kt:78`, `Withdraw.kt:81`, `Pay.kt:92`, `Compress.kt:50`), so a player who is picking up items is told "You are currently blocked from using /deposit." (`Deposit.kt:155`, `Withdraw.kt:178`, `Pay.kt:198`, `Compress.kt:260`). With auto-compress on, each pickup (and each container close, `Events.kt:376-379`) runs `AutoCompress.compress`, which takes both locks and waits a tick for the snapshot even when nothing converts (`AutoCompress.kt:37-39`, `159`); while the inventory lock is held, `Events.kt:175`, `226`, `271`, `314`, `355` cancel every pickup, drop, click and block or entity interaction, and a cancelled `LEFT_CLICK_BLOCK` makes the server resend the block (`ServerPlayerGameMode.java:198-209`), so the dig is lost. A player with auto-deposit or auto-compress on who lacks `diamondbank-og.deposit` or `diamondbank-og.compress` is messaged on every pickup (`AutoDeposit.kt:22-25` before the type check at `28-37`; `AutoCompress.kt:30-34`). Repro: `/autocompress` on, mine a row of stone with an Efficiency V pickaxe: blocks whose break starts while the previous drop is being picked up reappear. *Fix: repeat the four-type currency test after the lock checks at `Events.kt:178` and `229` and return for any other item; in `AutoCompress.compress` decide on the main thread, from the live inventory, whether anything can be compressed, and take no lock when nothing can.* |
| `onPlayerJoin` | 🟠 legacy balance lost, false message, migration rounding | **Migration runs only for a player without a database row, so a legacy balance is skipped for good once anything creates that row, every new player is told a balance was migrated, and a fractional legacy balance is truncated.** (a) `Events.kt:56` gates the migration on `!hasEntry`. Paying an offline legacy player passes `hasPlayedBefore()` (`Pay.kt:58`) and the transfer's upsert inserts the receiver row (`PostgreSQL.kt:136-137` via `transferBankShards`), as does an API credit; at that player's next join `hasEntry` is true and `diamondbank:balance` is never read. (b) The key removal and the message at `Events.kt:70-75` sit outside `if (legacyBalance != null)` (`62-69`), so every first join prints "Your old balance has been successfully migrated". (c) The legacy value is a `DOUBLE` (`Events.kt:57-60`) converted with `legacyBalance.toLong() * 9` (`64`), so `10.7` becomes 90 shards, not 97 (`CommonOperations.kt:48-53`). Repro: for a player whose data still holds `diamondbank:balance` and who has not joined since the migration shipped, `/pay <them> 0.1`; after they join their bank holds 1 shard. *Fix: at `56` gate on the presence of the legacy key instead of `hasEntry`, credit it with `addToBankShards` instead of `setPlayerShards`, convert at `64` with `diamondsToShards`, and move the removal and the message inside the `legacyBalance != null` branch.* |

## 5. Startup, shutdown, reload, configuration, and persistence setup

| Entry point | Severity | Note |
|---|---|---|
| `DiamondBankReload` (`/diamondbankreload`) and the executors it leaves behind (`/deposit`, `/withdraw`, `/pay`, `/compress`, `/balance`, `/bal`, `/balancetop`, `/baltop`, `/setbankbalance`, `/setbankbal`) | 🟠 stale balances after reload, pool leak, partial reload | **Reload leaves command executors bound to stale state, leaks the previous pool, and keeps the old Redis configuration.** (a) Seven command executors, behind ten command labels, keep the manager and config that existed at enable, while events, auto features and both APIs move to the new manager. `Deposit.kt:22-23`, `Withdraw.kt:25-26`, `Pay.kt:23-24`, `Compress.kt:33-34`, `Balance.kt:18-19`, `Balancetop.kt:20-21`, `SetBankBalance.kt:20-21` take `DiamondBankOG.config` and `DiamondBankOG.balanceManager` as constructor defaults, evaluated once by `setUpCommands` (`DiamondBankOG.kt:103`, `124-134`). The reload swaps the companion (`DiamondBankReload.kt:23`, `35`) without calling `setUpCommands` or `shutdown()`, so the two caches never see each other's writes and the old pool lives until restart, one more per run. Repro: a player online since before the reload has 100 bank shards; another plugin debits 90 through `J.subtractFromPlayerBankShards`; the player runs `/balance`: it still shows 100, and `/withdraw 5` then passes the stale check at `Withdraw.kt:106` (see the `/withdraw`, `/pay` row). (b) Redis is never rebuilt. `redis` is created once (`DiamondBankOG.kt:86`) and reads the config only at construction (`Redis.kt:9-15`); after editing `config.yml:14-17` the command still says "Successfully reloaded the config." (`DiamondBankReload.kt:47`). *Fix: after the swap at `DiamondBankReload.kt:35-45`, call `plugin.setUpCommands()` so the executors bind the new manager and config; shut the previous manager down only after the transactions running against it finish; rebuild `redis` (shutting down the old client) in the same command.* |
| `Redis.getValue`, `Redis.setValue` via `onEntityPickupItem`, `onInventoryClose`, `/autocompress`, `/autodeposit` | 🟠 main-thread stall per pickup, exception on outage, half-applied toggle | **Every call opens a new connection and catches nothing, the pickup handler makes that call on the main thread, and the toggle commands write their two keys separately.** `Redis.kt:28-33` and `36-41` connect, handshake, `SELECT`, run one command and close, on every call. Main-thread callers: `Events.kt:180` (each pickup that reaches the auto-deposit check), `command/AutoCompress.kt:33-47`, `command/AutoDeposit.kt:33-47`. Async callers: `Events.kt:185`, `377`. When Redis is down, `RedisConnectionException` escapes the pickup handler (the pickup completes and the recount at `184-197` is skipped), kills the `onInventoryClose` recount coroutine (`376-401`), and reaches the two commands as "An internal error occurred"; a failure between the two `setValue` calls (`:45`, `:47` in both commands) clears one toggle without setting the other. When the host is unreachable, each pickup costs a 10 s connect timeout on the main thread (`SocketOptions.java:37`). A throw after `connect()` leaks the connection. Repro: drop routing to the Redis host, then pick up a diamond: the server stalls 10 s. *Fix: use a managed long-lived connection and return explicit command failures separately from missing keys; stop and report a failed toggle at `command/AutoCompress.kt:33-48` and `command/AutoDeposit.kt:33-48` instead of continuing to a success message; serve the read at `Events.kt:180` from a cache or off the main thread; move both toggle commands' Redis work into `scope.launch` and write the mutually exclusive key pair in one `MSET`.* |
| Recipes across `/minecraft:reload`, and the craft guard | 🟠 shard recipes lost, currency crafted away | **Recipes are registered only in `onEnable`, so a datapack reload removes them until restart, and the craft guard misses prismarine bricks.** `Shard.kt:33`, `39` add both recipes once (`DiamondBankOG.kt:105`); `/minecraft:reload` installs a fresh `RecipeManager` (`MinecraftServer.java:2086`) without the plugin recipes and nothing re-registers them, so diamonds no longer split into shards and 9 Diamond Shards in a 3x3 craft vanilla prismarine bricks, which `onPrepareItemCraft` does not block (`Events.kt:408-410` list only prismarine, dark prismarine, sea lantern). The bricks recipe also wins without a reload whenever one shard in the grid was renamed, because the plugin recipe's exact ingredient compares full NBT (`Ingredient.java:85-86`). Repro: `/minecraft:reload`, then 9 Diamond Shards in a crafting table: prismarine bricks. *Fix: re-register both recipes on `ServerResourcesReloadedEvent`; add `PRISMARINE_BRICKS` to the list at `Events.kt:408-410`.* |
| `onEnable` and `DiamondBankReload` configuration loading (`YamlConfig.create`, `PostgreSQL.create`) | 🟠 plugin disabled with valid configuration | **Configuration loading omits upgrade defaults and environment expansion, rejects numeric credentials, and refuses pre-Flyway databases.** (a) `saveDefaultConfig` (`YamlConfig.kt:23`) leaves existing files untouched (`JavaPlugin.java:194-197`), and `YamlConfig.kt:25` loads without defaults. A deployed config predating `postgresMaxActiveConnections` lacks that key; `parseKeyAs<Int>` returns null (`YamlConfig.kt:71-76`), the guard at `47` rejects it, and `onEnable` disables the plugin (`DiamondBankOG.kt:70-75`). (b) `YamlConfig.kt:25`, `72` never resolves the `^{ENV}` placeholders at `config.yml:9-10`, `17`. Bukkit reads and constructs the scalars unchanged (`FileConfiguration.java:123-128`, `152-160`, `YamlConfiguration.java:149-164`, `YamlConstructor.java:25-27`), so even defined environment variables reach PostgreSQL and Redis as literal placeholder credentials (`PostgreSQL.kt:37-38`, `49`, `Redis.kt:12`), causing authentication failure and plugin disable. Numeric literal credentials are also rejected by `as? String` (`YamlConfig.kt:72`); an all-digit password parsed as an Integer becomes null and the connection is attempted without it. (c) `PostgreSQL.kt:40-41` configures Flyway without a baseline; a database created by a pre-Flyway build has the existing `diamond` and `diamond_log` tables but no schema history, so migration fails and `PostgreSQL.create` returns null (`53-59`). Repro: start with the bundled placeholder credentials and the corresponding environment variables set: the literal placeholder is used as the PostgreSQL user. *Fix: attach the bundled YAML as defaults after `YamlConfig.kt:25`, use typed scalar getters that stringify credentials, and resolve `^{ENV}` in parsed string values before constructing `YamlConfig`; report missing variables explicitly; at `PostgreSQL.kt:40` baseline at version 1 only after validating the known existing schema and confirming no history table exists.* |
| `plugin.yml` | 🟡 wrong help text | **The debug command help names a nonexistent command.** `plugin.yml:56` gives `/dbtoggledebug` as the usage for `dbogtoggledebug`, so `/help dbogtoggledebug` shows the wrong command. Repro: `/help dbogtoggledebug`. *Fix: write `/dbogtoggledebug` at `56`.* |

## 6. Shared internals

| Component | Severity | Note |
|---|---|---|
| `TransactionLock.withLockSuspend` | 🔴 economy freeze, shutdown hang | Reached from: every K balance method except `getBaltop`, every J balance method except `getBaltop` when called from a `Dispatchers.Default` thread (including J calls made inside a `PlayerBalanceChangedListener`, `EventManager.kt:23-27`), `/deposit`, `/withdraw`, `/pay`, `/compress` (holders through `tryWithLockSuspend`), `onPlayerJoin`, `onPlayerQuit`, `onPlayerRespawn`, `onEntityPickupItem`, `onPlayerDropItem`, `onInventoryClose`, `AutoCompress.compress`, `AutoDeposit.deposit`, `onDisable`. **Waiting parks a `Dispatchers.Default` thread, so enough waiters starve the holder and nothing in the scope ever runs again.** `TransactionLock.kt:26-34` calls the blocking `Semaphore.acquire()` inside a suspend function; the scope is `Dispatchers.Default` (`DiamondBankOG.kt:36`), capped at `max(2, nCPU)` workers (`Tasks.kt:29-33`), and a parked thread counts as busy. Once `nCPU` waiters for one UUID are parked while the holder is suspended (next-tick `runOnMainThread` or a database await), the holder never resumes and never releases: every command, recount, K call and the jasync pool actor (also on `Dispatchers.Default`, `ActorBasedObjectPool.kt:147-148`) hang; `economyDisabled` stays false; when the stuck holder owns the inventory lock, the guards at `Events.kt:175`, `226`, `271`, `314`, `355` cancel every pickup, drop, click and interaction of that player until restart; and auto-deposit keeps emptying picked-up stacks on the main thread (`AutoDeposit.kt:39-40`) while their credits wait forever (`AutoDeposit.kt:43`). The pickup and drop handlers launch a waiter for every item of any type (`Events.kt:184-197`, `231-239`), two per pickup with auto-compress on (`Events.kt:186`, `AutoCompress.kt:24`, `37`). `/stop` then hangs: `onDisable` joins the scope (`DiamondBankOG.kt:150`), a waiter blocked in `acquire()` ignores cancellation, the watchdog is already stopped (`MinecraftServer.java:931`), and players are never saved (`:971`); the join hangs even with a single waiter when the holder is an outside K or J call suspended in `runOnMainThread`, because the main thread is inside `onDisable`. Repro on 8 cores: break a chest holding 27 different stacks and stand in the drops so that nine are picked up in one tick; afterwards no economy command replies and `/stop` never finishes. *Fix: replace `java.util.concurrent.Semaphore` with `kotlinx.coroutines.sync.Semaphore` or `Mutex` at `26-34` and `49-60` so waiting suspends and honors cancellation; J's `runBlocking` keeps its blocking behavior on top.* |
| `InventorySnapshot.restoreTo` and the lock guards in `Events.kt` | 🔴 item dup and loss | Reached from: `K`/`J` `consumeFromPlayer`, `K`/`J` `playerPayPlayer`, `/deposit`, `/withdraw`, `/pay`, `/compress`, `AutoCompress.compress` (from every item pickup and container close). **The restore overwrites all 41 live slots from a snapshot that can span ticks, while incomplete guards allow intervening item changes.** `InventorySnapshot.kt:81` (`player.inventory.contents = newContents`) never compares the live inventory with what `from` copied. Commands and auto features take and restore snapshots across main-thread hops (`MainThreadBlock.kt:14-26`); bank-backed operations restore after their bank write, while compression rewrites without one (`Compress.kt:242`, `AutoCompress.kt:159`). The guards cancel pickups, drops, clicks and block and entity interactions while locked (`Events.kt:175`, `226`, `271`, `314`, `355`), but: `onPlayerInteract` returns at `283` before the lock check when no block was clicked, so a right-click at air still throws ender pearls, snowballs, splash potions and experience bottles (`ServerGamePacketListenerImpl.java:2098-2101`); armor stand swaps fire `PlayerInteractAtEntityEvent`, which keeps its own handler list (`PlayerInteractAtEntityEvent.java:21`, `ServerGamePacketListenerImpl.java:2955-2959`); a recipe book click moves ingredients into the crafting grid and fires only `PlayerRecipeBookClickEvent` (`ServerGamePacketListenerImpl.java:3397-3401`); releasing a bow or trident, finishing food or a potion, a totem pop and dying are not cancelled, and a death clears the same `ServerPlayer` that the restore then refills and respawn reuses (`ServerPlayer.java:992-998`, `PlayerList.java:815`); every handler returns before its lock check outside the three vanilla worlds (`Events.kt:152`, `203`, `246`, `281`, `324`). Each leaves the item both in the world or the grid and back in its slot. `InventoryDragEvent` is unguarded, so a stack already on the cursor dragged into the inventory is overwritten; arrow and trident pickups fire only `PlayerPickupArrowEvent`, which extends `PlayerPickupItemEvent` rather than `EntityPickupItemEvent` (`AbstractArrow.java:565-580`), so the guard at `Events.kt:175` never sees them and the restore deletes the picked-up item; items another plugin adds in the window are deleted the same way. For offline `consumeFromPlayer` the restore writes the stored profile through `UtilitiesOG.setInventoryData` (`InventorySnapshot.kt:78-79`), which throws when TrueOG Purpur rejects the RocksDB write (`PlayerDataStorage.java:181-192`): the bank debit and any excess-change credit stay committed, the profile keeps every removed item, and the audit row (`DiamondBankAPIKotlin.kt:195`, `DiamondBankAPIJava.kt:252`) is skipped. With `players.disable-saving` enabled (`SpigotConfig.java:406-408`), the same offline restore silently writes nothing (`PlayerDataStorage.java:183`), although Utilities-OG returns true (`UtilitiesOG.java:627-628`); consuming from a previously saved offline inventory can therefore report success without removing its currency. Repro: spam `/deposit 0.1` while a repeating command block runs `/kill <player>`: the death pile and the restored inventory both exist. *Fix: replace the unconditional write at `InventorySnapshot.kt:78-82` with live-slot reconciliation and apply only changed slots on the main thread; for bank-backed operations apply the item change before the bank write and safely return the removed items if that write fails, preserving intervening slot changes; refuse offline rewriting when playerdata saving is disabled, write the profile before the bank transaction, and confirm the persisted inventory instead of trusting the helper boolean; skip unchanged snapshots.* |
| `CommonOperations.getPlayerUsingUuidOrName` and its callers' existence check | 🔴 wrong recipient, new players unpayable, main-thread stall | Reached from: `/pay`, `/balance <player>`, `/bal <player>`, `/balancetop <player>`, `/baltop <player>`, `/setbankbalance`, `/setbankbal`. **Name resolution prefix-matches online players, then treats a first-session player as nonexistent, and `/pay` runs it on the main thread.** (a) `CommonOperations.kt:68` calls `Bukkit.getPlayer(String)`, which returns the online player whose name merely starts with the argument (`CraftServer.java:619-641`). Repro: Bob offline, Bobby online, `/pay Bob 10` pays Bobby; `/setbankbal Bob 0` zeroes Bobby. (b) Each caller then uses `hasPlayedBefore()` as the existence test (`Pay.kt:58`, `Balance.kt:66`, `Balancetop.kt:62`, `SetBankBalance.kt:61`); for an online player that is `CraftPlayer.hasPlayedBefore`, a field set only by `readExtraData` when saved data existed at login (`CraftPlayer.java:2110-2111`, `ServerPlayer.java:497`), so a player in their first session gets "That player doesn't exist" from all four commands until they relog. (c) `Pay.kt:52` resolves before `scope.launch` (`90`); on a user-cache miss `Bukkit.getOfflinePlayer(String)` performs a synchronous Mojang profile lookup (`CraftServer.java:1914-1940`), stalling the server for an unknown name. *Fix: use `Bukkit.getPlayerExact(value)` at `68` before the `getOfflinePlayer` fallback; in the four callers accept the target when it `is Player` and apply `hasPlayedBefore()` only to the `OfflinePlayer` fallback; move `Pay.kt:52` into the `scope.launch` block.* |
| `PostgreSQL.internalAddToPlayerShards` | 🟠 half-transfer loss | Reached from: `/pay`, `K`/`J` `playerPayPlayer` (via `transferBankShards`). **A nested `inTransaction` on the caller's connection commits the outer transaction early.** When the guarded upsert (`PostgreSQL.kt:136-137`) returns no row, `:143` calls `internalGetShardTypeShards(uuid, type, conn)`, which runs `conn.inTransaction` (`:290`); jasync sends `BEGIN`, the query and `COMMIT` on that connection (`ConcreteConnectionBase.kt:22-31`). PostgreSQL 16 warns "there is already a transaction in progress", commits the outer work, and answers the outer `ROLLBACK` with "there is no transaction in progress". In `transferBankShards` the writes are ordered by UUID (`PostgreSQL.kt:191`) and only the sender is pre-checked (`CachingBalanceManager.kt:100-106`), so a receiver with a negative bank fails the guard after the sender's debit is committed: the sender loses the amount, the receiver gets nothing, the sender's cache keeps the old balance (`CachingBalanceManager.kt:113` runs only on success), `/pay` disables the economy (`Pay.kt:142`), and `K.playerPayPlayer` returns `InsufficientBalanceException`, which reads as "nothing moved". A negative bank can be set through `/setbankbalance` (`SetBankBalance.kt:68`, `74`). Repro: `/setbankbal Bob -900`, then `/pay Bob 1` from a sender whose UUID sorts first: the sender's bank drops by 9 and Bob's stays at -900. *Fix: at `143` run the balance `SELECT` on `conn` with `sendPreparedStatement` instead of a nested `inTransaction`, so the guard failure rolls back inside the outer transaction; optionally pre-check the receiver in `CachingBalanceManager.transferBankShards`.* |
| `PostgreSQL.getBaltopWithUuid` | 🟠 dead feature | Reached from: `/balancetop <player>`, `/baltop <player>`. **The query is a syntax error.** `PostgreSQL.kt:356-361`: the `target AS (...)` CTE is closed at `357`, `paged AS` follows at `358` with no comma and no opening parenthesis, and a stray `)` remains; PostgreSQL 16 answers `syntax error at or near "paged"`. Every call returns `DatabaseException`, and the command replies "Something went wrong while trying to get the information for balancetop." (`Balancetop.kt:75-80`). Repro: `/baltop Steve` for any player who has joined. *Fix: write `target AS (...), paged AS (...)` at `357-358`.* |
| `InventoryExtensions.removeItemWithSameKey` on an `InventorySnapshot` | 🟠 transactions fail with offhand shards | Reached from: `/deposit all`, `/deposit <amount>`, `/pay`, `/compress`, `AutoCompress.compress`, `K`/`J` `consumeFromPlayer`, `K`/`J` `playerPayPlayer`. **The shard remover finds the slot in the 41-slot `getContents()` layout but clears it through the snapshot's 37-slot array, so emptying an offhand shard stack throws.** `InventoryExtensions.kt:44`, `51` and `67-71` read `contents`, which on `InventorySnapshot` is `getContents()` with armor at 36 to 39 and the offhand at 40 (`InventorySnapshot.kt:259-261`); `InventoryExtensions.kt:57-58` then calls `setItem(firstSlot, null)`, which writes the snapshot's own 37-slot array with the offhand at 36 (`InventorySnapshot.kt:51`, `128`). Emptying the offhand stack throws `ArrayIndexOutOfBoundsException: Index 40 out of bounds for length 37`: the coroutine dies before any bank write or restore, the player gets no reply, K throws instead of returning a `Result`, and J throws an undeclared exception. `/deposit all` always empties it (`InventorySnapshotUtils.kt:105-107`; the count includes the offhand, `PlayerInventoryExtensions.kt:10`); `/deposit <amount>`, `/pay`, the API, `/compress` and auto-compress do when the part they take from the offhand empties that stack (`InventorySnapshotUtils.kt:29`, `Compress.kt:171`, `AutoCompress.kt:103`). Repro: hold 5 Diamond Shards in the offhand, `/deposit all`: no reply, nothing deposited, a stack trace in the console. *Fix: read `storageContents` instead of `contents` at `InventoryExtensions.kt:51` and `68`; on `InventorySnapshot` that is the same 37-slot array `setItem` writes (`InventorySnapshot.kt:267`), and on a shulker inventory it is all 27 slots.* |
| `InventorySnapshotUtils.removeShards` | 🟠 excess dup on failure | Reached from: `K`/`J` `consumeFromPlayer`, `K`/`J` `playerPayPlayer`, `/pay`, `/deposit <amount>`. **Excess change is credited in its own transaction before the operation's bank write.** `InventorySnapshotUtils.kt:41`, `67` and `74` call `addToBankShards` for change that does not fit in the snapshot. If the later write fails, `restoreTo` is skipped, so the inventory keeps every item while up to 8 shards (80 after a broken diamond block) stay credited, unlogged. The later write is `Deposit.kt:121`, the transfer at `Pay.kt:134-149`, or the debit or transfer at `DiamondBankAPIKotlin.kt:182-186`, `247-252` (J at `DiamondBankAPIJava.kt:243-246`, `306-310`); it fails through a negative receiver, a stale cache after reload, or a database error. Repro: `/setbankbal Bob -900`; Alice has an empty bank and a full inventory with 2 diamonds in one slot; `/pay Bob 0.1`: Alice keeps both diamonds and her bank gains 8 shards. *Fix: return the excess together with the removed count, and let the caller's single bank write carry it.* |
| Submitted bank writes and `runOnMainThread { restoreTo }` completion | 🟠 committed money without cache or inventory completion | Reached from: caller cancellation of K `addToPlayerBankShards`, `subtractFromPlayerBankShards`, `consumeFromPlayer`, `playerPayPlayer`; interruption of the same J calls; `onDisable` during those API calls, `/deposit`, `/withdraw`, `/pay`, `/setbankbalance`, `/setbankbal`, `onPlayerJoin` migration and `AutoDeposit.deposit`. **Cancellation or shutdown can abandon cache and inventory completion after a submitted bank write still commits.** `PostgreSQL.kt:73-94`, `109-110` and `169-195` await transactions whose completion jasync owns; addition and transfer bodies also use DiamondBank's `scope.future`. Cancelling the outer awaited future does not cancel the independent inner completion chain (`AsyncObjectPool.kt:66-84`, `ConcreteConnectionBase.kt:23-34`, coroutines `future/Future.kt:168-173`). Once submitted, a successful write can commit while the caller receives failure and skips cache updates (`CachingBalanceManager.kt:50-53`, `72-76`, `108-113`). Inventory-backed commands then have a further cancellable hop (`Deposit.kt:131`, `Withdraw.kt:154`, `Pay.kt:151`), as do K/J consume and pay when called off the main thread (`DiamondBankAPIKotlin.kt:188`, `255`, `DiamondBankAPIJava.kt:249`, `313`, `MainThreadBlock.kt:11-26`). (a) `onDisable` cancels and joins the plugin scope (`DiamondBankOG.kt:148-150`); pending command restores are abandoned, and Paper cancels queued tasks after that (`PaperPluginInstanceManager.java:244`). External K/J calls are outside that scope, so a dropped scheduled task can leave them suspended or blocked holding their locks. Plugins disable before players are saved (`MinecraftServer.java:960`, `971`), so a committed deposit can leave both bank credit and the original items, while a withdrawal can debit the bank without adding items. Auto-deposit has already emptied the entity before launching its credit (`AutoDeposit.kt:39-44`), which cancellation before submission can also abandon. (b) Caller cancellation at a queued restore releases the inventory lock before the task runs; its later lock check fails (`InventorySnapshot.kt:67-69`), and the audit is skipped (`DiamondBankAPIKotlin.kt:195`, `263`, `DiamondBankAPIJava.kt:252`, `317`). Repro: run `/withdraw 10` and stop the server while its committed debit is awaiting the main-thread restore: after restart the bank is lower without the diamonds. *Fix: track in-flight plugin and API transactions, stop accepting new work at shutdown, and protect each accepted write through database outcome, cache, inventory reconciliation and audit with `withContext(NonCancellable)` beginning before submission; include auto-deposit's entity removal and credit in the tracked operation; drain pending main-thread work from a plugin-owned queue in `DiamondBankOG.kt:148-150` until those transactions finish, then close persistence and cancel idle work.* |
| Material-based currency counts versus meta-sensitive removal | 🟡 valid currency refused | Reached from: `/compress`, `/compress yes`, `AutoCompress.compress` (every item pickup and container close), `/deposit <amount>`, `/pay`, `K`/`J` `consumeFromPlayer`, `K`/`J` `playerPayPlayer`. **Currency counts include renamed Diamonds and Diamond Blocks that the removal rule refuses when they are needed for the conversion or payment.** `PlayerInventoryExtensions.kt:26`, `31` count by material, while `InventorySnapshot.kt:308-309` matches only stacks similar to the plain `ItemStack` passed to `removeItem`. If ordinary stacks cannot cover the requested removal, compression aborts (`Compress.kt:200-207`, `AutoCompress.kt:125-133`) and payment removal returns too little (`InventorySnapshotUtils.kt:36`, `62`) despite the higher count. `/compress yes` selects the shulker inventory (`Compress.kt:85-88`, `103-109`), whose counters also ignore metadata (`InventoryExtensions.kt:26-32`), while Bukkit removal uses similarity (`CraftInventory.java:232-242`, `359-383`, `ItemStack.java:302-310`). A renamed surplus item does not prevent a conversion covered by plain stacks. No value moves. Repro: rename one Diamond, hold nine Diamonds including it, and run `/compress`: the command reports "Something went wrong"; the same nine items in a held undyed shulker fail `/compress yes`. *Fix: share a material-based remover across snapshots and shulker inventories at `InventorySnapshot.kt:308-309` and `Compress.kt:200`, or make both counter implementations count only stacks accepted by the corresponding removal rule.* |
| `PostgreSQL.insertTransactionLog` and `fifo_limit_trigger` | 🟡 audit exceeds 500 rows | Reached from: K/J `addToPlayerBankShards`, `subtractFromPlayerBankShards`, `consumeFromPlayer`, `playerPayPlayer`, `/deposit`, `/withdraw`, `/pay`, `/setbankbalance`, `/setbankbal`. **Concurrent audit inserts can commit more than the guaranteed 500 rows because each trim sees only its own transaction and already committed rows.** `PostgreSQL.kt:416-424` submits each log INSERT in a separate transaction with no shared serialization; `V1__init.sql:17-22` trims using the visible row at offset 500. With 499 committed rows, two concurrent inserts each see 500 and delete nothing, leaving 501 after both commit; the configured pool permits ten concurrent connections (`config.yml:11`, `PostgreSQL.kt:50`). Repro: seed 499 audit rows, start two transactions that each insert a log row before either commits, then commit both: PostgreSQL 16 retains 501. *Fix: at `PostgreSQL.kt:417-419` acquire one common transaction-scoped advisory lock in a separate statement before INSERT, then execute INSERT and trimming in that transaction, so each INSERT starts with a snapshot taken after the previous log transaction commits.* |
| `EventManager.queueFor` | 🟡 events lost for a player | Reached from: `K`/`J` `registerEventListener`, and every balance change that emits an event (`PostgreSQL.kt:95`, `112`, `198`). **A listener that throws an `Error` ends that player's consumer coroutine, so no listener receives another event for that player until restart.** `EventManager.kt:28` catches only `Exception`; an `Error` (for example `NoClassDefFoundError` from a listener whose plugin was unloaded, since listeners cannot be unregistered, `:41-43`) ends the `for` loop at `:24`, the `SupervisorJob` absorbs it, `queueFor` keeps returning the dead channel from `computeIfAbsent` (`:21`), and `trySend` (`:38`) keeps buffering into the unlimited channel. Repro: register a listener that calls `TODO()` once, change a player's balance twice: no listener sees the second event. *Fix: at `EventManager.kt:28` catch `Throwable` and rethrow `CancellationException`, or remove the queue from `queues` when its consumer completes.* |
| `CommonOperations.diamondsToShards` | 🟡 crash on NaN, wrong or refused amounts | Reached from: `K`/`J` `diamondsToShards` (`DiamondBankAPIKotlin.kt:276`, `279`; `DiamondBankAPIJava.kt:335`, `343`), `/deposit`, `/withdraw`, `/pay`. **Diamond conversion throws on non-finite input and misreads or refuses other representable amounts.** At `CommonOperations.kt:49-53`, conversion goes through `Float.toString()`; `NaN` and `Infinity` contain no `.`, so `split[1]` at `:50` throws; both pass the `amount <= 0` checks (`Deposit.kt:55`, `Withdraw.kt:58`, `Pay.kt:70`) and are converted before `scope.launch` (`Deposit.kt:66`, `Withdraw.kt:69`, `Pay.kt:81`), so `/deposit NaN` reaches the player as "An internal error occurred", and `K.diamondsToShards(NaN)` throws instead of returning a `Result`. Amounts of 1e7 and above format as `1.0E7` and are refused as "more than one decimal digit". From 1048576 up, Float spacing changes the tenth digit silently: `/pay Bob 1048576.3` pays 1048576.2. Negative fractions convert wrongly through the API: `-0.5` yields `+5` and `-1.5` yields `-4`. The Double overloads narrow to Float first (`DiamondBankAPIKotlin.kt:276`, `DiamondBankAPIJava.kt:335`). Repro: `/withdraw NaN`. *Fix: parse numerically at `49-53`: reject non-finite input, take the whole part and the tenth digit from the original string or by `BigDecimal`, apply the sign to both, and have the Double overloads parse the Double directly.* |

Verified against commit `3df19f636773d113bcb6b58d715d3e6c2df852d3`.