From 3b914d4a7f304e2eab6f52c4a777c79ba019b2de Mon Sep 17 00:00:00 2001 From: CalDescent Date: Wed, 3 Nov 2021 19:27:56 +0000 Subject: [PATCH] Improved trade bot backups so that the current order being bought is included. This should fix any key recovery issues if the node crashes or otherwise fails when buying an offer. --- .../tradebot/DogecoinACCTv1TradeBot.java | 5 +- .../tradebot/DogecoinACCTv2TradeBot.java | 5 +- .../tradebot/LitecoinACCTv1TradeBot.java | 5 +- .../tradebot/LitecoinACCTv2TradeBot.java | 5 +- .../qortal/controller/tradebot/TradeBot.java | 5 +- .../repository/hsqldb/HSQLDBImportExport.java | 29 ++++++-- .../repository/hsqldb/HSQLDBRepository.java | 2 +- .../org/qortal/test/ImportExportTests.java | 74 +++++++++++++++++-- 8 files changed, 108 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv1TradeBot.java b/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv1TradeBot.java index e7b60b25..d37a6650 100644 --- a/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv1TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv1TradeBot.java @@ -201,7 +201,7 @@ public class DogecoinACCTv1TradeBot implements AcctTradeBot { TradeBot.updateTradeBotState(repository, tradeBotData, () -> String.format("Built AT %s. Waiting for deployment", atAddress)); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + TradeBot.backupTradeBotData(repository, null); // Return to user for signing and broadcast as we don't have their Qortal private key try { @@ -276,7 +276,8 @@ public class DogecoinACCTv1TradeBot implements AcctTradeBot { crossChainTradeData.expectedForeignAmount, xprv58, null, lockTimeA, receivingPublicKeyHash); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + // Include tradeBotData as an additional parameter, since it's not in the repository yet + TradeBot.backupTradeBotData(repository, Arrays.asList(tradeBotData)); // Check we have enough funds via xprv58 to fund P2SH to cover expectedForeignAmount long p2shFee; diff --git a/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv2TradeBot.java b/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv2TradeBot.java index a85f0be1..96dfd1b1 100644 --- a/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv2TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/DogecoinACCTv2TradeBot.java @@ -201,7 +201,7 @@ public class DogecoinACCTv2TradeBot implements AcctTradeBot { TradeBot.updateTradeBotState(repository, tradeBotData, () -> String.format("Built AT %s. Waiting for deployment", atAddress)); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + TradeBot.backupTradeBotData(repository, null); // Return to user for signing and broadcast as we don't have their Qortal private key try { @@ -276,7 +276,8 @@ public class DogecoinACCTv2TradeBot implements AcctTradeBot { crossChainTradeData.expectedForeignAmount, xprv58, null, lockTimeA, receivingPublicKeyHash); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + // Include tradeBotData as an additional parameter, since it's not in the repository yet + TradeBot.backupTradeBotData(repository, Arrays.asList(tradeBotData)); // Check we have enough funds via xprv58 to fund P2SH to cover expectedForeignAmount long p2shFee; diff --git a/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv1TradeBot.java b/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv1TradeBot.java index 686b675e..fd0682b6 100644 --- a/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv1TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv1TradeBot.java @@ -212,7 +212,7 @@ public class LitecoinACCTv1TradeBot implements AcctTradeBot { TradeBot.updateTradeBotState(repository, tradeBotData, () -> String.format("Built AT %s. Waiting for deployment", atAddress)); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + TradeBot.backupTradeBotData(repository, null); // Return to user for signing and broadcast as we don't have their Qortal private key try { @@ -287,7 +287,8 @@ public class LitecoinACCTv1TradeBot implements AcctTradeBot { crossChainTradeData.expectedForeignAmount, xprv58, null, lockTimeA, receivingPublicKeyHash); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + // Include tradeBotData as an additional parameter, since it's not in the repository yet + TradeBot.backupTradeBotData(repository, Arrays.asList(tradeBotData)); // Check we have enough funds via xprv58 to fund P2SH to cover expectedForeignAmount long p2shFee; diff --git a/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv2TradeBot.java b/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv2TradeBot.java index f4e06299..6261339a 100644 --- a/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv2TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/LitecoinACCTv2TradeBot.java @@ -201,7 +201,7 @@ public class LitecoinACCTv2TradeBot implements AcctTradeBot { TradeBot.updateTradeBotState(repository, tradeBotData, () -> String.format("Built AT %s. Waiting for deployment", atAddress)); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + TradeBot.backupTradeBotData(repository, null); // Return to user for signing and broadcast as we don't have their Qortal private key try { @@ -276,7 +276,8 @@ public class LitecoinACCTv2TradeBot implements AcctTradeBot { crossChainTradeData.expectedForeignAmount, xprv58, null, lockTimeA, receivingPublicKeyHash); // Attempt to backup the trade bot data - TradeBot.backupTradeBotData(repository); + // Include tradeBotData as an additional parameter, since it's not in the repository yet + TradeBot.backupTradeBotData(repository, Arrays.asList(tradeBotData)); // Check we have enough funds via xprv58 to fund P2SH to cover expectedForeignAmount long p2shFee; diff --git a/src/main/java/org/qortal/controller/tradebot/TradeBot.java b/src/main/java/org/qortal/controller/tradebot/TradeBot.java index ea390f16..1eb837bf 100644 --- a/src/main/java/org/qortal/controller/tradebot/TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/TradeBot.java @@ -31,6 +31,7 @@ import org.qortal.gui.SysTray; import org.qortal.repository.DataException; import org.qortal.repository.Repository; import org.qortal.repository.RepositoryManager; +import org.qortal.repository.hsqldb.HSQLDBImportExport; import org.qortal.settings.Settings; import org.qortal.transaction.PresenceTransaction; import org.qortal.transaction.PresenceTransaction.PresenceType; @@ -267,11 +268,11 @@ public class TradeBot implements Listener { return secret; } - /*package*/ static void backupTradeBotData(Repository repository) { + /*package*/ static void backupTradeBotData(Repository repository, List additional) { // Attempt to backup the trade bot data. This an optional step and doesn't impact trading, so don't throw an exception on failure try { LOGGER.info("About to backup trade bot data..."); - repository.exportNodeLocalData(); + HSQLDBImportExport.backupTradeBotStates(repository, additional); } catch (DataException e) { LOGGER.info(String.format("Repository issue when exporting trade bot data: %s", e.getMessage())); } diff --git a/src/main/java/org/qortal/repository/hsqldb/HSQLDBImportExport.java b/src/main/java/org/qortal/repository/hsqldb/HSQLDBImportExport.java index c5881c01..3e6dd534 100644 --- a/src/main/java/org/qortal/repository/hsqldb/HSQLDBImportExport.java +++ b/src/main/java/org/qortal/repository/hsqldb/HSQLDBImportExport.java @@ -28,9 +28,9 @@ public class HSQLDBImportExport { private static final Logger LOGGER = LogManager.getLogger(Bootstrap.class); - public static void backupTradeBotStates(Repository repository) throws DataException { - HSQLDBImportExport.backupCurrentTradeBotStates(repository); - HSQLDBImportExport.backupArchivedTradeBotStates(repository); + public static void backupTradeBotStates(Repository repository, List additional) throws DataException { + HSQLDBImportExport.backupCurrentTradeBotStates(repository, additional); + HSQLDBImportExport.backupArchivedTradeBotStates(repository, additional); LOGGER.info("Exported sensitive/node-local data: trade bot states"); } @@ -47,14 +47,23 @@ public class HSQLDBImportExport { /** * Backs up the trade bot states currently in the repository, without combining them with past ones * @param repository + * @param additional - any optional extra trade bot states to include in the backup * @throws DataException */ - private static void backupCurrentTradeBotStates(Repository repository) throws DataException { + private static void backupCurrentTradeBotStates(Repository repository, List additional) throws DataException { try { Path backupDirectory = HSQLDBImportExport.getExportDirectory(true); // Load current trade bot data List allTradeBotData = repository.getCrossChainRepository().getAllTradeBotData(); + + + // Add any additional entries if specified + if (additional != null && !additional.isEmpty()) { + allTradeBotData.addAll(additional); + } + + // Convert them to JSON objects JSONArray currentTradeBotDataJson = new JSONArray(); for (TradeBotData tradeBotData : allTradeBotData) { JSONObject tradeBotDataJson = tradeBotData.toJson(); @@ -82,14 +91,22 @@ public class HSQLDBImportExport { * Backs up the trade bot states currently in the repository to a separate "archive" file, * making sure to combine them with any unique states already present in the archive. * @param repository + * @param additional - any optional extra trade bot states to include in the backup * @throws DataException */ - private static void backupArchivedTradeBotStates(Repository repository) throws DataException { + private static void backupArchivedTradeBotStates(Repository repository, List additional) throws DataException { try { Path backupDirectory = HSQLDBImportExport.getExportDirectory(true); // Load current trade bot data List allTradeBotData = repository.getCrossChainRepository().getAllTradeBotData(); + + // Add any additional entries if specified + if (additional != null && !additional.isEmpty()) { + allTradeBotData.addAll(additional); + } + + // Convert them to JSON objects JSONArray allTradeBotDataJson = new JSONArray(); for (TradeBotData tradeBotData : allTradeBotData) { JSONObject tradeBotDataJson = tradeBotData.toJson(); @@ -263,7 +280,7 @@ public class HSQLDBImportExport { * @param jsonString * @return Triple (type, dataset, data) */ - private static Triple parseJSONString(String jsonString) throws DataException { + public static Triple parseJSONString(String jsonString) throws DataException { String type = null; String dataset = null; JSONArray data = null; diff --git a/src/main/java/org/qortal/repository/hsqldb/HSQLDBRepository.java b/src/main/java/org/qortal/repository/hsqldb/HSQLDBRepository.java index 1c025ae2..61f4b76f 100644 --- a/src/main/java/org/qortal/repository/hsqldb/HSQLDBRepository.java +++ b/src/main/java/org/qortal/repository/hsqldb/HSQLDBRepository.java @@ -471,7 +471,7 @@ public class HSQLDBRepository implements Repository { @Override public void exportNodeLocalData() throws DataException { - HSQLDBImportExport.backupTradeBotStates(this); + HSQLDBImportExport.backupTradeBotStates(this, null); HSQLDBImportExport.backupMintingAccounts(this); } diff --git a/src/test/java/org/qortal/test/ImportExportTests.java b/src/test/java/org/qortal/test/ImportExportTests.java index c7a5062f..2306d484 100644 --- a/src/test/java/org/qortal/test/ImportExportTests.java +++ b/src/test/java/org/qortal/test/ImportExportTests.java @@ -25,12 +25,15 @@ import org.qortal.repository.hsqldb.HSQLDBImportExport; import org.qortal.settings.Settings; import org.qortal.test.common.Common; import org.qortal.utils.NTP; +import org.qortal.utils.Triple; import java.io.FileWriter; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static org.junit.Assert.*; @@ -68,7 +71,7 @@ public class ImportExportTests extends Common { assertEquals(10, repository.getCrossChainRepository().getAllTradeBotData().size()); // Export them - HSQLDBImportExport.backupTradeBotStates(repository); + HSQLDBImportExport.backupTradeBotStates(repository, null); // Delete them from the repository for (TradeBotData tradeBotData : tradeBots) { @@ -117,7 +120,7 @@ public class ImportExportTests extends Common { assertEquals(10, repository.getCrossChainRepository().getAllTradeBotData().size()); // Export them - HSQLDBImportExport.backupTradeBotStates(repository); + HSQLDBImportExport.backupTradeBotStates(repository, null); // Delete them from the repository for (TradeBotData tradeBotData : tradeBots) { @@ -136,7 +139,7 @@ public class ImportExportTests extends Common { } // Export again - HSQLDBImportExport.backupTradeBotStates(repository); + HSQLDBImportExport.backupTradeBotStates(repository, null); // Import current states only Path exportPath = HSQLDBImportExport.getExportDirectory(false); @@ -184,7 +187,7 @@ public class ImportExportTests extends Common { assertEquals(10, repository.getCrossChainRepository().getAllTradeBotData().size()); // Export them - HSQLDBImportExport.backupTradeBotStates(repository); + HSQLDBImportExport.backupTradeBotStates(repository, null); // Delete them from the repository for (TradeBotData tradeBotData : tradeBots) { @@ -203,7 +206,7 @@ public class ImportExportTests extends Common { } // Export again - HSQLDBImportExport.backupTradeBotStates(repository); + HSQLDBImportExport.backupTradeBotStates(repository, null); // Import all states from the archive Path exportPath = HSQLDBImportExport.getExportDirectory(false); @@ -263,6 +266,67 @@ public class ImportExportTests extends Common { } } + @Test + public void testArchiveTradeBotStateOnTradeFailure() throws DataException, IOException { + try (final Repository repository = RepositoryManager.getRepository()) { + + // Create a trade bot and save it in the repository + TradeBotData tradeBotData = this.createTradeBotData(repository); + + // Ensure it doesn't exist in the repository + assertTrue(repository.getCrossChainRepository().getAllTradeBotData().isEmpty()); + + // Export trade bot states, passing in the newly created trade bot as an additional parameter + // This is needed because it hasn't been saved to the db yet + HSQLDBImportExport.backupTradeBotStates(repository, Arrays.asList(tradeBotData)); + + // Ensure it is still not present in the repository + assertTrue(repository.getCrossChainRepository().getAllTradeBotData().isEmpty()); + + // Export all local node data again, but this time without including the trade bot data + // This simulates the behaviour of a node shutdown + repository.exportNodeLocalData(); + + // The TradeBotStates.json file should contain no entries + Path backupDirectory = HSQLDBImportExport.getExportDirectory(false); + Path tradeBotStatesBackup = Paths.get(backupDirectory.toString(), "TradeBotStates.json"); + assertTrue(Files.exists(tradeBotStatesBackup)); + String jsonString = new String(Files.readAllBytes(tradeBotStatesBackup)); + Triple parsedJSON = HSQLDBImportExport.parseJSONString(jsonString); + JSONArray tradeBotDataJson = parsedJSON.getC(); + assertTrue(tradeBotDataJson.isEmpty()); + + // .. but the TradeBotStatesArchive.json should contain the trade bot data + Path tradeBotStatesArchiveBackup = Paths.get(backupDirectory.toString(), "TradeBotStatesArchive.json"); + assertTrue(Files.exists(tradeBotStatesArchiveBackup)); + jsonString = new String(Files.readAllBytes(tradeBotStatesArchiveBackup)); + parsedJSON = HSQLDBImportExport.parseJSONString(jsonString); + JSONObject tradeBotDataJsonObject = (JSONObject) parsedJSON.getC().get(0); + assertEquals(tradeBotData.toJson().toString(), tradeBotDataJsonObject.toString()); + + // Now try importing local data (to simulate a node startup) + String exportPath = Settings.getInstance().getExportPath(); + Path importPath = Paths.get(exportPath, "TradeBotStates.json"); + repository.importDataFromFile(importPath.toString()); + + // The trade should be missing since it's not present in TradeBotStates.json + assertTrue(repository.getCrossChainRepository().getAllTradeBotData().isEmpty()); + + // The user now imports TradeBotStatesArchive.json + Path archiveImportPath = Paths.get(exportPath, "TradeBotStatesArchive.json"); + repository.importDataFromFile(archiveImportPath.toString()); + + // The trade should be present in the database + assertEquals(1, repository.getCrossChainRepository().getAllTradeBotData().size()); + + // The trade bot data in the repository should match the one that was originally created + byte[] tradePrivateKey = tradeBotData.getTradePrivateKey(); + TradeBotData repositoryTradeBotData = repository.getCrossChainRepository().getTradeBotData(tradePrivateKey); + assertNotNull(repositoryTradeBotData); + assertEquals(tradeBotData.toJson().toString(), repositoryTradeBotData.toJson().toString()); + } + } + @Test public void testExportAndImportMintingAccountData() throws DataException, IOException { try (final Repository repository = RepositoryManager.getRepository()) {