crowetic/qortal
1
0
Fork 0
forked from Qortal/qortal
Code Pull Requests Activity

Clean up Transaction.isStillValidUnconfirmed()

Browse Source 
Brought more into line with isValidUnconfirmed().
No need to update creator's lastReference under new last-ref scheme.

Correspondingly, no need to acquire blockchain lock or repository
shenanigans in getUnconfirmedTransactions() and getInvalidTransactions()
for the same reason.

getInvalidTransactions() seems to be unused and may well be cleaned up
in a future commit.
This commit is contained in:
catbref 2020-05-04 09:14:52 +01:00
parent 71e80bd02f
commit 538e117abd
1 changed files with 48 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

145
src/main/java/org/qortal/transaction/Transaction.java
View File

@ -486,9 +486,6 @@ public abstract class Transaction {
/** /**
* Returns whether transaction can be added to unconfirmed transactions. * Returns whether transaction can be added to unconfirmed transactions.
* <p>
* NOTE: temporarily updates accounts' lastReference to check validity.<br>
* To do this, blockchain lock is obtained and pending repository changes are discarded.
* *
* @return transaction validation result, e.g. OK * @return transaction validation result, e.g. OK
* @throws DataException * @throws DataException
@ -502,7 +499,7 @@ public abstract class Transaction {
if (now >= this.getDeadline()) if (now >= this.getDeadline())
return ValidationResult.TIMESTAMP_TOO_OLD; return ValidationResult.TIMESTAMP_TOO_OLD;
// Transactions with a timestamp prior to latest block's timestamp are too old // Transactions with a expiry prior to latest block's timestamp are too old
BlockData latestBlock = repository.getBlockRepository().getLastBlock(); BlockData latestBlock = repository.getBlockRepository().getLastBlock();
if (this.getDeadline() <= latestBlock.getTimestamp()) if (this.getDeadline() <= latestBlock.getTimestamp())
return ValidationResult.TIMESTAMP_TOO_OLD; return ValidationResult.TIMESTAMP_TOO_OLD;
@ -595,9 +592,6 @@ public abstract class Transaction {
/** /**
* Returns sorted, unconfirmed transactions, excluding invalid. * Returns sorted, unconfirmed transactions, excluding invalid.
* <p>
* NOTE: temporarily updates accounts' lastReference to check validity.<br>
* To do this, blockchain lock is obtained and pending repository changes are discarded.
* *
* @return sorted, unconfirmed transactions * @return sorted, unconfirmed transactions
* @throws DataException * @throws DataException
@ -609,35 +603,15 @@ public abstract class Transaction {
unconfirmedTransactions.sort(getDataComparator()); unconfirmedTransactions.sort(getDataComparator());
/* Iterator<TransactionData> unconfirmedTransactionsIterator = unconfirmedTransactions.iterator();
* We have to grab the blockchain lock because we're updating while (unconfirmedTransactionsIterator.hasNext()) {
* when we fake the creator's last reference, TransactionData transactionData = unconfirmedTransactionsIterator.next();
* even though we throw away the update when we rollback the Transaction transaction = Transaction.fromData(repository, transactionData);
* savepoint.
*/
ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock();
blockchainLock.lock();
try {
// Clear repository's "in transaction" state so we don't cause a repository deadlock
repository.discardChanges();
try { if (transaction.isStillValidUnconfirmed(latestBlockData.getTimestamp()) != ValidationResult.OK)
Iterator<TransactionData> unconfirmedTransactionsIterator = unconfirmedTransactions.iterator(); continue;
while (unconfirmedTransactionsIterator.hasNext()) {
TransactionData transactionData = unconfirmedTransactionsIterator.next();
if (isStillValidUnconfirmed(repository, transactionData, latestBlockData.getTimestamp())) unconfirmedTransactionsIterator.remove();
continue;
unconfirmedTransactionsIterator.remove();
}
} finally {
// Throw away temporary updates to account lastReference
repository.discardChanges();
}
} finally {
// In separate finally block just in case rollback throws
blockchainLock.unlock();
} }
return unconfirmedTransactions; return unconfirmedTransactions;
@ -645,9 +619,6 @@ public abstract class Transaction {
/** /**
* Returns invalid, unconfirmed transactions. * Returns invalid, unconfirmed transactions.
* <p>
* NOTE: temporarily updates accounts' lastReference to check validity.<br>
* To do this, blockchain lock is obtained and pending repository changes are discarded.
* *
* @return sorted, invalid, unconfirmed transactions * @return sorted, invalid, unconfirmed transactions
* @throws DataException * @throws DataException
@ -660,36 +631,16 @@ public abstract class Transaction {
unconfirmedTransactions.sort(getDataComparator()); unconfirmedTransactions.sort(getDataComparator());
/* Iterator<TransactionData> unconfirmedTransactionsIterator = unconfirmedTransactions.iterator();
* We have to grab the blockchain lock because we're updating while (unconfirmedTransactionsIterator.hasNext()) {
* when we fake the creator's last reference, TransactionData transactionData = unconfirmedTransactionsIterator.next();
* even though we throw away the update when we rollback the Transaction transaction = Transaction.fromData(repository, transactionData);
* savepoint.
*/
ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock();
blockchainLock.lock();
try {
// Clear repository's "in transaction" state so we don't cause a repository deadlock
repository.discardChanges();
try { if (transaction.isStillValidUnconfirmed(latestBlockData.getTimestamp()) != ValidationResult.OK)
Iterator<TransactionData> unconfirmedTransactionsIterator = unconfirmedTransactions.iterator(); continue;
while (unconfirmedTransactionsIterator.hasNext()) {
TransactionData transactionData = unconfirmedTransactionsIterator.next();
if (isStillValidUnconfirmed(repository, transactionData, latestBlockData.getTimestamp())) invalidTransactions.add(transactionData);
continue; unconfirmedTransactionsIterator.remove();
invalidTransactions.add(transactionData);
unconfirmedTransactionsIterator.remove();
}
} finally {
// Throw away temporary updates to account lastReference
repository.discardChanges();
}
} finally {
// In separate finally block just in case rollback throws
blockchainLock.unlock();
} }
return invalidTransactions; return invalidTransactions;
@ -698,50 +649,50 @@ public abstract class Transaction {
/** /**
* Returns whether transaction is still a valid unconfirmed transaction. * Returns whether transaction is still a valid unconfirmed transaction.
* <p> * <p>
* NOTE: temporarily updates creator's lastReference to that from * This is like {@link #isValidUnconfirmed()} but only needs to perform
* unconfirmed transactions, and hence caller should use a repository * a subset of those checks.
* 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 * @return transaction validation result, e.g. OK
* @throws DataException * @throws DataException
*/ */
private static boolean isStillValidUnconfirmed(Repository repository, TransactionData transactionData, long blockTimestamp) throws DataException { private ValidationResult isStillValidUnconfirmed(long blockTimestamp) throws DataException {
final Long now = NTP.getTime(); final Long now = NTP.getTime();
if (now == null) if (now == null)
return false; return ValidationResult.CLOCK_NOT_SYNCED;
Transaction transaction = Transaction.fromData(repository, transactionData); // Expired already?
if (now >= this.getDeadline())
return ValidationResult.TIMESTAMP_TOO_OLD;
// Check transaction has not expired // Transactions with a expiry prior to latest block's timestamp are too old
if (transaction.getDeadline() <= blockTimestamp || transaction.getDeadline() < now) if (this.getDeadline() <= blockTimestamp)
return false; return ValidationResult.TIMESTAMP_TOO_OLD;
// Is transaction is past max approval period? // Transactions with a timestamp too far into future are too new
if (transaction.needsGroupApproval()) { // Skipped because this test only applies at instant of submission
int txGroupId = transactionData.getTxGroupId();
GroupData groupData = repository.getGroupRepository().fromGroupId(txGroupId);
int creationBlockHeight = repository.getBlockRepository().getHeightFromTimestamp(transactionData.getTimestamp()); // Check fee is sufficient
int currentBlockHeight = repository.getBlockRepository().getBlockchainHeight(); // Skipped because this is checked upon submission and the result would be the same now
if (currentBlockHeight > creationBlockHeight + groupData.getMaximumBlockDelay())
return false;
}
// Check transaction is currently valid // Reject if unconfirmed pile already has X transactions from same creator
if (transaction.isValid() != Transaction.ValidationResult.OK) // Skipped because this test only applies at instant of submission
return false;
// Good for adding to a block // Check transaction's txGroupId
// Temporarily update sender's last reference so that subsequent transactions validations work // Skipped because this is checked upon submission and the result would be the same now
// These updates should be discarded by some caller further up stack
PublicKeyAccount creator = new PublicKeyAccount(repository, transactionData.getCreatorPublicKey());
creator.setLastReference(transactionData.getSignature());
return true; // Check transaction references
if (!this.hasValidReference())
return ValidationResult.INVALID_REFERENCE;
// Check transaction is valid
ValidationResult result = this.isValid();
if (result != ValidationResult.OK)
return result;
// Check transaction is processable
result = this.isProcessable();
return result;
} }
/** /**