vm: split preparation and running of a contract #11667
Conversation
This is a small one. `VMContext` already contains information specific to a single function call, most notably the input "arguments". So it seems only sensible that it would also include the method to be called, largely encapsulating everything[^1] that's part of the inputs to the function call. (I think `promise_results` should also be a part of that, so maybe not quite *everything* yet, that's the next commit...)
Topically this is very similar to the `input`, even down to the detail that this is exposed via similar means to the contract. So it makes a lot of sense that it is treated in a very similar way in code as well.
This now technically kinda allows us to pipeline these two steps externally. Finally!
| compute_usage = tracing::field::Empty, | ||
| ))] | ||
| pub fn prepare( | ||
| ext: &(dyn External + Send), |
There was a problem hiding this comment.
This actually no longer requires Send from this API specifically, but External will need to be sendable if we want to allow preparing in a different thread anyway. So I'm keeping this bound here to make sure implementors of this type don't go rogue...
In a future change it may be possible to pass in a different trait that's focused on contract loading specifically. We could implement that for "just" TrieUpdate rather than the whole of Externals implementation.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11667 +/- ##
==========================================
+ Coverage 71.62% 71.79% +0.16%
==========================================
Files 787 790 +3
Lines 161191 161951 +760
Branches 161191 161951 +760
==========================================
+ Hits 115461 116278 +817
+ Misses 40697 40636 -61
- Partials 5033 5037 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -71,7 +66,9 @@ fn get_context() -> VMContext { | |||
| signer_account_id: "bob.near".parse().unwrap(), | |||
| signer_account_pk: vec![0, 1, 2, 3, 4], | |||
| predecessor_account_id: "carol.near".parse().unwrap(), | |||
| method: "VMLogicBuilder::method_not_specified".into(), | |||
There was a problem hiding this comment.
Should get_context take a method parameter to avoid hardcoding this, or are there known use cases where the method does not make sense?
There was a problem hiding this comment.
Pretty much none of the VMLogic tests use the method at all, so it is just a noise.
This is a very exciting step forward! Finally we got up to the point where we can do some work in preparing the contract to run separately from actual running of the contract. And all of this is encapsulated in a very neat API that gives out
Send + 'statictypes for users to pass around between threads or whatever so that they can pipeline these processes.It will remain to see whether the requirement to have
&Externaland&VMContextin both calls is a problem, and how much of a problem it is, but that might be very well solvable with some scoped threads or smart use of channels, or even justArc<Mutex>, especially since both of these structures generally tend to be unique to a contract execution...Part of #11319