Fix for chain-stall relating to freshly cancelled reward-shares.

In some cases, a freshly cancelled reward-share could still have
an associated signed timestamp. Block.mint() failed to spot this
and used an incorrect "online account" index when building the
to-be-minted block.

Block.mint() now checks that AccountRepository.getRewardShareIndex()
doesn't return null, i.e. indicating that the associated reward-share
for that "online account" no longer exists.

In turn, AccountRepository.getRewardShareIndex() didn't fulfill its
contract of returning null when the passed public key wasn't present
in the repository. So this method has been corrected also.

AccountRepository.rewardShareExists(byte[] publicKey) : boolean added.

BlockMinter had another bug where it didn't check the return from
Block.remint() for null properly. This has been fixed.

BlockMinter now has additional logging, with cool-off to prevent log
spam, for situations where minting could not happen.

Unit test (DisagreementTests) added to cover cancelled reward-share
case above. BlockMinter testing support slightly modified to help.

src/main/java/org/qortal/block/Block.java

@@ -323,7 +323,11 @@ public class Block {
 			if (onlineAccountData.getTimestamp() != onlineAccountsTimestamp)
 				continue;
 
-			int accountIndex = repository.getAccountRepository().getRewardShareIndex(onlineAccountData.getPublicKey());
+			Integer accountIndex = repository.getAccountRepository().getRewardShareIndex(onlineAccountData.getPublicKey());
+			if (accountIndex == null)
+				// Online account (reward-share) with current timestamp but reward-share cancelled
+				continue;
+
 			indexedOnlineAccounts.put(accountIndex, onlineAccountData);
 		}
 		List<Integer> accountIndexes = new ArrayList<>(indexedOnlineAccounts.keySet());

src/main/java/org/qortal/block/BlockMinter.java

