Switch _builder -> _decoder in ChainDB, plus tests

Add tests for getting a transaction receipt, since that code path was
not exercised in testing, and caused a fatal bug when using Trinity.

Note that ChainDB uses the keyword receipt_decoder instead of
receipt_builder everywhere (and the same for transaction_decoder). At
the DB level, the logic only cares to decode from bytestrings (as
opposed to the p2p layer, which always needs to be rlp-encodable, even
if it's an opaque bytestring). Since the database only uses the "decode
from bytestring" API, in can use the "tighter" ReceiptDecoderAPI instead
of the broader ReceiptBuilderAPI. Enforcing that tighter usage seems to
make sense.

Author: carver

eth/abc.py

@@ -1078,7 +1078,7 @@ def add_transaction(self,
     def get_block_transactions(
             self,
             block_header: BlockHeaderAPI,
-            transaction_builder: Type[TransactionBuilderAPI]) -> Tuple[SignedTransactionAPI, ...]:
+            transaction_decoder: Type[TransactionDecoderAPI]) -> Tuple[SignedTransactionAPI, ...]:
         """
         Return an iterable of transactions for the block speficied by the
         given block header.
@@ -1096,7 +1096,7 @@ def get_block_transaction_hashes(self, block_header: BlockHeaderAPI) -> Tuple[Ha
     def get_receipt_by_index(self,
                              block_number: BlockNumber,
                              receipt_index: int,
-                             receipt_builder: Type[ReceiptBuilderAPI]) -> ReceiptAPI:
+                             receipt_decoder: Type[ReceiptDecoderAPI]) -> ReceiptAPI:
         """
         Return the receipt of the transaction at specified index
         for the block header obtained by the specified block number
@@ -1106,7 +1106,7 @@ def get_receipt_by_index(self,
     @abstractmethod
     def get_receipts(self,
                      header: BlockHeaderAPI,
-                     receipt_class: Type[ReceiptBuilderAPI]) -> Tuple[ReceiptAPI, ...]:
+                     receipt_decoder: Type[ReceiptDecoderAPI]) -> Tuple[ReceiptAPI, ...]:
         """
         Return a tuple of receipts for the block specified by the given
         block header.
@@ -1118,7 +1118,7 @@ def get_transaction_by_index(
             self,
             block_number: BlockNumber,
             transaction_index: int,
-            transaction_builder: Type[TransactionBuilderAPI]) -> SignedTransactionAPI:
+            transaction_decoder: Type[TransactionDecoderAPI]) -> SignedTransactionAPI:
         """
         Return the transaction at the specified `transaction_index` from the
         block specified by `block_number` from the canonical chain.

eth/chains/base.py

@@ -427,9 +427,9 @@ def get_transaction_receipt_by_index(self,
         vm = self.get_vm_class_for_block_number(block_number)
 
         receipt = self.chaindb.get_receipt_by_index(
-            block_number=block_number,
-            receipt_index=index,
-            receipt_builder=vm.get_receipt_builder(),
+            block_number,
+            index,
+            vm.get_receipt_builder(),
         )
 
         return receipt

eth/db/chain.py

@@ -31,7 +31,7 @@
     ReceiptAPI,
     ReceiptDecoderAPI,
     SignedTransactionAPI,
-    TransactionBuilderAPI,
+    TransactionDecoderAPI,
 )
 from eth.constants import (
     EMPTY_UNCLE_HASH,
@@ -307,8 +307,8 @@ def add_transaction(self,
     def get_block_transactions(
             self,
             header: BlockHeaderAPI,
-            transaction_builder: Type[TransactionBuilderAPI]) -> Tuple[SignedTransactionAPI, ...]:
-        return self._get_block_transactions(header.transaction_root, transaction_builder)
+            transaction_decoder: Type[TransactionDecoderAPI]) -> Tuple[SignedTransactionAPI, ...]:
+        return self._get_block_transactions(header.transaction_root, transaction_decoder)
 
     def get_block_transaction_hashes(self, block_header: BlockHeaderAPI) -> Tuple[Hash32, ...]:
         """
@@ -347,7 +347,7 @@ def get_transaction_by_index(
             self,
             block_number: BlockNumber,
             transaction_index: int,
-            transaction_builder: Type[TransactionBuilderAPI]) -> SignedTransactionAPI:
+            transaction_decoder: Type[TransactionDecoderAPI]) -> SignedTransactionAPI:
         try:
             block_header = self.get_canonical_block_header_by_number(block_number)
         except HeaderNotFound:
@@ -356,7 +356,7 @@ def get_transaction_by_index(
         encoded_index = rlp.encode(transaction_index)
         encoded_transaction = transaction_db[encoded_index]
         if encoded_transaction != b'':
-            return transaction_builder.decode(encoded_transaction)
+            return transaction_decoder.decode(encoded_transaction)
         else:
             raise TransactionNotFound(
                 f"No transaction is at index {transaction_index} of block {block_number}"
@@ -413,12 +413,12 @@ def _get_block_transaction_data(db: DatabaseAPI, transaction_root: Hash32) -> It
     def _get_block_transactions(
             self,
             transaction_root: Hash32,
-            transaction_builder: Type[TransactionBuilderAPI]) -> Iterable[SignedTransactionAPI]:
+            transaction_decoder: Type[TransactionDecoderAPI]) -> Iterable[SignedTransactionAPI]:
         """
         Memoizable version of `get_block_transactions`
         """
         for encoded_transaction in self._get_block_transaction_data(self.db, transaction_root):
-            yield transaction_builder.decode(encoded_transaction)
+            yield transaction_decoder.decode(encoded_transaction)
 
     @staticmethod
     def _remove_transaction_from_canonical_chain(db: DatabaseAPI, transaction_hash: Hash32) -> None:

newsfragments/1994.bugfix.rst

@@ -0,0 +1,3 @@
+Fix a crash in :meth:`eth.chains.base.Chain.get_transaction_receipt` and
+:meth:`eth.chains.base.Chain.get_transaction_receipt_by_index` that resulted in this exception:
+``TypeError: get_receipt_by_index() got an unexpected keyword argument 'receipt_builder'``

tests/core/chain-object/test_chain.py

@@ -158,3 +158,45 @@ def test_mainnet_genesis_hash():
 def test_ropsten_genesis_hash():
     assert ROPSTEN_GENESIS_HEADER.hash == decode_hex(
         '0x41941023680923e0fe4d74a34bdac8141f2540e3ae90623718e47d66d1ca4a2d')
+
+
+def test_get_canonical_transaction_by_index(chain, tx):
+    if hasattr(chain, 'apply_transaction'):
+        # working on a Mining chain which can directly apply transactions
+        new_block, _, computation = chain.apply_transaction(tx)
+        computation.raise_if_error()
+    else:
+        # working on a non-mining chain, so we have to build the block to apply manually
+        new_block, receipts, computations = chain.build_block_with_transactions([tx])
+        assert len(computations) == 1
+        computations[0].raise_if_error()
+
+    block_import_result = chain.import_block(new_block)
+    block = block_import_result.imported_block
+
+    assert block.transactions == (tx,)
+    # transaction at block 1, idx 0
+    assert chain.get_canonical_transaction_index(tx.hash) == (1, 0)
+    assert chain.get_canonical_transaction_by_index(1, 0) == tx
+
+
+def test_get_transaction_receipt(chain, tx):
+    if hasattr(chain, 'apply_transaction'):
+        # working on a Mining chain which can directly apply transactions
+        new_block, expected_receipt, computation = chain.apply_transaction(tx)
+        computation.raise_if_error()
+    else:
+        # working on a non-mining chain, so we have to build the block to apply manually
+        new_block, receipts, computations = chain.build_block_with_transactions([tx])
+        assert len(computations) == 1
+        computations[0].raise_if_error()
+        expected_receipt = receipts[0]
+
+    block_import_result = chain.import_block(new_block)
+    block = block_import_result.imported_block
+
+    assert block.transactions == (tx,)
+    # receipt at block 1, idx 0
+    assert chain.get_canonical_transaction_index(tx.hash) == (1, 0)
+    assert chain.get_transaction_receipt_by_index(1, 0) == expected_receipt
+    assert chain.get_transaction_receipt(tx.hash) == expected_receipt