Fix long overflow in Block.distributeBlockRewardToQoraHolders()

Sadly no native 128bit integer support in Java 11 so resorting to using BigInteger. Added/improved unit tests to cover.
2025-11-04 00:57:03 +00:00 · 2020-05-07 16:37:40 +01:00
parent c7c419a3cd
commit 6d8f41ab05
5 changed files with 120 additions and 11 deletions
--- a/src/main/java/org/qortal/block/Block.java
+++ b/src/main/java/org/qortal/block/Block.java
@@ -1693,6 +1693,8 @@ public class Block {
 		final boolean isProcessingNotOrphaning = totalAmount >= 0;

 		long qoraPerQortReward = BlockChain.getInstance().getQoraPerQortReward();
+		BigInteger qoraPerQortRewardBI = BigInteger.valueOf(qoraPerQortReward);
+
 		List<AccountBalanceData> qoraHolders = this.repository.getAccountRepository().getEligibleLegacyQoraHolders(isProcessingNotOrphaning ? null : this.blockData.getHeight());

 		long totalQoraHeld = 0;
@@ -1706,10 +1708,18 @@ public class Block {
 		if (totalQoraHeld <= 0)
 			return sharedAmount;

+		// Could do with a faster 128bit integer library, but until then...
+		BigInteger qoraHoldersAmountBI = BigInteger.valueOf(qoraHoldersAmount);
+		BigInteger totalQoraHeldBI = BigInteger.valueOf(totalQoraHeld);
+
 		for (int h = 0; h < qoraHolders.size(); ++h) {
 			AccountBalanceData qoraHolder = qoraHolders.get(h);
+			BigInteger qoraHolderBalanceBI = BigInteger.valueOf(qoraHolder.getBalance());
+
+			// This is where a 128bit integer library could help:
+			// long holderReward = (qoraHoldersAmount * qoraHolder.getBalance()) / totalQoraHeld;
+			long holderReward = qoraHoldersAmountBI.multiply(qoraHolderBalanceBI).divide(totalQoraHeldBI).longValue();

-			long holderReward = (qoraHoldersAmount * qoraHolder.getBalance()) / totalQoraHeld;
 			long finalHolderReward = holderReward;
 			LOGGER.trace(() -> String.format("QORA holder %s has %s / %s QORA so share: %s",
 					qoraHolder.getAddress(), Amounts.prettyAmount(qoraHolder.getBalance()), finalTotalQoraHeld, Amounts.prettyAmount(finalHolderReward)));
@@ -1724,7 +1734,7 @@ public class Block {

 			// If processing, make sure we don't overpay
 			if (isProcessingNotOrphaning) {
-				long maxQortFromQora = qoraHolder.getBalance() / qoraPerQortReward;
+				long maxQortFromQora = Amounts.scaledDivide(qoraHolderBalanceBI, qoraPerQortRewardBI);

 				if (newQortFromQoraBalance >= maxQortFromQora) {
 					// Reduce final QORT-from-QORA payment to match max
--- a/src/main/java/org/qortal/utils/Amounts.java
+++ b/src/main/java/org/qortal/utils/Amounts.java
@@ -64,8 +64,12 @@ public abstract class Amounts {
 		return roundDownScaledMultiply(BigInteger.valueOf(multiplicand), BigInteger.valueOf(multiplier));
 	}

+	public static long scaledDivide(BigInteger dividend, BigInteger divisor) {
+		return dividend.multiply(Amounts.MULTIPLIER_BI).divide(divisor).longValue();
+	}
+
 	public static long scaledDivide(long dividend, long divisor) {
-		return BigInteger.valueOf(dividend).multiply(Amounts.MULTIPLIER_BI).divide(BigInteger.valueOf(divisor)).longValue();
+		return scaledDivide(BigInteger.valueOf(dividend), BigInteger.valueOf(divisor));
 	}

 }
--- a/src/test/java/org/qortal/test/minting/RewardTests.java
+++ b/src/test/java/org/qortal/test/minting/RewardTests.java
@@ -2,6 +2,7 @@ package org.qortal.test.minting;

 import static org.junit.Assert.*;

+import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -104,21 +105,26 @@ public class RewardTests extends Common {

 	@Test
 	public void testLegacyQoraReward() throws DataException {
-		Common.useSettings("test-settings-v2-qora-holder.json");
+		Common.useSettings("test-settings-v2-qora-holder-extremes.json");

 		long qoraHoldersShare = BlockChain.getInstance().getQoraHoldersShare();
+		BigInteger qoraHoldersShareBI = BigInteger.valueOf(qoraHoldersShare);
+
 		long qoraPerQort = BlockChain.getInstance().getQoraPerQortReward();
+		BigInteger qoraPerQortBI = BigInteger.valueOf(qoraPerQort);

 		try (final Repository repository = RepositoryManager.getRepository()) {
 			Map<String, Map<Long, Long>> initialBalances = AccountUtils.getBalances(repository, Asset.QORT, Asset.LEGACY_QORA, Asset.QORT_FROM_QORA);

 			Long blockReward = BlockUtils.getNextBlockReward(repository);
+			BigInteger blockRewardBI = BigInteger.valueOf(blockReward);

 			// Fetch all legacy QORA holder balances
 			List<AccountBalanceData> qoraHolders = repository.getAccountRepository().getAssetBalances(Asset.LEGACY_QORA, true);
 			long totalQoraHeld = 0L;
 			for (AccountBalanceData accountBalanceData : qoraHolders)
 				totalQoraHeld += accountBalanceData.getBalance();
+			BigInteger totalQoraHeldBI = BigInteger.valueOf(totalQoraHeld);

 			BlockUtils.mintBlock(repository);

@@ -141,14 +147,19 @@ public class RewardTests extends Common {
 			 */

 			// Expected reward
-			long qoraHoldersReward = (blockReward * qoraHoldersShare) / Amounts.MULTIPLIER;
+			long qoraHoldersReward = blockRewardBI.multiply(qoraHoldersShareBI).divide(Amounts.MULTIPLIER_BI).longValue();
 			assertTrue("QORA-holders share of block reward should be less than total block reward", qoraHoldersReward < blockReward);
+			assertFalse("QORA-holders share of block reward should not be negative!", qoraHoldersReward < 0);
+			BigInteger qoraHoldersRewardBI = BigInteger.valueOf(qoraHoldersReward);

 			long ourQoraHeld = initialBalances.get("chloe").get(Asset.LEGACY_QORA);
-			long ourQoraReward = (qoraHoldersReward * ourQoraHeld) / totalQoraHeld;
+			BigInteger ourQoraHeldBI = BigInteger.valueOf(ourQoraHeld);
+			long ourQoraReward = qoraHoldersRewardBI.multiply(ourQoraHeldBI).divide(totalQoraHeldBI).longValue();
 			assertTrue("Our QORA-related reward should be less than total QORA-holders share of block reward", ourQoraReward < qoraHoldersReward);
+			assertFalse("Our QORA-related reward should not be negative!", ourQoraReward < 0);

-			long ourQortFromQoraCap = ourQoraHeld / qoraPerQort;
+			long ourQortFromQoraCap = Amounts.scaledDivide(ourQoraHeldBI, qoraPerQortBI);
+			assertTrue("Our QORT-from-QORA cap should be greater than zero", ourQortFromQoraCap > 0);

 			long expectedReward = Math.min(ourQoraReward, ourQortFromQoraCap);
 			AccountUtils.assertBalance(repository, "chloe", Asset.QORT, initialBalances.get("chloe").get(Asset.QORT) + expectedReward);
@@ -170,10 +181,10 @@ public class RewardTests extends Common {
 			for (int i = 0; i < 100; ++i)
 				BlockUtils.mintBlock(repository);

-			// Expected balances to be limited by Chloe's legacy QORA amount
-			long expectedBalance = initialBalances.get("chloe").get(Asset.LEGACY_QORA) / qoraPerQort;
-			AccountUtils.assertBalance(repository, "chloe", Asset.QORT, initialBalances.get("chloe").get(Asset.QORT) + expectedBalance);
-			AccountUtils.assertBalance(repository, "chloe", Asset.QORT_FROM_QORA, initialBalances.get("chloe").get(Asset.QORT_FROM_QORA) + expectedBalance);
+			// Expected balances to be limited by Dilbert's legacy QORA amount
+			long expectedBalance = Amounts.scaledDivide(initialBalances.get("dilbert").get(Asset.LEGACY_QORA), qoraPerQort);
+			AccountUtils.assertBalance(repository, "dilbert", Asset.QORT, initialBalances.get("dilbert").get(Asset.QORT) + expectedBalance);
+			AccountUtils.assertBalance(repository, "dilbert", Asset.QORT_FROM_QORA, initialBalances.get("dilbert").get(Asset.QORT_FROM_QORA) + expectedBalance);
 		}
 	}

--- a/src/test/resources/test-chain-v2-qora-holder-extremes.json
+++ b/src/test/resources/test-chain-v2-qora-holder-extremes.json
@@ -0,0 +1,77 @@
+{
+	"isTestChain": true,
+	"blockTimestampMargin": 500,
+	"transactionExpiryPeriod": 86400000,
+	"maxBlockSize": 2097152,
+	"maxBytesPerUnitFee": 1024,
+	"unitFee": "0.1",
+	"requireGroupForApproval": false,
+	"minAccountLevelToRewardShare": 5,
+	"maxRewardSharesPerMintingAccount": 20,
+	"founderEffectiveMintingLevel": 10,
+	"onlineAccountSignaturesMinLifetime": 3600000,
+	"onlineAccountSignaturesMaxLifetime": 86400000,
+	"rewardsByHeight": [
+		{ "height": 1, "reward": 100 },
+		{ "height": 11, "reward": 10 },
+		{ "height": 21, "reward": 1 }
+	],
+	"sharesByLevel": [
+		{ "levels": [ 1, 2 ], "share": 0.05 },
+		{ "levels": [ 3, 4 ], "share": 0.10 },
+		{ "levels": [ 5, 6 ], "share": 0.15 },
+		{ "levels": [ 7, 8 ], "share": 0.20 },
+		{ "levels": [ 9, 10 ], "share": 0.25 }
+	],
+	"qoraHoldersShare": 0.20,
+	"qoraPerQortReward": 250,
+	"blocksNeededByLevel": [ 10, 20, 30, 40, 50, 60, 70, 80, 90, 100 ],
+	"blockTimingsByHeight": [
+		{ "height": 1, "target": 60000, "deviation": 30000, "power": 0.2 }
+	],
+	"ciyamAtSettings": {
+		"feePerStep": "0.0001",
+		"maxStepsPerRound": 500,
+		"stepsPerFunctionCall": 10,
+		"minutesPerBlock": 1
+	},
+	"featureTriggers": {
+		"messageHeight": 0,
+		"atHeight": 0,
+		"assetsTimestamp": 0,
+		"votingTimestamp": 0,
+		"arbitraryTimestamp": 0,
+		"powfixTimestamp": 0,
+		"qortalTimestamp": 0,
+		"newAssetPricingTimestamp": 0,
+		"groupApprovalTimestamp": 0
+	},
+	"genesisInfo": {
+		"version": 4,
+		"timestamp": 0,
+		"transactions": [
+			{ "type": "ISSUE_ASSET", "owner": "QdSnUy6sUiEnaN87dWmE92g1uQjrvPgrWG", "assetName": "QORT", "description": "QORT native coin", "data": "", "quantity": 0, "isDivisible": true, "fee": 0 },
+			{ "type": "ISSUE_ASSET", "owner": "QdSnUy6sUiEnaN87dWmE92g1uQjrvPgrWG", "assetName": "Legacy-QORA", "description": "Representative legacy QORA", "quantity": 0, "isDivisible": true, "data": "{}", "isUnspendable": true },
+			{ "type": "ISSUE_ASSET", "owner": "QdSnUy6sUiEnaN87dWmE92g1uQjrvPgrWG", "assetName": "QORT-from-QORA", "description": "QORT gained from holding legacy QORA", "quantity": 0, "isDivisible": true, "data": "{}", "isUnspendable": true },
+
+			{ "type": "GENESIS", "recipient": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "amount": "1000000000" },
+			{ "type": "GENESIS", "recipient": "QixPbJUwsaHsVEofJdozU9zgVqkK6aYhrK", "amount": "1000000" },
+			{ "type": "GENESIS", "recipient": "QaUpHNhT3Ygx6avRiKobuLdusppR5biXjL", "amount": "1000000" },
+			{ "type": "GENESIS", "recipient": "Qci5m9k4rcwe4ruKrZZQKka4FzUUMut3er", "amount": "1000000" },
+
+			{ "type": "GENESIS", "recipient": "QaUpHNhT3Ygx6avRiKobuLdusppR5biXjL", "amount": "637557960.49687541", "assetId": 1 },
+			{ "type": "GENESIS", "recipient": "Qci5m9k4rcwe4ruKrZZQKka4FzUUMut3er", "amount": "0.666", "assetId": 1 },
+
+			{ "type": "CREATE_GROUP", "creatorPublicKey": "2tiMr5LTpaWCgbRvkPK8TFd7k63DyHJMMFFsz9uBf1ZP", "owner": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "groupName": "dev-group", "description": "developer group", "isOpen": false, "approvalThreshold": "PCT100", "minimumBlockDelay": 0, "maximumBlockDelay": 1440 },
+
+			{ "type": "ISSUE_ASSET", "owner": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "assetName": "TEST", "description": "test asset", "data": "", "quantity": 1000000, "isDivisible": true, "fee": 0 },
+			{ "type": "ISSUE_ASSET", "owner": "QixPbJUwsaHsVEofJdozU9zgVqkK6aYhrK", "assetName": "OTHER", "description": "other test asset", "data": "", "quantity": 1000000, "isDivisible": true, "fee": 0 },
+			{ "type": "ISSUE_ASSET", "owner": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "assetName": "GOLD", "description": "gold test asset", "data": "", "quantity": 1000000, "isDivisible": true, "fee": 0 },
+
+			{ "type": "ACCOUNT_FLAGS", "target": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "andMask": -1, "orMask": 1, "xorMask": 0 },
+			{ "type": "REWARD_SHARE", "minterPublicKey": "2tiMr5LTpaWCgbRvkPK8TFd7k63DyHJMMFFsz9uBf1ZP", "recipient": "QgV4s3xnzLhVBEJxcYui4u4q11yhUHsd9v", "rewardSharePublicKey": "7PpfnvLSG7y4HPh8hE7KoqAjLCkv7Ui6xw4mKAkbZtox", "sharePercent": 100 },
+
+			{ "type": "ACCOUNT_LEVEL", "target": "Qci5m9k4rcwe4ruKrZZQKka4FzUUMut3er", "level": 8 }
+		]
+	}
+}
--- a/src/test/resources/test-settings-v2-qora-holder-extremes.json
+++ b/src/test/resources/test-settings-v2-qora-holder-extremes.json
@@ -0,0 +1,7 @@
+{
+  "restrictedApi": false,
+  "blockchainConfig": "src/test/resources/test-chain-v2-qora-holder.json",
+  "wipeUnconfirmedOnStart": false,
+  "testNtpOffset": 0,
+  "minPeers": 0
+}