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.
This commit is contained in:
catbref 2019-04-24 12:46:50 +01:00
parent d33ffee3ba
commit 126e651f27
11 changed files with 202 additions and 34 deletions

View File

@ -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.
* <p>
* 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<TransactionData> 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;
}
/**

View File

@ -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) {

View File

@ -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;

View File

@ -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);
}
}
}

View File

@ -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<Block> 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();
}
}
}

View File

@ -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;
}

View File

@ -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<byte[]> 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;
}

View File

@ -12,4 +12,6 @@ public interface NetworkRepository {
public int delete(PeerData peerData) throws DataException;
public int deleteAllPeers() throws DataException;
}

View File

@ -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);
}
}
}

View File

@ -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.
* <p>

View File

@ -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.
* <p>
* This is not done normally because we don't want unconfirmed transactions affecting validity of transactions already included in a block.
* <p>
* 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.
* <p>
* NOTE: temporarily updates accounts' lastReference to that from
* unconfirmed transactions, hence uses a repository savepoint.
* <p>
* 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.
* <p>
* NOTE: temporarily updates accounts' lastReference to that from
* unconfirmed transactions, hence uses a repository savepoint.
* <p>
* Also temporarily acquires blockchain lock.
*
* @return sorted unconfirmed transactions
* @throws DataException
*/
public static List<TransactionData> 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 <tt>repository.discardChanges()</tt>.
* <p>
* 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