AppleDash / SaneEconomy

Finally, a sane economy plugin for Bukkit.
https://www.spigotmc.org/resources/saneeconomy-simple-but-featureful-economy.26223/
GNU General Public License v3.0
19 stars 24 forks source link

Bug of money duplication #100

Open EvanSchleret opened 4 years ago

EvanSchleret commented 4 years ago

Hi,

You can duplicate money with MoneyNote (it's not a moneynote issue). The bug : https://www.youtube.com/watch?v=n6wT8ny9Ld0

The message of MoneyNote's developer : image

Tell me if you need more information.

AppleDash commented 4 years ago

More information would be ideal - apologies, I have been kind of busy due to COVID.

A248 commented 4 years ago

So I did some debugging on this and apparently it's because SaneEconomy uses BigDecimal, so, when MoneyNote gets the player's balance as a double, SE returns the value of bigDecimal.doubleValue(). Then when MoneyNote attempts to withdraw this balance, it is re-converted to a BigDecimal via new BigDecimal(double).

The result is a loss of precision in converting to and from BigDecimal, which causes this comparison to fail some of the time: https://github.com/AppleDash/SaneEconomy/blob/master/SaneEconomyCore/src/main/java/org/appledash/saneeconomy/economy/EconomyManager.java#L87.

This can be reproduced with a basic test case, no testing framework necessary. Note that the test does not always fail. Sometimes the #compareTo produces 1, sometimes 0, sometimes -1.

public class DecimalTester {

    public static void main(String[] args) {
        for (int n = 0; n < 20; n++) {

            BigDecimal bigDecimal = randomDecimal();

            // We MUST modify the BigDecimal in some way otherwise the test will always succeed
            for (int m = 0; m < 20; m++) {
                bigDecimal = bigDecimal.add(randomDecimal()).subtract(randomDecimal());
            }
            //

            System.out.println(bigDecimal.compareTo(new BigDecimal(bigDecimal.doubleValue())));
        }
    }

    private static BigDecimal randomDecimal() {
        return new BigDecimal(ThreadLocalRandom.current().nextDouble());
    }

}

Output:

-1
0
1
1
1
0
0
0
0
0
-1
0
0
1
1
0
-1
-1
0
0
AppleDash commented 4 years ago

How do you suggest I fix this?

A248 commented 4 years ago

Compare the BigDecimals to a degree of accuracy:

    private static final BigDecimal PRECISION = new BigDecimal("0.0001");

    private static boolean hasBalance(BigDecimal accountBalance, BigDecimal requiredBalance) {
        return (accountBalance.compareTo(requiredBalance) >= 0)
                || requiredBalance.subtract(accountBalance).compareTo(PRECISION) < 0; // difference < PRECISION
    }