filecoin-project / ref-fvm

Reference implementation of the Filecoin Virtual Machine
https://fvm.filecoin.io/
Other
380 stars 136 forks source link

prototype minimal Wasm actor deployment flow #444

Closed raulk closed 2 years ago

raulk commented 2 years ago

Context

Having a minimal, experimental protoype of an actor deployment flow will unlock several downstream community projects. What's described here is most certainly not the definitive flow, but it's doubly useful:

This implementation should sit in a branch, ideally behind a Cargo feature flag, and should carry prominent EXPERIMENTAL warnings.

InitActor enhancements

See https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0030.md#actor-deployment for reference.

State object

Add an actor_registry: Hamt<Cid, ActorSpec> field to the state object, where:

struct ActorSpec {
    // The actor that installed this actor.
    creator: ActorID,
    // The originator of the message that triggered creator to install this actor.
    // Probably needs https://github.com/filecoin-project/ref-fvm/issues/300
    originator: ActorID,
    // The version of the invocation container.
    container_version: u32,
}

Operations

Add an InstallActor(bytecode: RawBytes) -> Result<InstallActorReturn, ActorError> method, where:

struct InstallActorReturn {
    // The CodeCID with which this actor can be constructed.
    code_cid: Cid,
    // Whether the bytecode was effectively installed in this call or not.
    // False when the CodeCID already existed.
    // Alternatively we could fail, but that would break API idempotency.
    installed: bool,
}

This method:

  1. Computes the Cid of the parameter block (Wasm bytecode).
  2. Checks if an entry already exists inactor_registry. If yes, it shortcircuits and returns the CodeCID and installed=false.
  3. Adds the bytecode to the blockstore.
  4. Calls the actor::install_actor(block_id) -> Result<Cid> syscall to apply the installation. Aborts on failure.
  5. Adds an entry in actor_registry, filling in the active container version, creator, and originator.
  6. Returns the CodeCID and installed=true

ref-fvm enhancements

actor::install_actor syscall

  1. Persists the Wasm bytecode to the blockstore, and remembers the resulting Cid as the code_cid.
  2. Charge the actor installation gas defined in https://github.com/filecoin-project/FIPs/pull/317. (Can be 0 for now)
  3. Calls kernel.call_manager().machine().engine().preload(blockstore, code_cid) => this compiles the bytecode and caches the instantiated module in memory (cache will be entirely revisited for M2).

In the future, this will perform bytecode validation, structural analysis to derive complexity factors, patching to replace floating point operations, and more.

Note: this approach leaks cached modules in case of reorgs.

Caveats

Stebalien commented 2 years ago

:+1:

container_version

Take a look at the drawbacks/alternatives in https://github.com/filecoin-project/ref-fvm/issues/744 with respect to where/how to store metadata.

I'm also slightly concerned about not putting this into the code CID (something something CID squatting before an upgrade? seems unlikely).

creator/originator

What's the use-case? The only one I can think of is tracking down where/how exactly the actor was deployed. In that case, I'd want the originator and the nonce from that message.

// Whether the bytecode was effectively installed in this call or not.
// False when the CodeCID already existed.
// Alternatively we could fail, but that would break API idempotency.
installed: bool,

Do we really need the extra flag to say that the module was new? It shouldn't matter (and isn't idempotent).

InstallActor

I'd make the parameters a CBOR object and include the container_version:

  1. Users will get an error if they try to deploy to the wrong container version. This is important for both on-chain deployment (an actor may dynamically deploy code on-chain) and for network upgrades (where messages may already be in-flight).
  2. It makes it a CBOR object. At the moment, no other method has "raw" parameters.
anorth commented 2 years ago

Looks broadly good, the code size limits will be interesting to think about.

I find the terminology confusing, using "actor" to refer to both the instance (current usage) and code. Can we change everything here to refer to "actor code", or a new term?

raulk commented 2 years ago

What's the use-case? The only one I can think of is tracking down where/how exactly the actor was deployed.

It's also useful for permissioning use cases, e.g. I want to make sure that my actor can only interact with actors installed by X account. Down the line we'd provide an actor::get_actor_spec(code_cid: Cid) syscall so actors can obtain this info.

In that case, I'd want the originator and the nonce from that message.

Maybe? Do we have precedents of storing message-level information in the state tree?

Do we really need the extra flag to say that the module was new? It shouldn't matter (and isn't idempotent).

My thinking is that the gas cost will vary wildly between cases where the actor didn't exist (and we incurred the actor installation fee), and cases where it already existed. In fact, in the former case, the cost is highly dnamic. So I'd rather have a deterministic way of conveying whether this call performed the installation side-effect, or not.

I'd make the parameters a CBOR object and include the container_version:

