Interim commit with new AccountRefCache, but no tests and no Block support

src/main/java/org/qortal/account/Account.java

@@ -1,7 +1,6 @@
 package org.qortal.account;
 
 import java.math.BigDecimal;
-import java.util.List;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -12,10 +11,8 @@ import org.qortal.block.BlockChain;
 import org.qortal.data.account.AccountBalanceData;
 import org.qortal.data.account.AccountData;
 import org.qortal.data.account.RewardShareData;
-import org.qortal.data.transaction.TransactionData;
 import org.qortal.repository.DataException;
 import org.qortal.repository.Repository;
-import org.qortal.transaction.Transaction;
 import org.qortal.utils.Base58;
 
 @XmlAccessorType(XmlAccessType.NONE) // Stops JAX-RS errors when unmarshalling blockchain config
@@ -109,38 +106,11 @@ public class Account {
 	 * @throws DataException
 	 */
 	public byte[] getLastReference() throws DataException {
-		byte[] reference = this.repository.getAccountRepository().getLastReference(this.address);
+		byte[] reference = AccountRefCache.getLastReference(this.repository, this.address);
 		LOGGER.trace(() -> String.format("Last reference for %s is %s", this.address, reference == null ? "null" : Base58.encode(reference)));
 		return reference;
 	}
 
-	/**
-	 * Fetch last reference for account, considering unconfirmed transactions only, or return null.
-	 * <p>
-	 * NOTE: calls Transaction.getUnconfirmedTransactions which discards uncommitted
-	 * repository changes.
-	 * 
-	 * @return byte[] reference, or null if no unconfirmed transactions for this account.
-	 * @throws DataException
-	 */
-	public byte[] getUnconfirmedLastReference() throws DataException {
-		// Newest unconfirmed transaction takes priority
-		List<TransactionData> unconfirmedTransactions = Transaction.getUnconfirmedTransactions(repository);
-
-		byte[] reference = null;
-
-		for (TransactionData transactionData : unconfirmedTransactions) {
-			String unconfirmedTransactionAddress = PublicKeyAccount.getAddress(transactionData.getCreatorPublicKey());
-
-			if (unconfirmedTransactionAddress.equals(this.address))
-				reference = transactionData.getSignature();
-		}
-
-		final byte[] loggingReference = reference;
-		LOGGER.trace(() -> String.format("Last unconfirmed reference for %s is %s", this.address, loggingReference == null ? "null" : Base58.encode(loggingReference)));
-		return reference;
-	}
-
 	/**
 	 * Set last reference for account.
 	 * 
@@ -153,7 +123,7 @@ public class Account {
 
 		AccountData accountData = this.buildAccountData();
 		accountData.setReference(reference);
-		this.repository.getAccountRepository().setLastReference(accountData);
+		AccountRefCache.setLastReference(this.repository, accountData);
 	}
 
 	// Default groupID manipulations

src/main/java/org/qortal/account/AccountRefCache.java

@@ -0,0 +1,124 @@
+package org.qortal.account;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.function.BiFunction;
+
+import org.qortal.data.account.AccountData;
+import org.qortal.repository.DataException;
+import org.qortal.repository.Repository;
+import org.qortal.utils.Pair;
+
+public class AccountRefCache implements AutoCloseable {
+
+	private static final Map<Repository, RefCache> CACHE = new HashMap<Repository, RefCache>();
+
+	private static class RefCache {
+		private final Map<String, byte[]> getLastReferenceValues = new HashMap<String, byte[]>();
+		private final Map<String, Pair<byte[], byte[]>> setLastReferenceValues = new HashMap<String, Pair<byte[], byte[]>>();
+
+		public byte[] getLastReference(Repository repository, String address) throws DataException {
+			synchronized (this.getLastReferenceValues) {
+				byte[] lastReference = getLastReferenceValues.get(address);
+				if (lastReference != null)
+					// address is present in map, lastReference not null
+					return lastReference;
+
+				// address is present in map, just lastReference is null
+				if (getLastReferenceValues.containsKey(address))
+					return null;
+
+				lastReference = repository.getAccountRepository().getLastReference(address);
+				this.getLastReferenceValues.put(address, lastReference);
+				return lastReference;
+			}
+		}
+
+		public void setLastReference(AccountData accountData) {
+			BiFunction<String, Pair<byte[], byte[]>, Pair<byte[], byte[]>> mergePublicKey = (key, oldPair) -> {
+				byte[] mergedPublicKey = accountData.getPublicKey() != null ? accountData.getPublicKey() : oldPair.getB();
+				return new Pair<>(accountData.getReference(), mergedPublicKey);
+			};
+
+			synchronized (this.setLastReferenceValues) {
+				setLastReferenceValues.computeIfPresent(accountData.getAddress(), mergePublicKey);
+			}
+		}
+
+		Map<String, Pair<byte[], byte[]>> getNewLastReferences() {
+			return setLastReferenceValues;
+		}
+	}
+
+	private Repository repository;
+
+	public AccountRefCache(Repository repository) {
+		RefCache refCache = new RefCache();
+
+		synchronized (CACHE) {
+			if (CACHE.putIfAbsent(repository, refCache) != null)
+				throw new IllegalStateException("Account reference cache entry already exists");
+		}
+
+		this.repository = repository;
+	}
+
+	public void commit() throws DataException {
+		RefCache refCache;
+
+		synchronized (CACHE) {
+			refCache = CACHE.remove(this.repository);
+		}
+
+		if (refCache == null)
+			throw new IllegalStateException("Tried to commit non-existent account reference cache");
+
+		Map<String, Pair<byte[], byte[]>> newLastReferenceValues = refCache.getNewLastReferences();
+
+		for (Entry<String, Pair<byte[], byte[]>> entry : newLastReferenceValues.entrySet()) {
+			AccountData accountData = new AccountData(entry.getKey());
+
+			accountData.setReference(entry.getValue().getA());
+
+			if (entry.getValue().getB() != null)
+				accountData.setPublicKey(entry.getValue().getB());
+
+			this.repository.getAccountRepository().setLastReference(accountData);
+		}
+	}
+
+	@Override
+	public void close() throws Exception {
+		synchronized (CACHE) {
+			CACHE.remove(this.repository);
+		}
+	}
+
+	/*package*/ static byte[] getLastReference(Repository repository, String address) throws DataException {
+		RefCache refCache;
+
+		synchronized (CACHE) {
+			refCache = CACHE.get(repository);
+		}
+
+		if (refCache == null)
+			return repository.getAccountRepository().getLastReference(address);
+
+		return refCache.getLastReference(repository, address);
+	}
+
+	/*package*/ static void setLastReference(Repository repository, AccountData accountData) throws DataException {
+		RefCache refCache;
+
+		synchronized (CACHE) {
+			refCache = CACHE.get(repository);
+		}
+
+		if (refCache == null)
+			repository.getAccountRepository().setLastReference(accountData);
+
+		refCache.setLastReference(accountData);
+	}
+
+}

