Merge pull request #7152 from HenrikJannsen/apply-ExcludeForHash-annotation-to-filter

[3] Apply exclude for hash annotation to filter

common/src/main/java/bisq/common/ExcludeForHashAwareProto.java

@@ -103,7 +103,7 @@ public interface ExcludeForHashAwareProto extends Proto {
     default <B extends Message.Builder> B clearAnnotatedFields(B builder) {
         Set<String> excludedFields = getExcludedFields();
         if (!excludedFields.isEmpty()) {
-            getLogger().error("Clear fields in builder annotated with @ExcludeForHash: {}", excludedFields);
+            getLogger().debug("Clear fields in builder annotated with @ExcludeForHash: {}", excludedFields);
         }
         for (Descriptors.FieldDescriptor fieldDesc : builder.getAllFields().keySet()) {
             if (excludedFields.contains(fieldDesc.getName())) {

core/src/main/java/bisq/core/filter/Filter.java

@@ -20,6 +20,8 @@ package bisq.core.filter;
 import bisq.network.p2p.storage.payload.ExpirablePayload;
 import bisq.network.p2p.storage.payload.ProtectedStoragePayload;
 
+import bisq.common.ExcludeForHash;
+import bisq.common.ExcludeForHashAwareProto;
 import bisq.common.crypto.Sig;
 import bisq.common.proto.ProtoUtil;
 import bisq.common.proto.network.GetDataResponsePriority;
@@ -27,6 +29,8 @@ import bisq.common.util.CollectionUtils;
 import bisq.common.util.ExtraDataMapValidator;
 import bisq.common.util.Utilities;
 
+import protobuf.StoragePayload;
+
 import com.google.protobuf.ByteString;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -49,7 +53,7 @@ import javax.annotation.Nullable;
 @Slf4j
 @Getter
 @EqualsAndHashCode
-public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
+public final class Filter implements ProtectedStoragePayload, ExpirablePayload, ExcludeForHashAwareProto {
     public static final long TTL = TimeUnit.DAYS.toMillis(180);
 
     private final List<String> bannedOfferIds;
@@ -124,8 +128,14 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
     private final List<PaymentAccountFilter> delayedPayoutPaymentAccounts;
 
     // Added at v 1.9.16
+    @ExcludeForHash
     private final List<String> addedBtcNodes;
+    @ExcludeForHash
     private final List<String> addedSeedNodes;
+    // As we might add more ExcludeForHash we want to ensure to have a unique identifier.
+    // The hash of the data is not unique anymore if the only change have been at
+    // the ExcludeForHash annotated fields.
+    private final String uid;
 
     // After we have created the signature from the filter data we clone it and apply the signature
     static Filter cloneWithSig(Filter filter, String signatureAsBase64) {
@@ -166,7 +176,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 filter.getTakerFeeBsq(),
                 filter.getDelayedPayoutPaymentAccounts(),
                 filter.getAddedBtcNodes(),
-                filter.getAddedSeedNodes());
+                filter.getAddedSeedNodes(),
+                filter.getUid());
     }
 
     // Used for signature verification as we created the sig without the signatureAsBase64 field we set it to null again
@@ -208,7 +219,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 filter.getTakerFeeBsq(),
                 filter.getDelayedPayoutPaymentAccounts(),
                 filter.getAddedBtcNodes(),
-                filter.getAddedSeedNodes());
+                filter.getAddedSeedNodes(),
+                filter.getUid());
     }
 
     public Filter(List<String> bannedOfferIds,
@@ -245,7 +257,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                   long takerFeeBsq,
                   List<PaymentAccountFilter> delayedPayoutPaymentAccounts,
                   List<String> addedBtcNodes,
-                  List<String> addedSeedNodes) {
+                  List<String> addedSeedNodes,
+                  String uid) {
         this(bannedOfferIds,
                 nodeAddressesBannedFromTrading,
                 bannedPaymentAccounts,
@@ -283,7 +296,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 takerFeeBsq,
                 delayedPayoutPaymentAccounts,
                 addedBtcNodes,
-                addedSeedNodes);
+                addedSeedNodes,
+                uid);
     }
 
 
@@ -329,7 +343,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                   long takerFeeBsq,
                   List<PaymentAccountFilter> delayedPayoutPaymentAccounts,
                   List<String> addedBtcNodes,
