Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3] Apply exclude for hash annotation to filter #7152

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ default Set<String> getExcludedFields() {
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())) {
Expand Down
55 changes: 46 additions & 9 deletions core/src/main/java/bisq/core/filter/Filter.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@
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;
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;
Expand All @@ -49,7 +53,7 @@
@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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -166,7 +176,8 @@ static Filter cloneWithSig(Filter filter, String signatureAsBase64) {
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
Expand Down Expand Up @@ -208,7 +219,8 @@ static Filter cloneWithoutSig(Filter filter) {
filter.getTakerFeeBsq(),
filter.getDelayedPayoutPaymentAccounts(),
filter.getAddedBtcNodes(),
filter.getAddedSeedNodes());
filter.getAddedSeedNodes(),
filter.getUid());
}

public Filter(List<String> bannedOfferIds,
Expand Down Expand Up @@ -245,7 +257,8 @@ public Filter(List<String> bannedOfferIds,
long takerFeeBsq,
List<PaymentAccountFilter> delayedPayoutPaymentAccounts,
List<String> addedBtcNodes,
List<String> addedSeedNodes) {
List<String> addedSeedNodes,
String uid) {
this(bannedOfferIds,
nodeAddressesBannedFromTrading,
bannedPaymentAccounts,
Expand Down Expand Up @@ -283,7 +296,8 @@ public Filter(List<String> bannedOfferIds,
takerFeeBsq,
delayedPayoutPaymentAccounts,
addedBtcNodes,
addedSeedNodes);
addedSeedNodes,
uid);
}


Expand Down Expand Up @@ -329,7 +343,8 @@ public Filter(List<String> bannedOfferIds,
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;
Expand Down Expand Up @@ -368,6 +383,7 @@ public Filter(List<String> bannedOfferIds,
this.delayedPayoutPaymentAccounts = delayedPayoutPaymentAccounts;
this.addedBtcNodes = addedBtcNodes;
this.addedSeedNodes = addedSeedNodes;
this.uid = uid;

// ownerPubKeyBytes can be null when called from tests
if (ownerPubKeyBytes != null) {
Expand All @@ -379,6 +395,24 @@ public Filter(List<String> bannedOfferIds,

@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());
Expand Down Expand Up @@ -421,12 +455,13 @@ public protobuf.StoragePayload toProtoMessage() {
.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) {
Expand Down Expand Up @@ -474,7 +509,8 @@ public static Filter fromProto(protobuf.Filter proto) {
proto.getTakerFeeBsq(),
delayedPayoutPaymentAccounts,
ProtoUtil.protocolStringListToList(proto.getAddedBtcNodesList()),
ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList())
ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList()),
proto.getUid()
);
}

Expand Down Expand Up @@ -534,6 +570,7 @@ public String toString() {
",\n takerFeeBsq=" + takerFeeBsq +
",\n addedBtcNodes=" + addedBtcNodes +
",\n addedSeedNodes=" + addedSeedNodes +
",\n uid=" + uid +
"\n}";
}
}
3 changes: 1 addition & 2 deletions core/src/main/java/bisq/core/filter/FilterManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,7 @@ private ECKey toECKey(String privKeyString) {
}

private Sha256Hash getSha256Hash(Filter filter) {
byte[] filterData = filter.serializeForHash();
return Sha256Hash.of(filterData);
return Sha256Hash.of(filter.serializeForHash());
}

private String getPubKeyAsHex(ECKey ecKey) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
6 changes: 4 additions & 2 deletions core/src/test/java/bisq/core/filter/TestFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

import java.util.Collections;
import java.util.List;
import java.util.UUID;

import static org.bitcoinj.core.Utils.HEX;

Expand Down Expand Up @@ -103,8 +104,9 @@ public static Filter createFilter(PublicKey ownerPublicKey, String signerPubKeyA
1,
1,
Collections.emptyList(),
Collections.emptyList(),
Collections.emptyList()
List.of("test1.onion:1221"),
List.of("test2.onion:1221"),
UUID.randomUUID().toString()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.common.collect.Lists;

import java.util.HashSet;
import java.util.UUID;

import org.junit.jupiter.api.Disabled;

Expand Down Expand Up @@ -79,7 +80,8 @@ public void testRoundtripFull() {
0,
Lists.newArrayList(),
Lists.newArrayList(),
Lists.newArrayList()));
Lists.newArrayList(),
UUID.randomUUID().toString()));

vo.setRegisteredArbitrator(ArbitratorTest.getArbitratorMock());
vo.setRegisteredMediator(MediatorTest.getMediatorMock());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
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;
Expand Down Expand Up @@ -129,6 +130,8 @@ private void addContent() {

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());
}
Expand Down Expand Up @@ -212,6 +215,7 @@ private void addContent() {

Filter filter = filterManager.getDevFilter();
if (filter != null) {
uidTF.setText(filter.getUid());
setupFieldFromList(offerIdsTF, filter.getBannedOfferIds());
setupFieldFromList(bannedFromTradingTF, filter.getNodeAddressesBannedFromTrading());
setupFieldFromList(bannedFromNetworkTF, filter.getNodeAddressesBannedFromNetwork());
Expand Down Expand Up @@ -247,6 +251,8 @@ private void addContent() {
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"));
Expand Down Expand Up @@ -292,7 +298,8 @@ private void addContent() {
ParsingUtils.parseToCoin(takerFeeBsqTF.getText(), bsqFormatter).value,
readAsPaymentAccountFiltersList(delayedPayoutTF),
readAsList(addedBtcNodesTF),
readAsList(addedSeedNodesTF)
readAsList(addedSeedNodesTF),
uidTF.getText()
);

// We remove first the old filter
Expand Down
35 changes: 28 additions & 7 deletions p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
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;
Expand All @@ -71,6 +72,8 @@
import bisq.common.util.Tuple2;
import bisq.common.util.Utilities;

import protobuf.StoragePayload;

import com.google.protobuf.ByteString;

import com.google.inject.name.Named;
Expand Down Expand Up @@ -1253,8 +1256,8 @@ public static byte[] get32ByteHash(NetworkPayload data) {
*/
@EqualsAndHashCode
@ToString
public static final class DataAndSeqNrPair implements NetworkPayload {
// data are only used for calculating cryptographic hash from both values so they are kept private
public static final class DataAndSeqNrPair implements NetworkPayload, ExcludeForHashAwareProto {
// data are only used for calculating cryptographic hash from both values, so they are kept private
private final ProtectedStoragePayload protectedStoragePayload;
private final int sequenceNumber;

Expand All @@ -1263,13 +1266,31 @@ public DataAndSeqNrPair(ProtectedStoragePayload protectedStoragePayload, int seq
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())
.setSequenceNumber(sequenceNumber)
.build();
.setPayload(toStoragePayloadProto(serializeForHash))
.setSequenceNumber(sequenceNumber);
}

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();
}
}
}

Expand Down
Loading
Loading