Skip to content

Commit

Permalink
Fix API OfferInfo.isActivated setting for bsqswaps
Browse files Browse the repository at this point in the history 
I think this bug was introduced when deprecating GrpcOffersService
 .getMyOffer(id), in favor of using only getOffer(id) for 'my'
and 'available' offers.  This change explicitly sets the proto's
isActivated flag in the OfferInfo factory methods, and adds checks
to api offer test cases.

Based on branch `2-improve-grpc-exception-status-code-mapping`,
PR bisq-network#6088
  • Loading branch information
@ghubstan
ghubstan committed Mar 7, 2022
1 parent df2cac3 commit 1ba2b6c
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 17 deletions.
5 changes: 4 additions & 1 deletion apitest/src/test/java/bisq/apitest/method/offer/BsqSwapOfferTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import static java.lang.String.format;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static protobuf.OfferDirection.BUY;

Expand All @@ -45,7 +46,7 @@ public class BsqSwapOfferTest extends AbstractOfferTest {

@BeforeAll
public static void setUp() {
AbstractOfferTest.setUp();
AbstractOfferTest.setUp(false);
}

@BeforeEach
Expand Down Expand Up @@ -137,6 +138,7 @@ private void testGetMyBsqSwapOffer(OfferInfo bsqSwapOffer) {
numFetchAttempts++;
var fetchedBsqSwapOffer = aliceClient.getOffer(bsqSwapOffer.getId());
assertEquals(bsqSwapOffer.getId(), fetchedBsqSwapOffer.getId());
assertTrue(fetchedBsqSwapOffer.getIsActivated());
log.debug("Alice found her (my) new bsq swap offer on attempt # {}.", numFetchAttempts);
break;
} catch (Exception ex) {
Expand All @@ -159,6 +161,7 @@ private void testGetBsqSwapOffer(OfferInfo bsqSwapOffer) {
numFetchAttempts++;
var fetchedBsqSwapOffer = bobClient.getOffer(bsqSwapOffer.getId());
assertEquals(bsqSwapOffer.getId(), fetchedBsqSwapOffer.getId());
assertTrue(fetchedBsqSwapOffer.getIsActivated());
log.debug("Bob found new available bsq swap offer on attempt # {}.", numFetchAttempts);
break;
} catch (Exception ex) {
Expand Down
8 changes: 8 additions & 0 deletions apitest/src/test/java/bisq/apitest/method/offer/CreateBSQOffersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public void testCreateBuy1BTCFor20KBSQOffer() {
log.debug("Sell BSQ (Buy BTC) Offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -89,6 +90,7 @@ public void testCreateBuy1BTCFor20KBSQOffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -119,6 +121,7 @@ public void testCreateSell1BTCFor20KBSQOffer() {
log.debug("SELL 20K BSQ Offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -140,6 +143,7 @@ public void testCreateSell1BTCFor20KBSQOffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -170,6 +174,7 @@ public void testCreateBuyBTCWith1To2KBSQOffer() {
log.debug("BUY 1-2K BSQ Offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -191,6 +196,7 @@ public void testCreateBuyBTCWith1To2KBSQOffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -221,6 +227,7 @@ public void testCreateSellBTCFor5To10KBSQOffer() {
log.debug("SELL 5-10K BSQ Offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -242,6 +249,7 @@ public void testCreateSellBTCFor5To10KBSQOffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down
6 changes: 6 additions & 0 deletions apitest/src/test/java/bisq/apitest/method/offer/CreateOfferUsingFixedPriceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public void testCreateAUDBTCBuyOfferUsingFixedPrice16000() {
log.debug("Offer #1:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -79,6 +80,7 @@ public void testCreateAUDBTCBuyOfferUsingFixedPrice16000() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -109,6 +111,7 @@ public void testCreateUSDBTCBuyOfferUsingFixedPrice100001234() {
log.debug("Offer #2:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -128,6 +131,7 @@ public void testCreateUSDBTCBuyOfferUsingFixedPrice100001234() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -158,6 +162,7 @@ public void testCreateEURBTCSellOfferUsingFixedPrice95001234() {
log.debug("Offer #3:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -177,6 +182,7 @@ public void testCreateEURBTCSellOfferUsingFixedPrice95001234() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down
10 changes: 10 additions & 0 deletions apitest/src/test/java/bisq/apitest/method/offer/CreateOfferUsingMarketPriceMarginTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public void testCreateUSDBTCBuyOffer5PctPriceMargin() {
log.debug("Offer #1:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -94,6 +95,7 @@ public void testCreateUSDBTCBuyOffer5PctPriceMargin() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -126,6 +128,7 @@ public void testCreateNZDBTCBuyOfferMinus2PctPriceMargin() {
log.debug("Offer #2:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -143,6 +146,7 @@ public void testCreateNZDBTCBuyOfferMinus2PctPriceMargin() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -175,6 +179,7 @@ public void testCreateGBPBTCSellOfferMinus1Point5PctPriceMargin() {
log.debug("Offer #3:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -192,6 +197,7 @@ public void testCreateGBPBTCSellOfferMinus1Point5PctPriceMargin() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -224,6 +230,7 @@ public void testCreateBRLBTCSellOffer6Point55PctPriceMargin() {
log.debug("Offer #4:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -241,6 +248,7 @@ public void testCreateBRLBTCSellOffer6Point55PctPriceMargin() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -273,12 +281,14 @@ public void testCreateUSDBTCBuyOfferWithTriggerPrice() {
triggerPrice);
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

genBtcBlocksThenWait(1, 4000); // give time to add to offer book
newOffer = aliceClient.getOffer(newOffer.getId());
log.debug("Offer #5:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(triggerPrice, newOffer.getTriggerPrice());
}

Expand Down
8 changes: 8 additions & 0 deletions apitest/src/test/java/bisq/apitest/method/offer/CreateXMROffersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public void testCreateFixedPriceBuy1BTCFor200KXMROffer() {
log.debug("Sell XMR (Buy BTC) offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -90,6 +91,7 @@ public void testCreateFixedPriceBuy1BTCFor200KXMROffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -118,6 +120,7 @@ public void testCreateFixedPriceSell1BTCFor200KXMROffer() {
log.debug("Buy XMR (Sell BTC) offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -137,6 +140,7 @@ public void testCreateFixedPriceSell1BTCFor200KXMROffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertFalse(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -168,6 +172,7 @@ public void testCreatePriceMarginBasedBuy1BTCOfferWithTriggerPrice() {
log.debug("Pending Sell XMR (Buy BTC) offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -191,6 +196,7 @@ public void testCreatePriceMarginBasedBuy1BTCOfferWithTriggerPrice() {
log.debug("Available Sell XMR (Buy BTC) offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(BUY.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down Expand Up @@ -224,6 +230,7 @@ public void testCreatePriceMarginBasedSell1BTCOffer() {
log.debug("Buy XMR (Sell BTC) offer:\n{}", toOfferTable.apply(newOffer));
assertTrue(newOffer.getIsMyOffer());
assertTrue(newOffer.getIsMyPendingOffer());
assertFalse(newOffer.getIsActivated());

String newOfferId = newOffer.getId();
assertNotEquals("", newOfferId);
Expand All @@ -242,6 +249,7 @@ public void testCreatePriceMarginBasedSell1BTCOffer() {
newOffer = aliceClient.getOffer(newOfferId);
assertTrue(newOffer.getIsMyOffer());
assertFalse(newOffer.getIsMyPendingOffer());
assertTrue(newOffer.getIsActivated());
assertEquals(newOfferId, newOffer.getId());
assertEquals(SELL.name(), newOffer.getDirection());
assertTrue(newOffer.getUseMarketBasedPrice());
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/bisq/core/api/model/CanceledTradeInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import bisq.core.offer.OpenOffer;

import static bisq.core.api.model.ContractInfo.emptyContract;
import static bisq.core.api.model.OfferInfo.toMyOfferInfo;
import static bisq.core.api.model.OfferInfo.toMyInactiveOfferInfo;
import static bisq.core.offer.OpenOffer.State.CANCELED;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.capitalize;
Expand All @@ -37,7 +37,7 @@ public static TradeInfo toCanceledTradeInfo(OpenOffer myCanceledOpenOffer) {
throw new IllegalArgumentException(format("offer '%s' is not canceled", myCanceledOpenOffer.getId()));

Offer offer = myCanceledOpenOffer.getOffer();
OfferInfo offerInfo = toMyOfferInfo(offer);
OfferInfo offerInfo = toMyInactiveOfferInfo(offer);

return new TradeInfoV1Builder()
.withOffer(offerInfo)
Expand Down
21 changes: 14 additions & 7 deletions core/src/main/java/bisq/core/api/model/OfferInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,18 @@ public OfferInfo(OfferInfoBuilder builder) {
this.protocolVersion = builder.getProtocolVersion();
}

public static OfferInfo toMyOfferInfo(Offer offer) {
return getBuilder(offer, true).build();
public static OfferInfo toMyInactiveOfferInfo(Offer offer) {
return getBuilder(offer, true)
.withIsActivated(false)
.build();
}

public static OfferInfo toOfferInfo(Offer offer) {
// Assume the offer is not mine, but isMyOffer can be reset to true, i.e., when
// calling TradeInfo toTradeInfo(Trade trade, String role, boolean isMyOffer);
return getBuilder(offer, false).build();
return getBuilder(offer, false)
.withIsActivated(true)
.build();
}

public static OfferInfo toMyPendingOfferInfo(Offer myNewOffer) {
Expand All @@ -130,21 +134,24 @@ public static OfferInfo toMyPendingOfferInfo(Offer myNewOffer) {
// column that will show a PENDING value when this.isMyPendingOffer = true.
return getBuilder(myNewOffer, true)
.withIsMyPendingOffer(true)
.withIsActivated(false)
.build();
}

public static OfferInfo toMyOfferInfo(OpenOffer openOffer) {
// An OpenOffer is always my offer.
var currencyCode = openOffer.getOffer().getCurrencyCode();
Optional<Price> optionalTriggerPrice = openOffer.getTriggerPrice() > 0
var offer = openOffer.getOffer();
var currencyCode = offer.getCurrencyCode();
var isActivated = !openOffer.isDeactivated();
Optional<Price> optionalTriggerPrice = (!offer.isBsqSwapOffer() && openOffer.getTriggerPrice() > 0)
? Optional.of(Price.valueOf(currencyCode, openOffer.getTriggerPrice()))
: Optional.empty();
var preciseTriggerPrice = optionalTriggerPrice
.map(value -> reformatMarketPrice(value.toPlainString(), currencyCode))
.orElse("0");
return getBuilder(openOffer.getOffer(), true)
return getBuilder(offer, true)
.withTriggerPrice(preciseTriggerPrice)
.withIsActivated(!openOffer.isDeactivated())
.withIsActivated(isActivated)
.build();
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/bisq/core/api/model/TradeInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import lombok.Getter;

import static bisq.core.api.model.BsqSwapTradeInfo.toBsqSwapTradeInfo;
import static bisq.core.api.model.OfferInfo.toMyOfferInfo;
import static bisq.core.api.model.OfferInfo.toMyInactiveOfferInfo;
import static bisq.core.api.model.OfferInfo.toOfferInfo;
import static bisq.core.api.model.PaymentAccountPayloadInfo.toPaymentAccountPayloadInfo;
import static bisq.core.offer.OfferDirection.BUY;
Expand All @@ -50,7 +50,7 @@ public class TradeInfo implements Payload {
// view and interact with trades.

private static final BiFunction<TradeModel, Boolean, OfferInfo> toOfferInfo = (tradeModel, isMyOffer) ->
isMyOffer ? toMyOfferInfo(tradeModel.getOffer()) : toOfferInfo(tradeModel.getOffer());
isMyOffer ? toMyInactiveOfferInfo(tradeModel.getOffer()) : toOfferInfo(tradeModel.getOffer());

private static final Function<TradeModel, String> toPeerNodeAddress = (tradeModel) ->
tradeModel.getTradingPeerNodeAddress() == null
Expand Down
Loading

0 comments on commit 1ba2b6c

Please sign in to comment.