Skip to content

Commit

Permalink
client/asset/dcr: coin not found not an error for spv
Browse files Browse the repository at this point in the history
Do not return asset.CoinNotFoundError from SwapConfirmations with an
SPV wallet since this is normal for mempool transactions in SPV.

For a non-SPV wallet, if the swap appears spent but it cannot be
located in a block with a cfilters scan, this will return
asset.CoinNotFoundError. For SPV wallets, it is not an error if the
transaction cannot be located SPV wallets cannot see non-wallet
transactions until they are mined.

Fix a few logs where arguments were orderd strangely.

Also silence a warning that happens each time a transaction is
found in a block while looking for a spending transaction immediately
afterward.
  • Loading branch information
chappjc committed Jan 18, 2022
1 parent 5149ee2 commit 013abc2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 20 deletions.
2 changes: 2 additions & 0 deletions client/asset/btc/spv.go
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,8 @@ func (w *spvWallet) swapConfirmations(txHash *chainhash.Hash, vout uint32, pkScr
if assumedMempool {
w.log.Tracef("swapConfirmations - scanFilters did not find %v:%d, assuming in mempool.",
txHash, vout)
// NOT asset.CoinNotFoundError since this is normal for mempool
// transactions with an SPV wallet.
return 0, false, nil
}
return 0, false, fmt.Errorf("output %s:%v not found with search parameters startTime = %s, pkScript = %x",
Expand Down
26 changes: 18 additions & 8 deletions client/asset/dcr/dcr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1599,14 +1599,14 @@ func (dcr *ExchangeWallet) lookupTxOutput(ctx context.Context, txHash *chainhash
output, err := dcr.wallet.UnspentOutput(ctx, txHash, vout, wire.TxTreeUnknown)
if err == nil {
return output.TxOut, output.Confirmations, false, nil
} else if err != asset.CoinNotFoundError {
} else if !errors.Is(err, asset.CoinNotFoundError) {
return nil, 0, false, err
}

// Check wallet transactions.
tx, err := dcr.wallet.GetTransaction(ctx, txHash)
if err != nil {
return nil, 0, false, err
return nil, 0, false, err // asset.CoinNotFoundError if not found
}
msgTx, err := msgTxFromHex(tx.Hex)
if err != nil {
Expand Down Expand Up @@ -2275,8 +2275,10 @@ func (dcr *ExchangeWallet) ValidateSecret(secret, secretHash []byte) bool {
// the specified swap. The contract and matchTime are provided so that wallets
// may search for the coin using light filters.
//
// If the swap was not funded by this wallet, and it is already spent, this
// method may return asset.CoinNotFoundError. Compare dcr.externalTxOut.
// For a non-SPV wallet, if the swap appears spent but it cannot be located in a
// block with a cfilters scan, this will return asset.CoinNotFoundError. For SPV
// wallets, it is not an error if the transaction cannot be located SPV wallets
// cannot see non-wallet transactions until they are mined.
//
// If the coin is located, but recognized as spent, no error is returned.
func (dcr *ExchangeWallet) SwapConfirmations(ctx context.Context, coinID, contract dex.Bytes, matchTime time.Time) (confs uint32, spent bool, err error) {
Expand All @@ -2289,7 +2291,7 @@ func (dcr *ExchangeWallet) SwapConfirmations(ctx context.Context, coinID, contra
_, confs, spent, err = dcr.lookupTxOutput(ctx, txHash, vout)
if err == nil {
return confs, spent, nil
} else if err != asset.CoinNotFoundError {
} else if !errors.Is(err, asset.CoinNotFoundError) {
return 0, false, err
}

Expand All @@ -2300,9 +2302,17 @@ func (dcr *ExchangeWallet) SwapConfirmations(ctx context.Context, coinID, contra
}
_, p2shScript := scriptAddr.PaymentScript()

// Find the contract and it's spend status using block filters.
// Find the contract and its spend status using block filters.
dcr.log.Debugf("Contract output %s:%d NOT yet found, will attempt finding it with block filters.", txHash, vout)
return dcr.lookupTxOutWithBlockFilters(ctx, newOutPoint(txHash, vout), p2shScript, matchTime)
confs, spent, err = dcr.lookupTxOutWithBlockFilters(ctx, newOutPoint(txHash, vout), p2shScript, matchTime)
// Don't trouble the caller if we're using an SPV wallet and the transaction
// cannot be located.
if errors.Is(err, asset.CoinNotFoundError) && dcr.wallet.SpvMode() {
dcr.log.Debugf("SwapConfirmations - cfilters scan did not find %v:%d. "+
"Assuming in mempool.", txHash, vout)
err = nil
}
return confs, spent, err
}

// RegFeeConfirmations gets the number of confirmations for the specified
Expand Down Expand Up @@ -2962,7 +2972,7 @@ func (dcr *ExchangeWallet) isMainchainBlock(ctx context.Context, block *block) (
}
nextBlockHash, err := chainhash.NewHashFromStr(blockHeader.NextHash)
if err != nil {
return false, fmt.Errorf("block %s has invalid nexthash value %s: %v",
return false, fmt.Errorf("block %s has invalid nexthash value %s: %w",
block.hash, blockHeader.NextHash, err)
}
nextBlockHeader, err := dcr.wallet.GetBlockHeaderVerbose(ctx, nextBlockHash)
Expand Down
29 changes: 17 additions & 12 deletions client/asset/dcr/externaltx.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ func (dcr *ExchangeWallet) lookupTxOutWithBlockFilters(ctx context.Context, op o

output, outputBlock, err := dcr.externalTxOutput(ctx, op, pkScript, earliestTxTime)
if err != nil {
return 0, false, err
return 0, false, err // may be asset.CoinNotFoundError
}

spent, err := dcr.isOutputSpent(ctx, output)
if err != nil {
return 0, false, fmt.Errorf("error checking if output %s is spent: %v", op, err)
return 0, false, fmt.Errorf("error checking if output %s is spent: %w", op, err)
}

// Get the current tip height to calculate confirmations.
Expand All @@ -82,7 +82,7 @@ func (dcr *ExchangeWallet) externalTxOutput(ctx context.Context, op outPoint, pk
tx := dcr.externalTxCache[op.txHash]
if tx == nil {
tx = &externalTx{hash: &op.txHash}
dcr.externalTxCache[op.txHash] = tx
dcr.externalTxCache[op.txHash] = tx // never deleted (TODO)
}
dcr.externalTxMtx.Unlock()

Expand All @@ -98,14 +98,14 @@ func (dcr *ExchangeWallet) externalTxOutput(ctx context.Context, op outPoint, pk
// First check if the tx block is cached.
txBlock, err := dcr.txBlockFromCache(ctx, tx)
if err != nil {
return nil, nil, fmt.Errorf("error checking if tx %s is known to be mined: %v", tx.hash, err)
return nil, nil, fmt.Errorf("error checking if tx %s is known to be mined: %w", tx.hash, err)
}

// Scan block filters to find the tx block if it is yet unknown.
if txBlock == nil {
txBlock, err = dcr.scanFiltersForTxBlock(ctx, tx, [][]byte{pkScript}, earliestTxTime)
if err != nil {
return nil, nil, fmt.Errorf("error checking if tx %s is mined: %v", tx.hash, err)
return nil, nil, fmt.Errorf("error checking if tx %s is mined: %w", tx.hash, err)
}
if txBlock == nil {
return nil, nil, asset.CoinNotFoundError
Expand Down Expand Up @@ -171,11 +171,11 @@ func (dcr *ExchangeWallet) scanFiltersForTxBlock(ctx context.Context, tx *extern
// to block just before earliestTxTime.
currentTip := dcr.cachedBestBlock()
if lastScannedBlock == nil {
dcr.log.Debugf("Searching for tx %s in blocks between block %d (%s) to the block just before %s.",
dcr.log.Debugf("Searching for tx %s in blocks between best block %d (%s) and the block just before %s.",
tx.hash, currentTip.height, currentTip.hash, earliestTxTime)
} else if lastScannedBlock.height < currentTip.height {
dcr.log.Debugf("Searching for tx %s in blocks %d (%s) to %d (%s).", tx.hash,
currentTip.height, currentTip.hash, lastScannedBlock.height, lastScannedBlock.hash)
lastScannedBlock.height, lastScannedBlock.hash, currentTip.height, currentTip.hash)
} else {
if lastScannedBlock.height > currentTip.height {
dcr.log.Warnf("Previous cfilters look up for tx %s stopped at block %d but current tip is %d?",
Expand All @@ -192,7 +192,7 @@ func (dcr *ExchangeWallet) scanFiltersForTxBlock(ctx context.Context, tx *extern
scanCompletedWithoutResults := func() (*block, error) {
tx.lastScannedBlock = currentTip.hash
dcr.log.Debugf("Tx %s NOT found in blocks %d (%s) to %d (%s).", tx.hash,
currentTip.height, currentTip.hash, iHeight, iHash)
iHeight, iHash, currentTip.height, currentTip.hash)
return nil, nil
}

Expand Down Expand Up @@ -272,8 +272,8 @@ func (dcr *ExchangeWallet) findTxInBlock(ctx context.Context, txHash *chainhash.
return nil, nil, fmt.Errorf("invalid hex for tx %s: %v", txHash, err)
}

// We have the txs in this block, check if any them spends an output
// from the original tx.
// We have the txs in this block, check if any of them spends an output from
// the original tx.
outputSpenders := make([]*outputSpenderFinder, len(msgTx.TxOut))
for i, txOut := range msgTx.TxOut {
outputSpenders[i] = &outputSpenderFinder{
Expand All @@ -286,7 +286,7 @@ func (dcr *ExchangeWallet) findTxInBlock(ctx context.Context, txHash *chainhash.
for t := range blockTxs {
blkTx := &blockTxs[t]
if blkTx.Txid == txHash.String() {
continue // oriignal tx, ignore
continue // original tx, ignore
}
for i := range blkTx.Vin {
input := &blkTx.Vin[i]
Expand Down Expand Up @@ -340,7 +340,12 @@ func (dcr *ExchangeWallet) isOutputSpent(ctx context.Context, output *outputSpen

bestBlock := dcr.cachedBestBlock()
if nextScanHeight >= bestBlock.height {
if nextScanHeight > bestBlock.height {
// When a transaction is initially found in a block, that block is also
// scanned for spending transactions (see findTxInBlock) as an
// optimization since the block was pulled. As such, it is normal for
// the lastScannedHeight for newly located transactions to be the best
// block, so there is no need to warn, only return.
if nextScanHeight > bestBlock.height+1 {
dcr.log.Warnf("Attempted to look for output spender in block %d but current tip is %d!",
nextScanHeight, bestBlock.height)
}
Expand Down

0 comments on commit 013abc2

Please sign in to comment.