src/main/java/org/qortal/api/resource/AddressesResource.java

@@ -66,32 +66,18 @@ public class AddressesResource {
 			)
 		}
 	)
-	@ApiErrors({ApiError.INVALID_ADDRESS, ApiError.REPOSITORY_ISSUE})
+	@ApiErrors({ApiError.INVALID_ADDRESS, ApiError.ADDRESS_UNKNOWN, ApiError.REPOSITORY_ISSUE})
 	public AccountData getAccountInfo(@PathParam("address") String address) {
 		if (!Crypto.isValidAddress(address))
 			throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_ADDRESS);
 
 		try (final Repository repository = RepositoryManager.getRepository()) {
 			AccountData accountData = repository.getAccountRepository().getAccount(address);
-
 			// Not found?
 			if (accountData == null)
-				accountData = new AccountData(address);
-			else {
-				// Unconfirmed transactions could update lastReference
-				Account account = new Account(repository, address);
-
-				// Use last reference based on unconfirmed transactions if possible
-				byte[] unconfirmedLastReference = account.getUnconfirmedLastReference();
-
-				if (unconfirmedLastReference != null)
-					// There are unconfirmed transactions so modify returned data
-					accountData.setReference(unconfirmedLastReference);
-			}
+				throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.ADDRESS_UNKNOWN);
 
 			return accountData;
-		} catch (ApiException e) {
-			throw e;
 		} catch (DataException e) {
 			throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.REPOSITORY_ISSUE, e);
 		}
@@ -100,42 +86,37 @@ public class AddressesResource {
 	@GET
 	@Path("/lastreference/{address}")
 	@Operation(
-		summary = "Fetch reference for next transaction to be created by address, considering unconfirmed transactions",
-		description = "Returns the base58-encoded signature of the last confirmed/unconfirmed transaction created by address, failing that: the first incoming transaction. Returns \"false\" if there is no transactions.",
+		summary = "Fetch reference for next transaction to be created by address",
+		description = "Returns the base58-encoded signature of the last confirmed transaction created by address, failing that: the first incoming transaction. Returns \"false\" if there is no last-reference.",
 		responses = {
 			@ApiResponse(
-				description = "the base58-encoded transaction signature",
+				description = "the base58-encoded last-reference",
 				content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string"))
 			)
 		}
 	)
-	@ApiErrors({ApiError.INVALID_ADDRESS, ApiError.REPOSITORY_ISSUE})
-	public String getLastReferenceUnconfirmed(@PathParam("address") String address) {
+	@ApiErrors({ApiError.INVALID_ADDRESS, ApiError.ADDRESS_UNKNOWN, ApiError.REPOSITORY_ISSUE})
+	public String getLastReference(@PathParam("address") String address) {
 		if (!Crypto.isValidAddress(address))
 			throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_ADDRESS);
 
 		byte[] lastReference = null;
 
 		try (final Repository repository = RepositoryManager.getRepository()) {
-			Account account = new Account(repository, address);
+			AccountData accountData = repository.getAccountRepository().getAccount(address);
+			// Not found?
+			if (accountData == null)
+				throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.ADDRESS_UNKNOWN);
 
-			// Use last reference based on unconfirmed transactions if possible
-			lastReference = account.getUnconfirmedLastReference();
-
-			if (lastReference == null)
-				// No unconfirmed transactions so fallback to using one save in account data
-				lastReference = account.getLastReference();
-		} catch (ApiException e) {
-			throw e;
+			lastReference = accountData.getReference();
 		} catch (DataException e) {
 			throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.REPOSITORY_ISSUE, e);
 		}
 
-		if(lastReference == null || lastReference.length == 0) {
+		if (lastReference == null || lastReference.length == 0)
 			return "false";
-		} else {
-			return Base58.encode(lastReference);
-		}
+
+		return Base58.encode(lastReference);
 	}
 
 	@GET

src/main/java/org/qortal/transaction/Transaction.java

@@ -537,54 +537,31 @@ public abstract class Transaction {
 		if (feeValidationResult != ValidationResult.OK)
 			return feeValidationResult;
 
-		/*
-		 * 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 discard changes.
-		 */
-		ReentrantLock blockchainLock = Controller.getInstance().getBlockchainLock();
-		blockchainLock.lock();
-		try {
-			// Clear repository's "in transaction" state so we don't cause a repository deadlock
-			repository.discardChanges();
+		PublicKeyAccount creator = this.getCreator();
+		if (creator == null)
+			return ValidationResult.MISSING_CREATOR;
 
-			try {
-				PublicKeyAccount creator = this.getCreator();
-				if (creator == null)
-					return ValidationResult.MISSING_CREATOR;
+		// Reject if unconfirmed pile already has X transactions from same creator
+		if (countUnconfirmedByCreator(creator) >= Settings.getInstance().getMaxUnconfirmedPerAccount())
+			return ValidationResult.TOO_MANY_UNCONFIRMED;
 
-				// Reject if unconfirmed pile already has X transactions from same creator
-				if (countUnconfirmedByCreator(creator) >= Settings.getInstance().getMaxUnconfirmedPerAccount())
-					return ValidationResult.TOO_MANY_UNCONFIRMED;
+		// Check transaction's txGroupId
+		if (!this.isValidTxGroupId())
+			return ValidationResult.INVALID_TX_GROUP_ID;
 
-				// Check transaction's txGroupId
-				if (!this.isValidTxGroupId())
-					return ValidationResult.INVALID_TX_GROUP_ID;
+		// Check transaction references
+		if (!this.hasValidReference())
+			return ValidationResult.INVALID_REFERENCE;
 
-				byte[] unconfirmedLastReference = creator.getUnconfirmedLastReference();
-				if (unconfirmedLastReference != null)
-					creator.setLastReference(unconfirmedLastReference);
+		// Check transaction is valid
+		ValidationResult result = this.isValid();
+		if (result != ValidationResult.OK)
+			return result;
 
-				// Check transaction is valid
-				ValidationResult result = this.isValid();
-				if (result != ValidationResult.OK)
-					return result;
+		// Check transaction is processable
+		result = this.isProcessable();
 
-				// Check transaction references
-				if (!this.hasValidReference())
-					return ValidationResult.INVALID_REFERENCE;
-
-				// Check transaction is processable
-				result = this.isProcessable();
-
-				return result;
-			} finally {
-				repository.discardChanges();
-			}
-		} finally {
-			// In separate finally block just in case rollback throws
-			blockchainLock.unlock();
-		}
+		return result;
 	}
 
 	/** Returns whether transaction's fee is valid. Might be overriden in transaction subclasses. */

