SaneEconomy icon indicating copy to clipboard operation
SaneEconomy copied to clipboard

Bug of money duplication

Open EvanSchleret opened this issue 4 years ago • 4 comments

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.

EvanSchleret avatar Apr 16 '20 23:04 EvanSchleret

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

AppleDash avatar Jun 03 '20 11:06 AppleDash

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

A248 avatar Jun 03 '20 14:06 A248

How do you suggest I fix this?

AppleDash avatar Jun 05 '20 16:06 AppleDash

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
	}

A248 avatar Jun 06 '20 12:06 A248