Introduce v2 actors #3936

arajasek · 2020-09-21T06:56:55Z

Mostly opening this for illustrative purposes, but we could merge into #3781 fairly safely. Not gonna work correctly until all the TODO: ActorUpgrades are resolved, though.

Big TODOs:

Deal with payment channel parameter changes. This may require some retry logic, and will definitely require picking the right parameters across upgrades.
Fix "chain get". Currently, this command assumes exactly one version of specs-actors.
TESTING.

Some calls/operations will fail across the upgrade. Any funds associated with these operations (except the gas) will be returned to the sender.

If a user submits a message to create a multisig/paych actor, the message will not be accepted after the upgrade.
If a user submits a message to update a payment channel before the upgrade, it will not be accepted after the upgrade. HOWEVER, vouchers will remain valid and can be re-submitted after the upgrade.
If a multisig transaction to perform any operation listed here is proposed before the upgrade, it will fail if approved after the upgrade.

Open Concerns:

Performance. We currently run the state migration on gas estimation. This obviously has absolutely abysmal performance.
- We could cache migrations.
- We could just fail. Unfortunately, I'm concerned that lotus may not retry aggressively enough.
- We could "pretend" that we're still in the previous epoch, and estimate gas there (won't work for all messages).

chain/vm/invoker.go

chain/actors/version.go

chain/actors/builtin/builtin.go

Stebalien · 2020-09-24T01:54:09Z

I'm not sure what to do about the genesis code. It does some pretty hacky things to deal with the lack of power (adds fake power, adds sectors, then removes the fake power). I guess we can add a miner.SetEffectivePower function (to set the "this epoch" power), but that seems kind of sketch.

I'm not actually sure if we're doing this correctly anyways. We're setting TotalQualityAdjPower, TotalRawBytePower, and the "this epoch" variants. However, when computing things like pledge, we use the smoothed power, as far as I can tell.

Also, correctly handle multiple ADT versions.

Otherwise, we're going to end up with an import cycle between the adt and this version.

This will make it easier to load arbitrary actors. We can: * Type switch (sort of unsafe, may want marker methods?) * Use this with `vm.MutateActorState`.

We're probably going to want to change some of these design decisions down the road, but this is a good starting point. * We may want to use a more general test for "is actor valid at epoch". Maybe just a function? * I'd like to push some of the actor metadata down into the actor types themselves. Ideally, we'd be able to register actors with a simple `Register(validation, manyActors...)` call.

The actual actors version is v2, not v1. Using Version1 internally was really confusing.

build/params_shared_funcs.go

build/params_testnet.go

arajasek · 2020-10-05T19:51:39Z

chain/stmgr/forks.go

+		// This error is going to be really nasty.
+		return cid.Undef, xerrors.Errorf("network upgrade failed: %v", msgs.Messages())


+1 to just halting the chain, I think that's significantly better than allowing a (likely very broken) chain to continue and break state further.

(The answer might be different post-liftoff)

chain/messagepool/gasguess/guessgas.go

arajasek · 2020-10-05T20:57:03Z

Generally looks good, flagged some nits that i can address

arajasek · 2020-10-05T20:57:28Z

Submodules might need to be updated (or they're wrong in next)

filecoin-project/lotus#3936

dirkmc

Payment channel changes LGTM 👍

magik6k · 2020-10-06T11:57:46Z

Performance. We currently run the state migration on gas estimation. This obviously has absolutely abysmal performance.

We could cache migrations.

We could just fail. Unfortunately, I'm concerned that lotus may not retry aggressively enough.

We could "pretend" that we're still in the previous epoch, and estimate gas there (won't work for all messages).

I feel like the last option is probably the most conservative here

magik6k

Other than the gas estimation thing, this looks shippable

filecoin-project/lotus#3936

* update oni for specs-actors v2 upgrade filecoin-project/lotus#3936 * Update lotus with wallet changes Co-authored-by: Steven Allen <steven@stebalien.com>

arajasek requested review from Kubuxu, magik6k and whyrusleeping as code owners September 21, 2020 06:56

arajasek mentioned this pull request Sep 21, 2020

Network upgrade support #3781

Merged

7 tasks

magik6k reviewed Sep 21, 2020

View reviewed changes

chain/vm/invoker.go Outdated Show resolved Hide resolved

Stebalien force-pushed the refactor/net-upgrade branch from 26e68a7 to 24ae920 Compare September 21, 2020 19:50

arajasek force-pushed the refactor/net-upgrade branch from cc99ecd to 1dc69e3 Compare September 22, 2020 05:35

Base automatically changed from refactor/net-upgrade to master September 23, 2020 09:24

Stebalien force-pushed the asr/spec-v1 branch from 2b22715 to 62ae8ef Compare September 23, 2020 22:37

Stebalien reviewed Sep 24, 2020

View reviewed changes

chain/actors/version.go Show resolved Hide resolved

