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

[Idea] Upgradable actors #717

Open
raulk opened this issue Oct 5, 2021 · 7 comments
Open

[Idea] Upgradable actors #717

raulk opened this issue Oct 5, 2021 · 7 comments

Comments

@raulk
Copy link
Member

raulk commented Oct 5, 2021

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
Copy link
Member

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
Copy link
Member

Stebalien commented Dec 2, 2021

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
Copy link
Member

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
Copy link
Member

anorth commented Jun 21, 2022

The approach to upgradeability interacts with other items, such as #721, #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).

@raulk raulk changed the title upgradable actors Idea: Upgradable actors Aug 18, 2022
@raulk raulk transferred this issue from filecoin-project/fvm-specs Aug 18, 2022
@raulk raulk changed the title Idea: Upgradable actors [Idea] Upgradable actors Aug 18, 2022
@raulk raulk added this to the M2.1 Iron (r4) milestone Aug 18, 2022
@maciejwitowski
Copy link
Collaborator

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

@Stebalien
Copy link
Member

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

@maciejwitowski maciejwitowski removed this from the M2.1 (r4) Iron milestone Sep 12, 2022
@maciejwitowski
Copy link
Collaborator

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants