Move make_receipt to BaseTransactionExecutor

[onyb]

What is wrong?

#383 was recently merged, bringing a generic transaction executor to be used as mixin in different VM state classes, thereby allowing us to achieve better code reuse across forks.

A good candidate that can benefit from a similar refactoring is the BaseVMState.make_receipt method. This method is currently called inside BaseVMState.add_transaction(transaction, computation, block), though receipt generation has nothing to do with adding the transaction to a block. A better place to make the call is in BaseTransactionExecutor.execute_transaction. It therefore also makes logical sense to move BaseVMState.make_receipt to the BaseTransactionExecutor class, which has all the necessary information for receipt generation, i.e. vm state (via self), computation, and transaction.

How can it be fixed

1. Add make_receipt as an abstractmethod to evm.vm_state.BaseTransactionExecutor.
2. Move the call to self.make_receipt in the evm.vm_state.BaseVMState.add_transaction, to evm.vm_state.BaseTransactionExecutor.execute_transaction.
3. Implement make_receipt in evm.vm.forks.frontier.vm_state.FrontierTransactionExecutor.
4. Use proper inheritance in other forks, notably in ByzantiumVMState.

[pipermerriam]

So would execute_transaction now return a 2-tuple of computation, receipt?

[onyb]

So would execute_transaction now return a 2-tuple of computation, receipt?

I think this should be the preferred way of doing it. Alternately, we can call the add_receipt method in execute_transaction and modify this line to be: block.bloom_filter |= self.receipts[-1].bloom.

[pipermerriam]

I'm inclined to instead modify add_transaction to take the extra receipt parameter (along with having execute_transaction return the receipt.