So that was my original design. I really want the container version to be an explicit user-defined attribute (otherwise we open ourselves up to unsafety and nasty compatibility issues). But I decided to simplify it to make the parameters block align with the syscall argument, thus avoid the overhead from having to extract the bytecode from the struct field, add it as a dedicated block, to then pass it on to install_actor syscall.

However, on second thought, we could make the install_actor syscall take a block ID to the CBOR-encoded InstallActorParams (and it makes sense, because that syscall needs to check if the desired container version is supported). However, I'm not sure I like coupling the actor method params object with the syscall params.

Maybe they could be different structs in code that share the CBOR representation?

raulk commented 2 years ago

I find the terminology confusing, using "actor" to refer to both the instance (current usage) and code. Can we change everything here to refer to "actor code", or a new term?

Yes, I'm open to that. Let's brainstorm terms, and ultimately fall back on "actor code" if none of them satisfy us?

Some proposals:

Stebalien commented 2 years ago

It's also useful for permissioning use cases, e.g. I want to make sure that my actor can only interact with actors installed by X account. Down the line we'd provide an actor::get_actor_spec(code_cid: Cid) syscall so actors can obtain this info

That's why I asked :). I'm strongly against using it for that:

  1. Actor creation may have been triggered by some random callback, so the "originator" is unreliable/insecure.
  2. It encourages users to re-deploy the same actor code multiple times (once per "author").
  3. It's either insecure (actors are deployed by accounts) or expensive (actors deployed by multisig).
  4. There's no way to "adopt" some actor code into your ecosystem.
  5. There's no way to untrust actors.

Maybe? Do we have precedents of storing message-level information in the state tree?

Nonces are stored in actors (also used for address creation).

But I was just spit-balling potential use-cases. I don't know if this is actually useful (which is why I asked about use-cases).

My thinking is that the gas cost will vary wildly between cases where the actor didn't exist (and we incurred the actor installation fee), and cases where it already existed. In fact, in the former case, the cost is highly dnamic. So I'd rather have a deterministic way of conveying whether this call performed the installation side-effect, or not.

I just don't see how I'd use that information (automatically, at least). But I don't feel too strongly about it (although it will slightly increase gas costs).

Maybe they could be different structs in code that share the CBOR representation?

I wouldn't do that, but I also don't see any real issue with just making the params and the code the same type. That is, you call InstallActor with the actor code which is defined to be this special object that includes the code as a byte array.

In the future, for larger code blobs, I expect the "code" will actually be a link to some sharded on-chain datastructure.

raulk commented 2 years ago

@Stebalien

  1. Actor creation may have been triggered by some random callback, so the "originator" is unreliable/insecure.

What is this mystical callback you're talking about that wouldn't get initiated by a chain message signed by an account actor? The only case where lines would be fuzzed AFAIK is an actor installation performed by a multisig actor, because you'd probably capture the final approving account actor as the originator.

If we're talking about future cases of cron or event callbacks, this is a much larger topic to cover and I'm not even sure that such events should be allowed to install actor code.

  1. It encourages users to re-deploy the same actor code multiple times (once per "author").

How would that happen? We deduplicate at the CodeCID level. This is a more legitimate concern if we embed the author/originator/creator metatada in the Wasm bytecode itself, as unique originators would lead to unique CIDs.

  1. It's either insecure (actors are deployed by accounts) or expensive (actors deployed by multisig).

What is? Saving the originator actor ID?

  1. There's no way to "adopt" some actor code into your ecosystem.
  2. There's no way to untrust actors.

I suspect you're referring to the permissioning use case here and considering potential requirements for more dynamic permissioning schemes. I'm not claiming that the originator itself is the only attribute you'd take into account in your permissioning scheme; but it certainly sounds like something useful to use if you want to.


IMO the weaknesses of this originator idea have to do with the consistency/authenticity of the ActorID you'd find here. There are two main edge cases:

raulk commented 2 years ago

@Stebalien

Nonces are stored in actors (also used for address creation).

Yeah ;-) I meant in actor state objects, sorry. AFAIK there is no precedent of actor state saving message-level metadata in state. I'm not sure how it would be useful given that actor code cannot lookup past messages. The only applicability is to serve as a poor mans' index for observability tools, but that's a tremendously suboptimal usage of state tree space.

I wouldn't do that, but I also don't see any real issue with just making the params and the code the same type. That is, you call InstallActor with the actor code which is defined to be this special object that includes the code as a byte array.

Concerned in terms of naming and the implicit coupling. But naming is solvable through aliases, and the coupling is acceptable iff the InitActor#InstallActor method is the only thing that's authorized to call the actor::install_actor() syscall. That way the coupling does not leak.

raulk commented 2 years ago

This has been merged to master in ref-fvm: https://github.com/filecoin-project/ref-fvm/pull/658 As well as into next in builtin-actors: https://github.com/filecoin-project/builtin-actors/pull/491