From f112760432bc59f7682f9daf1be5afdb99ce56fe Mon Sep 17 00:00:00 2001 From: woodser Date: Wed, 16 Aug 2023 09:25:39 -0400 Subject: [PATCH] remove and repost offers if signing arbitrator unregistered --- .../haveno/core/offer/OfferFilterService.java | 16 ++---- .../java/haveno/core/offer/OfferPayload.java | 14 ++--- .../haveno/core/offer/OpenOfferManager.java | 52 +++++++++++++++---- .../java/haveno/core/trade/HavenoUtils.java | 16 +++--- 4 files changed, 61 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/haveno/core/offer/OfferFilterService.java b/core/src/main/java/haveno/core/offer/OfferFilterService.java index 433e9f3a56..f9816582ca 100644 --- a/core/src/main/java/haveno/core/offer/OfferFilterService.java +++ b/core/src/main/java/haveno/core/offer/OfferFilterService.java @@ -131,9 +131,6 @@ public class OfferFilterService { if (isMyInsufficientTradeLimit(offer)) { return Result.IS_MY_INSUFFICIENT_TRADE_LIMIT; } - if (!hasValidArbitrator(offer)) { - return Result.ARBITRATOR_NOT_VALIDATED; - } if (!hasValidSignature(offer)) { return Result.SIGNATURE_NOT_VALIDATED; } @@ -222,26 +219,23 @@ public class OfferFilterService { return result; } - public boolean hasValidArbitrator(Offer offer) { + public boolean hasValidSignature(Offer offer) { + + // get accepted arbitrator by address Arbitrator arbitrator = user.getAcceptedArbitratorByAddress(offer.getOfferPayload().getArbitratorSigner()); - if (arbitrator != null) return true; // accepted arbitrator is null if we are the signing arbitrator - if (offer.getOfferPayload().getArbitratorSigner() != null) { + if (arbitrator == null && offer.getOfferPayload().getArbitratorSigner() != null) { Arbitrator thisArbitrator = user.getRegisteredArbitrator(); if (thisArbitrator != null && thisArbitrator.getNodeAddress().equals(offer.getOfferPayload().getArbitratorSigner())) { - if (thisArbitrator.getNodeAddress().equals(p2PService.getNetworkNode().getNodeAddress())) return true; // TODO: unnecessary to compare arbitrator and p2pservice address? + if (thisArbitrator.getNodeAddress().equals(p2PService.getNetworkNode().getNodeAddress())) arbitrator = thisArbitrator; // TODO: unnecessary to compare arbitrator and p2pservice address? } // otherwise log warning List arbitratorAddresses = user.getAcceptedArbitrators().stream().map(Arbitrator::getNodeAddress).collect(Collectors.toList()); log.warn("No arbitrator is registered with offer's signer. offerId={}, arbitrator signer={}, accepted arbitrators={}", offer.getId(), offer.getOfferPayload().getArbitratorSigner(), arbitratorAddresses); } - return false; - } - public boolean hasValidSignature(Offer offer) { - Arbitrator arbitrator = user.getAcceptedArbitratorByAddress(offer.getOfferPayload().getArbitratorSigner()); if (arbitrator == null) return false; // invalid arbitrator return HavenoUtils.isArbitratorSignatureValid(offer.getOfferPayload(), arbitrator); } diff --git a/core/src/main/java/haveno/core/offer/OfferPayload.java b/core/src/main/java/haveno/core/offer/OfferPayload.java index 074fbdd708..79db5ae315 100644 --- a/core/src/main/java/haveno/core/offer/OfferPayload.java +++ b/core/src/main/java/haveno/core/offer/OfferPayload.java @@ -160,7 +160,7 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay public OfferPayload(String id, long date, - NodeAddress ownerNodeAddress, + @Nullable NodeAddress ownerNodeAddress, PubKeyRing pubKeyRing, OfferDirection direction, long price, @@ -249,7 +249,7 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay OfferPayload signee = new OfferPayload( id, date, - ownerNodeAddress, + null, pubKeyRing, direction, price, @@ -317,7 +317,6 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay protobuf.OfferPayload.Builder builder = protobuf.OfferPayload.newBuilder() .setId(id) .setDate(date) - .setOwnerNodeAddress(ownerNodeAddress.toProtoMessage()) .setPubKeyRing(pubKeyRing.toProtoMessage()) .setDirection(OfferDirection.toProtoMessage(direction)) .setPrice(price) @@ -342,7 +341,7 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay .setUpperClosePrice(upperClosePrice) .setIsPrivateOffer(isPrivateOffer) .setProtocolVersion(protocolVersion); - Optional.ofNullable(arbitratorSigner).ifPresent(e -> builder.setArbitratorSigner(arbitratorSigner.toProtoMessage())); + Optional.ofNullable(ownerNodeAddress).ifPresent(e -> builder.setOwnerNodeAddress(ownerNodeAddress.toProtoMessage())); Optional.ofNullable(offerFeeTxId).ifPresent(builder::setOfferFeeTxId); Optional.ofNullable(countryCode).ifPresent(builder::setCountryCode); Optional.ofNullable(bankId).ifPresent(builder::setBankId); @@ -350,6 +349,7 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay Optional.ofNullable(acceptedCountryCodes).ifPresent(builder::addAllAcceptedCountryCodes); Optional.ofNullable(hashOfChallenge).ifPresent(builder::setHashOfChallenge); Optional.ofNullable(extraDataMap).ifPresent(builder::putAllExtraData); + Optional.ofNullable(arbitratorSigner).ifPresent(e -> builder.setArbitratorSigner(arbitratorSigner.toProtoMessage())); Optional.ofNullable(arbitratorSignature).ifPresent(e -> builder.setArbitratorSignature(ByteString.copyFrom(e))); Optional.ofNullable(reserveTxKeyImages).ifPresent(builder::addAllReserveTxKeyImages); @@ -367,7 +367,7 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay return new OfferPayload(proto.getId(), proto.getDate(), - NodeAddress.fromProto(proto.getOwnerNodeAddress()), + proto.hasOwnerNodeAddress() ? NodeAddress.fromProto(proto.getOwnerNodeAddress()) : null, PubKeyRing.fromProto(proto.getPubKeyRing()), OfferDirection.fromProto(proto.getDirection()), proto.getPrice(), @@ -443,8 +443,8 @@ public final class OfferPayload implements ProtectedStoragePayload, ExpirablePay ",\r\n upperClosePrice=" + upperClosePrice + ",\r\n isPrivateOffer=" + isPrivateOffer + ",\r\n hashOfChallenge='" + hashOfChallenge + '\'' + - ",\n arbitratorSigner=" + arbitratorSigner + - ",\n arbitratorSignature=" + Utilities.bytesAsHexString(arbitratorSignature) + + ",\r\n arbitratorSigner=" + arbitratorSigner + + ",\r\n arbitratorSignature=" + Utilities.bytesAsHexString(arbitratorSignature) + "\r\n} "; } diff --git a/core/src/main/java/haveno/core/offer/OpenOfferManager.java b/core/src/main/java/haveno/core/offer/OpenOfferManager.java index b645450868..c9b891416e 100644 --- a/core/src/main/java/haveno/core/offer/OpenOfferManager.java +++ b/core/src/main/java/haveno/core/offer/OpenOfferManager.java @@ -629,7 +629,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe addOpenOffer(editedOpenOffer); - if (!editedOpenOffer.isDeactivated()) + if (editedOpenOffer.isAvailable()) republishOffer(editedOpenOffer); offersToBeEdited.remove(openOffer.getId()); @@ -1138,7 +1138,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } // verify offer not seen before - Optional openOfferOptional = getOpenOfferById(request.offerId); + Optional openOfferOptional = getOpenOfferById(request.offerId); // TODO: check if offer is on books, not open offer if (openOfferOptional.isPresent()) { errorMessage = "We already got a request to sign offer id " + request.offerId; log.info(errorMessage); @@ -1431,8 +1431,6 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe // Update persisted offer if a new capability is required after a software update /////////////////////////////////////////////////////////////////////////////////////////// - // TODO (woodser): arbitrator signature will be invalid if offer updated (exclude updateable fields from signature? re-sign?) - private void maybeUpdatePersistedOffers() { // We need to clone to avoid ConcurrentModificationException List openOffersClone = getOpenOffers(); @@ -1570,7 +1568,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe synchronized (openOffers) { contained = openOffers.contains(openOffer); } - if (contained && !openOffer.isDeactivated() && openOffer.getOffer().getOfferPayload().getReserveTxKeyImages() != null && !openOffer.getOffer().getOfferPayload().getReserveTxKeyImages().isEmpty()) { + if (contained && openOffer.isAvailable()) { // TODO It is not clear yet if it is better for the node and the network to send out all add offer // messages in one go or to spread it over a delay. With power users who have 100-200 offers that can have // some significant impact to user experience and the network @@ -1591,10 +1589,16 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } private void republishOffer(OpenOffer openOffer, @Nullable Runnable completeHandler) { - offerBookService.addOffer(openOffer.getOffer(), + + // re-add to offer book if signature is valid + Arbitrator arbitrator = user.getAcceptedArbitratorByAddress(openOffer.getOffer().getOfferPayload().getArbitratorSigner()); + boolean isValid = arbitrator != null && HavenoUtils.isArbitratorSignatureValid(openOffer.getOffer().getOfferPayload(), arbitrator); + if (isValid) { + offerBookService.addOffer(openOffer.getOffer(), () -> { if (!stopped) { - // Refresh means we send only the data needed to refresh the TTL (hash, signature and sequence no.) + + // refresh means we send only the data needed to refresh the TTL (hash, signature and sequence no.) if (periodicRefreshOffersTimer == null) { startPeriodicRefreshOffersTimer(); } @@ -1609,12 +1613,38 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe stopRetryRepublishOffersTimer(); retryRepublishOffersTimer = UserThread.runAfter(OpenOfferManager.this::republishOffers, RETRY_REPUBLISH_DELAY_SEC); - - if (completeHandler != null) { - completeHandler.run(); - } + if (completeHandler != null) completeHandler.run(); } }); + return; + } + + // cancel and recreate offer + log.warn("Offer {} has invalid arbitrator signature, reposting", openOffer.getId()); + onCancelled(openOffer); + Offer updatedOffer = new Offer(openOffer.getOffer().getOfferPayload()); + updatedOffer.setPriceFeedService(priceFeedService); + OpenOffer updatedOpenOffer = new OpenOffer(updatedOffer, openOffer.getTriggerPrice()); + + // repost offer + new Thread(() -> { + synchronized (processOffersLock) { + CountDownLatch latch = new CountDownLatch(1); + processUnpostedOffer(getOpenOffers(), updatedOpenOffer, (transaction) -> { + addOpenOffer(updatedOpenOffer); + requestPersistence(); + latch.countDown(); + if (completeHandler != null) completeHandler.run(); + }, (errorMessage) -> { + log.warn("Error reposting offer {} with invalid signature: {}", updatedOpenOffer.getId(), errorMessage); + onCancelled(updatedOpenOffer); + updatedOffer.setErrorMessage(errorMessage); + latch.countDown(); + if (completeHandler != null) completeHandler.run(); + }); + HavenoUtils.awaitLatch(latch); + } + }).start(); } private void startPeriodicRepublishOffersTimer() { diff --git a/core/src/main/java/haveno/core/trade/HavenoUtils.java b/core/src/main/java/haveno/core/trade/HavenoUtils.java index f55503939d..c41a96d55f 100644 --- a/core/src/main/java/haveno/core/trade/HavenoUtils.java +++ b/core/src/main/java/haveno/core/trade/HavenoUtils.java @@ -245,17 +245,17 @@ public class HavenoUtils { return sign(keyRing.getSignatureKeyPair().getPrivate(), message); } - public static byte[] sign(KeyRing keyRing, byte[] bytes) { - return sign(keyRing.getSignatureKeyPair().getPrivate(), bytes); + public static byte[] sign(KeyRing keyRing, byte[] message) { + return sign(keyRing.getSignatureKeyPair().getPrivate(), message); } public static byte[] sign(PrivateKey privateKey, String message) { return sign(privateKey, message.getBytes(Charsets.UTF_8)); } - public static byte[] sign(PrivateKey privateKey, byte[] bytes) { + public static byte[] sign(PrivateKey privateKey, byte[] message) { try { - return Sig.sign(privateKey, bytes); + return Sig.sign(privateKey, message); } catch (Exception e) { throw new IllegalArgumentException(e); } @@ -265,9 +265,9 @@ public class HavenoUtils { verifySignature(pubKeyRing, message.getBytes(Charsets.UTF_8), signature); } - public static void verifySignature(PubKeyRing pubKeyRing, byte[] bytes, byte[] signature) { + public static void verifySignature(PubKeyRing pubKeyRing, byte[] message, byte[] signature) { try { - boolean isValid = Sig.verify(pubKeyRing.getSignaturePubKey(), bytes, signature); + boolean isValid = Sig.verify(pubKeyRing.getSignaturePubKey(), message, signature); if (!isValid) throw new IllegalArgumentException("Signature verification failed."); } catch (CryptoException e) { throw new IllegalArgumentException(e); @@ -278,9 +278,9 @@ public class HavenoUtils { return isSignatureValid(pubKeyRing, message.getBytes(Charsets.UTF_8), signature); } - public static boolean isSignatureValid(PubKeyRing pubKeyRing, byte[] bytes, byte[] signature) { + public static boolean isSignatureValid(PubKeyRing pubKeyRing, byte[] message, byte[] signature) { try { - verifySignature(pubKeyRing, bytes, signature); + verifySignature(pubKeyRing, message, signature); return true; } catch (Exception e) { return false;