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

FIP-0031: Atomic switch to non-programmable FVM #294

Merged
merged 8 commits into from
Feb 24, 2022

Conversation

raulk
Copy link
Member

@raulk raulk commented Feb 8, 2022

A few days ago, we submitted FIP-0030 (Introducing the Filecoin Virtual Machine). That FIP served as a baseline specification for the FVM, and did not propose concrete network actions.

This FIP that proposes the activation of a non-programmable version of the FVM in the Filecoin network.

In terms of deployment, this FIP should be coupled with an upcoming FIP that proposes a rearchitecture of the gas model to prepare for arbitrary code.

@@ -0,0 +1,265 @@
---
fip: <to be assigned>
Copy link
Collaborator

Choose a reason for hiding this comment

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

FIP0031

Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

Looking good so far.

## Change Motivation

Swapping out core components in the execution layer of a blockchain is a complex
operation. It's analogous to replacing the engine of an airplane while airborne.
Copy link
Member

Choose a reason for hiding this comment

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

The overly dramatic analogy is not helpful here.

For this reason, its advisable to manage risk carefully. Monolithic, big-bang
deployments are risky. It's appropriate to break up changes in smaller units
that can be deployed in an orderly, incremental fashion, eventually leading up
to the final goal: on-chain user-programmability.
Copy link
Member

Choose a reason for hiding this comment

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

Style nit: prefer first-person. "We advise...."

| `fil/7/verifiedregistry` | TBD |

> Notes:
> What would a premigration look like here?
Copy link
Member

Choose a reason for hiding this comment

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

It's not that big, probably unnecessary.

WIP.

## Backwards Compatibility

Copy link
Member

Choose a reason for hiding this comment

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

Note whether or not a node running the intrinsic actors is expected to be able to maintain sync after this epoch, after simple changes like changing the code CIDs.

@anorth
Copy link
Member

anorth commented Feb 14, 2022

A couple of questions to answer:

  • What will be the impact on block validation times? What levers to we have to pull if that impact is non-zero?
  • How well prepared will we be to quickly fix a critical error in the built-in actors, should one be discovered after the switch? How would the process and timelines change? (There is a subtext here about the thoroughness of testing that would enable us to be confident deploying a small change direct to the network with very little testnet time).

@Stebalien
Copy link
Member

What will be the impact on block validation times? What levers to we have to pull if that impact is non-zero?

We're not sure yet. Unptimized, I'm seeing my lotus/FVM node go from 1.5-3s block validation times, to 3-6s. Luckily:

  1. We can be benchmark before we actually upgrade.
  2. We have room for optimization.

How well prepared will we be to quickly fix a critical error in the built-in actors, should one be discovered after the switch? How would the process and timelines change? (There is a subtext here about the thoroughness of testing that would enable us to be confident deploying a small change direct to the network with very little testnet time).

I'm planning on running a mock upgrade on a test network to force us to get everything in-place. We have 3 upgrade points:

  1. We can replace some or all of the actor implementations. The FVM itself won't actually care about this as it'll just execute the actors as-specified by the chain state.
  2. We can swap out the FVM Kernel, Machine, etc. for different network versions (or add conditionals). Although I'd prefer to avoid when possible.
  3. We can import multiple versions of the entire FVM (like we do with specs-actors) and construct the correct version for the current epoch.

I'd like to test 1 and 3 (an upgrade that swaps out both the actors and the FVM itself) in a test network before calling the upgrade "ready".

There is a subtext here about the thoroughness of testing that would enable us to be confident deploying a small change direct to the network with very little testnet time

Our current best source of test coverage is our test vectors. Once we get some test coverage information (see filecoin-project/ref-fvm#314) we can determine how dire the situation really is.

I'd also like to "foreward-port" our test vectors to newer network versions by re-running them while ignoring anything related to gas.

But I'm actually less concerned about test coverage than I am about test frameworks. We need to be able to write tests quickly if we want to be able to fix bugs quickly.

@anorth
Copy link
Member

anorth commented Feb 16, 2022

Thanks @Stebalien. By asking the question I really meant that the FIP text should answer it. I get that the validation cost question may be "we don't know" at present, but, if you want to rush the FIP, we could still outline the policy options.

@anorth anorth changed the title FIP: Atomic switch to non-programmable FVM FIP-0031: Atomic switch to non-programmable FVM Feb 22, 2022
@raulk raulk marked this pull request as ready for review February 22, 2022 23:52
FIPS/fip-0031.md Outdated Show resolved Hide resolved
@jennijuju
Copy link
Member

jennijuju commented Feb 24, 2022

Hi @fips-editors - from a FIP DRAFT completion perspective, i think its good to merge? authors may address the comments in follow up PRs and work towards last call?

@anorth
Copy link
Member

anorth commented Feb 24, 2022

Fine to merge as a draft. I don't think it can move to last call with the WIPs in place for some important sections.

Co-authored-by: Jiaying Wang <[email protected]>
Copy link
Member Author

@raulk raulk left a comment

Choose a reason for hiding this comment

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

That works for me. In that case, I'd appreciate an approval so I can merge this PR as a draft FIP.

@jennijuju jennijuju merged commit 4327d93 into master Feb 24, 2022
@jennijuju jennijuju deleted the raulk/fvm-atomic-switch branch February 24, 2022 13:51
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.

5 participants