@@ -40,6 +40,8 @@ public class BlockMinter extends Thread {
 
 	// Other properties
 	private static final Logger LOGGER = LogManager.getLogger(BlockMinter.class);
+	private static Long lastLogTimestamp;
+	private static Long logTimeout;
 
 	// Constructors
 
@@ -151,6 +153,9 @@ public class BlockMinter extends Thread {
 				if (previousBlock == null || !Arrays.equals(previousBlock.getSignature(), lastBlockData.getSignature())) {
 					previousBlock = new Block(repository, lastBlockData);
 					newBlocks.clear();
+
+					// Reduce log timeout
+					logTimeout = 10 * 1000L;
 				}
 
 				// Discard accounts we have already built blocks with
@@ -163,19 +168,23 @@ public class BlockMinter extends Thread {
 					// First block does the AT heavy-lifting
 					if (newBlocks.isEmpty()) {
 						Block newBlock = Block.mint(repository, previousBlock.getBlockData(), mintingAccount);
-						if (newBlock == null)
+						if (newBlock == null) {
 							// For some reason we can't mint right now
+							moderatedLog(() -> LOGGER.error("Couldn't build a to-be-minted block"));
 							continue;
+						}
 
 						newBlocks.add(newBlock);
 					} else {
 						// The blocks for other minters require less effort...
-						Block newBlock = newBlocks.get(0);
+						Block newBlock = newBlocks.get(0).remint(mintingAccount);
-						if (newBlock == null)
+						if (newBlock == null) {
 							// For some reason we can't mint right now
+							moderatedLog(() -> LOGGER.error("Couldn't rebuild a to-be-minted block"));
 							continue;
+						}
 
-						newBlocks.add(newBlock.remint(mintingAccount));
+						newBlocks.add(newBlock);
 					}
 				}
 
@@ -193,9 +202,14 @@ public class BlockMinter extends Thread {
 				boolean newBlockMinted = false;
 
 				try {
-					// Clear repository's "in transaction" state so we don't cause a repository deadlock
+					// Clear repository session state so we have latest view of data
 					repository.discardChanges();
 
+					// Now that we have blockchain lock, do final check that chain hasn't changed
+					BlockData latestBlockData = blockRepository.getLastBlock();
+					if (!Arrays.equals(lastBlockData.getSignature(), latestBlockData.getSignature()))
+						continue;
+
 					List<Block> goodBlocks = new ArrayList<>();
 					for (Block testBlock : newBlocks) {
 						// Is new block's timestamp valid yet?
@@ -204,8 +218,12 @@ public class BlockMinter extends Thread {
 							continue;
 
 						// Is new block valid yet? (Before adding unconfirmed transactions)
-						if (testBlock.isValid() != ValidationResult.OK)
+						ValidationResult result = testBlock.isValid();
+						if (result != ValidationResult.OK) {
+							moderatedLog(() -> LOGGER.error(String.format("To-be-minted block invalid '%s' before adding transactions?", result.name())));
+
 							continue;
+						}
 
 						goodBlocks.add(testBlock);
 					}
@@ -356,10 +374,14 @@ public class BlockMinter extends Thread {
 		// Ensure mintingAccount is 'online' so blocks can be minted
 		Controller.getInstance().ensureTestingAccountsOnline(mintingAndOnlineAccounts);
 
-		BlockData previousBlockData = repository.getBlockRepository().getLastBlock();
-
 		PrivateKeyAccount mintingAccount = mintingAndOnlineAccounts[0];
 
+		mintTestingBlockRetainingTimestamps(repository, mintingAccount);
+	}
+
+	public static void mintTestingBlockRetainingTimestamps(Repository repository, PrivateKeyAccount mintingAccount) throws DataException {
+		BlockData previousBlockData = repository.getBlockRepository().getLastBlock();
+
 		Block newBlock = Block.mint(repository, previousBlockData, mintingAccount);
 
 		// Make sure we're the only thread modifying the blockchain
@@ -387,4 +409,15 @@ public class BlockMinter extends Thread {
 		}
 	}
 
+	private static void moderatedLog(Runnable logFunction) {
+		// We only log if logging at TRACE or previous log timeout has expired
+		if (!LOGGER.isTraceEnabled() && lastLogTimestamp != null && lastLogTimestamp + logTimeout > System.currentTimeMillis())
+			return;
+
+		lastLogTimestamp = System.currentTimeMillis();
+		logTimeout = 2 * 60 * 1000L; // initial timeout, can be reduced if new block appears
+
+		logFunction.run();
+	}
+
 }

src/main/java/org/qortal/repository/AccountRepository.java

@@ -139,6 +139,8 @@ public interface AccountRepository {
 	 */
 	public RewardShareData getRewardShareByIndex(int index) throws DataException;
 
+	public boolean rewardShareExists(byte[] rewardSharePublicKey) throws DataException;
+
 	public void save(RewardShareData rewardShareData) throws DataException;
 
 	/** Delete reward-share from repository using passed minting account's public key and recipient's address. */

src/main/java/org/qortal/repository/hsqldb/HSQLDBAccountRepository.java

@@ -689,13 +689,13 @@ public class HSQLDBAccountRepository implements AccountRepository {
 	}
 
 	@Override
-	public Integer getRewardShareIndex(byte[] publicKey) throws DataException {
+	public Integer getRewardShareIndex(byte[] rewardSharePublicKey) throws DataException {
-		String sql = "SELECT COUNT(*) FROM RewardShares WHERE reward_share_public_key < ?";
+		if (!this.rewardShareExists(rewardSharePublicKey))
-
-		try (ResultSet resultSet = this.repository.checkedExecute(sql, publicKey)) {
-			if (resultSet == null)
 			return null;
 
+		String sql = "SELECT COUNT(*) FROM RewardShares WHERE reward_share_public_key < ?";
+
+		try (ResultSet resultSet = this.repository.checkedExecute(sql, rewardSharePublicKey)) {
 			return resultSet.getInt(1);
 		} catch (SQLException e) {
 			throw new DataException("Unable to determine reward-share index in repository", e);
@@ -724,6 +724,15 @@ public class HSQLDBAccountRepository implements AccountRepository {
 		}
 	}
 
+	@Override
+	public boolean rewardShareExists(byte[] rewardSharePublicKey) throws DataException {
+		try {
+			return this.repository.exists("RewardShares", "reward_share_public_key = ?", rewardSharePublicKey);
+		} catch (SQLException e) {
+			throw new DataException("Unable to check reward-share exists in repository", e);
+		}
+	}
+
 	@Override
 	public void save(RewardShareData rewardShareData) throws DataException {
 		HSQLDBSaver saveHelper = new HSQLDBSaver("RewardShares");

src/test/java/org/qortal/test/minting/DisagreementTests.java

@@ -0,0 +1,125 @@
+package org.qortal.test.minting;
+
+import static org.junit.Assert.*;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.qortal.account.PrivateKeyAccount;
+import org.qortal.block.BlockMinter;
+import org.qortal.controller.Controller;
+import org.qortal.data.account.RewardShareData;
+import org.qortal.data.block.BlockData;
+import org.qortal.data.transaction.TransactionData;
+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.Common;
+import org.qortal.test.common.TestAccount;
+import org.qortal.test.common.TransactionUtils;
+import org.qortal.transform.block.BlockTransformer;
+import org.roaringbitmap.IntIterator;
+
+import io.druid.extendedset.intset.ConciseSet;
+
+public class DisagreementTests extends Common {
+
+	private static final BigDecimal CANCEL_SHARE_PERCENT = BigDecimal.ONE.negate();
+
+	@Before
+	public void beforeTest() throws DataException {
+		Common.useDefaultSettings();
+	}
+
+	@After
+	public void afterTest() throws DataException {
+		Common.orphanCheck();
+	}
+
+	/**
+	 * Testing minting a block when there is a signed online account timestamp present
+	 * that no longer has a corresponding reward-share in DB.
+	 * <p>
+	 * Something like:
+	 * <ul>
+	 * <li>Mint block, with tx to create reward-share R</li>
+	 * <li>Sign current timestamp with R</li>
+	 * <li>Mint block including R as online account</li>
+	 * <li>Mint block, with tx to cancel reward-share R</li>
+	 * <li>Mint another block: R's timestamp should be excluded</li>
+	 * </ul>
+	 * 
+	 * @throws DataException
+	 */
+	@Test
+	public void testOnlineAccounts() throws DataException {
+		final BigDecimal sharePercent = new BigDecimal("12.8");
+
+		try (final Repository repository = RepositoryManager.getRepository()) {
+			TestAccount mintingAccount = Common.getTestAccount(repository, "alice-reward-share");
+			TestAccount signingAccount = Common.getTestAccount(repository, "alice");
+
+			// Create reward-share
+			byte[] testRewardSharePrivateKey = AccountUtils.rewardShare(repository, "alice", "bob", sharePercent);
+			PrivateKeyAccount testRewardShareAccount = new PrivateKeyAccount(repository, testRewardSharePrivateKey);
+
+			// Confirm reward-share info set correctly
+			RewardShareData testRewardShareData = repository.getAccountRepository().getRewardShare(testRewardShareAccount.getPublicKey());
+			assertNotNull(testRewardShareData);
+
+			// Create signed timestamps
+			Controller.getInstance().ensureTestingAccountsOnline(mintingAccount, testRewardShareAccount);
+
+			// Mint another block
+			BlockMinter.mintTestingBlockRetainingTimestamps(repository, mintingAccount);
+
+			// Confirm reward-share's signed timestamp is included
+			BlockData blockData = repository.getBlockRepository().getLastBlock();
+			List<RewardShareData> rewardSharesData = fetchRewardSharesForBlock(repository, blockData);
+			boolean doesContainRewardShare = rewardSharesData.stream().anyMatch(rewardShareData -> Arrays.equals(rewardShareData.getRewardSharePublicKey(), testRewardShareData.getRewardSharePublicKey()));
+			assertTrue(doesContainRewardShare);
+
+			// Cancel reward-share
+			TransactionData cancelRewardShareTransactionData = AccountUtils.createRewardShare(repository, "alice", "bob", CANCEL_SHARE_PERCENT);
+			TransactionUtils.signAsUnconfirmed(repository, cancelRewardShareTransactionData, signingAccount);
+			BlockMinter.mintTestingBlockRetainingTimestamps(repository, mintingAccount);
+
+			// Confirm reward-share no longer exists in repository
+			RewardShareData cancelledRewardShareData = repository.getAccountRepository().getRewardShare(testRewardShareAccount.getPublicKey());
+			assertNull("Reward-share shouldn't exist", cancelledRewardShareData);
+
+			// Attempt to mint with cancelled reward-share
+			BlockMinter.mintTestingBlockRetainingTimestamps(repository, mintingAccount);
+
+			// Confirm reward-share's signed timestamp is NOT included
+			blockData = repository.getBlockRepository().getLastBlock();
+			rewardSharesData = fetchRewardSharesForBlock(repository, blockData);
+			doesContainRewardShare = rewardSharesData.stream().anyMatch(rewardShareData -> Arrays.equals(rewardShareData.getRewardSharePublicKey(), testRewardShareData.getRewardSharePublicKey()));
+			assertFalse(doesContainRewardShare);
+		}
+	}
+
+	private List<RewardShareData> fetchRewardSharesForBlock(Repository repository, BlockData blockData) throws DataException {
+		byte[] encodedOnlineAccounts = blockData.getEncodedOnlineAccounts();
+		ConciseSet accountIndexes = BlockTransformer.decodeOnlineAccounts(encodedOnlineAccounts);
+
+		List<RewardShareData> rewardSharesData = new ArrayList<>();
+
+		IntIterator iterator = accountIndexes.iterator();
+		while (iterator.hasNext()) {
+			int accountIndex = iterator.next();
+
+			RewardShareData rewardShareData = repository.getAccountRepository().getRewardShareByIndex(accountIndex);
+			rewardSharesData.add(rewardShareData);
+		}
+
+		return rewardSharesData;
+	}
+
+}