From 126e651f27210ef3077e8b0be17c59d7cdc05350 Mon Sep 17 00:00:00 2001 From: catbref Date: Wed, 24 Apr 2019 12:46:50 +0100 Subject: [PATCH] Change sync consensus to favour lower-value block sigs + other changes API /addresses/{address} now returns lastReference taking unconfirmed into account. Added DELETE /peers/known to remove all known peers from repository. Added blockchain locking around Transaction methods like isValidUnconfirmed as they (temporarily) update account lastReference. Ditto getInvalidTransactions, etc. --- src/main/java/org/qora/account/Account.java | 26 +++++++--- .../qora/api/resource/AddressesResource.java | 7 +++ .../org/qora/api/resource/AdminResource.java | 6 +++ .../org/qora/api/resource/PeersResource.java | 33 ++++++++++++ .../java/org/qora/block/BlockGenerator.java | 44 ++++++++-------- .../java/org/qora/controller/Controller.java | 5 +- .../org/qora/controller/Synchronizer.java | 43 +++++++++++++++- .../qora/repository/NetworkRepository.java | 2 + .../hsqldb/HSQLDBNetworkRepository.java | 8 +++ .../repository/hsqldb/HSQLDBRepository.java | 12 +++++ .../org/qora/transaction/Transaction.java | 50 ++++++++++++++++++- 11 files changed, 202 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/qora/account/Account.java b/src/main/java/org/qora/account/Account.java index 91acf47f..50da8c7b 100644 --- a/src/main/java/org/qora/account/Account.java +++ b/src/main/java/org/qora/account/Account.java @@ -171,10 +171,28 @@ public class Account { * @throws DataException */ public byte[] getUnconfirmedLastReference() throws DataException { + byte[] reference = getUnconfirmedLastReference(null); + + if (reference == null) + // No unconfirmed transactions + reference = getLastReference(); + + return reference; + } + + /** + * Fetch last reference for account, considering unconfirmed transactions only, or return defaultReference. + *

+ * NOTE: a repository savepoint may be used during execution. + * + * @return byte[] reference, or defaultReference if no unconfirmed transactions for this account. + * @throws DataException + */ + public byte[] getUnconfirmedLastReference(byte[] defaultReference) throws DataException { // Newest unconfirmed transaction takes priority List unconfirmedTransactions = Transaction.getUnconfirmedTransactions(repository); - byte[] reference = null; + byte[] reference = defaultReference; for (TransactionData transactionData : unconfirmedTransactions) { String address = PublicKeyAccount.getAddress(transactionData.getCreatorPublicKey()); @@ -183,11 +201,7 @@ public class Account { reference = transactionData.getSignature(); } - if (reference != null) - return reference; - - // No unconfirmed transactions - return getLastReference(); + return reference; } /** diff --git a/src/main/java/org/qora/api/resource/AddressesResource.java b/src/main/java/org/qora/api/resource/AddressesResource.java index d11b0c03..4d3f4dc8 100644 --- a/src/main/java/org/qora/api/resource/AddressesResource.java +++ b/src/main/java/org/qora/api/resource/AddressesResource.java @@ -75,6 +75,13 @@ public class AddressesResource { // Not found? if (accountData == null) accountData = new AccountData(address); + else { + // Unconfirmed transactions could update lastReference + Account account = new Account(repository, address); + byte[] unconfirmedLastReference = account.getUnconfirmedLastReference(null); + if (unconfirmedLastReference != null) + accountData.setReference(unconfirmedLastReference); + } return accountData; } catch (ApiException e) { diff --git a/src/main/java/org/qora/api/resource/AdminResource.java b/src/main/java/org/qora/api/resource/AdminResource.java index 37b3d7db..5d1a64cc 100644 --- a/src/main/java/org/qora/api/resource/AdminResource.java +++ b/src/main/java/org/qora/api/resource/AdminResource.java @@ -26,6 +26,12 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.FileAppender; +import org.apache.logging.log4j.core.appender.RollingFileAppender; import org.qora.account.PrivateKeyAccount; import org.qora.api.ApiError; import org.qora.api.ApiErrors; diff --git a/src/main/java/org/qora/api/resource/PeersResource.java b/src/main/java/org/qora/api/resource/PeersResource.java index a46085f6..2bcacd6f 100644 --- a/src/main/java/org/qora/api/resource/PeersResource.java +++ b/src/main/java/org/qora/api/resource/PeersResource.java @@ -217,4 +217,37 @@ public class PeersResource { } } + @DELETE + @Path("/known") + @Operation( + summary = "Remove any known peers from database", + responses = { + @ApiResponse( + description = "true if any peers were removed, false if there were no peers to delete", + content = @Content( + schema = @Schema( + type = "string" + ) + ) + ) + } + ) + @ApiErrors({ + ApiError.INVALID_DATA, ApiError.REPOSITORY_ISSUE + }) + public String removeKnownPeers(String address) { + Security.checkApiCallAllowed(request); + + try (final Repository repository = RepositoryManager.getRepository()) { + int numDeleted = repository.getNetworkRepository().deleteAllPeers(); + repository.saveChanges(); + + return numDeleted != 0 ? "true" : "false"; + } catch (ApiException e) { + throw e; + } catch (DataException e) { + throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.REPOSITORY_ISSUE, e); + } + } + } diff --git a/src/main/java/org/qora/block/BlockGenerator.java b/src/main/java/org/qora/block/BlockGenerator.java index 0b93774a..bb535c15 100644 --- a/src/main/java/org/qora/block/BlockGenerator.java +++ b/src/main/java/org/qora/block/BlockGenerator.java @@ -4,7 +4,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Random; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -93,7 +93,7 @@ public class BlockGenerator extends Thread { } // Make sure we're the only thread modifying the blockchain - Lock blockchainLock = Controller.getInstance().getBlockchainLock(); + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); if (blockchainLock.tryLock()) generation: try { List goodBlocks = new ArrayList<>(); @@ -244,30 +244,30 @@ public class BlockGenerator extends Thread { Block newBlock = new Block(repository, previousBlockData, generator); // Make sure we're the only thread modifying the blockchain - Lock blockchainLock = Controller.getInstance().getBlockchainLock(); - if (blockchainLock.tryLock()) - try { - // Delete invalid transactions - deleteInvalidTransactions(repository); + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); + blockchainLock.lock(); + try { + // Delete invalid transactions + deleteInvalidTransactions(repository); - // Add unconfirmed transactions - addUnconfirmedTransactions(repository, newBlock); + // Add unconfirmed transactions + addUnconfirmedTransactions(repository, newBlock); - // Sign to create block's signature - newBlock.sign(); + // Sign to create block's signature + newBlock.sign(); - // Is newBlock still valid? - ValidationResult validationResult = newBlock.isValid(); - if (validationResult != ValidationResult.OK) - throw new IllegalStateException( - "Valid, generated block now invalid '" + validationResult.name() + "' after adding unconfirmed transactions?"); + // Is newBlock still valid? + ValidationResult validationResult = newBlock.isValid(); + if (validationResult != ValidationResult.OK) + throw new IllegalStateException( + "Valid, generated block now invalid '" + validationResult.name() + "' after adding unconfirmed transactions?"); - // Add to blockchain - newBlock.process(); - repository.saveChanges(); - } finally { - blockchainLock.unlock(); - } + // Add to blockchain + newBlock.process(); + repository.saveChanges(); + } finally { + blockchainLock.unlock(); + } } } diff --git a/src/main/java/org/qora/controller/Controller.java b/src/main/java/org/qora/controller/Controller.java index 751bb8e8..c414cc73 100644 --- a/src/main/java/org/qora/controller/Controller.java +++ b/src/main/java/org/qora/controller/Controller.java @@ -10,7 +10,6 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; import java.util.Properties; -import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import org.apache.logging.log4j.LogManager; @@ -66,7 +65,7 @@ public class Controller extends Thread { private final long buildTimestamp; /** Lock for only allowing one blockchain-modifying codepath at a time. e.g. synchronization or newly generated block. */ - private final Lock blockchainLock; + private final ReentrantLock blockchainLock; private Controller() { Properties properties = new Properties(); @@ -126,7 +125,7 @@ public class Controller extends Thread { } } - public Lock getBlockchainLock() { + public ReentrantLock getBlockchainLock() { return this.blockchainLock; } diff --git a/src/main/java/org/qora/controller/Synchronizer.java b/src/main/java/org/qora/controller/Synchronizer.java index b8acc483..cae32993 100644 --- a/src/main/java/org/qora/controller/Synchronizer.java +++ b/src/main/java/org/qora/controller/Synchronizer.java @@ -3,7 +3,7 @@ package org.qora.controller; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -48,7 +48,7 @@ public class Synchronizer { public boolean synchronize(Peer peer) { // Make sure we're the only thread modifying the blockchain // If we're already synchronizing with another peer then this will also return fast - Lock blockchainLock = Controller.getInstance().getBlockchainLock(); + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); if (blockchainLock.tryLock()) try { try (final Repository repository = RepositoryManager.getRepository()) { @@ -67,6 +67,8 @@ public class Synchronizer { // First signature is common block BlockData commonBlockData = this.repository.getBlockRepository().fromSignature(signatures.get(0)); + int commonBlockHeight = commonBlockData.getHeight(); + LOGGER.info(String.format("Common block with peer %s is at height %d", peer, commonBlockHeight)); signatures.remove(0); // If common block is too far behind us then we're on massively different forks so give up. @@ -76,6 +78,42 @@ public class Synchronizer { return false; } + // If we have blocks after common block then decide whether we want to sync (lowest block signature wins) + for (int height = commonBlockHeight + 1; height <= peerHeight && height <= this.ourHeight; ++height) { + int sigIndex = height - commonBlockHeight - 1; + + // Do we need more signatures? + if (signatures.size() - 1 < sigIndex) { + // Grab more signatures + byte[] previousSignature = sigIndex == 0 ? commonBlockData.getSignature() : signatures.get(sigIndex - 1); + List moreSignatures = this.getBlockSignatures(peer, previousSignature, MAXIMUM_BLOCK_STEP); + if (moreSignatures == null || moreSignatures.isEmpty()) { + LOGGER.info(String.format("Peer %s failed to respond with more block signatures after height %d", peer, height - 1)); + return false; + } + + signatures.addAll(moreSignatures); + } + + byte[] ourSignature = this.repository.getBlockRepository().fromHeight(height).getSignature(); + byte[] peerSignature = signatures.get(sigIndex); + + for (int i = 0; i < ourSignature.length; ++i) { + /* + * If our byte is lower, we don't synchronize with this peer, + * if their byte is lower, check next height, + * (if bytes are equal, try next byte). + */ + if (ourSignature[i] < peerSignature[i]) { + LOGGER.info(String.format("Not synchronizing with peer %s as we have better block at height %d", peer, height)); + return false; + } + + if (peerSignature[i] < ourSignature[i]) + break; + } + } + if (this.ourHeight > commonBlockData.getHeight()) { // Unwind to common block (unless common block is our latest block) LOGGER.debug(String.format("Orphaning blocks back to height %d", commonBlockData.getHeight())); @@ -217,6 +255,7 @@ public class Synchronizer { BlockData blockData = this.repository.getBlockRepository().fromSignature(blockSignatures.get(i)); if (blockData != null) { + // Note: index i isn't cleared: List.subList is fromIndex inclusive to toIndex exclusive blockSignatures.subList(0, i).clear(); break; } diff --git a/src/main/java/org/qora/repository/NetworkRepository.java b/src/main/java/org/qora/repository/NetworkRepository.java index 55abf726..8b52fbea 100644 --- a/src/main/java/org/qora/repository/NetworkRepository.java +++ b/src/main/java/org/qora/repository/NetworkRepository.java @@ -12,4 +12,6 @@ public interface NetworkRepository { public int delete(PeerData peerData) throws DataException; + public int deleteAllPeers() throws DataException; + } diff --git a/src/main/java/org/qora/repository/hsqldb/HSQLDBNetworkRepository.java b/src/main/java/org/qora/repository/hsqldb/HSQLDBNetworkRepository.java index 496955d1..7f718fb2 100644 --- a/src/main/java/org/qora/repository/hsqldb/HSQLDBNetworkRepository.java +++ b/src/main/java/org/qora/repository/hsqldb/HSQLDBNetworkRepository.java @@ -84,4 +84,12 @@ public class HSQLDBNetworkRepository implements NetworkRepository { } } + @Override + public int deleteAllPeers() throws DataException { + try { + return this.repository.delete("Peers"); + } catch (SQLException e) { + throw new DataException("Unable to delete peers from repository", e); + } + } } diff --git a/src/main/java/org/qora/repository/hsqldb/HSQLDBRepository.java b/src/main/java/org/qora/repository/hsqldb/HSQLDBRepository.java index 297a83ae..fc61cc21 100644 --- a/src/main/java/org/qora/repository/hsqldb/HSQLDBRepository.java +++ b/src/main/java/org/qora/repository/hsqldb/HSQLDBRepository.java @@ -339,6 +339,18 @@ public class HSQLDBRepository implements Repository { } } + /** + * Delete all rows from database table. + * + * @param tableName + * @throws SQLException + */ + public int delete(String tableName) throws SQLException { + try (PreparedStatement preparedStatement = this.connection.prepareStatement("DELETE FROM " + tableName)) { + return this.checkedExecuteUpdateCount(preparedStatement); + } + } + /** * Returns additional SQL "LIMIT" and "OFFSET" clauses. *

diff --git a/src/main/java/org/qora/transaction/Transaction.java b/src/main/java/org/qora/transaction/Transaction.java index 4b3258db..ad0182b7 100644 --- a/src/main/java/org/qora/transaction/Transaction.java +++ b/src/main/java/org/qora/transaction/Transaction.java @@ -8,6 +8,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.ReentrantLock; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -15,6 +16,7 @@ import org.qora.account.Account; import org.qora.account.PrivateKeyAccount; import org.qora.account.PublicKeyAccount; import org.qora.block.BlockChain; +import org.qora.controller.Controller; import org.qora.data.block.BlockData; import org.qora.data.group.GroupData; import org.qora.data.transaction.TransactionData; @@ -474,6 +476,8 @@ public abstract class Transaction { * unconfirmed transactions, and hence uses a repository savepoint. *

* This is not done normally because we don't want unconfirmed transactions affecting validity of transactions already included in a block. + *

+ * Also temporarily acquires blockchain lock. * * @return true if transaction can be added to unconfirmed transactions, false otherwise * @throws DataException @@ -493,6 +497,14 @@ public abstract class Transaction { if (!hasMinimumFee() || !hasMinimumFeePerByte()) return ValidationResult.INSUFFICIENT_FEE; + /* + * We have to grab the blockchain lock because we're updating + * when we fake the creator's last reference, + * even though we throw away the update when we rollback the + * savepoint. + */ + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); + blockchainLock.lock(); repository.setSavepoint(); try { PublicKeyAccount creator = this.getCreator(); @@ -513,6 +525,7 @@ public abstract class Transaction { return result; } finally { repository.rollbackToSavepoint(); + blockchainLock.unlock(); } } @@ -558,10 +571,12 @@ public abstract class Transaction { } /** - * Returns sorted, unconfirmed transactions, deleting invalid. + * Returns sorted, unconfirmed transactions, excluding invalid. *

* NOTE: temporarily updates accounts' lastReference to that from * unconfirmed transactions, hence uses a repository savepoint. + *

+ * Also temporarily acquires blockchain lock. * * @return sorted unconfirmed transactions * @throws DataException @@ -573,6 +588,14 @@ public abstract class Transaction { unconfirmedTransactions.sort(getDataComparator()); + /* + * We have to grab the blockchain lock because we're updating + * when we fake the creator's last reference, + * even though we throw away the update when we rollback the + * savepoint. + */ + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); + blockchainLock.lock(); repository.setSavepoint(); try { for (int i = 0; i < unconfirmedTransactions.size(); ++i) { @@ -587,11 +610,23 @@ public abstract class Transaction { } finally { // Throw away temporary updates to account lastReference repository.rollbackToSavepoint(); + blockchainLock.unlock(); } return unconfirmedTransactions; } + /** + * Returns invalid, unconfirmed transactions. + *

+ * NOTE: temporarily updates accounts' lastReference to that from + * unconfirmed transactions, hence uses a repository savepoint. + *

+ * Also temporarily acquires blockchain lock. + * + * @return sorted unconfirmed transactions + * @throws DataException + */ public static List getInvalidTransactions(Repository repository) throws DataException { BlockData latestBlockData = repository.getBlockRepository().getLastBlock(); @@ -600,6 +635,14 @@ public abstract class Transaction { unconfirmedTransactions.sort(getDataComparator()); + /* + * We have to grab the blockchain lock because we're updating + * when we fake the creator's last reference, + * even though we throw away the update when we rollback the + * savepoint. + */ + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); + blockchainLock.lock(); repository.setSavepoint(); try { for (int i = 0; i < unconfirmedTransactions.size(); ++i) { @@ -616,6 +659,7 @@ public abstract class Transaction { } finally { // Throw away temporary updates to account lastReference repository.rollbackToSavepoint(); + blockchainLock.unlock(); } return invalidTransactions; @@ -627,6 +671,10 @@ public abstract class Transaction { * NOTE: temporarily updates creator's lastReference to that from * unconfirmed transactions, and hence caller should use a repository * savepoint or invoke repository.discardChanges(). + *

+ * Caller should also hold the blockchain lock as we're 'updating' + * when we fake the transaction creator's last reference, even if + * it discarded at rollback. * * @return true if transaction can be added to unconfirmed transactions, false otherwise * @throws DataException