remove and repost offers if signing arbitrator unregistered

This commit is contained in:
woodser 2023-08-16 09:25:39 -04:00
parent b2b4706f14
commit f112760432
4 changed files with 61 additions and 37 deletions

View file

@ -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<NodeAddress> 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);
}

View file

@ -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} ";
}

View file

@ -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<OpenOffer> openOfferOptional = getOpenOfferById(request.offerId);
Optional<OpenOffer> 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<OpenOffer> 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() {

View file

@ -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;