runtime: start lifting contract preparation up through tx runtime layers #11810
Conversation
With this all receipts are accounted for within this structure. Next up will be moving the iteration over receipts to happen internally within this structure, which would then allow us to also peek, etc.
This now allows introducing some hooks for pipelined preparation into the receipt execution loop.
It appears that having the contract code access bundled into Externals wasn't particularly helping with lifting the contract preparation up. As a demo I have lifted the preparation to happen before the RuntimeExt is constructed...
|
@nagisa Hey, sorry, I won't have time to review this PR today, I'm going on PTO tomorrow and I'll be gone for a week. Can you find another reviewer or ping me again in a week? |
|
I'll find somebody else, enjoy your time off! |
|
@nagisa Sorry for jumping in, I'd like to invite you to participate in the Race of Sloths. Just mention @race-of-sloths user in your github comment or PR description to join! See how the flow works here: #11778 |
|
@nagisa Thank you for your contribution! Your pull request is now a part of the Race of Sloths! Current status: executed@nagisa check out your results on the Race of Sloths Leaderboard! and in the profile What is the Race of SlothsRace of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! Bot commands
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11810 +/- ##
==========================================
+ Coverage 71.78% 71.80% +0.02%
==========================================
Files 792 796 +4
Lines 162617 163325 +708
Branches 162617 163325 +708
==========================================
+ Hits 116729 117279 +550
- Misses 40844 40998 +154
- Partials 5044 5048 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This avoids a problem where in order to prepare a contract the NEAR balances of an account need to be known (in order to construct a `VMOutcome`) but also where the balance of an account might change by the time the VMOutcome is materialized to the caller as a result of them calling `run`.
`method` has been very recently moved into `VMContext` with the thought that `method` is closely related to the function call arguments which were already inside `VMContext`. However with `prepare` having been modified sufficiently to not rely on `VMContext`, `method` has again moved to be passed through a function argument. Keeping another instance of `method` inside `VMContext` would potentially allow for value confusion, as running a different method than what has been prepared is invalid.
nagisa
force-pushed
the
moves-contract-preparation
branch
from
8bb65dd
to
e6213f1
Compare
nagisa
force-pushed
the
moves-contract-preparation
branch
from
e6213f1
to
9b06c74
Compare
This is not, strictly speaking dead-dead code. It is going to appear in a future change regardless, but is unnecessary in this PR.
| @@ -359,16 +358,26 @@ impl<'a> External for RuntimeExt<'a> { | |||
| fn get_receipt_receiver(&self, receipt_index: ReceiptIndex) -> &AccountId { | |||
| self.receipt_manager.get_receipt_receiver(receipt_index) | |||
| } | |||
| } | |||
|
|
|||
| pub(crate) struct RuntimeContractExt<'a> { | |||
There was a problem hiding this comment.
nit. assuming Ext suffix represents "external", what does external contract mean here? (I miss the context for the structures in ext.rs.
There was a problem hiding this comment.
That's a good question. "External" here stands for the "External Interface to the Contract Runtime". For this structure it could really just also read "ContractGetter" or "ContractReader" or "ContractAccessor" or something similar to evoke the purpose of this data structure more clearly, but since we already have RuntimeExt as a precedent, I opted to go with a similar name here as well.
🔄 The PR has been merged.Important This pull request is a part of the Race of Sloths and has not been scored yet. Scoring will close in 24 hours! 🕰️ |
✅ PR is finalized!Your contribution is much appreciated with a final score of 0! Another weekly streak completed, well done @nagisa! To keep your weekly streak and get another bonus make pull request next week! Looking forward to see you in race-of-sloths |
Best reviewed commit-by-commit.
This ultimately lifts the contract preparation up through a several function call layers in the transaction runtime up to the layer where all the currently necessary data are available.
This PR also establishes in broad strokes where the pipelining decisions will be made (
ApplyProcessingReceiptState) and makes some minor changes to the type to have it contain local receipts (in addition to the previously contained delayed receipts etc) in a queue of sorts which would allow the pipelining code to look-ahead of the ongoing processing work and queue-up preparation of the upcoming contracts there.This work so far is intended to have no functional changes.
Part of #11319