Skip to content

Conversation

@rekmarks
Copy link
Member

@rekmarks rekmarks commented Apr 5, 2025

Progresses: #369

Introduces a new package, @ocap/rpc-methods, whose primary affordances are two: RpcClient and RpcService. These classes are intended to represent the call-er and call-ee ends of an RPC relationship. In brief, you:

  1. Call RPC methods using RpcClient.call()
  2. pipe the resulting payload to RpcService.execute()
  3. and pipe the result back to the caller.

These two classes consume "method" and "handler" specifications. The idea is that you specify your methods individually, gather these specifications together into a "specification" or "handler" record, and then let the rest of your RPC types follow from there.

Our first target for this pattern is the "kernel control methods", where we are able to replace the MessageResolver and CommandRegistry abstractions, as well as the hated request / response structs. See packages/extension/src/kernel-integration/handlers for where most of this action occurred. Although this PR consists of equal amounts of additions and deletions, you'll note the kernel control method implementation in the extension is significantly simplified (+700 / -1300).

The application's behavior should be virtually unaffected by this change. Following this PR, we will target other RPC channels in order to get rid of their request / response structs as well.

@socket-security
Copy link

socket-security bot commented Apr 5, 2025

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@eslint/[email protected]0.20.0 None 0 319 kB eslintbot
npm/@eslint/[email protected]9.24.0 None 0 14.4 kB eslintbot
npm/[email protected]9.24.0 None +1 3.39 MB eslintbot

View full report↗︎

@rekmarks rekmarks force-pushed the rekm/rpc-methods-package branch from 041f8bc to 5fe6aef Compare April 7, 2025 03:52
@rekmarks rekmarks force-pushed the rekm/rpc-methods-package branch from b231d66 to 505aed9 Compare April 8, 2025 18:52
Comment on lines +132 to +137
{
files: ['**/test/**/*', '**/*.test.ts', '**/*.test.tsx'],
rules: {
'@typescript-eslint/explicit-function-return-type': 'off',
},
},
Copy link
Member Author

@rekmarks rekmarks Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incidental change: This is a common pattern in our tests that is completely fine.

@rekmarks rekmarks marked this pull request as ready for review April 8, 2025 19:15
@rekmarks rekmarks requested a review from a team as a code owner April 8, 2025 19:15
@rekmarks rekmarks requested a review from sirtimid April 8, 2025 19:18
@rekmarks rekmarks mentioned this pull request Mar 31, 2025
5 tasks
Copy link
Contributor

@sirtimid sirtimid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! 🤩

@rekmarks rekmarks merged commit 4c78148 into main Apr 9, 2025
21 checks passed
@rekmarks rekmarks deleted the rekm/rpc-methods-package branch April 9, 2025 16:30
rekmarks added a commit that referenced this pull request Apr 11, 2025
…#481)

Progresses: #369 

Continues the work of #369 by replacing the "vat worker" command / reply
types with the pattern introduced in #474. This primarily affects the
extension, since only its `VatWorkerManager` (so renamed from
`VatWorkerService`) logically crosses process boundaries. Its Node.js
counterpart is just a class that lives next to the kernel, and is only
affected by some renamed types.
rekmarks added a commit that referenced this pull request Apr 22, 2025
…#487)

Closes: #369 

Replaces all kernel and vat "message" types with the new `RpcClient` /
`RpcService` pattern. For related work and the justification for this,
see:
- #369
- #474 
- #481 

Note that, in the course of implementing this refactor, it was
discovered that we are never returning _any_ responses to syscalls
originating from vat supervisors. This PR maintains a provisional
solution using the new pattern, and #488 tracks the work to actually fix
this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants