Commit 1c4f126
committed
refactor(pxe): decompose PXEOracleInterface
Deprecate `PXEOracleInterface` in favor of more granular and explicit dependencies.
I apologize in advance for the length of this PR, the commits show the progression that led to this final form, and prove that the end result didn't come to me OTOH, but it was the result of dozens of stepwise refactors. At the current state it is possible to spin multiple little PRs off this one if that's deemed saner, it just wasn't clear that would be the case when I originally set out to do this.
## New service layer
To replace `PXEOracleInterface` (aka `ExecutionDataProvider`), and at the same time keep oracle classes relatively lean and concerns separated, I'm introducing a service layer on top of the existing providers layer.
Due to this PR being big enough as is, and there being more pressing issues to address, I'm not applying this idea to completion, just using it where it helps me untangle things. I do think going further in this direction would help make everything be more unit testable.
The core rules of thumb behind this (long term) are:
- Providers shouldn't talk to each other. This is a simplicity forcing function. Providers are database abstractions.
- Services can talk to each other, and to the provider(s) they encapsulate. Each provider should only be accessed by one service, but one service can manage multiple data providers. So: services get other read/write data needs met by talking to other services.
- The rest of the app talks to services instead of directly to providers. Again, I'm not tackling that here because the PR is big enough as is, but it's an architecture we could slowly move towards.
## Capsules as plumbing
Something that became clear along the way of this refactor is that capsule state management in many cases is a transport concern, and as such code becomes much more cohesive if we separate the code that deals with that, from the code that actually operates on the contents. Maybe the nicest example of that can be seen in `utilityBulkRetrieveLogs`.
## Working with Anchor blocks
I think most of the places where we now consume `AnchorBlockDataProvider` to get the current anchor block, should be modified to instead receive the anchor block as param, which will make it easier to see that in reality the anchor block is pinned for a given `ContractFunctionSimulator` activation. That, in turn, should make the work on DB integrity easier, and it will also be helpful to eventually adopt a less conservative approach towards concurrency.
## Temporary complexity increase (waiting for new tagging sync work)
There are some parts of the app that might look a bit more convoluted with this refactor, mostly due to porting sync tagging functionality to make things compile and pass tests. Note that @benesjan is overhauling sync tagging, so I'm deliberately not investing effort on simplifying those parts and will instead wait and see what's necessary once that work is available.
Some specific cases of this situation are found on:
- Test setup of `oracle_version_is_checked.test.ts`: it got much more complex given `ContractFunctionSimulator` now receives all the data providers it needs to operate instead of hiding everything under `PXEOracleInterface`. A lot of this complexity is probably derived from the old tag sync'ing logic and I'm expecting it to just disintegrate.
- `private_execution.test.ts`: I'm hoping a lot of the boilerplate will be unnecessary once the new work on tagging lands.
- `LogService` and its tests
Fixes #177761 parent 1776329 commit 1c4f126
File tree
35 files changed
+2848
-2677
lines changed- yarn-project
- pxe/src
- contract_function_simulator
- oracle
- debug
- entrypoints
- client
- bundle
- lazy
- server
- events
- logs
- notes
- private_kernel
- public_storage
- storage
- capsule_data_provider
- contract_data_provider
- tree_membership
- stdlib/src/messaging
- txe/src
- oracle
35 files changed
+2848
-2677
lines changedLines changed: 59 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
| |||
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| 48 | + | |
47 | 49 | | |
48 | 50 | | |
49 | 51 | | |
| |||
69 | 71 | | |
70 | 72 | | |
71 | 73 | | |
72 | | - | |
73 | | - | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
74 | 82 | | |
75 | 83 | | |
76 | 84 | | |
77 | 85 | | |
78 | 86 | | |
79 | 87 | | |
80 | 88 | | |
| 89 | + | |
81 | 90 | | |
82 | 91 | | |
83 | 92 | | |
| |||
86 | 95 | | |
87 | 96 | | |
88 | 97 | | |
89 | | - | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
90 | 108 | | |
91 | 109 | | |
92 | 110 | | |
| |||
116 | 134 | | |
117 | 135 | | |
118 | 136 | | |
119 | | - | |
| 137 | + | |
120 | 138 | | |
121 | | - | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
122 | 143 | | |
123 | 144 | | |
124 | 145 | | |
| |||
154 | 175 | | |
155 | 176 | | |
156 | 177 | | |
157 | | - | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
158 | 188 | | |
159 | 189 | | |
160 | 190 | | |
| |||
226 | 256 | | |
227 | 257 | | |
228 | 258 | | |
229 | | - | |
| 259 | + | |
230 | 260 | | |
231 | | - | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
232 | 265 | | |
233 | 266 | | |
234 | 267 | | |
| |||
239 | 272 | | |
240 | 273 | | |
241 | 274 | | |
242 | | - | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
243 | 285 | | |
244 | 286 | | |
245 | 287 | | |
| |||
274 | 316 | | |
275 | 317 | | |
276 | 318 | | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
277 | 323 | | |
278 | | - | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
279 | 328 | | |
280 | 329 | | |
281 | 330 | | |
| |||
0 commit comments