From b8ac128d5cde7a325360d61fec1987f05db36275 Mon Sep 17 00:00:00 2001 From: catbref Date: Fri, 21 Aug 2020 12:27:06 +0100 Subject: [PATCH] Improve comparing chains where some blocks signed with cancelled reward-share Symptoms include this in logs: Unexpected zero effective minter level for reward-share %s - using 1 instead! This occurs when Synchronizer compares two sub-chains from a common block, and one of the blocks is signed by a reward-share key that has subsequently been cancelled. Although this is catered for, excessive log-spam is emited. So in addition to demoting the log level from WARN to DEBUG, more code has been added to try harder to find the actual data needed, thus preventing the logging in the first place. New repository transaction search method added to support above, along with corresponding tests. --- .../java/org/qortal/block/BlockMinter.java | 2 +- .../org/qortal/controller/Synchronizer.java | 29 ++++++- .../repository/TransactionRepository.java | 16 ++++ .../HSQLDBTransactionRepository.java | 63 +++++++++++++++ .../qortal/test/TransactionSearchTests.java | 76 +++++++++++++++++++ 5 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/qortal/test/TransactionSearchTests.java diff --git a/src/main/java/org/qortal/block/BlockMinter.java b/src/main/java/org/qortal/block/BlockMinter.java index 279b0bc8..cde8617a 100644 --- a/src/main/java/org/qortal/block/BlockMinter.java +++ b/src/main/java/org/qortal/block/BlockMinter.java @@ -115,7 +115,7 @@ public class BlockMinter extends Thread { RewardShareData rewardShareData = repository.getAccountRepository().getRewardShare(mintingAccountData.getPublicKey()); if (rewardShareData == null) { - // Reward-share doesn't even exist - probably not a good sign + // Reward-share doesn't exist - probably cancelled but not yet removed from node's list of minting accounts madi.remove(); continue; } diff --git a/src/main/java/org/qortal/controller/Synchronizer.java b/src/main/java/org/qortal/controller/Synchronizer.java index b155a547..d55fef47 100644 --- a/src/main/java/org/qortal/controller/Synchronizer.java +++ b/src/main/java/org/qortal/controller/Synchronizer.java @@ -4,6 +4,7 @@ import java.math.BigInteger; import java.text.DecimalFormat; import java.text.NumberFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -11,11 +12,13 @@ import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.qortal.account.Account; +import org.qortal.account.PublicKeyAccount; import org.qortal.block.Block; import org.qortal.block.Block.ValidationResult; import org.qortal.data.block.BlockData; import org.qortal.data.block.BlockSummaryData; import org.qortal.data.network.PeerChainTipData; +import org.qortal.data.transaction.RewardShareTransactionData; import org.qortal.data.transaction.TransactionData; import org.qortal.network.Peer; import org.qortal.network.message.BlockMessage; @@ -550,16 +553,34 @@ public class Synchronizer { } private void populateBlockSummariesMinterLevels(Repository repository, List blockSummaries) throws DataException { + final int firstBlockHeight = blockSummaries.get(0).getHeight(); + for (int i = 0; i < blockSummaries.size(); ++i) { BlockSummaryData blockSummary = blockSummaries.get(i); // Qortal: minter is always a reward-share, so find actual minter and get their effective minting level int minterLevel = Account.getRewardShareEffectiveMintingLevel(repository, blockSummary.getMinterPublicKey()); if (minterLevel == 0) { - // We don't want to throw, or use zero, as this will kill Controller thread and make client unstable. - // So we log this but use 1 instead - LOGGER.warn(String.format("Unexpected zero effective minter level for reward-share %s - using 1 instead!", Base58.encode(blockSummary.getMinterPublicKey()))); - minterLevel = 1; + // It looks like this block's minter's reward-share has been cancelled. + // So search for REWARD_SHARE transactions since common block to find missing minter info + List transactionSignatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(Transaction.TransactionType.REWARD_SHARE, null, firstBlockHeight, null); + + for (byte[] transactionSignature : transactionSignatures) { + RewardShareTransactionData transactionData = (RewardShareTransactionData) repository.getTransactionRepository().fromSignature(transactionSignature); + + if (transactionData != null && Arrays.equals(transactionData.getRewardSharePublicKey(), blockSummary.getMinterPublicKey())) { + Account rewardShareMinter = new PublicKeyAccount(repository, transactionData.getMinterPublicKey()); + minterLevel = rewardShareMinter.getEffectiveMintingLevel(); + break; + } + } + + if (minterLevel == 0) { + // We don't want to throw, or use zero, as this will kill Controller thread and make client unstable. + // So we log this but use 1 instead + LOGGER.debug(() -> String.format("Unexpected zero effective minter level for reward-share %s - using 1 instead!", Base58.encode(blockSummary.getMinterPublicKey()))); + minterLevel = 1; + } } blockSummary.setMinterLevel(minterLevel); diff --git a/src/main/java/org/qortal/repository/TransactionRepository.java b/src/main/java/org/qortal/repository/TransactionRepository.java index 38856d24..0cb97f9e 100644 --- a/src/main/java/org/qortal/repository/TransactionRepository.java +++ b/src/main/java/org/qortal/repository/TransactionRepository.java @@ -91,6 +91,22 @@ public interface TransactionRepository { public List getSignaturesMatchingCriteria(TransactionType txType, byte[] publicKey, ConfirmationStatus confirmationStatus, Integer limit, Integer offset, Boolean reverse) throws DataException; + /** + * Returns signatures for transactions that match search criteria. + *

+ * Simpler version that only checks accepts one (optional) transaction type, + * and one (optional) public key, within an block height range. + * + * @param txType + * @param publicKey + * @param minBlockHeight + * @param maxBlockHeight + * @return + * @throws DataException + */ + public List getSignaturesMatchingCriteria(TransactionType txType, byte[] publicKey, + Integer minBlockHeight, Integer maxBlockHeight) throws DataException; + /** * Returns signature for latest auto-update transaction. *

diff --git a/src/main/java/org/qortal/repository/hsqldb/transaction/HSQLDBTransactionRepository.java b/src/main/java/org/qortal/repository/hsqldb/transaction/HSQLDBTransactionRepository.java index bf9c88aa..359940a7 100644 --- a/src/main/java/org/qortal/repository/hsqldb/transaction/HSQLDBTransactionRepository.java +++ b/src/main/java/org/qortal/repository/hsqldb/transaction/HSQLDBTransactionRepository.java @@ -586,6 +586,69 @@ public class HSQLDBTransactionRepository implements TransactionRepository { } } + @Override + public List getSignaturesMatchingCriteria(TransactionType txType, byte[] publicKey, + Integer minBlockHeight, Integer maxBlockHeight) throws DataException { + List signatures = new ArrayList<>(); + + StringBuilder sql = new StringBuilder(1024); + sql.append("SELECT signature FROM Transactions "); + + List whereClauses = new ArrayList<>(); + List bindParams = new ArrayList<>(); + + if (txType != null) { + whereClauses.add("type = ?"); + bindParams.add(txType.value); + } + + if (publicKey != null) { + whereClauses.add("creator = ?"); + bindParams.add(publicKey); + } + + if (minBlockHeight != null) { + whereClauses.add("Transactions.block_height >= ?"); + bindParams.add(minBlockHeight); + } + + if (maxBlockHeight != null) { + whereClauses.add("Transactions.block_height <= ?"); + bindParams.add(maxBlockHeight); + } + + if (!whereClauses.isEmpty()) { + sql.append(" WHERE "); + + final int whereClausesSize = whereClauses.size(); + for (int wci = 0; wci < whereClausesSize; ++wci) { + if (wci != 0) + sql.append(" AND "); + + sql.append(whereClauses.get(wci)); + } + } + + sql.append(" ORDER BY Transactions.created_when"); + + LOGGER.trace(() -> String.format("Transaction search SQL: %s", sql)); + + try (ResultSet resultSet = this.repository.checkedExecute(sql.toString(), bindParams.toArray())) { + if (resultSet == null) + return signatures; + + do { + byte[] signature = resultSet.getBytes(1); + + signatures.add(signature); + } while (resultSet.next()); + + return signatures; + } catch (SQLException e) { + throw new DataException("Unable to fetch matching transaction signatures from repository", e); + } + } + @Override public byte[] getLatestAutoUpdateTransaction(TransactionType txType, int txGroupId, Integer service) throws DataException { StringBuilder sql = new StringBuilder(1024); diff --git a/src/test/java/org/qortal/test/TransactionSearchTests.java b/src/test/java/org/qortal/test/TransactionSearchTests.java new file mode 100644 index 00000000..8cc75646 --- /dev/null +++ b/src/test/java/org/qortal/test/TransactionSearchTests.java @@ -0,0 +1,76 @@ +package org.qortal.test; + +import static org.junit.Assert.*; + +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.qortal.account.PrivateKeyAccount; +import org.qortal.repository.DataException; +import org.qortal.repository.Repository; +import org.qortal.repository.RepositoryManager; +import org.qortal.test.common.AccountUtils; +import org.qortal.test.common.BlockUtils; +import org.qortal.test.common.Common; +import org.qortal.transaction.Transaction.TransactionType; + +public class TransactionSearchTests extends Common { + + @Before + public void beforeTest() throws DataException { + Common.useDefaultSettings(); + } + + @Test + public void testFindingSpecificTransactionsWithinHeight() throws DataException { + try (final Repository repository = RepositoryManager.getRepository()) { + PrivateKeyAccount alice = Common.getTestAccount(repository, "alice"); + PrivateKeyAccount chloe = Common.getTestAccount(repository, "chloe"); + + // Block 2 + BlockUtils.mintBlock(repository); + + // Block 3 + AccountUtils.pay(repository, alice, chloe.getAddress(), 1234L); + + // Block 4 + AccountUtils.pay(repository, chloe, alice.getAddress(), 5678L); + + // Block 5 + BlockUtils.mintBlock(repository); + + List signatures; + + // No transactions with this type + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(TransactionType.GROUP_KICK, null, null, null); + assertEquals(0, signatures.size()); + + // 2 payments + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(TransactionType.PAYMENT, null, null, null); + assertEquals(2, signatures.size()); + + // 1 payment by Alice + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(TransactionType.PAYMENT, alice.getPublicKey(), null, null); + assertEquals(1, signatures.size()); + + // 1 transaction by Chloe + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(null, chloe.getPublicKey(), null, null); + assertEquals(1, signatures.size()); + + // 1 transaction from blocks 4 onwards + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(null, null, 4, null); + assertEquals(1, signatures.size()); + + // 1 transaction from blocks 2 to 3 + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(null, null, 2, 3); + assertEquals(1, signatures.size()); + + // No transaction of this type from blocks 2 to 5 + signatures = repository.getTransactionRepository().getSignaturesMatchingCriteria(TransactionType.ISSUE_ASSET, null, 2, 5); + assertEquals(0, signatures.size()); + } + + } + +}