[darbyjack]

Vault's Economy methods answer with an EconomyResponse that says whether the money moved. Guilds ignored that answer and committed the guild side of the transaction anyway.

Three commands did this:

• CommandBank.deposit withdrew from the player and then set guild.balance = total. A provider that refused the withdrawal left the player where they were and credited the bank with money nobody paid for.
• CommandBank.withdraw set guild.balance = bal - rounded and then paid the player. A provider that refused the deposit took the money out of the bank and never handed it over.
• CommandCreate.accept() added the guild to the handler and then withdrew the creation cost. A refused withdrawal left the player with a guild they did not pay for, and a provider that threw let the exception escape the command with the guild already in the handler.

Reproduction

FakeEconomy is a Vault provider that can succeed, refuse a transaction or throw. Driving the real commands with it against the code on master:

EconomyTransactionTest > a refused deposit leaves the player and the bank where they were FAILED
    AssertionFailedError: a refused withdrawal must not credit the bank, or the money exists twice ==> expected: <0.0> but was: <250.0>

EconomyTransactionTest > a refused withdrawal leaves the player and the bank where they were FAILED
    AssertionFailedError: a refused deposit must not debit the bank, or the money is gone ==> expected: <500.0> but was: <350.0>

EconomyTransactionTest > a guild creation the provider refuses leaves no guild, no membership and no charge FAILED
    org.mockito.exceptions.verification.NeverWantedButInvoked:
    guildHandler.addGuild(<any>);
    But invoked here:
    -> at me.glaremasters.guilds.commands.management.CommandCreate$create$1.accept(CommandCreate.kt:137)

EconomyTransactionTest > a deposit the provider throws on changes nothing FAILED
    java.lang.IllegalStateException: the economy provider blew up
        at me.glaremasters.guilds.commands.bank.CommandBank.deposit(CommandBank.kt:77)

The fix

EconomyUtils gained withdraw and deposit. Both call the provider, read EconomyResponse.transactionSuccess() and report the provider's own errorMessage to the player when the transaction failed. A provider that throws is caught, logged and reported the same way. A NullPointerException still sends error.economy-required, the message hasEnough has always sent for a missing economy plugin.

Each command commits its guild side only after the transaction succeeded:

• deposit takes the money out of the player's account first, then credits the bank
• withdraw pays the player first, then debits the bank
• guild creation takes the creation cost before the guild goes into the handler, so a refused payment leaves no guild and no membership

A failed transaction leaves both sides as they were and drops the pending confirmation. An amount of zero is not sent to the provider at all, since the shipped default for the guild creation cost is 0.0 and there is no money to move then.

CommandAdminBank and CommandUpgrade were checked too. Both only move money inside the guild's own ledger and never call the economy provider, so there is no response for them to read.

How this composes with #851

#851 re-runs the command-time checks when the pending confirmation is accepted. It edits the same file, and the two merge cleanly: this branch's hunks are in the payment and guild-building block, #851's are in the check block, and git merge-tree against origin/master and against fix/guild-creation-confirm-revalidation reports no conflict.

The order the two changes leave behind is:

1. the command checks everything and quotes a price
2. the player confirms
3. the confirmation re-runs the same checks, so a guild that was taken or renamed in the meantime is refused before anything is charged
4. the guild is built and GuildCreateEvent fires, and a cancelled event stops the creation before the payment
5. the creation cost is taken, and the provider's response decides whether it was taken
6. the guild goes into the handler and the player becomes its member

A refused check at step 3 leaves no guild and charges nothing. A refused payment at step 5 leaves no guild and no membership, and voids the offer so a later /guild confirm cannot charge for a guild that was never created.

The two changes do not undo each other. #851 re-runs EconomyUtils.hasEnough, which reads the balance, and the amount it checks is the same quoted cost that this branch charges, so the check and the charge are about the same amount. The read can go stale between the check and the transaction, and that costs nothing: the provider's response, not the check, decides. A player who spends the money in the meantime gets the withdrawal refused, the failure reported, and no guild.

I merged this branch with #851 and ran the tests against the merged tree, and again against the merged trees of every other open PR. All of them merge cleanly, compile, and pass the eleven tests below.

The new message

The catalogue had nothing for a refused transaction, so error.economy-transaction-failed was added with a {reason} placeholder. It goes into en-US.yml only.

crowdin.yml declares en-US.yml as the single source and every other locale file as a Crowdin-managed translation, so the key reaches the other locales on the next sync. Writing an English line into the translated files by hand would be reverted by that sync.

Until the sync lands, ACF resolves a key that the player's locale does not have against the manager's default locale. The plugin sets that default from settings.messagesLanguage, which is en-US out of the box. A server on another language sees the English string, and a server that points settings.messagesLanguage at a locale the sync has not reached yet sees the key placeholder, the same as it does today for the keys still waiting on Crowdin. PR #805 added admin.cant-remove-master and transfer.failed the same way.

Tests

New files under src/test/java/me/glaremasters/guilds/economy/:

• FakeEconomy.java, a Vault provider that can succeed, refuse or throw and that tracks the balance
• AcfCommandContext.java, which puts a command issuer where ACF's commands read it from so a test can drive a command directly
• EconomyTransactionTest.java, eleven tests: a successful deposit and withdrawal, a refused deposit and withdrawal, a provider that throws on either side, a refused and a throwing guild creation payment, a paid guild creation, a guild creation that costs nothing, and a guild creation the player can no longer afford when the confirmation is accepted

The last one is the order between the checks and the payment. The player is quoted a price they can pay and spends the money before they confirm, and the creation is refused with nothing charged and no guild made.

Results

./gradlew test testJava11 spotlessCheck    BUILD SUCCESSFUL
./gradlew check                             BUILD SUCCESSFUL
    Legacy API probe: 2 Material constant(s) checked against 1.8.8, 0 allow-listed.
./gradlew plugwrightTest                    local  144 passed, 0 failed, 0 skipped  (7m 2s)

The E2E suite was run twice locally on this tree, once before the message was trimmed back to en-US.yml and once on the final head, with the same result both times.

This branch was also merged with each open PR and the eleven tests were run against every merged tree. All of them merge cleanly and pass:

git merge-tree --write-tree fix/economy-transaction-safety <branch>
  fix/guild-creation-confirm-revalidation   9a22e9d4   11 passed, 0 failed
  fix/guild-startup-loading-reliability      1137469a   11 passed, 0 failed
  fix/json-storage-filtering                 e47f126c   11 passed, 0 failed
  fix/vault-serialization-data-integrity     950151f0   11 passed, 0 failed
  fix/blacklist-matching                     00710c29   11 passed, 0 failed
  fix/invite-code-usage-validation           c231e10b   11 passed, 0 failed
  fix/case-insensitive-guild-lookup          e29cc1a7   11 passed, 0 failed
  fix/invite-code-membership-consistency     bb1eed5f   11 passed, 0 failed
  test/minecraft-version-smoke-coverage      58d31909   11 passed, 0 failed

git merge-tree --write-tree origin/master fix/economy-transaction-safety reports no conflict either.

The E2E check on this PR

The Plugwright check on this branch failed on "a second teleport home cannot happen until the cooldown expires" in homes.spec.ts, with "pw_be4d's connection had already ended (socketClosed); /guild home would never have been sent". The bot's socket dropped, so the command never left the client. The assertion is not about this change, and this branch does not touch homes.spec.ts.

That test is flaky on the runner rather than on any one branch:

• master, run 37211243837 (2026-10-04, a manual E2E run on master): the same test failed, 135 passed and 1 failed.
• fix/blacklist-matching, run 39727120928: the same test failed the same way, with the same "Connection ended: socketClosed" on "/guild home".
• fix/invite-code-usage-validation, run 39728538651: the same test failed the same way.
• fix/invite-code-membership-consistency, run 39730173487: the same test failed the same way, and that branch went green on a re-run of the same head.

Runs:
https://github.com/guilds-plugin/Guilds/actions/runs/37211243837
https://github.com/guilds-plugin/Guilds/actions/runs/39727120928
https://github.com/guilds-plugin/Guilds/actions/runs/39728538651
https://github.com/guilds-plugin/Guilds/actions/runs/39730173487
https://github.com/guilds-plugin/Guilds/actions/runs/39742006003

I left homes.spec.ts alone and did not weaken the assertion. The suite is green locally on this head, 144 passed, 0 failed, 0 skipped (7m 2s), and the re-run of the failed job on the same head is green as well: run 39742006003, attempt 2, passed.

Compatibility

No new dependency, no new API and no config key. The plugin still compiles to Java 11 bytecode and the 1.8.8 Material probe passes. No Material constant was touched. EconomyResponse has been part of the Vault API since Vault 1.0, so this works with every provider the plugin already supports.

Limitations

• EconomyUtils.hasEnough still catches only NullPointerException. A provider that throws something else while the balance is read lets the exception out of the command, before any state has changed.
• The {reason} placeholder carries the economy provider's own errorMessage. Provider text is not translated, so a player on a non-English server sees an English fragment inside an otherwise translated sentence. It is a failure path, and the alternative was a message that says nothing about what went wrong.
• GuildCreateEvent is still fired before the creation cost is taken. Charging first would mean refunding when the event is cancelled, and a refund that fails loses the player's money, so the event order is deliberate. A listener that reacts to GuildCreateEvent can therefore see a guild creation that then fails to be paid for.
• A provider that refuses a transaction now stops the deposit or the guild creation instead of letting the money disappear. That is the point of the change, but it does make a misbehaving provider visible to players.
• A guild reaches the disk through the periodic save, a minute after the creation by default, or when the server stops. If that write fails, the failure is in the server log and the player is not told, and the guild is gone after the restart. That is how every guild change has behaved, a rename or a tier upgrade as much as a creation, and the storage work in fix(vaults): keep unreadable vaults in place and stop losing items #845, fix: only read .json files as records in the JSON storage #846 and fix: refuse to start when the guild data cannot be read #850 does not change it, since that work is about reading the data and about the files that cannot be read. A creation that cannot be persisted is the case where the player has paid for something that then disappears, so it is worth a follow-up that reports a failed save to the players it affects.