Files
DiamondBank-OG/DEFECT-CHART.md
T
2026-10-20 09:57:13 -04:00

43 KiB

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.