chain/actors/builtin/builtin.go Show resolved Hide resolved

arajasek requested a review from raulk as a code owner September 25, 2020 00:53

arajasek and others added 8 commits September 25, 2020 12:49

Introduce v1 actors

ebad0de

fixup v1 actors for new methods

35562bd

Also, correctly handle multiple ADT versions.

compile fix

858f119

fix runtime reflection for upgrade

8806f27

correctly load new actors

edb31e6

move version to actors, from builtin

87351fa

Otherwise, we're going to end up with an import cycle between the adt and this version.

add a generic load method for actor state

d9656f5

This will make it easier to load arbitrary actors. We can: * Type switch (sort of unsafe, may want marker methods?) * Use this with `vm.MutateActorState`.

Stebalien force-pushed the asr/spec-v1 branch from 1438904 to 8b35f48 Compare September 25, 2020 19:49

Stebalien added 7 commits September 25, 2020 12:51

rebase fixup

7d3bd14

fix vm version

b8d9d78

add v1 support to the state tree

271ceb9

continue expanding vm abstraction layer

5f3160c

Merge branch 'master' into asr/spec-v1

947d1e2

fix actor imports

2049daa

rename actors v1 -> actors v2

ca9448b

The actual actors version is v2, not v1. Using Version1 internally was really confusing.

Stebalien changed the title ~~Introduce v1 actors~~ Introduce v2 actors Sep 28, 2020

use actor state abstraction in payment channel test

6fcf57e

Stebalien force-pushed the asr/spec-v1 branch from 60bc131 to f8b2022 Compare October 5, 2020 18:14

fix tests to use v2 types

9621dbb

arajasek commented Oct 5, 2020

View reviewed changes

add some more policy toggles for testground

17846aa

Stebalien added a commit to filecoin-project/oni that referenced this pull request Oct 5, 2020

update oni for specs-actors v2 upgrade

6b0e3bd

filecoin-project/lotus#3936

Stebalien added a commit to filecoin-project/oni that referenced this pull request Oct 5, 2020

update oni for specs-actors v2 upgrade

dbee44e

filecoin-project/lotus#3936

fix lotus soup build

1e78cd2

Stebalien force-pushed the asr/spec-v1 branch from 48c9c31 to 1e78cd2 Compare October 5, 2020 21:37

arajasek and others added 3 commits October 5, 2020 18:39

Add some comments

6d25f3d

fix conformance tests

89b88c1

make state diffing work with partial state trees

16243b9

Stebalien mentioned this pull request Oct 6, 2020

update oni for specs-actors v2 upgrade filecoin-project/oni#263

Merged

Add type assertion for v2 runtime

14ad91c

arajasek mentioned this pull request Oct 6, 2020

Bugfix: Runtime's Receiver() should only return ID addresses #3589

Merged

dirkmc reviewed Oct 6, 2020

View reviewed changes

This was referenced Oct 6, 2020

Add funds that have left FilReserve to circ supply #4160

Merged

Update miner eligibility checks for v2 actors #4188

Merged

Set WinningPoStSectorSetLookback to finality post-v2 actors #4190

Merged

Add a CLI command to set a miner's owner address #4189

Merged

magik6k approved these changes Oct 6, 2020

View reviewed changes

nonsense pushed a commit to filecoin-project/oni that referenced this pull request Oct 6, 2020

update oni for specs-actors v2 upgrade (#263)

0017ea8

filecoin-project/lotus#3936

Stebalien added 3 commits October 6, 2020 11:42

set genesis name in test

4a6e9c9

only enable gas tampering if the breeze upgrade is enabled

256be28

fix base fee tests for different smoke heights

b1818a1

arajasek merged commit 8739b93 into next Oct 6, 2020

arajasek deleted the asr/spec-v1 branch October 6, 2020 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce v2 actors #3936

Introduce v2 actors #3936

arajasek commented Sep 21, 2020 •

edited by Stebalien

Stebalien commented Sep 24, 2020

arajasek Oct 5, 2020

arajasek commented Oct 5, 2020

arajasek commented Oct 5, 2020

dirkmc left a comment

magik6k commented Oct 6, 2020

magik6k left a comment

		// This error is going to be really nasty.
		return cid.Undef, xerrors.Errorf("network upgrade failed: %v", msgs.Messages())

Introduce v2 actors #3936

Introduce v2 actors #3936

Conversation

arajasek commented Sep 21, 2020 • edited by Stebalien

Stebalien commented Sep 24, 2020

arajasek Oct 5, 2020

Choose a reason for hiding this comment

arajasek commented Oct 5, 2020

arajasek commented Oct 5, 2020

dirkmc left a comment

Choose a reason for hiding this comment

magik6k commented Oct 6, 2020

magik6k left a comment

Choose a reason for hiding this comment

arajasek commented Sep 21, 2020 •

edited by Stebalien