Pluggable System Calls & Kernel

[Stebalien]

Right now, the FVM provides a single system call API that's difficult to extend. Use-cases include:

1. Other blockchains (e.g., for IPC). Said chains may have their own syscalls and definitely don't want some of our special syscalls (proof validation, etc.).
2. Development tools. E.g., this would allow users to easily add special debugging syscalls, compile the FVM without proof validation support, etc.

Proposal:

1. Make the syscalls::bind_syscalls function pluggable. E.g., consider making it a possibly static method on the kernel?
2. Make the Kernel extensible. I.e., define a base kernel that must be implemented (along with a basic bind_syscalls function) and let the user define custom "mixins".

[yourarj]

@Stebalien Would like to discuss the approach and impl.

[Stebalien]

impl Kernel for ... where Kernel
The "naive" approach is to:

1. Add some form of bind_syscalls method to the kernel.
2. Let the user "wrap" the default kernel in a custom kernel that extends bind_syscalls.

However, this has a few problems:

1. We need the price list to be extensible.
2. The kernel doesn't expose the call manager. I guess we could allow this?
3. Really, the kernel only exposes methods needed by syscalls, by design.

We'll probably need to:

1. Expose all the kernel internals. IMO, this is probably fine? Although we may want to mark the DefaultKernel struct as non-exhaustive.
2. Make the price list pluggable. I think the right answer here is to make the PriceList an associated type of the Machine.
3. For wrapping, we could use something like ambassador, but that's still not great. I think it may be better to use newtypes. I.e.:

pub trait IpldBlockOps {
    fn block_open(&mut self, cid: &Cid) -> Result<(BlockId, BlockStat)>;
    // ...
}
pub trait BaseKernel {
    type IpldBlockOps: IpldBlockOps;
    type CryptoOps: CryptoOps;

    pub fn ipld(&mut self) -> &mut Self::IpldBlockOps;
    pub fn crypto(&mut self) -> &mut Self::CryptoOps;
    // ...
}

pub trait MyKernelExtension: BaseKernel {
    type CustomOps: CustomOps;

    pub fn custom(&mut self) -> &mut Self::CustomOps;
}

This way, the user can override specific features/modules without having to wrap the entire kernel trait.

[Stebalien]

So, first some rust context and a discussion of "patterns".

• Coming from an OO language, you might expect to be able to "extend" some base class. E.g., extend the DefaultKernel by overriding/adding a few methods. You can't do that in rust.
• Coming from Go, you'll also be a bit disappointed. In go, you can compose multiple objects/structs by "embedding" them. There's no way to do that in rust either.

In rust, you'd usually do something like:

trait MyTrait {
    fn a(&self);
    fn b(&self);
    fn c(&self);
    fn d(&self);
    fn e(&self);
    fn f(&self);
}
struct Base;
impl MyTrait for Base {
    fn a(&self) { /* do something */ }
    fn b(&self) { /* do something */ }
    fn c(&self) { /* do something */ }
    fn d(&self) { /* do something */ }
    fn e(&self) { /* do something */ }
    fn f(&self) { /* do something */ }
}
struct SubClass(Base);
impl MyTrait for Base {
    fn a(&self) { self.0.a() }
    fn b(&self) { self.0.b() }
    fn c(&self) { self.0.c() }
    fn d(&self) { self.0.d() }
    fn e(&self) { self.0.e() }
    fn f(&self) { /* override */ }
}

That is, manually delegate every method you're not overriding. Or, you'd use something like ambassador to do this automatically (kind of).

But... there's another pattern we can explore. We can break the trait up into multiple traits (as we do in the Kernel today), but then, instead of defining the Kernel as Kernel: Trait1 + Trait2 + Trait3, we define the Kernel as an object that returns references to objects implementing Trait1, etc. E.g.

trait GroupOne {
    fn a(&self);
    fn b(&self);
    fn c(&self);
}
trait GroupTwo {
    fn d(&self);
    fn e(&self);
    fn f(&self);
}
trait MyTrait {
    type GroupOne: GroupOne;
    type GroupTwo: GroupTwo;
    fn group_one(&self) -> &Self::GroupOne;
    fn group_two(&self) -> &Self::GroupTwo;
}

struct Base;
impl MyTrait for Base {
    type GroupOne: GroupOne = Self;
    type GroupTwo: GroupTwo = Self;
    fn group_one(&self) -> &Self::GroupOne { self }
    fn group_two(&self) -> &Self::GroupTwo { self }
}
impl GroupOne for Base {
    fn a(&self) { /* do something */ }
    fn b(&self) { /* do something */ }
    fn c(&self) { /* do something */ }
}
impl GroupTwo for Base {
    fn d(&self) { /* do something */ }
    fn e(&self) { /* do something */ }
    fn f(&self) { /* do something */ }
}

struct SubClass(Base);

