Change txdb zap to use layoutp

[rozaydin]

This PR changes the txdb zap method, instead of retrieving records from layout.m (which iterates over all txes i assume), it retrieves from layout.p (pending txes) to prevent unnecessary looping over all txs.

My understanding might be wrong, I am not quite sure about layout.m if it's used to store all txes with a timestamp or not. (if there is some documentation about leveldb layout for hsd that would be awesome)
However if it makes sense, i would like to suggest the zap method to return (instead of just success: true) the list of txes it zapped. Which i can add to this PR if it makes sense ...

• p[hash] -> dummy (pending flag)
• m[time][hash] -> dummy (tx by time)

If you are on it, would be great to know what should be the relation between the mempool and zapped/abondaned txes

Thanks for the amazing software you are building ...

[pinheadmz]

Hey thanks for the contribution and for looking in to this. I'm not sure this is an improvement actually. The existing zap() function uses the m index in the DB which is transactions ordered by TIME, which is the only parameter accepted by the API call. The p index is ALL pending transactions ordered by txid (so, random). By passing a time value to the function, only transactions added to the DB within that window will be processed (not "all" txs).

I couldn't tell you why the "zap" function is constrained by TX age but I think instead of breaking it, if you want to ADD a "delete all pending txs" API using the method you started implementing here, that might make more sense for a pull request.

[pinheadmz]

I think things like this should go in a global gitignore on your own machine.

[rozaydin]

By passing a time value to the function, only transactions added to the DB within that window will be processed (not "all" txs)

I see, my bad missed the gte, lte params on getRangeHashes, My reasoning weakened ... (but pls see below)

/**
   * Get TX hashes by timestamp range.
   * @param {Number} acct
   * @param {Object} options
   * @param {Number} options.start - Start height.
   * @param {Number} options.end - End height.
   * @param {Number?} options.limit - Max number of records.
   * @param {Boolean?} options.reverse - Reverse order.
   * @returns {Promise} - Returns {@link Hash}[].
   */

  getRangeHashes(acct, options) {
    assert(typeof acct === 'number');

    if (acct !== -1)
      return this.getAccountRangeHashes(acct, options);

    const start = options.start || 0;
    const end = options.end || 0xffffffff;

    return this.bucket.keys({
      gte: layout.m.min(start),
      lte: layout.m.max(end),
      limit: options.limit,
      reverse: options.reverse,
      parse: (key) => {
        const [, hash] = layout.m.decode(key);
        return hash;
      }
    });
  }

but still the api docs for zap https://hsd-dev.org/api-docs/#zap-transactions states that:

but here we are retrieving txes (using layout.m) without checking if they are pending or not ? Or is there a guard that prevents the zap to remove non-pending txes ?

 async zap(acct, age) {
    assert((age >>> 0) === age);

    const now = util.now();

    const txs = await this.getRange(acct, {
      start: 0,
      end: now - age
    });

    const hashes = [];

    for (const wtx of txs) {
      if (wtx.height !== -1)
        continue;

      assert(now - wtx.mtime >= age);

      this.logger.debug('Zapping TX: %x (%d)',
        wtx.tx.hash(), this.wid);

      await this.remove(wtx.hash);

      hashes.push(wtx.hash);
    }

    return hashes;
  }

[pinheadmz]

Yes, the guard is what I mention here. What we could do is migrate the DB so we store all pending TXs by time, and then just use the p index. That might be a worthwhile optimization but requires a migration or adding a new index altogether. I guess the gains depend on how much time the user enters, and how many confirmed TXs they've broadcasted in that timespan.

[rozaydin]

Yeap there is indeed a guard, let me go over your feedback ... thanks

[pinheadmz]

I think if you continue working on this with the p index, you probably can remove this filter:

hsd/lib/wallet/txdb.js

Lines 3194 to 3195 in afe8b43

	if (wtx.height !== -1)
	continue;

[coveralls]

Pull Request Test Coverage Report for Build 1454650398

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.7%) to 62.361%

---

Totals
Change from base Build 1391687210:	0.7%
Covered Lines:	21238
Relevant Lines:	31825

---

💛 - Coveralls