Merge pull request #6997 from alvasw/fix_fee_check_invalid_json_crash

Fix fee check crash on invalid JSON response

core/src/main/java/bisq/core/offer/OpenOffer.java

@@ -91,13 +91,6 @@ public OpenOffer(Offer offer) {
         this(offer, 0);
     }
 
-    public OpenOffer(Offer offer, State state) {
-        this.offer = offer;
-        this.triggerPrice = 0;
-        this.state = state;
-        arbitratorNodeAddress = null;
-    }
-
     public OpenOffer(Offer offer, long triggerPrice) {
         this.offer = offer;
         this.triggerPrice = triggerPrice;

core/src/main/java/bisq/core/offer/bsq_swap/OpenBsqSwapOfferService.java

@@ -367,7 +367,7 @@ private void redoProofOfWorkAndRepublish(OpenOffer openOffer) {
 
                         checkArgument(!openOffer.isDeactivated(),
                                 "We must not get called at redoProofOrWorkAndRepublish if offer was deactivated");
-                        OpenOffer newOpenOffer = new OpenOffer(newOffer, OpenOffer.State.AVAILABLE);
+                        OpenOffer newOpenOffer = new OpenOffer(newOffer);
                         if (!newOpenOffer.isDeactivated()) {
                             openOfferManager.maybeRepublishOffer(newOpenOffer);
                         }

core/src/main/java/bisq/core/provider/mempool/TxValidator.java

@@ -94,8 +94,9 @@ public TxValidator(DaoStateService daoStateService, String txId, FilterManager f
 
     public TxValidator parseJsonValidateMakerFeeTx(String jsonTxt, List<String> btcFeeReceivers) {
         this.jsonTxt = jsonTxt;
-        FeeValidationStatus status = initialSanityChecks(txId, jsonTxt);
+        FeeValidationStatus status;
         try {
+            status = initialSanityChecks(txId, jsonTxt);
             if (status.pass()) {
                 status = checkFeeAddressBTC(jsonTxt, btcFeeReceivers);
                 if (status.pass()) {
@@ -131,8 +132,9 @@ public TxValidator validateBsqFeeTx(boolean isMaker) {
 
     public TxValidator parseJsonValidateTakerFeeTx(String jsonTxt, List<String> btcFeeReceivers) {
         this.jsonTxt = jsonTxt;
-        FeeValidationStatus status = initialSanityChecks(txId, jsonTxt);
+        FeeValidationStatus status;
         try {
+            status = initialSanityChecks(txId, jsonTxt);
             if (status.pass()) {
                 status = checkFeeAddressBTC(jsonTxt, btcFeeReceivers);
                 if (status.pass()) {
@@ -148,10 +150,14 @@ public TxValidator parseJsonValidateTakerFeeTx(String jsonTxt, List<String> btcF
     }
 
     public long parseJsonValidateTx() {
-        if (!initialSanityChecks(txId, jsonTxt).pass()) {
+        try {
+            if (!initialSanityChecks(txId, jsonTxt).pass()) {
+                return -1;
+            }
+            return getTxConfirms(jsonTxt, chainHeight);
+        } catch (JsonSyntaxException e) {
             return -1;
         }
-        return getTxConfirms(jsonTxt, chainHeight);
     }
 
     ///////////////////////////////////////////////////////////////////////////////////////////

core/src/test/java/bisq/core/fee/FeeValidationTests.java

@@ -0,0 +1,71 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.fee;
+
+import bisq.core.dao.state.DaoStateService;
+import bisq.core.filter.FilterManager;
+import bisq.core.offer.Offer;
+import bisq.core.offer.OpenOffer;
+import bisq.core.provider.mempool.FeeValidationStatus;
+import bisq.core.provider.mempool.TxValidator;
+
+import org.bitcoinj.core.Coin;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsEqual.equalTo;
+import static org.mockito.Mockito.mock;
+
+public class FeeValidationTests {
+    @Test
+    void createOpenBsqOffer() {
+        var openOffer = new OpenOffer(mock(Offer.class));
+        assertThat(openOffer.getFeeValidationStatus(), is(equalTo(FeeValidationStatus.NOT_CHECKED_YET)));
+    }
+
+    @Test
+    void createOpenOfferWithTriggerPrice() {
+        var openOffer = new OpenOffer(mock(Offer.class), 42_000);
+        assertThat(openOffer.getFeeValidationStatus(), is(equalTo(FeeValidationStatus.NOT_CHECKED_YET)));
+    }
+
+    @Test
+    void notCheckedYetStatusIsNotFail() {
+        assertThat(FeeValidationStatus.NOT_CHECKED_YET.fail(), is(false));
+    }
+
+    @Test
+    void txValidatorInitialStateIsNotCheckedYet() {
+        var txValidator = new TxValidator(mock(DaoStateService.class),
+                "a_tx_id",
+                mock(Coin.class),
+                true,
+                106,
+                mock(FilterManager.class));
+
+        assertThat(txValidator.getStatus(), is(equalTo(FeeValidationStatus.NOT_CHECKED_YET)));
+    }
+
+    @Test
+    void txValidatorSecondConstructorInitialStateIsNotCheckedYet() {
+        var txValidator = new TxValidator(mock(DaoStateService.class), "a_tx_id", mock(FilterManager.class));
+        assertThat(txValidator.getStatus(), is(equalTo(FeeValidationStatus.NOT_CHECKED_YET)));
+    }
+}

core/src/test/java/bisq/core/fee/TxValidatorSanityCheckTests.java

@@ -0,0 +1,137 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+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 TxValidatorSanityCheckTests {
+    private final List<String> 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)));
+    }
+
+    private JsonObject getValidBtcMakerFeeMempoolJsonResponse() throws IOException {
+        URL resource = getClass().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);
+    }
+}

core/src/test/java/bisq/core/offer/OpenOfferConstructionTest.java

@@ -0,0 +1,39 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.offer;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.mock;
+
+public class OpenOfferConstructionTest {
+    @Test
+    void openOfferInitialStateTest() {
+        var openOffer = new OpenOffer(mock(Offer.class));
+        assertThat(openOffer.getState(), is(equalTo(OpenOffer.State.AVAILABLE)));
+    }
+
+    @Test
+    void openOfferWithTriggerPriceInitialStateTest() {
+        var openOffer = new OpenOffer(mock(Offer.class), 42_000);
+        assertThat(openOffer.getState(), is(equalTo(OpenOffer.State.AVAILABLE)));
+    }
+}

core/src/test/resources/mempool_test_data/valid_btc_maker_fee.json

@@ -0,0 +1 @@
+{"vsize":173,"feePerVsize":85.21739130434783,"effectiveFeePerVsize":85.21739130434783,"txid":"e3607e971ead7d03619e3a9eeaa771ed5adba14c448839e0299f857f7bb4ec07","version":1,"locktime":0,"size":255,"weight":690,"fee":14700,"vin":[{"is_coinbase":false,"prevout":{"value":996859150,"scriptpubkey":"00142a8e35db962721056ca515971992206bd9dffee0","scriptpubkey_address":"bcrt1q928rtkukyuss2m99zkt3ny3qd0vallhqvrma78","scriptpubkey_asm":"OP_0 OP_PUSHBYTES_20 2a8e35db962721056ca515971992206bd9dffee0","scriptpubkey_type":"v0_p2wpkh"},"scriptsig":"","scriptsig_asm":"","sequence":4294967295,"txid":"b03fe206f5cb00db3c995d7929a10e34f77630f31f8ba16adf90c4f60a9bae30","vout":2,"witness":["3045022100ba3503559189431ed643100d78d8b90c7abdb79c62d7d7b79ec3fc8bc45888db02204ab67155186f258a12051d2b578c79d752458c1c58c23952aacf8ef217a3162901","026ed3ec4d8736f6b67eaf3d656558926ceebd0c7c9e498d5b7d393d353894b035"],"inner_redeemscript_asm":"","inner_witnessscript_asm":""}],"vout":[{"value":5000,"scriptpubkey":"a9144c0e4893237f85479f489b32c8ff0faf3ee2e1c987","scriptpubkey_address":"2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w","scriptpubkey_asm":"OP_HASH160 OP_PUSHBYTES_20 4c0e4893237f85479f489b32c8ff0faf3ee2e1c9 OP_EQUAL","scriptpubkey_type":"p2sh"},{"value":200000,"scriptpubkey":"001404fc5217348d331051c94e5044eeb9c711e0631a","scriptpubkey_address":"bcrt1qqn79y9e535e3q5wffegyfm4ecug7qcc6e3pkc5","scriptpubkey_asm":"OP_0 OP_PUSHBYTES_20 04fc5217348d331051c94e5044eeb9c711e0631a","scriptpubkey_type":"v0_p2wpkh"},{"value":996639450,"scriptpubkey":"001414a63aafad1f000d4e1f4e2148815a1d43de83e5","scriptpubkey_address":"bcrt1qzjnr4tadruqq6nslfcs53q26r4paaql99m3drh","scriptpubkey_asm":"OP_0 OP_PUSHBYTES_20 14a63aafad1f000d4e1f4e2148815a1d43de83e5","scriptpubkey_type":"v0_p2wpkh"}],"status":{"confirmed":true,"block_height":143,"block_hash":"3a9db50820638e6f5c45f252de509bcafebf4731f4e4ad14276b9715439670ee","block_time":1704107755}}