Refactor current VM computations to be message computations #2097
Conversation
fselmo
force-pushed
the
refactor-computation-class
branch
3 times, most recently
from
c762500
to
bf7f3aa
Compare
…utation``: - The current implementation of the computation classes (and the base computation class) will become stale when EOF is introduced because it always assumes a "message" which has transaction fields. We should refactor the actual base computation class to have only the inherent properties of base computations. - The ``MessageComputation`` class then inherits from a simpler, stripped-down ``BaseComputation`` class. Existing VM computation classes are now ``MessageComputation`` classes since they inherently require a ``message`` and ``transaction_context`` starting at the ``__init__()`` method and stretching across some of its methods and properties. - This hierarchy paves the way for EOF computations, and possibly other computations, to exist within a more flexible model. EOF computations are likely to be properties of message computations since an EOF computation cannot be executed without a message / transaction.
fselmo
force-pushed
the
refactor-computation-class
branch
3 times, most recently
from
f2a916e
to
c640856
Compare
- Define computation children in the ``BaseComputation`` class and narrow the type as needed for subclasses - e.g. for ``MessageComputation`` narrow the children to be ``MessageComputationAPI``. - Some minor cleanup along the way. - Add newsfragment
fselmo
force-pushed
the
refactor-computation-class
branch
from
c640856
to
c7bc4cf
Compare
fselmo
force-pushed
the
refactor-computation-class
branch
from
fc27ebb
to
0c30193
Compare
| C = TypeVar("C", bound="ComputationAPI") | ||
|
|
||
|
|
||
| class BaseComputation(ComputationAPI, Configurable, Generic[C]): |
There was a problem hiding this comment.
This changes the MRO from the old Computation class. Is that intentional?
There was a problem hiding this comment.
Do you mean since Configurable and ComputationAPI were swapped in order? I don't think it matters much (since there are breaking changes here already) but I can restore the order. I was going for API (abstract class) -> inherited non-abstract class -> and then the generic.
There was a problem hiding this comment.
I'm going to merge this since it was mostly approved and this seems like a smaller change. I'd like to be able to start rebasing the EOF PR from these changes. I can address some more computation refactoring in the that PR as well. Happy to move this conversation over there.
There was a problem hiding this comment.
Do you mean since Configurable and ComputationAPI were swapped in order?
Yep, that's what I was looking at. I was more worried about it just being a copy/pasta rather than a breaking change for users. I didn't look into it too much, so if it doesn't matter this seems fine to me!
What was wrong?
Related to Issue #2089
Computations currently require a message and a transaction context. This is because, up until EOF, computations were built from a message with transaction-like fields. This PR opens support for computations that do not come from messages. One such example will be an EOF computation which is built from instructions in the bytecode of a message computation.
How was it fixed?
To make way for the above changes, the
BaseComputationclass is refactored in this PR to be more basic. It strips out the the idea that all computations rely on a message. This is now a property of theMessageComputationclass that we are more familiar with, which is still configured as part of all VMs at present, and inherits from and builds upon theBaseComputation.This PR does not attempt to add any new computation classes. It is simply paving the way for them by splitting the current
BaseComputationclass into these two classes.Todo:
Cute Animal Picture