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

add readme, instructions, versioning policy, and licenses. #12

Merged
merged 4 commits into from
Mar 1, 2022

Conversation

raulk
Copy link
Member

@raulk raulk commented Mar 1, 2022

No description provided.

@raulk raulk requested a review from Stebalien March 1, 2022 14:57
README.md Outdated
Comment on lines 68 to 76
## Versioning

With the transition to Wasm actors, every network version that modifies built-in
actor code will result in a release of this repo. In practice, this breaks the
distinction between network version and actor versions. For further details on
this, refer to [FIP-0031](https://github.com/filecoin-project/FIPs/tree/master/FIPS/fip-0031.md#non-versioned-changes-and-state-tree-migrations).

For this reason, releases of this repo will be made with version numbers that
correlate to network versions, starting from v14.0.
Copy link
Member Author

Choose a reason for hiding this comment

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

Please pay extra attention to this proposal, and let's discuss explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, see above. Basically, the network version is an out-of-band "change behavior" signal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed above.

@raulk raulk changed the title add readme and licenses. add readme, instructions, versioning policy, and licenses. Mar 1, 2022
Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

all this sounds good to me.

LICENSE-MIT Outdated Show resolved Hide resolved
README.md Outdated
```
This line is only printed as a warning due to limitations in the Cargo build
script mechanisms (preceded by other logs).
4. Embed the CAR file bytes into your binary.
Copy link
Member

Choose a reason for hiding this comment

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

This will likely be implementation dependent (some might download, cache, etc.).

README.md Outdated

## Versioning

With the transition to Wasm actors, every network version that modifies built-in
Copy link
Member

Choose a reason for hiding this comment

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

Not entirely:

  1. We can still use the network version for non-actor changes.
  2. We use back-to-back network version changes for staged upgrades. E.g., when we deprecated the old proofs format, we had an upgrade that added support for the new format, and an upgrade that removed support for the old. We used a single actors version for both.

Copy link
Member Author

Choose a reason for hiding this comment

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

Chatted with @Stebalien about this. I was approaching this as an opportunity to simplify protocol versioning by unifying the network version and the actor version (since the latter will no longer bump only on ABI changes as today, but on any code change, no matter how minor). However, medium-term we want to decouple actor versioning and network versioning even further, since FVM opens up the opportunity to conduct built-in actor upgrades through on-chain governance.

So I'm backing out the proposal and instead sticking with actor versions, knowing that they'll very likely bump with most network versions.

README.md Outdated
Comment on lines 68 to 76
## Versioning

With the transition to Wasm actors, every network version that modifies built-in
actor code will result in a release of this repo. In practice, this breaks the
distinction between network version and actor versions. For further details on
this, refer to [FIP-0031](https://github.com/filecoin-project/FIPs/tree/master/FIPS/fip-0031.md#non-versioned-changes-and-state-tree-migrations).

For this reason, releases of this repo will be made with version numbers that
correlate to network versions, starting from v14.0.
Copy link
Member

Choose a reason for hiding this comment

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

Ah, see above. Basically, the network version is an out-of-band "change behavior" signal.

`ActorVersion`. We adopt a policy similar to specs-actors:

- Major number in crate version correlates with `ActorVersion`.
- We generally don't use minor versions; these are always set to `0`.
Copy link
Member

Choose a reason for hiding this comment

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

nit: I expect we may use patch versions, but the network will use the latest patch version.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, patch versions often used late in the dev/test cycle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is stated in the point below.

@raulk raulk merged commit bb5ed4b into master Mar 1, 2022
@raulk raulk deleted the raulk/readme branch March 1, 2022 18:14
- We strive for round major crate versions to denote the definitive release for
a given network upgrade. However, due to the inability to predict certain
aspects of software engineering, this is not a hard rule and further releases
may be made by bumping the patch number.
Copy link
Member

Choose a reason for hiding this comment

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

This wasn't a goal of specs-actors at all. Many releases were branches with multiple minor version tags and the one released to the network was not 0. The major version bump doesn't signal the end of a development stream, but the beginning of the next one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. Looking at the specs-actors version history, v3 was the last major line that used minor versions. All subsequent versions hit the network as vN.0.M. I assumed this was an intentional decision since version strings like v4.1 would be rather confusing in a world where clients model the actor version as a single int (denoting the major version)?

- Definitive release: 10.0.0.
- Patched definitive release: 10.0.1.
- Patched definitive release: 10.0.2.
- Network upgrade goes live with 10.0.2.
Copy link
Member

Choose a reason for hiding this comment

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

We will probably never have reason to use patch versions in FVM. They were usable in Go because changing the code didn't necessarily change its behaviour, but with compilation through WASM there'll be very few changes that don't change the output code and hence gas calculation at all. We bumped the minor version for any detectable behaviour change.

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.

The versioning discussion appears to be a discussion of its own, so I hoisted it to its own issue: #18. @anorth @Stebalien, you're invited to post your thoughts, as is everyone else.

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.

None yet

4 participants