-                  List<String> addedSeedNodes) {
+                  List<String> addedSeedNodes,
+                  String uid) {
         this.bannedOfferIds = bannedOfferIds;
         this.nodeAddressesBannedFromTrading = nodeAddressesBannedFromTrading;
         this.bannedPaymentAccounts = bannedPaymentAccounts;
@@ -368,6 +383,7 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
         this.delayedPayoutPaymentAccounts = delayedPayoutPaymentAccounts;
         this.addedBtcNodes = addedBtcNodes;
         this.addedSeedNodes = addedSeedNodes;
+        this.uid = uid;
 
         // ownerPubKeyBytes can be null when called from tests
         if (ownerPubKeyBytes != null) {
@@ -379,6 +395,24 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
 
     @Override
     public protobuf.StoragePayload toProtoMessage() {
+        return toProto(false);
+    }
+
+    @Override
+    public protobuf.StoragePayload toProto(boolean serializeForHash) {
+        return resolveProto(serializeForHash);
+    }
+
+    @Override
+    public protobuf.StoragePayload.Builder getBuilder(boolean serializeForHash) {
+        return StoragePayload.newBuilder().setFilter(toFilterProto(serializeForHash));
+    }
+
+    private protobuf.Filter toFilterProto(boolean serializeForHash) {
+        return resolveBuilder(getFilterBuilder(serializeForHash), serializeForHash).build();
+    }
+
+    private protobuf.Filter.Builder getFilterBuilder(boolean serializeForHash) {
         List<protobuf.PaymentAccountFilter> paymentAccountFilterList = bannedPaymentAccounts.stream()
                 .map(PaymentAccountFilter::toProtoMessage)
                 .collect(Collectors.toList());
@@ -421,12 +455,13 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 .setTakerFeeBsq(takerFeeBsq)
                 .addAllDelayedPayoutPaymentAccounts(delayedPayoutPaymentAccountList)
                 .addAllAddedBtcNodes(addedBtcNodes)
-                .addAllAddedSeedNodes(addedSeedNodes);
+                .addAllAddedSeedNodes(addedSeedNodes)
+                .setUid(uid);
 
         Optional.ofNullable(signatureAsBase64).ifPresent(builder::setSignatureAsBase64);
         Optional.ofNullable(extraDataMap).ifPresent(builder::putAllExtraData);
 
-        return protobuf.StoragePayload.newBuilder().setFilter(builder).build();
+        return builder;
     }
 
     public static Filter fromProto(protobuf.Filter proto) {
@@ -474,7 +509,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 proto.getTakerFeeBsq(),
                 delayedPayoutPaymentAccounts,
                 ProtoUtil.protocolStringListToList(proto.getAddedBtcNodesList()),
-                ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList())
+                ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList()),
+                proto.getUid()
         );
     }
 
@@ -534,6 +570,7 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload {
                 ",\n     takerFeeBsq=" + takerFeeBsq +
                 ",\n     addedBtcNodes=" + addedBtcNodes +
                 ",\n     addedSeedNodes=" + addedSeedNodes +
+                ",\n     uid=" + uid +
                 "\n}";
     }
 }

core/src/main/java/bisq/core/filter/FilterManager.java

