Convert apply_message to a classmethod

[carver]

What was wrong?

Computations are unnecessarily getting created and then thrown away. See:

Peteris Erins @Pet3ris Apr 07 08:58
Hi There! I'm noticing that when a contract CALL is issued by py-evm (internal CALL), two new computations are created immediately. Anyone know why is that the case?
49
Op: GAS
[384, 2, 672, 33, 33, 768, 33, 672, 0, 4]
49
Op: CALL
[384, 2, 672, 33, 33, 768, 33, 672, 0, 4, 113796]
New computation started
<eth.vm.forks.constantinople.computation.ConstantinopleComputation object at 0x10d237bd0>
New computation started
<eth.vm.forks.constantinople.computation.ConstantinopleComputation object at 0x10d32c2d0>
50
Op: PUSH2
[]

Peteris Erins @Pet3ris Apr 07 09:07
moreover they both seem to be using the same message computation.msg

Peteris Erins @Pet3ris Apr 07 09:22
one of the computations terminates immediately which suggests to me that it's calling a pre-compiled contract, but the other one is not terminated. By termination I mean that it's apply_computation function returns

Eth-Gitter-Bridge @Eth-Gitter-Bridge Apr 07 12:30
<carver> > I've tried to deploy contract without EthereumTester and web3.py. Is this possible (with only py-evm)?

It's easy to add a transaction in py-evm, but it's not easy to build that transaction to invoke a contract. Those transaction-building tools are mostly in web3.py. They help you build the data that goes into a transaction and then sign it. @ArtObr

<carver> @Pet3ris Hm, the double-computation thing doesn't sound familiar. Calling into a precompile doesn't sound right, I don't think computations are created at all for them: https://github.com/ethereum/py-evm/blob/086e0704a8d2da5f8f0a64d708d8830d1015895d/eth/vm/computation.py#L531-L534

Peteris Erins @Pet3ris 00:31
@Eth-Gitter-Bridge that's very helfpul re: precomputed contracts. Maybe this is just an internal call. Effectively what I'm trying to do is to keep track of the current computation by maintaining a stack of computations. When BaseComputation.__init__ is called, I add to the stack and when a BaseComputation.apply_computation is finished, I remove the top computation from the stack.

But I'm observing the following:

BaseComputation.__init__ called twice if the current Opcode is a CALL (and both times with the same message), only one of those computations then has a state that updates (_stack, etc.)
over time, there are more __init__ calls than there are apply_computation calls (does this mean __init__ is called twice for some subclasses of BaseComputation?
Any thoughts why that could be happening? Am I still monkey patching the right functions to track computations?


Peteris Erins @Pet3ris 01:50
Yeah, I can see how the logger would be an appealing place to hook into apply_computation, because the opcodes are dispersed, there's not a common path to inject in a single place. So yeah, I guess my next approach would be to patch Computationwith a custom apply_computation.

Just for context - I'm following your suggestion in terms of managing computations (and it is working well in general) but I couldn't use apply_computation for adds because it kicked in too early so needed to go exactly to the point where computation is constructed.


Eth-Gitter-Bridge @Eth-Gitter-Bridge 12:07
<carver> Hm, not sure why. It's plausible the double-init is some kind of bug (in the form of extra useless work that's happening). My next step would probably be a breakpoint, or a logger with the calling stack's info, like a self.logger.debug("BaseComputation.__init__ was called", stack_info=True)

Peteris Erins @Pet3ris 12:37
gotcha - will try that!

Peteris Erins @Pet3ris 14:25
interestingly both originate from the CALL opcode
the stack traces look absolutely identical
File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/chains/base.py", line 899, in apply_transaction
    receipt, computation = vm.apply_transaction(base_block.header, transaction)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/base.py", line 468, in apply_transaction
    computation = self.state.apply_transaction(transaction)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/state.py", line 323, in apply_transaction
    return self.execute_transaction(transaction)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/forks/frontier/state.py", line 200, in execute_transaction
    return executor(transaction)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/state.py", line 372, in __call__
    computation = self.build_computation(message, valid_transaction)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/forks/frontier/state.py", line 138, in build_computation
    transaction_context).apply_message()
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/forks/frontier/computation.py", line 76, in apply_message
    self.transaction_context,
  File "/Users/p/Dev/auditless/auditless-debugger/api/src/endpoints.py", line 399, in apply_computation
    computation = old_apply_computation(cls, state, message, transaction_context)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/computation.py", line 703, in apply_computation
    opcode_fn(computation=computation)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/logic/call.py", line 137, in __call__
    child_computation = computation.apply_child_computation(child_msg)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/computation.py", line 505, in apply_child_computation
    child_computation = self.generate_child_computation(child_msg)
  File "/Users/p/.local/share/virtualenvs/api-QuiCHkzb/lib/python3.7/site-packages/eth/vm/computation.py", line 520, in generate_child_computation
    self.transaction_context,
  File "/Users/p/Dev/auditless/auditless-debugger/api/src/endpoints.py", line 418, in new__init__
    logging.warning("Base Computation init was called.", stack_info=True)

Eth-Gitter-Bridge @Eth-Gitter-Bridge 17:10
<carver> Huh, interesting. So that maybe/probably means it's not a "extra useless work" bug. Any more information about the types of computations that are never updating their _stack and other variables that you're expecting? Is it the first of two sub-calls made by contracts? Is it contract creation calls? I guess I would move forward assuming that these are real calls, unless it becomes more obvious that something is broken.

Peteris Erins @Pet3ris 01:29
weirdly there is only one CALL
statement after which 2 computations are created, nearly identical, and only one of them goes on to update their state
the reason I suspect something is broken is that the computations don't end up being "closed", e.g., I have more computations open than closed even for terminated transactions so clearly the number of BaseComputation.init statements exceeds the number of apply_computation statements

Peteris Erins @Pet3ris 08:25
I've done some in-depth debugging and I think I know why there are 2 computations originating during the CALL:

1) one is the child computation that is immediately triggered by the call and generate_child_computation

https://github.com/ethereum/py-evm/blob/bc476b21928b5f732ce980da9cc8fb9e4fbd9f3a/eth/vm/computation.py#L366

2) this child computation then calls apply_message on itself

3) apply_message calls apply_computation

4) apply_computation then creates another new computation with the same parameters.

This bit is probably the confusing bit for me. We create a computation only to call apply_computation on it which then recreates the computation and runs it again? It does seem like this is not a bug, but intentional and I can definitely work around it :).


Peteris Erins @Pet3ris 09:55
managed to make a workaround and it's all good :)
thanks for the debugging tips!

How was it fixed?

A computation was being created here:

py-evm/eth/vm/computation.py

Lines 372 to 376 in 086e070

	child_computation = self.__class__(
	self.state,
	child_msg,
	self.transaction_context,
	).apply_create_message()

It's only purpose, as far as I can tell, was to carry the state, message, and transaction context to be used when later creating the actual computation that will be returned.

Instead, this PR makes apply_message() and apply_create_message() into class methods, and passes in the couple of variables they need to create the computation within.

To-Do

• Clean up commit history

• Add entry to the release notes

Cute Animal Picture

[pipermerriam]

Interesting find.

[carver]

Interesting find.

Yeah, nothing strictly broken, but I think it's a nice cleanup, and has some side-benefits for monkey-patching hackers.

[cburgdorf]

Nice cleanup 👍

[cburgdorf]

Great to see some additional docs here!