Discuss: Versioning and FVM M2 development

[raulk]

Context

The FVM M2 milestone requires changes to these actors:

• init actor, to introduce the ability to install code and instantiate arbitrary actors
• (maybe) system actor
• (presumably) account actor, to enable Account Abstraction to represent EVM accounts and validate Ethereum transactions

Furthermore, we propose bringing in EVM actors into the builtin-actors tree (#482).

We would like to figure out ways to support concurrent development of nv17 and FVM M2 (presumably nv18).

Proposal

• The master branch contains the actor changes that are confidently going to ship in the next network version, either because their FIPs have already been approved for inclusion in the next network version, or they are de facto on track for that.
• Rely on develop/<project name> branches for future changes. FVM M2 develoment would happen under develop/fvm-m2, but would still use the pull request process to merge changes there, so that the community has visibility and can review.

[ZenGround0]

Strong preference for developing directly on master. Linear commit history and one-thing-per-commit means that the worst case where we have to revert partial development on something that doesn't have time to land is not even that bad.

[raulk]

@ZenGround0 I didn't follow your line of reasoning. This isn't about merging strategy. This is about supporting concurrent development.

[ZenGround0]

Yes I understand that. I was trying to understand the reason behind a develop branch which I think mostly exists to separate out changes that we think might not land. If that's the motivation it's not sufficient given the nice properties of the linear commit history imo. Maybe I'm missing stronger motivation.

Doing development on one branch is simpler when there is not too much going on and my intuition is that these two streams combined are not big enough to violate that heuristic. If you'd strongly prefer a development branch we can make it work too.

[raulk]

I was trying to understand the reason behind a develop branch which I think mostly exists to separate out changes that we think might not land.

That isn't the motivation. The motivation is to enable concurrent development of network versions that are pipelined in time. Merging all changes to master will imply that changes slated for nv18 would end up shipping in nv17 (if master represented nv17).

[anorth]

I think you're both right, but maybe with different contexts.

I concur with @ZenGround0 a strong preference for development of the code that we expect to ship soon happens on master. At the moment "soon" means for nv17 (actors v8 has already branched off for release in nv16). So Zen this means all the work you're preparing for would be on master.

We do then need a way to develop code that's targeting a release further in the future. A long-lived branch that's treated as a target for PRs is a good way of doing this. Maintainers need to continually rebase on master. I would caution against maintaining more than one of these, as the cost of conflict between these branches runs high. But either "M2" or "nv18" or even just "next" are ok labels for this.

All that said, @raulk, if changes to the init actor or account actor are ready in time for nv17, I would tend to just release them with that, following the principle of minimising work in progress. The closer that M2 comes to just flicking a switch to enable already-implemented behaviour, the better.

[jennijuju]

I think the ask for developing in a dev branch was partially from me.

reason being:

• I think codes (protocol change) should only be included in a actor release unless an FIP is accepted & scoped to be finalized in a network upgrade.
• if everything is merged into master, it might be error prune for us to cherrypick the necessary PR that goes into the v(cur+1) actor for the next network upgrade. The error prune comes from: missing PRs, dealing with conflicts and so on.
  (we didnt have this issue before with spec actor cuz we have never had two network upgrade work stream going on in parallel, imho).

If we land on everything goes directly into master, then we need to establish stricter rules on the test coverage with each PR imho.

[anorth]

we need to establish stricter rules on the test coverage with each PR imho

We should anyway, so if master is intimidating enough to make it actually happen, so much the better!

[arajasek]

Yeah, I like just having master and a next (and a release/vX branch that preserves old releases and supports any quick changes needed there). My opinion (which is really just a mashup of @anorth and @ZenGround0's stated opinions) is:

• master represents stuff we expect to go into the next network upgrade, as people have said. The precise definition of "expect to go into the next network upgrade" is a little unclear, but I think we've generally found enough consensus on what that means (and when we've had to quickly add / remove a change, it's not been too hard).

• next represents a dev branch with more future-looking changes. We should aim to have next "always" track master so that future work is always on top of latest master. I agree with @anorth that work should only live in next if there is some compelling reason why it can't be merged into master instead (eg. "it needs a new syscall that will only be provided by the FVM in M2").

This isn't really that different from the method we've used in specs-actors in the past, except that we are expecting more future-looking dev work happening in parallel, and so need some new place for that work (when absolutely necessary). A single next branch serves that purpose.

[raulk]

@arajasek sounds like the approach I suggested in the original post, but limiting concurrency to 1 in a next branch.

[raulk]

@anorth is also aligned with that approach.

Re: the suggestions of shipping FVM M2-related changes (init, system, and account actors) in nv17 if they're ready, I'm strongly opposed.

1. There is no value in shipping unused features to the network. This relies on a fake assumption that they're secure even though they haven't been subjected to... usage.
2. There's the risk that later phases of FVM M2 (incentivised testnet, auditing) will unveil vulnerabilities in code that we've shipped in nv17. We don't want to be in that situation.

[jennijuju]

Here is the latest proposal from the FVM perspective, assuming nv17 is for addressing selected non-FVM FIPs, an nv18 will be reserved for nv18.

FVM Milestone	Network version	Actors version	builtin-actors branch
FVM M1	nv16	v8	release/v8
—	nv17	v9	master
FVM M2 EVM	nv18	v10	next
FVM M2 Native	(tbd)	(tbd)	next with feature flags

with this proposal, question:

• For FIPs that were being accepted & implemented but not scoped into v17, does the code go to next as well?

[jennijuju]

cc @lemmih - how does the forest used to manage this? any thoughts on the proposals above & how we should go forward?

[lemmih]

In short, we essentially didn't manage network versions so I have no insight to offer. :) I might be reading this discussion wrong but which branches to use is an internal matter. As long as you publish your crates and use semver, forest will be happy.

[raulk]

For the record, my immediate interest is coming out with an actionable item here that we all mildly agree on. That seems to be to target all "projected" (vs. confirmed) changes to a next branch. This outcome limits our abillity to project changes into the future beyond the next network version, which basically results in clipping our own wings to fly and achieve higher velocities and rates of innovation in the long-term. I do not agree with that outcome and develop/fvm-m2 would be a ton better, but this (in my view, defective) policy can be reversed very quickly when the time comes.

[vyzo]

M2 native should go in next with feature flags, not master imo.

[raulk]

Well spotted, @vyzo. I've also updated it in the original mapping table which includes ref-fvm version alignment.

[arajasek]

I might be reading this discussion wrong but which branches to use is an internal matter. As long as you publish your crates and use semver, forest will be happy.

@lemmih No such thing as an internal matter -- your (team's) input on things like this is very welcome, since it'll affect anyone building an implementation of Filecoin :)

[anorth]

Re: the suggestions of shipping FVM M2-related changes (init, system, and account actors) in nv17 if they're ready, I'm strongly opposed

I will note some arguments in favour of shipping code when it's ready, though we can defer a decision until we have a concrete case to evaluate.

• There is continual merge/integration cost to maintaining completed work in a branch, because subsequent work cannot be written directly on top of it, and must be merged instead
• Spreading the work out over time lets us pay more attention to it. Loading up M2 with a lot of other changes makes it a very complex thing to comprehend and test, IMO more likely to have an error. I liked the earlier idea that enabling M2 would be basically a flag flip.
• The complexity of continually merging changes is itself an avenue for errors, especially subtle ones related to unforseen interactions between changes that were written in isolation, vs in the context of each other. Devs are less switched on during this chore work.
• Code on master will be subject to more testing and I think we're more likely to find bugs
• Merging to master carries the appropriate fear of errors to encourage robust testing and skepticism of ones own changes, that's a bit harder to bring to a dev branch
• The counter to @raulk's (2) is that by delaying shipping, there's a risk we discover bugs in the code that's both deployed and in use, due to the increased amount of complexity landing all at once limiting our validation efforts.

[jennijuju]

we are set for m2.1 - open new issue when m2.2 comes if needed.