From 6990766f7593751f4d872a53286e369c8a15605a Mon Sep 17 00:00:00 2001 From: CalDescent Date: Sat, 14 May 2022 12:43:54 +0100 Subject: [PATCH 1/6] Speed up unconfirmed transaction propagation. Currently, new transactions take a very long time to be included in each block (or reach the intended recipient), because each node has to obtain a repository lock and import the transaction before it notifies its peers. This can take a long time due to the lock being held by the block minter or synchronizer, and this compounds with every peer that the transaction is routed through. Validating signatures doesn't require a lock, and so can take place very soon after receipt of a new transaction. This change causes each node to broadcast a new transaction to its peers as soon as its signature is validated, rather than waiting until after the import. When a notified peer then makes a request for the transaction data itself, this can now be loaded from the sig-valid import queue as an alternative to the repository (since they won't be in the repository until after the import, which likely won't have happened yet). One small downside to this approach is that each unconfirmed transaction is now notified twice - once after the signature is deemed valid, and again in Controller.onNewTransaction(), but this should be an acceptable trade off given the speed improvements it should achieve. Another downside is that it could cause invalid transactions (with valid signatures) to propagate, but these would quickly be added to each peer's invalidUnconfirmedTransactions list after the import failure, and therefore be ignored. --- .../controller/TransactionImporter.java | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/qortal/controller/TransactionImporter.java b/src/main/java/org/qortal/controller/TransactionImporter.java index 39f45a14..9e90a8f0 100644 --- a/src/main/java/org/qortal/controller/TransactionImporter.java +++ b/src/main/java/org/qortal/controller/TransactionImporter.java @@ -3,6 +3,7 @@ package org.qortal.controller; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.qortal.data.transaction.TransactionData; +import org.qortal.network.Network; import org.qortal.network.Peer; import org.qortal.network.message.GetTransactionMessage; import org.qortal.network.message.Message; @@ -127,8 +128,12 @@ public class TransactionImporter extends Thread { LOGGER.debug("Validating signatures in incoming transactions queue (size {})...", unvalidatedCount); } + // A list of all currently pending transactions that have valid signatures List sigValidTransactions = new ArrayList<>(); + // A list of signatures that became valid in this round + List newlyValidSignatures = new ArrayList<>(); + boolean isLiteNode = Settings.getInstance().isLite(); // Signature validation round - does not require blockchain lock @@ -147,6 +152,7 @@ public class TransactionImporter extends Thread { if (isLiteNode) { // Lite nodes can't easily validate transactions, so for now we will have to assume that everything is valid sigValidTransactions.add(transaction); + newlyValidSignatures.add(transactionData.getSignature()); // Add mark signature as valid if transaction still exists in import queue incomingTransactions.computeIfPresent(transactionData, (k, v) -> Boolean.TRUE); continue; @@ -167,15 +173,19 @@ public class TransactionImporter extends Thread { invalidUnconfirmedTransactions.put(signature58, expiry); } + // We're done with this transaction continue; } - else { - // Count the number that were validated in this round, for logging purposes - validatedCount++; - } + + // Count the number that were validated in this round, for logging purposes + validatedCount++; // Add mark signature as valid if transaction still exists in import queue incomingTransactions.computeIfPresent(transactionData, (k, v) -> Boolean.TRUE); + + // Signature validated in this round + newlyValidSignatures.add(transactionData.getSignature()); + } else { LOGGER.trace(() -> String.format("Transaction %s known to have valid signature", Base58.encode(transactionData.getSignature()))); } @@ -188,6 +198,12 @@ public class TransactionImporter extends Thread { LOGGER.debug("Finished validating signatures in incoming transactions queue (valid this round: {}, total pending import: {})...", validatedCount, sigValidTransactions.size()); } + if (!newlyValidSignatures.isEmpty()) { + LOGGER.debug("Broadcasting {} newly valid signatures ahead of import", newlyValidSignatures.size()); + Message newTransactionSignatureMessage = new TransactionSignaturesMessage(newlyValidSignatures); + Network.getInstance().broadcast(broadcastPeer -> newTransactionSignatureMessage); + } + } catch (DataException e) { LOGGER.error("Repository issue while processing incoming transactions", e); } @@ -325,8 +341,18 @@ public class TransactionImporter extends Thread { byte[] signature = getTransactionMessage.getSignature(); try (final Repository repository = RepositoryManager.getRepository()) { - TransactionData transactionData = repository.getTransactionRepository().fromSignature(signature); + // Firstly check the sig-valid transactions that are currently queued for import + TransactionData transactionData = this.getCachedSigValidTransactions().stream() + .filter(t -> Arrays.equals(signature, t.getSignature())) + .findFirst().orElse(null); + if (transactionData == null) { + // Not found in import queue, so try the database + transactionData = repository.getTransactionRepository().fromSignature(signature); + } + + if (transactionData == null) { + // Still not found - so we don't have this transaction LOGGER.debug(() -> String.format("Ignoring GET_TRANSACTION request from peer %s for unknown transaction %s", peer, Base58.encode(signature))); // Send no response at all??? return; From fa3a81575a6f1a7de38c4edc8db6085cbaf33f9c Mon Sep 17 00:00:00 2001 From: CalDescent Date: Sat, 14 May 2022 12:53:36 +0100 Subject: [PATCH 2/6] Reduce wasted time that could otherwise be spent validating queued transaction signatures. --- .../qortal/controller/TransactionImporter.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/qortal/controller/TransactionImporter.java b/src/main/java/org/qortal/controller/TransactionImporter.java index 9e90a8f0..5a2dab19 100644 --- a/src/main/java/org/qortal/controller/TransactionImporter.java +++ b/src/main/java/org/qortal/controller/TransactionImporter.java @@ -19,7 +19,6 @@ import org.qortal.utils.Base58; import org.qortal.utils.NTP; import java.util.*; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -62,7 +61,7 @@ public class TransactionImporter extends Thread { try { while (!Controller.isStopping()) { - Thread.sleep(1000L); + Thread.sleep(500L); // Process incoming transactions queue validateTransactionsInQueue(); @@ -226,14 +225,9 @@ public class TransactionImporter extends Thread { return; } - try { - ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); - if (!blockchainLock.tryLock(2, TimeUnit.SECONDS)) { - LOGGER.debug("Too busy to import incoming transactions queue"); - return; - } - } catch (InterruptedException e) { - LOGGER.debug("Interrupted when trying to acquire blockchain lock"); + ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); + if (!blockchainLock.tryLock()) { + LOGGER.debug("Too busy to import incoming transactions queue"); return; } @@ -304,7 +298,6 @@ public class TransactionImporter extends Thread { } } finally { LOGGER.debug("Finished importing {} incoming transaction{}", processedCount, (processedCount == 1 ? "" : "s")); - ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock(); blockchainLock.unlock(); } } catch (DataException e) { From f41fbb3b3d9b4b7f6ec44686fdd1f69ede08fb3e Mon Sep 17 00:00:00 2001 From: CalDescent Date: Fri, 20 May 2022 13:38:56 +0100 Subject: [PATCH 3/6] Removed "consecutive blocks" limitation in block minter. --- src/main/java/org/qortal/controller/BlockMinter.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/main/java/org/qortal/controller/BlockMinter.java b/src/main/java/org/qortal/controller/BlockMinter.java index 9966d6a9..d1637ad3 100644 --- a/src/main/java/org/qortal/controller/BlockMinter.java +++ b/src/main/java/org/qortal/controller/BlockMinter.java @@ -212,14 +212,6 @@ public class BlockMinter extends Thread { // Do we need to build any potential new blocks? List newBlocksMintingAccounts = mintingAccountsData.stream().map(accountData -> new PrivateKeyAccount(repository, accountData.getPrivateKey())).collect(Collectors.toList()); - // We might need to sit the next block out, if one of our minting accounts signed the previous one - final byte[] previousBlockMinter = previousBlockData.getMinterPublicKey(); - final boolean mintedLastBlock = mintingAccountsData.stream().anyMatch(mintingAccount -> Arrays.equals(mintingAccount.getPublicKey(), previousBlockMinter)); - if (mintedLastBlock) { - LOGGER.trace(String.format("One of our keys signed the last block, so we won't sign the next one")); - continue; - } - if (parentSignatureForLastLowWeightBlock != null) { // The last iteration found a higher weight block in the network, so sleep for a while // to allow is to sync the higher weight chain. We are sleeping here rather than when From 9e8d85285fa6569fbee3cb6afc9e2ba86bbdc846 Mon Sep 17 00:00:00 2001 From: CalDescent Date: Fri, 20 May 2022 13:36:56 +0100 Subject: [PATCH 4/6] Removed extra unnecessary digest after writing new data. --- .../java/org/qortal/arbitrary/ArbitraryDataFile.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/qortal/arbitrary/ArbitraryDataFile.java b/src/main/java/org/qortal/arbitrary/ArbitraryDataFile.java index 9be4f145..1e86ee98 100644 --- a/src/main/java/org/qortal/arbitrary/ArbitraryDataFile.java +++ b/src/main/java/org/qortal/arbitrary/ArbitraryDataFile.java @@ -93,17 +93,10 @@ public class ArbitraryDataFile { File outputFile = outputFilePath.toFile(); try (FileOutputStream outputStream = new FileOutputStream(outputFile)) { outputStream.write(fileContent); - outputStream.close(); this.filePath = outputFilePath; - // Verify hash - String digest58 = this.digest58(); - if (!this.hash58.equals(digest58)) { - LOGGER.error("Hash {} does not match file digest {} for signature: {}", this.hash58, digest58, Base58.encode(signature)); - this.delete(); - throw new DataException("Data file digest validation failed"); - } } catch (IOException e) { - throw new DataException("Unable to write data to file"); + this.delete(); + throw new DataException(String.format("Unable to write data with hash %s: %s", this.hash58, e.getMessage())); } } From b73c041cc30ed683ead635644d5e0ff8a54fd781 Mon Sep 17 00:00:00 2001 From: CalDescent Date: Mon, 23 May 2022 20:31:36 +0100 Subject: [PATCH 5/6] Bump version to 3.3.1 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 4cc06769..2ccf552e 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.qortal qortal - 3.3.0 + 3.3.1 jar true From 551686c2de163789a5607fe5571d8e4f0a79e8bf Mon Sep 17 00:00:00 2001 From: CalDescent Date: Mon, 23 May 2022 21:54:25 +0100 Subject: [PATCH 6/6] Updated AdvancedInstaller project for v3.3.1 --- WindowsInstaller/Qortal.aip | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WindowsInstaller/Qortal.aip b/WindowsInstaller/Qortal.aip index e922943d..33e5d5c0 100755 --- a/WindowsInstaller/Qortal.aip +++ b/WindowsInstaller/Qortal.aip @@ -17,10 +17,10 @@ - + - + @@ -212,7 +212,7 @@ - +