src/test/java/org/qortal/test/AccountRefCacheTests.java

@@ -0,0 +1,76 @@
+package org.qortal.test;
+
+public class AccountRefCacheTests {
+
+	// Test no cache in play (existing account):
+	// fetch 1st ref
+	// generate 2nd ref and call Account.setLastReference
+	// fetch 3rd ref
+	// 3rd ref should match 2st ref
+
+	// Test no cache in play (no account):
+	// fetch 1st ref
+	// generate 2nd ref and call Account.setLastReference
+	// fetch 3rd ref
+	// 3rd ref should match 2st ref
+
+	// Test cache in play (existing account, no commit):
+	// fetch 1st ref
+	// begin caching
+	// fetch 2nd ref
+	// 2nd ref should match 1st ref
+	// generate 3rd ref and call Account.setLastReference
+	// fetch 4th ref
+	// 4th ref should match 1st ref
+	// discard cache
+	// fetch 5th ref
+	// 5th ref should match 1st ref
+
+	// Test cache in play (existing account, with commit):
+	// fetch 1st ref
+	// begin caching
+	// fetch 2nd ref
+	// 2nd ref should match 1st ref
+	// generate 3rd ref and call Account.setLastReference
+	// fetch 4th ref
+	// 4th ref should match 1st ref
+	// commit cache
+	// fetch 5th ref
+	// 5th ref should match 3rd ref
+
+	// Test cache in play (new account, no commit):
+	// fetch 1st ref (null)
+	// begin caching
+	// fetch 2nd ref
+	// 2nd ref should match 1st ref
+	// generate 3rd ref and call Account.setLastReference
+	// fetch 4th ref
+	// 4th ref should match 1st ref
+	// discard cache
+	// fetch 5th ref
+	// 5th ref should match 1st ref
+
+	// Test cache in play (new account, with commit):
+	// fetch 1st ref (null)
+	// begin caching
+	// fetch 2nd ref
+	// 2nd ref should match 1st ref
+	// generate 3rd ref and call Account.setLastReference
+	// fetch 4th ref
+	// 4th ref should match 1st ref
+	// commit cache
+	// fetch 5th ref
+	// 5th ref should match 3rd ref
+
+	// Test Block support
+	// fetch 1st ref for Alice
+	// generate new payment from Alice to new account Ellen
+	// generate another payment from Alice to new account Ellen
+	// mint block containing payments
+	// confirm Ellen's ref is 1st payment's sig
+	// confirm Alice's ref if 2nd payment's sig
+	// orphan block
+	// confirm Ellen's ref is null
+	// confirm Alice's ref matches 1st ref
+
+}

src/test/java/org/qortal/test/common/transaction/TestTransaction.java

@@ -13,11 +13,7 @@ public abstract class TestTransaction {
 	protected static final Random random = new Random();
 
 	public static BaseTransactionData generateBase(PrivateKeyAccount account) throws DataException {
-		byte[] lastReference = account.getUnconfirmedLastReference();
-		if (lastReference == null)
-			lastReference = account.getLastReference();
-
-		return new BaseTransactionData(System.currentTimeMillis(), Group.NO_GROUP, lastReference, account.getPublicKey(), BlockChain.getInstance().getUnitFee(), null);
+		return new BaseTransactionData(System.currentTimeMillis(), Group.NO_GROUP, account.getLastReference(), account.getPublicKey(), BlockChain.getInstance().getUnitFee(), null);
 	}
 
 }