From 7bcb0ec5f1281742d18e639eaf0d49d135119eae Mon Sep 17 00:00:00 2001 From: JoeGruff Date: Fri, 22 Apr 2022 15:52:26 +0900 Subject: [PATCH] client/trade: Return counterPartyConfirms error. Because the return data cannot all be trusted if the method has an error, return that error up to the caller rather than just logging and returning with possibly false values. For swaps using contracts it is ok for the swap to be status none, in which case we cannot check the lock time or confirmations on chain yet. Do not log an error in this case. --- client/core/trade.go | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/client/core/trade.go b/client/core/trade.go index 0f378f0147..ebecc9f789 100644 --- a/client/core/trade.go +++ b/client/core/trade.go @@ -842,15 +842,18 @@ func (t *trackedTrade) processCancelMatch(msgMatch *msgjson.Match) error { // // This method accesses match fields and MUST be called with the trackedTrade // mutex lock held for reads. -func (t *trackedTrade) counterPartyConfirms(ctx context.Context, match *matchTracker) (have, needed uint32, changed, spent, expired bool) { +func (t *trackedTrade) counterPartyConfirms(ctx context.Context, match *matchTracker) (have, needed uint32, changed, spent, expired bool, err error) { + fail := func(err error) (uint32, uint32, bool, bool, bool, error) { + return 0, 0, false, false, false, err + } + // Counter-party's swap is the "to" asset. needed = t.wallets.toAsset.SwapConf // Check the confirmations on the counter-party's swap. If counterSwap is // not set, we shouldn't be here, but catch this just in case. if match.counterSwap == nil { - t.dc.log.Warnf("counterPartyConfirms: No AuditInfo available to check!") - return + return fail(errors.New("counterPartyConfirms: No AuditInfo available to check!")) } wallet := t.wallets.toWallet @@ -858,22 +861,15 @@ func (t *trackedTrade) counterPartyConfirms(ctx context.Context, match *matchTra _, lockTime, err := wallet.LocktimeExpired(match.MetaData.Proof.CounterContract) if err != nil { - t.dc.log.Errorf("Error checking if locktime has expired on taker's contract on order %s, "+ - "match %s: %v", t.ID(), match, err) + return fail(fmt.Errorf("error checking if locktime has expired on taker's contract on order %s, "+ + "match %s: %w", t.ID(), match, err)) } expired = time.Until(lockTime) < 0 // not necessarily refundable, but can be at any moment have, spent, err = wallet.SwapConfirmations(ctx, coin.ID(), match.MetaData.Proof.CounterContract, match.MetaData.Stamp) if err != nil { - if !errors.Is(err, asset.ErrSwapNotInitiated) { - // No need to log an error if swap not initiated as this - // is expected for newly made swaps involving contracts. - t.dc.log.Errorf("Failed to get confirmations of the counter-party's swap %s (%s) "+ - "for match %s, order %v: %v", - coin, t.wallets.toAsset.Symbol, match, t.UID(), err) - } - return + return fail(err) } // Log the pending swap status at new heights only. @@ -1076,7 +1072,16 @@ func (t *trackedTrade) isSwappable(ctx context.Context, match *matchTracker) boo coinIDString(toAssetID, match.MetaData.Proof.MakerSwap), unbip(toAssetID)) // If the maker is the counterparty, we can determine swappability // based on the confirmations. - confs, req, changed, spent, expired := t.counterPartyConfirms(ctx, match) + confs, req, changed, spent, expired, err := t.counterPartyConfirms(ctx, match) + if err != nil { + if !errors.Is(err, asset.ErrSwapNotInitiated) { + // We cannot get the swap data yet but there is no need + // to log an error if swap not initiated as this is + // expected for newly made swaps involving contracts. + t.dc.log.Errorf("isSwappable: unable to get counter party confirms: %v", err) + } + return false + } if spent { t.dc.log.Errorf("Counter-party's swap is spent before we could broadcast our own") match.MetaData.Proof.SelfRevoked = true @@ -1163,7 +1168,16 @@ func (t *trackedTrade) isRedeemable(ctx context.Context, match *matchTracker) bo case order.TakerSwapCast: if match.Side == order.Maker { // Check the confirmations on the taker's swap. - confs, req, changed, spent, expired := t.counterPartyConfirms(ctx, match) + confs, req, changed, spent, expired, err := t.counterPartyConfirms(ctx, match) + if err != nil { + if !errors.Is(err, asset.ErrSwapNotInitiated) { + // We cannot get the swap data yet but there is no need + // to log an error if swap not initiated as this is + // expected for newly made swaps involving contracts. + t.dc.log.Errorf("isRedeemable: unable to get counter party confirms: %v", err) + } + return false + } if spent { t.dc.log.Errorf("Order %s, match %s counter-party's swap is spent before we could redeem", t.ID(), match) match.MetaData.Proof.SelfRevoked = true