From d86583c6b783eabf89266079b64efc673b55748e Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Fri, 12 Jan 2024 12:41:56 +0100 Subject: [PATCH 1/3] Run TxValidatorSanityCheckTests for maker and taker --- .../fee/MakerTxValidatorSanityCheckTests.java | 138 ++++++++++++++++++ ... => TakerTxValidatorSanityCheckTests.java} | 12 +- 2 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java rename core/src/test/java/bisq/core/fee/{TxValidatorSanityCheckTests.java => TakerTxValidatorSanityCheckTests.java} (94%) diff --git a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java new file mode 100644 index 00000000000..16b6664e7f6 --- /dev/null +++ b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java @@ -0,0 +1,138 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.fee; + +import bisq.core.dao.state.DaoStateService; +import bisq.core.filter.FilterManager; +import bisq.core.provider.mempool.FeeValidationStatus; +import bisq.core.provider.mempool.TxValidator; + +import com.google.gson.Gson; +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; + +import java.net.URL; + +import java.nio.file.Files; +import java.nio.file.Path; + +import java.io.IOException; + +import java.util.List; +import java.util.Objects; + +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +@ExtendWith(MockitoExtension.class) +public class MakerTxValidatorSanityCheckTests { + public static final List FEE_RECEIVER_ADDRESSES = List.of("2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w"); + + private TxValidator txValidator; + + @BeforeEach + void setup(@Mock DaoStateService daoStateService, @Mock FilterManager filterManager) { + String txId = "e3607e971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07"; + txValidator = new TxValidator(daoStateService, txId, filterManager); + } + + @ParameterizedTest + @NullAndEmptySource + void nullAndEmptyMempoolResponse(String jsonText) { + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES); + FeeValidationStatus status = txValidator1.getStatus(); + assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); + } + + @Test + void invalidJsonResponse() { + String invalidJson = "in\"valid'json',"; + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES); + + FeeValidationStatus status = txValidator1.getStatus(); + assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); + } + + @ParameterizedTest + @ValueSource(strings = {"status", "txid"}) + void mempoolResponseWithMissingField(String missingField) throws IOException { + JsonObject json = getValidBtcMakerFeeMempoolJsonResponse(); + json.remove(missingField); + assertThat(json.has(missingField), is(false)); + + String jsonContent = new Gson().toJson(json); + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + + FeeValidationStatus status = txValidator1.getStatus(); + assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); + } + + @Test + void mempoolResponseWithoutConfirmedField() throws IOException { + JsonObject json = getValidBtcMakerFeeMempoolJsonResponse(); + json.get("status").getAsJsonObject().remove("confirmed"); + assertThat(json.get("status").getAsJsonObject().has("confirmed"), is(false)); + + String jsonContent = new Gson().toJson(json); + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + + FeeValidationStatus status = txValidator1.getStatus(); + assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); + } + + @Test + void responseHasDifferentTxId() throws IOException { + String differentTxId = "abcde971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07"; + + JsonObject json = getValidBtcMakerFeeMempoolJsonResponse(); + json.add("txid", new JsonPrimitive(differentTxId)); + assertThat(json.get("txid").getAsString(), is(differentTxId)); + + String jsonContent = new Gson().toJson(json); + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + + FeeValidationStatus status = txValidator1.getStatus(); + assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); + } + + public static JsonObject getValidBtcMakerFeeMempoolJsonResponse() throws IOException { + URL resource = MakerTxValidatorSanityCheckTests.class.getClassLoader() + .getResource("mempool_test_data/valid_btc_maker_fee.json"); + String path = Objects.requireNonNull(resource).getPath(); + + if (System.getProperty("os.name").toLowerCase().startsWith("win")) { + // We need to remove the first character on Windows because the path starts with a + // leading slash "/C:/Users/..." + path = path.substring(1); + } + + String jsonContent = Files.readString(Path.of(path)); + return new Gson().fromJson(jsonContent, JsonObject.class); + } +} diff --git a/core/src/test/java/bisq/core/fee/TxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java similarity index 94% rename from core/src/test/java/bisq/core/fee/TxValidatorSanityCheckTests.java rename to core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java index d7940b6ff13..18fea843023 100644 --- a/core/src/test/java/bisq/core/fee/TxValidatorSanityCheckTests.java +++ b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java @@ -51,7 +51,7 @@ import static org.hamcrest.MatcherAssert.assertThat; @ExtendWith(MockitoExtension.class) -public class TxValidatorSanityCheckTests { +public class TakerTxValidatorSanityCheckTests { private final List FEE_RECEIVER_ADDRESSES = List.of("2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w"); private TxValidator txValidator; @@ -65,7 +65,7 @@ void setup(@Mock DaoStateService daoStateService, @Mock FilterManager filterMana @ParameterizedTest @NullAndEmptySource void nullAndEmptyMempoolResponse(String jsonText) { - TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES); + TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonText, FEE_RECEIVER_ADDRESSES); FeeValidationStatus status = txValidator1.getStatus(); assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); } @@ -73,7 +73,7 @@ void nullAndEmptyMempoolResponse(String jsonText) { @Test void invalidJsonResponse() { String invalidJson = "in\"valid'json',"; - TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES); + TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(invalidJson, FEE_RECEIVER_ADDRESSES); FeeValidationStatus status = txValidator1.getStatus(); assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); @@ -87,7 +87,7 @@ void mempoolResponseWithMissingField(String missingField) throws IOException { assertThat(json.has(missingField), is(false)); String jsonContent = new Gson().toJson(json); - TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); FeeValidationStatus status = txValidator1.getStatus(); assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); @@ -100,7 +100,7 @@ void mempoolResponseWithoutConfirmedField() throws IOException { assertThat(json.get("status").getAsJsonObject().has("confirmed"), is(false)); String jsonContent = new Gson().toJson(json); - TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); FeeValidationStatus status = txValidator1.getStatus(); assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); @@ -115,7 +115,7 @@ void responseHasDifferentTxId() throws IOException { assertThat(json.get("txid").getAsString(), is(differentTxId)); String jsonContent = new Gson().toJson(json); - TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); + TxValidator txValidator1 = txValidator.parseJsonValidateTakerFeeTx(jsonContent, FEE_RECEIVER_ADDRESSES); FeeValidationStatus status = txValidator1.getStatus(); assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); From 9f4a4d69137d37ecf51b08dc726c8aa15fe505c0 Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Fri, 12 Jan 2024 12:41:56 +0100 Subject: [PATCH 2/3] TxValidator: Add missing vin and vout tests --- .../java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java | 2 +- .../java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java index 16b6664e7f6..6a147bfb5ca 100644 --- a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java +++ b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java @@ -80,7 +80,7 @@ void invalidJsonResponse() { } @ParameterizedTest - @ValueSource(strings = {"status", "txid"}) + @ValueSource(strings = {"status", "txid", "vin", "vout"}) void mempoolResponseWithMissingField(String missingField) throws IOException { JsonObject json = getValidBtcMakerFeeMempoolJsonResponse(); json.remove(missingField); diff --git a/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java index 18fea843023..546c03c988f 100644 --- a/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java +++ b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java @@ -80,7 +80,7 @@ void invalidJsonResponse() { } @ParameterizedTest - @ValueSource(strings = {"status", "txid"}) + @ValueSource(strings = {"status", "txid", "vin", "vout"}) void mempoolResponseWithMissingField(String missingField) throws IOException { JsonObject json = getValidBtcMakerFeeMempoolJsonResponse(); json.remove(missingField); From d7cd1e2c9255fb091225f53bef73371c6ad15dad Mon Sep 17 00:00:00 2001 From: Alva Swanson Date: Fri, 12 Jan 2024 12:41:56 +0100 Subject: [PATCH 3/3] TxValidator: Fix crash on invalid vin or vout JSON Array The `TxValidator`s `getVinAndVout(...)` method assumes that `json.get("field").getAsJsonArray()` returns null when the field is not a JSON array. The `getAsJsonArray()` throws an `IllegalStateException` exception, however. The `IllegalStateException` doesn't get caught by any caller. --- .../bisq/core/provider/mempool/TxValidator.java | 15 ++++++++++----- .../fee/MakerTxValidatorSanityCheckTests.java | 15 +++++++++++++++ .../fee/TakerTxValidatorSanityCheckTests.java | 15 +++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/bisq/core/provider/mempool/TxValidator.java b/core/src/main/java/bisq/core/provider/mempool/TxValidator.java index 479724f7db2..7c67343d973 100644 --- a/core/src/main/java/bisq/core/provider/mempool/TxValidator.java +++ b/core/src/main/java/bisq/core/provider/mempool/TxValidator.java @@ -302,12 +302,17 @@ private static Tuple2 getVinAndVout(String jsonTxt) throws if (json.get("vin") == null || json.get("vout") == null) { throw new JsonSyntaxException("missing vin/vout"); } - JsonArray jsonVin = json.get("vin").getAsJsonArray(); - JsonArray jsonVout = json.get("vout").getAsJsonArray(); - if (jsonVin == null || jsonVout == null || jsonVin.size() < 1 || jsonVout.size() < 2) { - throw new JsonSyntaxException("not enough vins/vouts"); + + try { + JsonArray jsonVin = json.get("vin").getAsJsonArray(); + JsonArray jsonVout = json.get("vout").getAsJsonArray(); + if (jsonVin == null || jsonVout == null || jsonVin.size() < 1 || jsonVout.size() < 2) { + throw new JsonSyntaxException("not enough vins/vouts"); + } + return new Tuple2<>(jsonVin, jsonVout); + } catch (IllegalStateException e) { + throw new JsonSyntaxException("vin/vout no as JSON Array", e); } - return new Tuple2<>(jsonVin, jsonVout); } private static FeeValidationStatus initialSanityChecks(String txId, String jsonTxt) { diff --git a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java index 6a147bfb5ca..874b580cf61 100644 --- a/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java +++ b/core/src/test/java/bisq/core/fee/MakerTxValidatorSanityCheckTests.java @@ -49,6 +49,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; @ExtendWith(MockitoExtension.class) public class MakerTxValidatorSanityCheckTests { @@ -106,6 +107,20 @@ void mempoolResponseWithoutConfirmedField() throws IOException { assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); } + @ParameterizedTest + @ValueSource(strings = {"vin", "vout"}) + void checkFeeAddressBtcTestVinOrVoutNotJsonArray(String vinOrVout) throws IOException { + JsonObject json = MakerTxValidatorSanityCheckTests.getValidBtcMakerFeeMempoolJsonResponse(); + json.add(vinOrVout, new JsonPrimitive(1234)); + assertThrows(IllegalStateException.class, () -> json.get(vinOrVout).getAsJsonArray()); + + String jsonContent = new Gson().toJson(json); + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, + MakerTxValidatorSanityCheckTests.FEE_RECEIVER_ADDRESSES); + + assertThat(txValidator1.getStatus(), is(FeeValidationStatus.NACK_JSON_ERROR)); + } + @Test void responseHasDifferentTxId() throws IOException { String differentTxId = "abcde971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07"; diff --git a/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java index 546c03c988f..f74d50592cc 100644 --- a/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java +++ b/core/src/test/java/bisq/core/fee/TakerTxValidatorSanityCheckTests.java @@ -49,6 +49,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; @ExtendWith(MockitoExtension.class) public class TakerTxValidatorSanityCheckTests { @@ -106,6 +107,20 @@ void mempoolResponseWithoutConfirmedField() throws IOException { assertThat(status, is(equalTo(FeeValidationStatus.NACK_JSON_ERROR))); } + @ParameterizedTest + @ValueSource(strings = {"vin", "vout"}) + void checkFeeAddressBtcTestVinOrVoutNotJsonArray(String vinOrVout) throws IOException { + JsonObject json = MakerTxValidatorSanityCheckTests.getValidBtcMakerFeeMempoolJsonResponse(); + json.add(vinOrVout, new JsonPrimitive(1234)); + assertThrows(IllegalStateException.class, () -> json.get(vinOrVout).getAsJsonArray()); + + String jsonContent = new Gson().toJson(json); + TxValidator txValidator1 = txValidator.parseJsonValidateMakerFeeTx(jsonContent, + MakerTxValidatorSanityCheckTests.FEE_RECEIVER_ADDRESSES); + + assertThat(txValidator1.getStatus(), is(FeeValidationStatus.NACK_JSON_ERROR)); + } + @Test void responseHasDifferentTxId() throws IOException { String differentTxId = "abcde971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07";