@@ -701,8 +701,7 @@ public class FilterManager {
     }
 
     private Sha256Hash getSha256Hash(Filter filter) {
-        byte[] filterData = filter.serializeForHash();
+        return Sha256Hash.of(filter.serializeForHash());
-        return Sha256Hash.of(filterData);
     }
 
     private String getPubKeyAsHex(ECKey ecKey) {

core/src/test/java/bisq/core/filter/FilterWithExcludeForHashFieldTests.java

@@ -0,0 +1,63 @@
+/*
+ * 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.filter;
+
+import bisq.common.crypto.Sig;
+
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.PublicKey;
+import java.security.Security;
+
+import java.util.Arrays;
+
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@ExtendWith(MockitoExtension.class)
+public class FilterWithExcludeForHashFieldTests {
+    static {
+        Security.addProvider(new BouncyCastleProvider());
+    }
+
+    private final PublicKey ownerPublicKey;
+
+    public FilterWithExcludeForHashFieldTests() throws NoSuchAlgorithmException, NoSuchProviderException {
+        KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(Sig.KEY_ALGO, "BC");
+        KeyPair ownerKeyPair = keyPairGenerator.generateKeyPair();
+        ownerPublicKey = ownerKeyPair.getPublic();
+    }
+
+    @Test
+    void testSerializeForHashAnnotation() {
+        Filter filterWithoutSig = TestFilter.createFilter(ownerPublicKey, "invalidPubKeyAsHex");
+        byte[] serializeForHashBytes = filterWithoutSig.serializeForHash();
+        byte[] serializeBytes = filterWithoutSig.serialize();
+        assertFalse(Arrays.equals(serializeForHashBytes, serializeBytes));
+        assertTrue(serializeBytes.length > serializeForHashBytes.length);
+    }
+}

core/src/test/java/bisq/core/filter/TestFilter.java

@@ -32,6 +32,7 @@ import java.nio.charset.StandardCharsets;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.UUID;
 
 import static org.bitcoinj.core.Utils.HEX;
 
@@ -103,8 +104,9 @@ public class TestFilter {
                 1,
                 1,
                 Collections.emptyList(),
-                Collections.emptyList(),
+                List.of("test1.onion:1221"),
-                Collections.emptyList()
+                List.of("test2.onion:1221"),
+                UUID.randomUUID().toString()
         );
     }
 

core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java

@@ -26,6 +26,7 @@ import bisq.core.proto.CoreProtoResolver;
 import com.google.common.collect.Lists;
 
 import java.util.HashSet;
+import java.util.UUID;
 
 import org.junit.jupiter.api.Disabled;
 
@@ -79,7 +80,8 @@ public class UserPayloadModelVOTest {
                 0,
                 Lists.newArrayList(),
                 Lists.newArrayList(),
-                Lists.newArrayList()));
+                Lists.newArrayList(),
+                UUID.randomUUID().toString()));
 
         vo.setRegisteredArbitrator(ArbitratorTest.getArbitratorMock());
         vo.setRegisteredMediator(MediatorTest.getMediatorMock());

desktop/src/main/java/bisq/desktop/main/overlays/windows/FilterWindow.java

@@ -60,6 +60,7 @@ import javafx.geometry.Insets;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 import static bisq.desktop.util.FormBuilder.addInputTextField;
@@ -129,6 +130,8 @@ public class FilterWindow extends Overlay<FilterWindow> {
 
         InputTextField keyTF = addInputTextField(gridPane, ++rowIndex,
                 Res.get("shared.unlock"), 10);
+        InputTextField uidTF = addInputTextField(gridPane, ++rowIndex,
+                "UID", 10);
         if (useDevPrivilegeKeys) {
             keyTF.setText(DevEnv.getDEV_PRIVILEGE_PRIV_KEY());
         }
@@ -212,6 +215,7 @@ public class FilterWindow extends Overlay<FilterWindow> {
 
         Filter filter = filterManager.getDevFilter();
         if (filter != null) {
+            uidTF.setText(filter.getUid());
             setupFieldFromList(offerIdsTF, filter.getBannedOfferIds());
             setupFieldFromList(bannedFromTradingTF, filter.getNodeAddressesBannedFromTrading());
             setupFieldFromList(bannedFromNetworkTF, filter.getNodeAddressesBannedFromNetwork());
@@ -247,6 +251,8 @@ public class FilterWindow extends Overlay<FilterWindow> {
             makerFeeBsqTF.setText(bsqFormatter.formatBSQSatoshis(filter.getMakerFeeBsq()));
             takerFeeBsqTF.setText(bsqFormatter.formatBSQSatoshis(filter.getTakerFeeBsq()));
             setupFieldFromPaymentAccountFiltersList(delayedPayoutTF, filter.getDelayedPayoutPaymentAccounts());
+        } else {
+            uidTF.setText(UUID.randomUUID().toString());
         }
 
         Button removeFilterMessageButton = new AutoTooltipButton(Res.get("filterWindow.remove"));
@@ -292,7 +298,8 @@ public class FilterWindow extends Overlay<FilterWindow> {
                         ParsingUtils.parseToCoin(takerFeeBsqTF.getText(), bsqFormatter).value,
                         readAsPaymentAccountFiltersList(delayedPayoutTF),
                         readAsList(addedBtcNodesTF),
-                        readAsList(addedSeedNodesTF)
+                        readAsList(addedSeedNodesTF),
+                        uidTF.getText()
                 );
 
                 // We remove first the old filter

p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java

@@ -55,6 +55,7 @@ import bisq.network.p2p.storage.persistence.RemovedPayloadsService;
 import bisq.network.p2p.storage.persistence.ResourceDataStoreService;
 import bisq.network.p2p.storage.persistence.SequenceNumberMap;
 
+import bisq.common.ExcludeForHashAwareProto;
 import bisq.common.Timer;
 import bisq.common.UserThread;
 import bisq.common.app.Capabilities;
@@ -71,6 +72,8 @@ import bisq.common.util.Hex;
 import bisq.common.util.Tuple2;
 import bisq.common.util.Utilities;
 
+import protobuf.StoragePayload;
+
 import com.google.protobuf.ByteString;
 
 import com.google.inject.name.Named;
@@ -1253,8 +1256,8 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
      */
     @EqualsAndHashCode
     @ToString
-    public static final class DataAndSeqNrPair implements NetworkPayload {
+    public static final class DataAndSeqNrPair implements NetworkPayload, ExcludeForHashAwareProto {
-        // data are only used for calculating cryptographic hash from both values so they are kept private
+        // data are only used for calculating cryptographic hash from both values, so they are kept private
         private final ProtectedStoragePayload protectedStoragePayload;
         private final int sequenceNumber;
 
@@ -1263,13 +1266,31 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
             this.sequenceNumber = sequenceNumber;
         }
 
-        // Used only for calculating hash of byte array from PB object
         @Override
-        public com.google.protobuf.Message toProtoMessage() {
+        public protobuf.DataAndSeqNrPair toProtoMessage() {
+            return toProto(false);
+        }
+
+        @Override
+        public protobuf.DataAndSeqNrPair toProto(boolean serializeForHash) {
+            return resolveProto(serializeForHash);
+        }
+
+        @Override
+        public protobuf.DataAndSeqNrPair.Builder getBuilder(boolean serializeForHash) {
             return protobuf.DataAndSeqNrPair.newBuilder()
-                    .setPayload((protobuf.StoragePayload) protectedStoragePayload.toProtoMessage())
+                    .setPayload(toStoragePayloadProto(serializeForHash))
-                    .setSequenceNumber(sequenceNumber)
+                    .setSequenceNumber(sequenceNumber);
-                    .build();
+        }
+
+        private protobuf.StoragePayload toStoragePayloadProto(boolean serializeForHash) {
+            if (protectedStoragePayload instanceof ExcludeForHashAwareProto) {
+                ExcludeForHashAwareProto proto = (ExcludeForHashAwareProto) protectedStoragePayload;
+                StoragePayload.Builder builder = (StoragePayload.Builder) proto.getBuilder(serializeForHash);
+                return resolveBuilder(builder, serializeForHash).build();
+            } else {
+                return (StoragePayload) protectedStoragePayload.toProtoMessage();
+            }
         }
     }
 

p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java

@@ -32,6 +32,8 @@ import com.google.protobuf.Message;
 
 import com.google.common.base.Preconditions;
 
+import org.bouncycastle.util.encoders.Hex;
+
 import java.security.PublicKey;
 
 import java.time.Clock;
@@ -215,7 +217,9 @@ public class ProtectedStorageEntry implements NetworkPayload, PersistablePayload
             boolean result = Sig.verify(this.ownerPubKey, hashOfDataAndSeqNr, this.signature);
 
             if (!result)
-                log.warn("ProtectedStorageEntry::isSignatureValid() failed.\n{}}", this);
+                log.warn("Invalid signature for {}.\nSerialized data as hex={}}",
+                        protectedStoragePayload.getClass().getSimpleName(),
+                        Hex.toHexString(protectedStoragePayload.toProtoMessage().toByteArray()));
 
             return result;
         } catch (CryptoException e) {

proto/src/main/proto/pb.proto

@@ -769,6 +769,7 @@ message Filter {
     repeated PaymentAccountFilter delayedPayoutPaymentAccounts = 36;
     repeated string addedBtcNodes = 37;
     repeated string addedSeedNodes = 38;
+    string uid = 39;
 }
 
 /* Deprecated */