filecoin-project / ref-fvm

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

[Idea] Upgradable actors #717

Open raulk opened 3 years ago

raulk commented 3 years ago

One idea that @Stebalien and I chatted about today is introducing first-class support for upgradable actors in the FVM. This conversation forked from the "how do we upgrade system actors?" question.

Most serious smart contract operators on Ethereum use the proxy pattern to enable contract logic to evolve. There are many ways of doing this, here are some resources:

Upgradability might be a bit polarizing. One could argue for either of these views:

  1. Immutability of user-deployed code bound to addresses is a basic guarantee of blockchains, and should not be dropped.
  2. Code mutability would be a beneficial first-class primitive to introduce at the protocol level; proof of that is that most serious contracts in other platforms resort to establish design patterns to achieve it in user land.

One solution would be to allow actors to implement an "Upgradable" trait, which implements the logic to authenticate an upgrade message.

Stebalien commented 3 years ago

IMO, this should simply be implemented as runtime-provided function. Actors can call the "replace code" function to upgrade themselves.

  1. I'm not convinced an external interface is what we're looking for. When an actor decides to upgrade really depends on the actor itself.
  2. It should be very easy to statically determine if an actor can upgrade itself by looking at the imports. If an actor doesn't import the "replace code" runtime function, the code CID cannot be changed.
Stebalien commented 2 years ago

We'll also likely need an "upgrade" method on deployed actor code. I.e., when some actor calls replace_code(new code cid), we'll want to:

  1. Run the upgrade (not constructor) function on new code cid).
  2. If the upgrade is successful, trap and return without running any of the old code.
  3. If the upgrade is unsuccessful, return an error without changing the state.

Without this, we'll have a security issue:

  1. Malicious user deploys a new actor with some custom state.
  2. Malicious user upgrades their actor to some well-known actor (e.g., a system actor).
  3. The system, a user, etc. trusts the actor because the code CID is in some "trusted actor" set.
Stebalien commented 2 years ago

So, one interesting issue will be re-entrancy. E.g.:

A -> B -> A (upgrade)
A -> B
A (upgraded state-tree but old code?)

We're going to need to forbid this. My thinking is, when B returns to A, we'd check to make sure the code hasn't changed. If it has, we'd revert and tell A that the call failed because it would create a re-entrant upgrade.

anorth commented 2 years ago

The approach to upgradeability interacts with other items, such as filecoin-project/ref-fvm#721, filecoin-project/ref-fvm#729 and Account Abstraction. I am firmly in the camp that it would be a beneficial first-class primitive, and we can make a better development environment by enshrining it, rather than force proxy patterns (which will in turn require more complex FVM primitives like delegatecall that we might otherwise avoid).

Another part of the upgrade story is the permission to execute an upgrade, often attributed to an owner of the proxy contract. Projects can establish a path to reduced trust by changing this owner to some governance contract, multisig, or even eventually setting an owner to 0x0. One idea I had is that Filecoin could implement this part, too, by adding an Owner address to the top-level actor object, and then requiring that the caller of an replace_code(address, new_code) syscall is the owner.

That may be unnecessary, though. The alternative is that every contract manages it's own ownership address (inline, rather than in a proxy) and is responsible for the access-control check before invoking replace_code(new_code) (with implicit self).

maciejwitowski commented 2 years ago

@raulk do we need the implementation of this to land in Iron? If not, which milestone would it be needed in?

Stebalien commented 2 years ago

No, assuming we can reach consensus on the current actor addressing proposals.

maciejwitowski commented 1 year ago

No, assuming we can reach consensus on the current actor addressing proposals. @Stebalien does it mean we don't need this now? Or we'll need it later?