From 783edb3447fc5b57c3512e64d84c387d3da9360d Mon Sep 17 00:00:00 2001 From: catbref Date: Fri, 21 Dec 2018 15:27:38 +0000 Subject: [PATCH] Wipe unconfirmed transactions on startup Fix issues with last reference and unconfirmed transactions. --- src/main/java/api/UnmarshalListener.java | 18 +++++++ .../java/api/resource/AssetsResource.java | 2 +- src/main/java/api/resource/NamesResource.java | 2 +- .../java/api/resource/PaymentsResource.java | 2 +- .../api/resource/TransactionsResource.java | 2 +- .../data/transaction/TransactionData.java | 5 ++ src/main/java/qora/block/BlockGenerator.java | 49 +++++++++++++++++-- .../transaction/IssueAssetTransaction.java | 4 ++ .../qora/transaction/PaymentTransaction.java | 4 ++ .../transaction/RegisterNameTransaction.java | 4 ++ .../java/qora/transaction/Transaction.java | 22 +++++++++ .../HSQLDBTransactionRepository.java | 5 ++ .../resources/i18n/ApiError_en.properties | 1 + 13 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 src/main/java/api/UnmarshalListener.java diff --git a/src/main/java/api/UnmarshalListener.java b/src/main/java/api/UnmarshalListener.java new file mode 100644 index 00000000..2296c2ef --- /dev/null +++ b/src/main/java/api/UnmarshalListener.java @@ -0,0 +1,18 @@ +package api; + +import javax.xml.bind.Unmarshaller.Listener; + +import data.transaction.TransactionData; + +public class UnmarshalListener extends Listener { + + @Override + public void afterUnmarshal(Object target, Object parent) { + if (!(target instanceof TransactionData)) + return; + + // do something + return; + } + +} diff --git a/src/main/java/api/resource/AssetsResource.java b/src/main/java/api/resource/AssetsResource.java index 97defed1..8860797f 100644 --- a/src/main/java/api/resource/AssetsResource.java +++ b/src/main/java/api/resource/AssetsResource.java @@ -258,7 +258,7 @@ public class AssetsResource { try (final Repository repository = RepositoryManager.getRepository()) { Transaction transaction = Transaction.fromData(repository, transactionData); - ValidationResult result = transaction.isValid(); + ValidationResult result = transaction.isValidUnconfirmed(); if (result != ValidationResult.OK) throw TransactionsResource.createTransactionInvalidException(request, result); diff --git a/src/main/java/api/resource/NamesResource.java b/src/main/java/api/resource/NamesResource.java index 2b6ebf37..e98ad23e 100644 --- a/src/main/java/api/resource/NamesResource.java +++ b/src/main/java/api/resource/NamesResource.java @@ -65,7 +65,7 @@ public class NamesResource { try (final Repository repository = RepositoryManager.getRepository()) { Transaction transaction = Transaction.fromData(repository, transactionData); - ValidationResult result = transaction.isValid(); + ValidationResult result = transaction.isValidUnconfirmed(); if (result != ValidationResult.OK) throw TransactionsResource.createTransactionInvalidException(request, result); diff --git a/src/main/java/api/resource/PaymentsResource.java b/src/main/java/api/resource/PaymentsResource.java index 5f2f26f0..08a74c3e 100644 --- a/src/main/java/api/resource/PaymentsResource.java +++ b/src/main/java/api/resource/PaymentsResource.java @@ -65,7 +65,7 @@ public class PaymentsResource { try (final Repository repository = RepositoryManager.getRepository()) { Transaction transaction = Transaction.fromData(repository, transactionData); - ValidationResult result = transaction.isValid(); + ValidationResult result = transaction.isValidUnconfirmed(); if (result != ValidationResult.OK) throw TransactionsResource.createTransactionInvalidException(request, result); diff --git a/src/main/java/api/resource/TransactionsResource.java b/src/main/java/api/resource/TransactionsResource.java index d20083ad..901e9132 100644 --- a/src/main/java/api/resource/TransactionsResource.java +++ b/src/main/java/api/resource/TransactionsResource.java @@ -319,7 +319,7 @@ public class TransactionsResource { if (!transaction.isSignatureValid()) throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_SIGNATURE); - ValidationResult result = transaction.isValid(); + ValidationResult result = transaction.isValidUnconfirmed(); if (result != ValidationResult.OK) throw createTransactionInvalidException(request, result); diff --git a/src/main/java/data/transaction/TransactionData.java b/src/main/java/data/transaction/TransactionData.java index f8a62394..45689d6d 100644 --- a/src/main/java/data/transaction/TransactionData.java +++ b/src/main/java/data/transaction/TransactionData.java @@ -111,6 +111,11 @@ public abstract class TransactionData { return Crypto.toAddress(this.creatorPublicKey); } + @XmlTransient + public void setCreatorPublicKey(byte[] creatorPublicKey) { + this.creatorPublicKey = creatorPublicKey; + } + // Comparison @Override diff --git a/src/main/java/qora/block/BlockGenerator.java b/src/main/java/qora/block/BlockGenerator.java index 233158a1..dd406c41 100644 --- a/src/main/java/qora/block/BlockGenerator.java +++ b/src/main/java/qora/block/BlockGenerator.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.Logger; import data.block.BlockData; import data.transaction.TransactionData; import qora.account.PrivateKeyAccount; +import qora.account.PublicKeyAccount; import qora.block.Block.ValidationResult; import qora.transaction.Transaction; import repository.BlockRepository; @@ -47,6 +48,12 @@ public class BlockGenerator extends Thread { Thread.currentThread().setName("BlockGenerator"); try (final Repository repository = RepositoryManager.getRepository()) { + // Wipe existing unconfirmed transactions + List unconfirmedTransactions = repository.getTransactionRepository().getAllUnconfirmedTransactions(); + for (TransactionData transactionData : unconfirmedTransactions) + repository.getTransactionRepository().delete(transactionData); + repository.saveChanges(); + generator = new PrivateKeyAccount(repository, generatorPrivateKey); // Going to need this a lot... @@ -109,10 +116,10 @@ public class BlockGenerator extends Thread { // Grab all unconfirmed transactions (already sorted) List unconfirmedTransactions = repository.getTransactionRepository().getAllUnconfirmedTransactions(); - // Remove transactions that have timestamp later than block's timestamp (not yet valid) - unconfirmedTransactions.removeIf(transactionData -> transactionData.getTimestamp() > newBlock.getBlockData().getTimestamp()); - // Remove transactions that have expired deadline for this block - unconfirmedTransactions.removeIf(transactionData -> Transaction.fromData(repository, transactionData).getDeadline() <= newBlock.getBlockData().getTimestamp()); + unconfirmedTransactions.removeIf(transactionData -> !isSuitableTransaction(repository, transactionData, newBlock)); + + // Discard last-reference changes used to aid transaction validity checks + repository.discardChanges(); // Attempt to add transactions until block is full, or we run out for (TransactionData transactionData : unconfirmedTransactions) @@ -120,6 +127,40 @@ public class BlockGenerator extends Thread { break; } + /** Returns true if transaction is suitable for adding to new block */ + private boolean isSuitableTransaction(Repository repository, TransactionData transactionData, Block newBlock) { + // Ignore transactions that have timestamp later than block's timestamp (not yet valid) + if (transactionData.getTimestamp() > newBlock.getBlockData().getTimestamp()) + return false; + + Transaction transaction = Transaction.fromData(repository, transactionData); + + // Ignore transactions that have expired deadline for this block + if (transaction.getDeadline() <= newBlock.getBlockData().getTimestamp()) + return false; + + // Ignore transactions that are currently not valid + try { + if (transaction.isValid() != Transaction.ValidationResult.OK) + return false; + } catch (DataException e) { + // Not good either + return false; + } + + // Good for adding to a block + // Temporarily update sender's last reference so that subsequent transactions validations work + // These updates will be discard on exit of addUnconfirmedTransactions() above + PublicKeyAccount creator = new PublicKeyAccount(repository, transactionData.getCreatorPublicKey()); + try { + creator.setLastReference(transactionData.getSignature()); + } catch (DataException e) { + // Not good + return false; + } + return true; + } + public void shutdown() { this.running = false; // Interrupt too, absorbed by HSQLDB but could be caught by Thread.sleep() diff --git a/src/main/java/qora/transaction/IssueAssetTransaction.java b/src/main/java/qora/transaction/IssueAssetTransaction.java index b18cdfc9..22effc19 100644 --- a/src/main/java/qora/transaction/IssueAssetTransaction.java +++ b/src/main/java/qora/transaction/IssueAssetTransaction.java @@ -32,6 +32,10 @@ public class IssueAssetTransaction extends Transaction { super(repository, transactionData); this.issueAssetTransactionData = (IssueAssetTransactionData) this.transactionData; + + // XXX This is horrible - thanks to JAXB unmarshalling not calling constructor + if (this.transactionData.getCreatorPublicKey() == null) + this.transactionData.setCreatorPublicKey(this.issueAssetTransactionData.getIssuerPublicKey()); } // More information diff --git a/src/main/java/qora/transaction/PaymentTransaction.java b/src/main/java/qora/transaction/PaymentTransaction.java index f7d124f9..05391d61 100644 --- a/src/main/java/qora/transaction/PaymentTransaction.java +++ b/src/main/java/qora/transaction/PaymentTransaction.java @@ -26,6 +26,10 @@ public class PaymentTransaction extends Transaction { super(repository, transactionData); this.paymentTransactionData = (PaymentTransactionData) this.transactionData; + + // XXX This is horrible - thanks to JAXB unmarshalling not calling constructor + if (this.transactionData.getCreatorPublicKey() == null) + this.transactionData.setCreatorPublicKey(this.paymentTransactionData.getSenderPublicKey()); } // More information diff --git a/src/main/java/qora/transaction/RegisterNameTransaction.java b/src/main/java/qora/transaction/RegisterNameTransaction.java index 29d12b18..c578fcd4 100644 --- a/src/main/java/qora/transaction/RegisterNameTransaction.java +++ b/src/main/java/qora/transaction/RegisterNameTransaction.java @@ -28,6 +28,10 @@ public class RegisterNameTransaction extends Transaction { super(repository, transactionData); this.registerNameTransactionData = (RegisterNameTransactionData) this.transactionData; + + // XXX This is horrible - thanks to JAXB unmarshalling not calling constructor + if (this.transactionData.getCreatorPublicKey() == null) + this.transactionData.setCreatorPublicKey(this.registerNameTransactionData.getRegistrantPublicKey()); } // More information diff --git a/src/main/java/qora/transaction/Transaction.java b/src/main/java/qora/transaction/Transaction.java index c586d2b6..6013b7f9 100644 --- a/src/main/java/qora/transaction/Transaction.java +++ b/src/main/java/qora/transaction/Transaction.java @@ -410,6 +410,28 @@ public abstract class Transaction { } } + /** + * Returns whether transaction can be added to unconfirmed transactions. + *

+ * NOTE: temporarily updates creator's lastReference to that from + * unconfirmed transactions, and hence calls repository.discardChanges() + * before exit. + *

+ * This is not done normally because we don't want unconfirmed transactions affecting validity of transactions already included in a block. + * + * @return true if transaction can be added to unconfirmed transactions, false otherwise + * @throws DataException + */ + public ValidationResult isValidUnconfirmed() throws DataException { + try { + Account creator = this.getCreator(); + creator.setLastReference(creator.getUnconfirmedLastReference()); + return this.isValid(); + } finally { + repository.discardChanges(); + } + } + /** * Returns whether transaction can be added to the blockchain. *

diff --git a/src/main/java/repository/hsqldb/transaction/HSQLDBTransactionRepository.java b/src/main/java/repository/hsqldb/transaction/HSQLDBTransactionRepository.java index 45fe2b89..0dda5dd5 100644 --- a/src/main/java/repository/hsqldb/transaction/HSQLDBTransactionRepository.java +++ b/src/main/java/repository/hsqldb/transaction/HSQLDBTransactionRepository.java @@ -517,6 +517,11 @@ public class HSQLDBTransactionRepository implements TransactionRepository { } catch (SQLException e) { throw new DataException("Unable to delete transaction from repository", e); } + try { + this.repository.delete("UnconfirmedTransactions", "signature = ?", transactionData.getSignature()); + } catch (SQLException e) { + throw new DataException("Unable to remove transaction from unconfirmed transactions repository", e); + } } } diff --git a/src/main/resources/i18n/ApiError_en.properties b/src/main/resources/i18n/ApiError_en.properties index 5a8c0c3c..efc816fc 100644 --- a/src/main/resources/i18n/ApiError_en.properties +++ b/src/main/resources/i18n/ApiError_en.properties @@ -47,6 +47,7 @@ BLOCK_NO_EXISTS=block does not exist # Transactions TRANSACTION_NO_EXISTS=transaction does not exist PUBLIC_KEY_NOT_FOUND=public key not found +TRANSACTION_INVALID=transaction invalid # Names NAME_NO_EXISTS=name does not exist