impl MyTrait for Base {
    type GroupOne: GroupOne = Self;
    type GroupTwo: GroupTwo = Self;
    fn group_one(&self) -> &Self::GroupOne { self }
    fn group_two(&self) -> &Self::GroupTwo { self }
}

impl GroupTwo for SubClass {
    fn d(&self) { self.0.d() }
    fn e(&self) { self.0.e() }
    fn f(&self) { /* override */ }
}

This is definitely more complex but, it makes it much easier to override/extend specific groups. With today's 12 traits, we can forward ~12 very simple methods instead of ~40. Even better, if we re-organize according to filecoin-project/FIPs#845 (approximately), we can forward 5/6 methods.

[Stebalien]

In terms of sequencing, I'd actually start with the price-list.

1. Make it possible to feed in a custom price-list as a hash map (no types, just data). IMO, the Machine should parse (well, read) this price-list into a typed PriceList.
2. Make the PriceList generic, an associated type on the Machine? I think we can make this "not terrible", but we'll have to discuss it a bit.

[Stebalien]

You know what? No, I change my mind. Let's not start with the price-list. I'd like to do that, but I'm not entirely sure how much of the price-list the user should actually care about and/or specify.

Let's start small and ignore that entire pattern I described above. That pattern is important for overriding functionality, but we're just extending functionality here:

1. Define a new Syscalls: Kernel trait with a bind_syscalls method on it.
2. Use this when linking syscalls in the Engine.
3. Apply the ambassador trait to the Kernel traits. Really, we might want to consolidate all these traits into one single trait for now (we can break them out in the future if/when we find a better organization).

Then, try extracting the "filcrypto" specific syscalls (compute_unsealed_sector_cid, verify_post, verify_consensus_fault, verify_aggregate_seals, verify_replica_update, and verify_batch_seals) into a new kernel as such:

#[delegatable_trait]
pub trait Kernel {
    // ...
}

#[derive(Delegate)]
#[delegate(Kernel)]
pub struct FilecoinKernel<K=DefaultKernel>(K) where K: Kernel;

impl<K> SyscallHandler for FilecoinKernel<K> where K: Kernel {
    fn bind_syscalls(&self, linker: &mut Linker<InvocationData<impl Kernel + 'static>>) -> Result<()> {
        // Bind the default syscalls.
        self.0.bind_syscalls(linker)?;
        // Now bind the crypto syscalls.
    }
}

// We _might_ want to make a `FilecoinKernel` trait and a separate implementation to make it easier to test... but let's start with a direct implementation if it's easier.
impl<K> FilecoinKernel<K> where K: Kernel {
    fn compute_unsealed_sector_cid(
        &self,
        proof_type: RegisteredSealProof,
        pieces: &[PieceInfo],
    ) -> Result<Cid> { ... }

    fn verify_post(&self, verify_info: &WindowPoStVerifyInfo) -> Result<bool> { ... }

    fn verify_consensus_fault(
        &self,
        h1: &[u8],
        h2: &[u8],
        extra: &[u8],
    ) -> Result<Option<ConsensusFault>> { ... }

    fn batch_verify_seals(&self, vis: &[SealVerifyInfo]) -> Result<Vec<bool>> { ... }

    fn verify_aggregate_seals(&self, aggregate: &AggregateSealVerifyProofAndInfos) -> Result<bool> { ... }

    fn verify_replica_update(&self, replica: &ReplicaUpdateInfo) -> Result<bool> { ... }

}

[Stebalien]

We now have the basic implementation. However:

1. We currently have a whole stack of traits that have to be delegated in order to customize the syscalls. We want to slim this down.
2. We need to reconsider the current trait dependency tree. E.g., Kernel: SyscallHandler may not make sense.
3. We likely need to rename some things. E.g., Rename DefaultKernel to BaseKernel #1930.
4. We need to make the kernel configurable which will likely require more changes to these types.

[Stebalien]

For context, we merged #1922 because it (a) works, (b) is progress, and (c) was getting large. Not because the feature is "complete".

The concrete next step is to test this change out with IPC to see if it does what we need it to do.

[maciejwitowski]

As this is being integrated to IPC I'd like to explore the discussion of adding this to stable FVM version. At the moment IPC is pointing to a branch in ref-fvm which isn't ideal.

The concrete next step is to test this change out with IPC to see if it does what we need it to do.

@Stebalien @fridrik01 Are there any other steps related to stability, API, testing, adding missing features that you have in mind before we mark it as stable?

[Stebalien]

Everything should be in master and I'm currently working on a release (trying to get some final changes into the AMT first, but I guess I should just release FVM itself).

[maciejwitowski]

@Stebalien thanks! Please ping @fridrik01 after you release it so he can use it in IPC instead of pointing to the branch.
If we could have it today, it would be great!

[Stebalien]

Fixed & released.