diff --git a/src/main/java/org/qortal/account/Account.java b/src/main/java/org/qortal/account/Account.java index 654698e2..62190133 100644 --- a/src/main/java/org/qortal/account/Account.java +++ b/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. - *

- * 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 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 diff --git a/src/main/java/org/qortal/account/AccountRefCache.java b/src/main/java/org/qortal/account/AccountRefCache.java new file mode 100644 index 00000000..674b8044 --- /dev/null +++ b/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 CACHE = new HashMap(); + + private static class RefCache { + private final Map getLastReferenceValues = new HashMap(); + private final Map> setLastReferenceValues = new HashMap>(); + + 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, Pair> 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> 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> newLastReferenceValues = refCache.getNewLastReferences(); + + for (Entry> 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); + } + +} diff --git a/src/main/java/org/qortal/api/resource/AddressesResource.java b/src/main/java/org/qortal/api/resource/AddressesResource.java index f13a622c..674ae2a1 100644 --- a/src/main/java/org/qortal/api/resource/AddressesResource.java +++ b/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 diff --git a/src/main/java/org/qortal/transaction/Transaction.java b/src/main/java/org/qortal/transaction/Transaction.java index 4edd8596..8317a5ea 100644 --- a/src/main/java/org/qortal/transaction/Transaction.java +++ b/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. */ diff --git a/src/test/java/org/qortal/test/AccountRefCacheTests.java b/src/test/java/org/qortal/test/AccountRefCacheTests.java new file mode 100644 index 00000000..a070bb5a --- /dev/null +++ b/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 + +} diff --git a/src/test/java/org/qortal/test/common/transaction/TestTransaction.java b/src/test/java/org/qortal/test/common/transaction/TestTransaction.java index ddb91939..00aa92de 100644 --- a/src/test/java/org/qortal/test/common/transaction/TestTransaction.java +++ b/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); } }