Client: separate TxIndex from ReceiptsManager

[ScottyPoi]

TxIndex

• issue No TX Index without ReceiptsManager #3926

1. New TxIndex class
2. Extract TxIndex logic from ReceiptsManager
3. Optimize eth_getTransactionByHash by removing unnecessary receipt lookup
4. Optimize debug_getRawTransaction by removing unnecessary receipt lookup
5. Fix miner.spec.ts test
6. Add txIndex.spec.ts test
7. Test txIndex initialization in VMExecution
8. Add missing test coverage

---

TxIndex refers to a database mapping of a transaction hash to its blockhash and position (index) within the block transactions array.

This allows lookup of transaction data by hash without directly storing each transaction by hash in the DB.

e.g.:

getTransactionByHash(txHash) {
  const [blockHash, index] = txIndex.getIndex(txHash)
  const block = getBlock(blockHash)
  const transaction = block.transactions[index]
  return transaction
}

---

The current setup only included a TxIndex as part of ReceiptsManager, which only exists if --saveReceipts is set to true. While it is the case that some methods in ReceiptsManager require a TxIndex, the inverse is not true. A user may want to keep a TxIndex without needing to save receipts.

This PR extracts all of the indexing logic out of ReceiptsManager and moves it to a new class called TxIndex. TxIndex can exist independently of ReceiptsManager as a separate attribute of VMExecution. TxIndex will initialize as long as this.MetaDB exists, and will still be restricted by --txLookupLimit.

The method receiptsManager.getReceiptByTxHash has been changed to getReceiptsByTxHashIndex. This method is only called by RPC getTransaction methods, which now separately call TxIndex.getIndex.

---

eth_getTransactionByHash and debug_getRawTransaction

Decoupling receipts and txIndex allowed for optimizing eth_getTransactionByHash and debug_getRawTransaction. These methods had involved an extra DB lookup for the block receipts, which was never used and not part of looking up the transaction.

---

Test fix:

The only test for saveReceipts existed in miner.spec.ts, and was producing false positives.

The test was calling receiptsManager.getReceipts using transaction hashes instead of block hashes. Even though this should have failed, the getReceipts method returns an empty array when fails, which allowed the test's assert.isDefined to pass, even though the expected result was not retrieved.

This test was fixed, and updated to also test the transaction indexing.

[codecov]

Codecov Report

Attention: Patch coverage is 94.56522% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.39%. Comparing base (09a83bd) to head (592fd65).
Report is 1 commits behind head on master.

Additional details and impacted files

Flag	Coverage Δ
block	84.33% <ø> (ø)
blockchain	89.32% <ø> (ø)
client	67.58% <94.56%> (+0.02%)	⬆️
common	97.51% <ø> (ø)
devp2p	86.78% <ø> (ø)
evm	73.11% <ø> (ø)
mpt	89.74% <ø> (-0.26%)	⬇️
statemanager	69.06% <ø> (ø)
static	99.11% <ø> (ø)
tx	89.89% <ø> (ø)
util	89.19% <ø> (ø)
vm	55.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Dargon789]

Client: separate TxIndex from ReceiptsManager #4012

[acolytec3]

LGTM!

[holgerd77]

Really nice rework and optimization, another LGTM! 🙂👍