From 2e6d33659c23b59bf68434dd1e1f27e3daa602a4 Mon Sep 17 00:00:00 2001 From: catbref Date: Sun, 16 Jun 2019 11:41:33 +0100 Subject: [PATCH] Fix CANCEL_ASSET_ORDER check for already closed, partially/fully matched, order. Additional unit tests to cover above case. --- .../CancelAssetOrderTransaction.java | 3 ++ .../org/qora/transaction/Transaction.java | 1 + .../org/qora/test/assets/CancellingTests.java | 44 +++++++++++++++++-- .../java/org/qora/test/common/AssetUtils.java | 12 ++++- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/qora/transaction/CancelAssetOrderTransaction.java b/src/main/java/org/qora/transaction/CancelAssetOrderTransaction.java index 84433757..66b1cf26 100644 --- a/src/main/java/org/qora/transaction/CancelAssetOrderTransaction.java +++ b/src/main/java/org/qora/transaction/CancelAssetOrderTransaction.java @@ -74,6 +74,9 @@ public class CancelAssetOrderTransaction extends Transaction { if (orderData == null) return ValidationResult.ORDER_DOES_NOT_EXIST; + if (orderData.getIsClosed()) + return ValidationResult.ORDER_ALREADY_CLOSED; + Account creator = getCreator(); // Check creator's public key results in valid address diff --git a/src/main/java/org/qora/transaction/Transaction.java b/src/main/java/org/qora/transaction/Transaction.java index 63ca63dd..12d7e18c 100644 --- a/src/main/java/org/qora/transaction/Transaction.java +++ b/src/main/java/org/qora/transaction/Transaction.java @@ -235,6 +235,7 @@ public abstract class Transaction { MAXIMUM_PROXY_RELATIONSHIPS(84), TRANSACTION_ALREADY_EXISTS(85), NO_BLOCKCHAIN_LOCK(86), + ORDER_ALREADY_CLOSED(87), NOT_YET_RELEASED(1000); public final int value; diff --git a/src/test/java/org/qora/test/assets/CancellingTests.java b/src/test/java/org/qora/test/assets/CancellingTests.java index a93e5a59..49149422 100644 --- a/src/test/java/org/qora/test/assets/CancellingTests.java +++ b/src/test/java/org/qora/test/assets/CancellingTests.java @@ -1,5 +1,7 @@ package org.qora.test.assets; +import static org.junit.Assert.*; + import java.math.BigDecimal; import java.math.RoundingMode; import java.util.Map; @@ -7,12 +9,15 @@ import java.util.Map; import org.junit.After; import org.junit.Before; import org.junit.Test; + import org.qora.repository.DataException; import org.qora.repository.Repository; import org.qora.repository.RepositoryManager; import org.qora.test.common.AccountUtils; import org.qora.test.common.AssetUtils; import org.qora.test.common.Common; +import org.qora.transaction.Transaction; +import org.qora.transaction.Transaction.ValidationResult; public class CancellingTests extends Common { @@ -51,6 +56,31 @@ public class CancellingTests extends Common { } } + @Test + public void testRepeatCancel() throws DataException { + BigDecimal amount = new BigDecimal("1234.87654321").setScale(8); + BigDecimal price = new BigDecimal("1.35615263").setScale(8); + + try (Repository repository = RepositoryManager.getRepository()) { + Map> initialBalances = AccountUtils.getBalances(repository, AssetUtils.testAssetId, AssetUtils.otherAssetId); + + byte[] aliceOrderId = AssetUtils.createOrder(repository, "alice", AssetUtils.testAssetId, AssetUtils.otherAssetId, amount, price); + AssetUtils.cancelOrder(repository, "alice", aliceOrderId); + + // Build 2nd cancel-order transaction to check it is invalid + assertCannotCancelClosedOrder(repository, "alice", aliceOrderId); + + // Check asset balances match pre-ordering values + BigDecimal expectedBalance; + + expectedBalance = initialBalances.get("alice").get(AssetUtils.testAssetId); + AccountUtils.assertBalance(repository, "alice", AssetUtils.testAssetId, expectedBalance); + + expectedBalance = initialBalances.get("bob").get(AssetUtils.otherAssetId); + AccountUtils.assertBalance(repository, "bob", AssetUtils.otherAssetId, expectedBalance); + } + } + @Test public void testPartialTargetMatchCancel() throws DataException { BigDecimal aliceAmount = new BigDecimal("1234").setScale(8); // OTHER @@ -79,7 +109,7 @@ public class CancellingTests extends Common { byte[] bobOrderId = AssetUtils.createOrder(repository, "bob", AssetUtils.otherAssetId, AssetUtils.testAssetId, bobAmount, bobPrice); AssetUtils.cancelOrder(repository, "alice", aliceOrderId); - AssetUtils.cancelOrder(repository, "bob", bobOrderId); + assertCannotCancelClosedOrder(repository, "bob", bobOrderId); // Check asset balances BigDecimal expectedBalance; @@ -127,7 +157,7 @@ public class CancellingTests extends Common { byte[] aliceOrderId = AssetUtils.createOrder(repository, "alice", AssetUtils.testAssetId, AssetUtils.otherAssetId, aliceAmount, alicePrice); byte[] bobOrderId = AssetUtils.createOrder(repository, "bob", AssetUtils.otherAssetId, AssetUtils.testAssetId, bobAmount, bobPrice); - AssetUtils.cancelOrder(repository, "alice", aliceOrderId); + assertCannotCancelClosedOrder(repository, "alice", aliceOrderId); AssetUtils.cancelOrder(repository, "bob", bobOrderId); // Check asset balances @@ -177,7 +207,7 @@ public class CancellingTests extends Common { byte[] bobOrderId = AssetUtils.createOrder(repository, "bob", AssetUtils.otherAssetId, AssetUtils.goldAssetId, bobAmount, bobPrice); AssetUtils.cancelOrder(repository, "alice", aliceOrderId); - AssetUtils.cancelOrder(repository, "bob", bobOrderId); + assertCannotCancelClosedOrder(repository, "bob", bobOrderId); // Check asset balances BigDecimal expectedBalance; @@ -225,7 +255,7 @@ public class CancellingTests extends Common { byte[] aliceOrderId = AssetUtils.createOrder(repository, "alice", AssetUtils.goldAssetId, AssetUtils.otherAssetId, aliceAmount, alicePrice); byte[] bobOrderId = AssetUtils.createOrder(repository, "bob", AssetUtils.otherAssetId, AssetUtils.goldAssetId, bobAmount, bobPrice); - AssetUtils.cancelOrder(repository, "alice", aliceOrderId); + assertCannotCancelClosedOrder(repository, "alice", aliceOrderId); AssetUtils.cancelOrder(repository, "bob", bobOrderId); // Check asset balances @@ -247,4 +277,10 @@ public class CancellingTests extends Common { } } + public void assertCannotCancelClosedOrder(Repository repository, String accountName, byte[] orderId) throws DataException { + Transaction transaction = AssetUtils.buildCancelOrder(repository, accountName, orderId); + ValidationResult validationResult = transaction.isValidUnconfirmed(); + assertEquals("CANCEL_ASSET_ORDER should be invalid due to already closed order", ValidationResult.ORDER_ALREADY_CLOSED, validationResult); + } + } diff --git a/src/test/java/org/qora/test/common/AssetUtils.java b/src/test/java/org/qora/test/common/AssetUtils.java index 332dec63..dabaf68f 100644 --- a/src/test/java/org/qora/test/common/AssetUtils.java +++ b/src/test/java/org/qora/test/common/AssetUtils.java @@ -21,6 +21,7 @@ import org.qora.group.Group; import org.qora.repository.DataException; import org.qora.repository.Repository; import org.qora.repository.RepositoryManager; +import org.qora.transaction.Transaction; public class AssetUtils { @@ -72,7 +73,7 @@ public class AssetUtils { return repository.getAssetRepository().getAccountsOrders(account.getPublicKey(), null, null, null, null, true).get(0).getOrderId(); } - public static void cancelOrder(Repository repository, String accountName, byte[] orderId) throws DataException { + public static Transaction buildCancelOrder(Repository repository, String accountName, byte[] orderId) throws DataException { PrivateKeyAccount account = Common.getTestAccount(repository, accountName); byte[] reference = account.getLastReference(); @@ -81,7 +82,14 @@ public class AssetUtils { BaseTransactionData baseTransactionData = new BaseTransactionData(timestamp, AssetUtils.txGroupId, reference, account.getPublicKey(), AssetUtils.fee, null); TransactionData transactionData = new CancelAssetOrderTransactionData(baseTransactionData, orderId); - TransactionUtils.signAndForge(repository, transactionData, account); + return Transaction.fromData(repository, transactionData); + } + + public static void cancelOrder(Repository repository, String accountName, byte[] orderId) throws DataException { + PrivateKeyAccount account = Common.getTestAccount(repository, accountName); + Transaction transaction = buildCancelOrder(repository, accountName, orderId); + + TransactionUtils.signAndForge(repository, transaction.getTransactionData(), account); } public static void genericTradeTest(long haveAssetId, long wantAssetId,