UXTO's disappearing in Wallet cache

[Galuf1]

it's been long since we visit the transaction builder, fee optimizers, and wallet cache, and there is a couple stuff we need to fix before this milestone, starting from the fees. We are paying higher fees than the current LTC client in testnet. We need to make sure that the builder is correct, and the wallet is caching all the unspent outputs, because in certain scenarios they get lost from the cache and you have to refresh the outputs by sending a dust to that address.

[patrickdugan]

this may be relevant:

Lines 732 on TradeLayer.cpp

/**

• Fetches transaction inputs and adds them to the coins view cache.
• @param tx[in] The transaction to fetch inputs for

• @return True, if all inputs were successfully added to the cache */ static bool FillTxInputCache(const CTransaction& tx) { LOCK(cs_tx_cache); static unsigned int nCacheSize = gArgs.GetArg("-tltxcache", 500000);

  if (view.GetCacheSize() > nCacheSize) { PrintToLog("%s(): clearing cache before insertion [size=%d, hit=%d, miss=%d]\n", func, view.GetCacheSize(), nCacheHits, nCacheMiss); view.Flush(); }

  for (std::vector::const_iterator it = tx.vin.begin(); it != tx.vin.end(); ++it) { const CTxIn& txIn = *it; unsigned int nOut = txIn.prevout.n; if (view.HaveCoin(txIn.prevout)){ ++nCacheHits; continue; } else{ ++nCacheMiss; }

  CTransactionRef txPrev;
  uint256 hashBlock = uint256();
  
  if (!GetTransaction(txIn.prevout.hash, txPrev, Params().GetConsensus(), hashBlock, true)) {
     return false;
  }
  
  if (txPrev.get()->vout.size() <= nOut){
      return false;
  }
  
  Coin newCoin(txPrev.get()->vout[nOut], 0, tx.IsCoinBase());
  view.AddCoin(txIn.prevout, std::move(newCoin), false);

  }

  return true; }

[patrickdugan]

For comparative reference, here is the same function in the current Omnicore.cpp:

/**

• Fetches transaction inputs and adds them to the coins view cache.
• Note: cs_tx_cache should be locked, when adding and accessing inputs!
• @param tx[in] The transaction to fetch inputs for

• @return True, if all inputs were successfully added to the cache */ static bool FillTxInputCache(const CTransaction& tx, const std::shared_ptr<std::map<COutPoint, Coin>> removedCoins) { static unsigned int nCacheSize = gArgs.GetArg("-omnitxcache", 500000);

  if (view.GetCacheSize() > nCacheSize) { PrintToLog("%s(): clearing cache before insertion [size=%d, hit=%d, miss=%d]\n", func, view.GetCacheSize(), nCacheHits, nCacheMiss); view.Flush(); }

  for (std::vector::const_iterator it = tx.vin.begin(); it != tx.vin.end(); ++it) { const CTxIn& txIn = *it; unsigned int nOut = txIn.prevout.n; const Coin& coin = view.AccessCoin(txIn.prevout);

  if (!coin.IsSpent()) {
      ++nCacheHits;
      continue;
  } else {
      ++nCacheMiss;
  }
  
  CTransactionRef txPrev;
  uint256 hashBlock;
  Coin newcoin;
  if (removedCoins && removedCoins->find(txIn.prevout) != removedCoins->end()) {
      newcoin = removedCoins->find(txIn.prevout)->second;
  } else if (GetTransaction(txIn.prevout.hash, txPrev, Params().GetConsensus(), hashBlock)) {
      newcoin.out.scriptPubKey = txPrev->vout[nOut].scriptPubKey;
      newcoin.out.nValue = txPrev->vout[nOut].nValue;
      BlockMap::iterator bit = mapBlockIndex.find(hashBlock);
      newcoin.nHeight = bit != mapBlockIndex.end() ? bit->second->nHeight : 1;
  } else {
      return false;
  }
  
  view.AddCoin(txIn.prevout, std::move(newcoin), true);

  }

  return true; }

[philmb3487]

Would it have to do with

        if (removedCoins && removedCoins->find(txIn.prevout) != removedCoins->end()) {
            newcoin = removedCoins->find(txIn.prevout)->second;
        }
`
??

[patrickdugan]

There's a strong probability that is the fix they implemented.

We'll run some bots that do a decent tx throughput for a day in an A/B test and then adopt this code if that pans out, or otherwise, it seems like a "why not" situation regarding replacing the old code with this.

[santos177]

side note: This bug hasn't be present in last functional tests. So we should create specific unit/functional tests only to check what's going on with the utxo set. If utxo aren't dissapearing , we should close this issue.

[patrickdugan]

This is not consensus affecting so we could ship with what we have and adopt the altered code if it ever occurred in the field. There's also an argument for adopting the more recent Omni code as a best practices thing.

We can close the issue now for launch purposes and re-open later if the value of this code difference manifests in a re-emergence of the bug.

On Mon, Sep 14, 2020 at 7:50 PM Santos notifications@github.com wrote:

side note: This bug hasn't be present in last functional tests. So we should create specific unit/functional tests only to check what's going on with the utxo set. If utxo aren't dissapearing , we should close this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BlockPo/BlockPo-to-Tradelayer/issues/220#issuecomment-692359676, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS2CBJMNFBCZGDOZBMDUALSF2M2ZANCNFSM4LSYVJ4Q .

[philmb3487]

ok, leme know if/how to test and what's the plan then.

[patrickdugan]

We will run a simple trade spammer bot next week and see if it runs into a missing input error or other crash. Then we'll deploy more sophisticated bots for trade channels and orderbook contract trades to get to a confidence threshold on those features.

On Thu, Sep 17, 2020 at 8:02 AM Philippe Michaud-Boudreault < notifications@github.com> wrote:

ok, leme know if/how to test and what's the plan then.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BlockPo/BlockPo-to-Tradelayer/issues/220#issuecomment-694158564, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS2CBJTVQ2S46WEKGX6FFDSGHUE3ANCNFSM4LSYVJ4Q .

[santos177]

Closing for now this issue. If this bug still there, please welcome to re-open this ticket.

[santos177]

@patrickdugan , @sinetek : under testing on testnet now, patch branch is input_cache.

[philmb3487]

@santos177 awesome, thanks for report. giving a look.

[santos177]

@sinetek : that would be great!