Fix potentially overflowing multiply in Block reward processing.

Change BlockChain config to use AmountTypeAdapter instead of
creating duplicated long versions of BigDecimal values.

Some tidying to Amounts class.
This commit is contained in:
catbref 2020-05-01 08:57:15 +01:00
parent a309f8de9e
commit e86143426b
6 changed files with 43 additions and 74 deletions

View File

@ -1313,10 +1313,10 @@ public class Block {
} }
protected void processBlockRewards() throws DataException { protected void processBlockRewards() throws DataException {
Long reward = BlockChain.getInstance().getRewardAtHeight(this.blockData.getHeight()); long reward = BlockChain.getInstance().getRewardAtHeight(this.blockData.getHeight());
// No reward for our height? // No reward for our height?
if (reward == null) if (reward == 0)
return; return;
distributeBlockReward(reward); distributeBlockReward(reward);
@ -1548,10 +1548,10 @@ public class Block {
} }
protected void orphanBlockRewards() throws DataException { protected void orphanBlockRewards() throws DataException {
Long reward = BlockChain.getInstance().getRewardAtHeight(this.blockData.getHeight()); long reward = BlockChain.getInstance().getRewardAtHeight(this.blockData.getHeight());
// No reward for our height? // No reward for our height?
if (reward == null) if (reward == 0)
return; return;
distributeBlockReward(0 - reward); distributeBlockReward(0 - reward);
@ -1656,7 +1656,7 @@ public class Block {
for (int s = 0; s < sharesByLevel.size(); ++s) { for (int s = 0; s < sharesByLevel.size(); ++s) {
final int binIndex = s; final int binIndex = s;
long binAmount = (totalAmount * sharesByLevel.get(binIndex).unscaledShare) / 100000000L; long binAmount = Amounts.roundDownScaledMultiply(totalAmount, sharesByLevel.get(binIndex).share);
LOGGER.trace(() -> String.format("Bin %d share of %s: %s", binIndex, Amounts.prettyAmount(totalAmount), Amounts.prettyAmount(binAmount))); LOGGER.trace(() -> String.format("Bin %d share of %s: %s", binIndex, Amounts.prettyAmount(totalAmount), Amounts.prettyAmount(binAmount)));
// Spread across all accounts in bin. getShareBin() returns -1 for minter accounts that are also founders, so they are effectively filtered out. // Spread across all accounts in bin. getShareBin() returns -1 for minter accounts that are also founders, so they are effectively filtered out.
@ -1677,12 +1677,12 @@ public class Block {
} }
private long distributeBlockRewardToQoraHolders(long totalAmount) throws DataException { private long distributeBlockRewardToQoraHolders(long totalAmount) throws DataException {
long qoraHoldersAmount = (BlockChain.getInstance().getQoraHoldersUnscaledShare() * totalAmount) / 100000000L; long qoraHoldersAmount = Amounts.roundDownScaledMultiply(totalAmount, BlockChain.getInstance().getQoraHoldersShare());
LOGGER.trace(() -> String.format("Legacy QORA holders share of %s: %s", Amounts.prettyAmount(totalAmount), Amounts.prettyAmount(qoraHoldersAmount))); LOGGER.trace(() -> String.format("Legacy QORA holders share of %s: %s", Amounts.prettyAmount(totalAmount), Amounts.prettyAmount(qoraHoldersAmount)));
final boolean isProcessingNotOrphaning = totalAmount >= 0; final boolean isProcessingNotOrphaning = totalAmount >= 0;
long qoraPerQortReward = BlockChain.getInstance().getUnscaledQoraPerQortReward(); long qoraPerQortReward = BlockChain.getInstance().getQoraPerQortReward();
List<AccountBalanceData> qoraHolders = this.repository.getAccountRepository().getEligibleLegacyQoraHolders(isProcessingNotOrphaning ? null : this.blockData.getHeight()); List<AccountBalanceData> qoraHolders = this.repository.getAccountRepository().getEligibleLegacyQoraHolders(isProcessingNotOrphaning ? null : this.blockData.getHeight());
long totalQoraHeld = 0; long totalQoraHeld = 0;

View File

@ -3,7 +3,6 @@ package org.qortal.block;
import java.io.File; import java.io.File;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.InputStream; import java.io.InputStream;
import java.math.BigDecimal;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
@ -55,13 +54,14 @@ public class BlockChain {
/** Transaction expiry period, starting from transaction's timestamp, in milliseconds. */ /** Transaction expiry period, starting from transaction's timestamp, in milliseconds. */
private long transactionExpiryPeriod; private long transactionExpiryPeriod;
private BigDecimal unitFee; @XmlJavaTypeAdapter(value = org.qortal.api.AmountTypeAdapter.class)
private long unscaledUnitFee; // calculated after unmarshal private long unitFee;
private int maxBytesPerUnitFee; private int maxBytesPerUnitFee;
/** Maximum acceptable timestamp disagreement offset in milliseconds. */ /** Maximum acceptable timestamp disagreement offset in milliseconds. */
private long blockTimestampMargin; private long blockTimestampMargin;
/** Maximum block size, in bytes. */ /** Maximum block size, in bytes. */
private int maxBlockSize; private int maxBlockSize;
@ -86,28 +86,26 @@ public class BlockChain {
/** Block rewards by block height */ /** Block rewards by block height */
public static class RewardByHeight { public static class RewardByHeight {
public int height; public int height;
public BigDecimal reward; @XmlJavaTypeAdapter(value = org.qortal.api.AmountTypeAdapter.class)
public long unscaledReward; // reward * 1e8, calculated after unmarshal public long reward;
} }
List<RewardByHeight> rewardsByHeight; List<RewardByHeight> rewardsByHeight;
/** Share of block reward/fees by account level */ /** Share of block reward/fees by account level */
public static class ShareByLevel { public static class ShareByLevel {
public List<Integer> levels; public List<Integer> levels;
public BigDecimal share; @XmlJavaTypeAdapter(value = org.qortal.api.AmountTypeAdapter.class)
public long unscaledShare; // share * 1e8, calculated after unmarshal public long share;
} }
List<ShareByLevel> sharesByLevel; List<ShareByLevel> sharesByLevel;
/** Share of block reward/fees to legacy QORA coin holders */ /** Share of block reward/fees to legacy QORA coin holders */
BigDecimal qoraHoldersShare; @XmlJavaTypeAdapter(value = org.qortal.api.AmountTypeAdapter.class)
/** Unscaled (* 1e8) share of block reward/fees to legacy QORA coin holders */ private Long qoraHoldersShare;
private long qoraHoldersUnscaledShare; // calculated after unmarshal
/** How many legacy QORA per 1 QORT of block reward. */ /** How many legacy QORA per 1 QORT of block reward. */
BigDecimal qoraPerQortReward; @XmlJavaTypeAdapter(value = org.qortal.api.AmountTypeAdapter.class)
/** How many legacy QORA per 1 QORT of block reward. Unscaled (* 1e8). */ private Long qoraPerQortReward;
private long unscaledQoraPerQortReward; // calculated after unmarshal
/** /**
* Number of minted blocks required to reach next level from previous. * Number of minted blocks required to reach next level from previous.
@ -268,14 +266,10 @@ public class BlockChain {
return this.isTestChain; return this.isTestChain;
} }
public BigDecimal getUnitFee() { public long getUnitFee() {
return this.unitFee; return this.unitFee;
} }
public long getUnscaledUnitFee() {
return this.unscaledUnitFee;
}
public int getMaxBytesPerUnitFee() { public int getMaxBytesPerUnitFee() {
return this.maxBytesPerUnitFee; return this.maxBytesPerUnitFee;
} }
@ -321,22 +315,14 @@ public class BlockChain {
return this.cumulativeBlocksByLevel; return this.cumulativeBlocksByLevel;
} }
public BigDecimal getQoraHoldersShare() { public long getQoraHoldersShare() {
return this.qoraHoldersShare; return this.qoraHoldersShare;
} }
public long getQoraHoldersUnscaledShare() { public long getQoraPerQortReward() {
return this.qoraHoldersUnscaledShare;
}
public BigDecimal getQoraPerQortReward() {
return this.qoraPerQortReward; return this.qoraPerQortReward;
} }
public long getUnscaledQoraPerQortReward() {
return this.unscaledQoraPerQortReward;
}
public int getMinAccountLevelToMint() { public int getMinAccountLevelToMint() {
return this.minAccountLevelToMint; return this.minAccountLevelToMint;
} }
@ -365,13 +351,13 @@ public class BlockChain {
// More complex getters for aspects that change by height or timestamp // More complex getters for aspects that change by height or timestamp
public Long getRewardAtHeight(int ourHeight) { public long getRewardAtHeight(int ourHeight) {
// Scan through for reward at our height // Scan through for reward at our height
for (int i = rewardsByHeight.size() - 1; i >= 0; --i) for (int i = rewardsByHeight.size() - 1; i >= 0; --i)
if (rewardsByHeight.get(i).height <= ourHeight) if (rewardsByHeight.get(i).height <= ourHeight)
return rewardsByHeight.get(i).unscaledReward; return rewardsByHeight.get(i).reward;
return null; return 0;
} }
public BlockTimingByHeight getBlockTimingByHeight(int ourHeight) { public BlockTimingByHeight getBlockTimingByHeight(int ourHeight) {
@ -431,10 +417,7 @@ public class BlockChain {
/** Minor normalization, cached value generation, etc. */ /** Minor normalization, cached value generation, etc. */
private void fixUp() { private void fixUp() {
this.unitFee = this.unitFee.setScale(8); // Calculate cumulative blocks required for each level
this.unscaledUnitFee = this.unitFee.unscaledValue().longValue();
// Pre-calculate cumulative blocks required for each level
int cumulativeBlocks = 0; int cumulativeBlocks = 0;
this.cumulativeBlocksByLevel = new ArrayList<>(this.blocksNeededByLevel.size() + 1); this.cumulativeBlocksByLevel = new ArrayList<>(this.blocksNeededByLevel.size() + 1);
for (int level = 0; level <= this.blocksNeededByLevel.size(); ++level) { for (int level = 0; level <= this.blocksNeededByLevel.size(); ++level) {
@ -444,20 +427,6 @@ public class BlockChain {
cumulativeBlocks += this.blocksNeededByLevel.get(level); cumulativeBlocks += this.blocksNeededByLevel.get(level);
} }
// Calculate unscaled long versions of block rewards by height
for (RewardByHeight rewardByHeight : this.rewardsByHeight)
rewardByHeight.unscaledReward = rewardByHeight.reward.setScale(8).unscaledValue().longValue();
// Calculate unscaled long versions of block reward shares-by-level
for (ShareByLevel shareByLevel : this.sharesByLevel)
shareByLevel.unscaledShare = shareByLevel.share.setScale(8).unscaledValue().longValue();
// Calculate unscaled long version of Qora-holders block reward share
this.qoraHoldersUnscaledShare = this.qoraHoldersShare.setScale(8).unscaledValue().longValue();
// Calculate unscaled long version of Qora-per-Qort block reward
this.unscaledQoraPerQortReward = this.qoraPerQortReward.setScale(8).unscaledValue().longValue();
// Convert collections to unmodifiable form // Convert collections to unmodifiable form
this.rewardsByHeight = Collections.unmodifiableList(this.rewardsByHeight); this.rewardsByHeight = Collections.unmodifiableList(this.rewardsByHeight);
this.sharesByLevel = Collections.unmodifiableList(this.sharesByLevel); this.sharesByLevel = Collections.unmodifiableList(this.sharesByLevel);

View File

@ -320,7 +320,7 @@ public abstract class Transaction {
/** Returns whether transaction's fee is at least minimum unit fee as specified in blockchain config. */ /** Returns whether transaction's fee is at least minimum unit fee as specified in blockchain config. */
public boolean hasMinimumFee() { public boolean hasMinimumFee() {
return this.transactionData.getFee() >= BlockChain.getInstance().getUnscaledUnitFee(); return this.transactionData.getFee() >= BlockChain.getInstance().getUnitFee();
} }
public long feePerByte() { public long feePerByte() {
@ -333,7 +333,7 @@ public abstract class Transaction {
/** Returns whether transaction's fee is at least amount needed to cover byte-length of transaction. */ /** Returns whether transaction's fee is at least amount needed to cover byte-length of transaction. */
public boolean hasMinimumFeePerByte() { public boolean hasMinimumFeePerByte() {
long unitFee = BlockChain.getInstance().getUnscaledUnitFee(); long unitFee = BlockChain.getInstance().getUnitFee();
int maxBytePerUnitFee = BlockChain.getInstance().getMaxBytesPerUnitFee(); int maxBytePerUnitFee = BlockChain.getInstance().getMaxBytesPerUnitFee();
return this.feePerByte() >= maxBytePerUnitFee / unitFee; return this.feePerByte() >= maxBytePerUnitFee / unitFee;
@ -351,7 +351,7 @@ public abstract class Transaction {
int unitFeeCount = ((dataLength - 1) / maxBytePerUnitFee) + 1; int unitFeeCount = ((dataLength - 1) / maxBytePerUnitFee) + 1;
return BlockChain.getInstance().getUnscaledUnitFee() * unitFeeCount; return BlockChain.getInstance().getUnitFee() * unitFeeCount;
} }
/** /**

View File

@ -14,13 +14,13 @@ public abstract class Amounts {
public static String prettyAmount(long amount) { public static String prettyAmount(long amount) {
StringBuilder stringBuilder = new StringBuilder(20); StringBuilder stringBuilder = new StringBuilder(20);
stringBuilder.append(amount / 100000000L); stringBuilder.append(amount / MULTIPLIER);
stringBuilder.append('.'); stringBuilder.append('.');
int dpLength = stringBuilder.length(); int dpLength = stringBuilder.length();
stringBuilder.append(Math.abs(amount % 100000000L)); stringBuilder.append(Math.abs(amount % MULTIPLIER));
int paddingRequired = 8 - (stringBuilder.length() - dpLength); int paddingRequired = 8 - (stringBuilder.length() - dpLength);
if (paddingRequired > 0) if (paddingRequired > 0)
@ -48,20 +48,20 @@ public abstract class Amounts {
return Math.abs(a); return Math.abs(a);
} }
public static long roundUpScaledMultiply(BigInteger amount, BigInteger price) { public static long roundUpScaledMultiply(BigInteger multiplicand, BigInteger multiplier) {
return amount.multiply(price).add(ROUNDING).divide(MULTIPLIER_BI).longValue(); return multiplicand.multiply(multiplier).add(ROUNDING).divide(MULTIPLIER_BI).longValue();
} }
public static long roundUpScaledMultiply(long amount, long price) { public static long roundUpScaledMultiply(long multiplicand, long multiplier) {
return roundUpScaledMultiply(BigInteger.valueOf(amount), BigInteger.valueOf(price)); return roundUpScaledMultiply(BigInteger.valueOf(multiplicand), BigInteger.valueOf(multiplier));
} }
public static long roundDownScaledMultiply(BigInteger amount, BigInteger price) { public static long roundDownScaledMultiply(BigInteger multiplicand, BigInteger multiplier) {
return amount.multiply(price).divide(MULTIPLIER_BI).longValue(); return multiplicand.multiply(multiplier).divide(MULTIPLIER_BI).longValue();
} }
public static long roundDownScaledMultiply(long amount, long price) { public static long roundDownScaledMultiply(long multiplicand, long multiplier) {
return roundDownScaledMultiply(BigInteger.valueOf(amount), BigInteger.valueOf(price)); return roundDownScaledMultiply(BigInteger.valueOf(multiplicand), BigInteger.valueOf(multiplier));
} }
public static long scaledDivide(long dividend, long divisor) { public static long scaledDivide(long dividend, long divisor) {

View File

@ -13,7 +13,7 @@ public abstract class TestTransaction {
protected static final Random random = new Random(); protected static final Random random = new Random();
public static BaseTransactionData generateBase(PrivateKeyAccount account) throws DataException { public static BaseTransactionData generateBase(PrivateKeyAccount account) throws DataException {
return new BaseTransactionData(System.currentTimeMillis(), Group.NO_GROUP, account.getLastReference(), account.getPublicKey(), BlockChain.getInstance().getUnscaledUnitFee(), null); return new BaseTransactionData(System.currentTimeMillis(), Group.NO_GROUP, account.getLastReference(), account.getPublicKey(), BlockChain.getInstance().getUnitFee(), null);
} }
} }

View File

@ -71,7 +71,7 @@ public class RewardTests extends Common {
BlockUtils.mintBlock(repository); BlockUtils.mintBlock(repository);
expectedBalance += rewardInfo.unscaledReward; expectedBalance += rewardInfo.reward;
} }
AccountUtils.assertBalance(repository, "alice", Asset.QORT, expectedBalance); AccountUtils.assertBalance(repository, "alice", Asset.QORT, expectedBalance);
@ -106,8 +106,8 @@ public class RewardTests extends Common {
public void testLegacyQoraReward() throws DataException { public void testLegacyQoraReward() throws DataException {
Common.useSettings("test-settings-v2-qora-holder.json"); Common.useSettings("test-settings-v2-qora-holder.json");
long qoraHoldersShare = BlockChain.getInstance().getQoraHoldersUnscaledShare(); long qoraHoldersShare = BlockChain.getInstance().getQoraHoldersShare();
long qoraPerQort = BlockChain.getInstance().getUnscaledQoraPerQortReward(); long qoraPerQort = BlockChain.getInstance().getQoraPerQortReward();
try (final Repository repository = RepositoryManager.getRepository()) { try (final Repository repository = RepositoryManager.getRepository()) {
Map<String, Map<Long, Long>> initialBalances = AccountUtils.getBalances(repository, Asset.QORT, Asset.LEGACY_QORA, Asset.QORT_FROM_QORA); Map<String, Map<Long, Long>> initialBalances = AccountUtils.getBalances(repository, Asset.QORT, Asset.LEGACY_QORA, Asset.QORT_FROM_QORA);
@ -161,7 +161,7 @@ public class RewardTests extends Common {
public void testMaxLegacyQoraReward() throws DataException { public void testMaxLegacyQoraReward() throws DataException {
Common.useSettings("test-settings-v2-qora-holder.json"); Common.useSettings("test-settings-v2-qora-holder.json");
long qoraPerQort = BlockChain.getInstance().getUnscaledQoraPerQortReward(); long qoraPerQort = BlockChain.getInstance().getQoraPerQortReward();
try (final Repository repository = RepositoryManager.getRepository()) { try (final Repository repository = RepositoryManager.getRepository()) {
Map<String, Map<Long, Long>> initialBalances = AccountUtils.getBalances(repository, Asset.QORT, Asset.LEGACY_QORA, Asset.QORT_FROM_QORA); Map<String, Map<Long, Long>> initialBalances = AccountUtils.getBalances(repository, Asset.QORT, Asset.LEGACY_QORA, Asset.QORT_FROM_QORA);