Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type parameter to Kernel::send specifying type for receiving kernel #1780

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

anorth
Copy link
Member

@anorth anorth commented May 19, 2023

This type parameter supports a kernel to be wrapped, and have the wrapped type also be instantiated on the other side of the send. Something like this is necessary to support injecting kernel behaviour for testing.

@alexytsu and I couldn't get the TestKernel/TestCallManager pattern to work from github.com/anorth/fvm-workbench (where we need to fake out proof verification, and long term wish to be able to inject arbitrary kernel behaviour). This new scheme avoids the coupled TestCallManager and associated funky transmutation.

It's limited in that, unless we can figure out the right type constraints, a TestCallManager itself can't be wrapped, because it ignores the type parameter provided to it.

This is a partial resolution of #1779 (we're unblocked for now). But as noted there, still very limited because there's no facility for dynamic behaviour of an injected kernel. The kernel is constructed internally and short-lived. Passing a kernel construction method (i.e. factory) at runtime would be far more flexible.

@anorth anorth changed the title try removing TestCallManager Try to resolve Kernel injection with an additional static type parameter to send May 19, 2023
@anorth anorth changed the title Try to resolve Kernel injection with an additional static type parameter to send Add type parameter to Kernel::send specifying type for receiving kernel May 29, 2023
@anorth anorth marked this pull request as ready for review May 29, 2023 02:04
@anorth anorth requested a review from Stebalien May 29, 2023 02:04
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2023

Codecov Report

Merging #1780 (ae371e6) into master (f4f3f34) will increase coverage by 0.01%.
The diff coverage is 98.03%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1780      +/-   ##
==========================================
+ Coverage   74.75%   74.76%   +0.01%     
==========================================
  Files         146      146              
  Lines       14106    14106              
==========================================
+ Hits        10545    10547       +2     
+ Misses       3561     3559       -2     
Impacted Files Coverage Δ
fvm/src/kernel/default.rs 53.10% <97.91%> (ø)
fvm/src/syscalls/send.rs 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

I wish there were a better way... but this definitely makes testing easier.

@Stebalien Stebalien merged commit db8c0b1 into master Jun 21, 2023
@Stebalien Stebalien deleted the anorth/kernel-static branch June 21, 2023 20:50
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