Information

Full output of /ess version:

http://prntscr.com/ke0z97

Details

There is a problem with rounding numbers from BigDecimal to double. When using vault to get current players balance if a player has ex.(49.999999999999999 it rounds to 50.0) and this causes issues with plugins that use this data to check if the player has enough money.

The issue was discovered recently with ChestShop where players could then generate infinite money as vault returned the player balance as 50.0 instead of 49.9999999 so the transaction went through but the money was not taken.

Not sure where to report this so I started here. I hope this is enough information.

So, to avoid the issues with /pay, insert the following code into one of your plugins:
`@EventHandler

public void onCommand(PlayerCommandPreprocessEvent event){

    String input = event.getMessage();

    String input1[] = input.split(" ");

    if(input1.length < 3){return;}

    if(!input1[0].equalsIgnoreCase("/pay")){return;}

    String value[] = input1[2].split("\\.");

    if (value.length != 2){return;}

    if(value[0].equalsIgnoreCase("")){

        value[0] = "0";

    }

    String decimal = value[1];

    decimal = decimal.substring(0, Math.min(decimal.length(), 2));

    String output = input1[0] + " " + input1[1] + " " + value[0] + "." + decimal;

    event.setMessage(output);

}`

It sets the places to the right of the decimal to 2. Adjust as needed for your commands you need to limit if the plugin author hasn't done it themselves.

There doesn't appear to be anything we can do about this - that's a limitation of converting BigDecimals to doubles. All I can suggest is to limit the precision of whatever transactions led to a player having a balance of 49.9999999 rather than 50.

Yes i know players glitch it with /pay 0.99999999999999999999 and it works.

Here is a test of this done on latest essentialsx http://prntscr.com/kedgpz

I think the pay command should be just limited to 4 decimals or something not really sure.

Check the balance in the config /bal trims the output

What about checking with the chestshop devs? https://github.com/ChestShop-authors/ChestShop-3/

It looks like their plugin was recently updated. https://ci.minebench.de/job/ChestShop-3/

Yeah I made a fix for that one ChestShop-authors/ChestShop-3#148

That's great! Thanks @simon44556

All I can suggest is to limit the precision of whatever transactions led to a player having a balance of 49.9999999 rather than 50.

@md678685 What do you suggest as a way to limit the precision?

Yes i know players glitch it with /pay 0.99999999999999999999 and it works.
@simon44556

I was unable to duplicate using the method listed above. In the image, checked two players balances, sent /pay 0.99999999999999999999. checked both balances again.

https://i.imgur.com/j8g4cva.png

I've noticed that unlike Vault, the Reserve API uses BigDecimals rather than doubles. Perhaps it's worth reviving #1707 to add support for Reserve, so that when plugins access EssentialsX's economy using Reserve, rounding conversions can be avoided.

@simon44556 , @NullCase: This is not something that plugins using the VaultAPI should fix individually, the economy plugin has to make sure that it doesn't return a bigger value than a user actually has in its account, in this case it needs to round down when converting from BigDecimal to double instead of rounding up.

It is still the burden of plugins using Vault to ensure transactions are successful, which Vault facilitates with EcononyResponse.

All plugins that support Vault should check the returned EconomyResponse and ensure that the transaction was successful (response.transactionSuccess()), as it's possible in theory for an Economy provider to reject a transaction for reasons other than a lack of funds.

ChestShop appears to assume that "having X in balance" is equivalent to "X can definitely be withdrawn", which is dangerous, as shown by this exploit.

As for PR #2135, I'm not sure as to whether deliberately underreporting balances is a valid solution to this. Yes, it prevents this exploit in ChestShop being abused on up-to-date EssentialsX servers, but it doesn't fix the root problem, which is that ChestShop doesn't do the necessary checks in the correct places. It also has the potential to cause unintended behaviour in other plugins. (What if a plugin checks that depositing a sum doesn't go over a certain threshold? In that instance, rounding up is preferable.)

Could you please grab build 636 on the CI server and see if the issue is resolved?

That's a good point and I'll look into making it properly use the response.

I still feel that always returning a rounded balance that is below the actually one is a better solution than blindly taking the rounded one ‒ no matter if it is smaller or bigger than the actual one. Granted there might be an argument for returning a bigger one but the main issue is that it's unpredictable. I also think that not returning more money than a player actually has is way more important than not returning less money.

fc62ab0823ccae1c1b874ff2b4ea1b7848b51d55 also aims to limit the extent of the issue on ChestShop's side by ensuring that non-successful transactions are properly acknowledged, however I don't know at what point this version will be made available see below for updated ChestShop builds.

If people could report back whether the latest EssentialsX build linked above solves the issue, it would be much appreciated.

however I don't know at what point this version will be made available.

The dev builds are available right now. ;)

Closing as I'm fairly certain this is no longer an issue.