Init actor sketch #169

whyrusleeping · 2019-03-05T01:09:43Z

Working on the init actor (we've only been talking about it for two years at this point). This will be the new way to create actors (answering questions for other PRs that needs to be able to create actors), it will also track and allocate actor IDs (though there may be some arguments for not tracking it here, i'll go through that later).

With this, we also remove the 'create actor' from the VM context, cleaning that logic up a bit. The weirdness now is that the init actor is a 'priveleged' actor, in that it can make calls that no other actor can make.

dignifiedquire · 2019-03-05T18:48:49Z

actors.md

+
+Parameters:
+
+- code Cid


where do I get this Cid from?

We should have a list of well known CIDs somewhere for the builtin actors

dignifiedquire · 2019-03-05T18:49:15Z

actors.md

+
+    addr := VM.CreateActor(code, msg.Value, DecodeParams(params))
+
+    // TODO: where is this mapping stored?


might as well store it in this actor?

yeah... that doesnt seem like the worst thing to do. I'll come up with some pros and cons in a bit

dignifiedquire · 2019-03-05T18:49:38Z

actors.md

+TODO: the type of params should probably just be variadic set of arguments. Making it be a `[]byte` here kinda sucks
+
+```go
+func Exec(code Cid, params []byte) Address {


is the gas cost for this clear?

no, its not a fixed value if thats what youre asking. This will have to pay gas for whatever code is in the actors constructor.

Should this be added, or should all gas related things be added in one go later?

Let's do gas related stuff later in a big gas push

dignifiedquire · 2019-03-05T18:49:54Z

actors.md

+    // TODO: where is this mapping stored?
+    SetIDMapping(actorID, addr)
+
+    return addr


how do I get the ID from the addr?

I was thinking that nodes would just create and store a reverse index locally. Storing a two-way mapping on-chain seems expensive.

alternatively, I was talking with @frrist about a setup where the primary state tree is a map[ID]Actor, and separately we store a map[Address]ID. This would allow users to shift around keys for their accounts, and maybe other things.

I think we said a map[PubKey]ID when we initially talked about this, is that what we mean by Address here?

I think using the ID as primary identifier in the state tree would be neat. Having a map[Address]ID around, should also address any reordering concerns that could appear due to different ID assignments. We could store this mapping in the InitActor, given it is the actor that actually adds entries to this mapping.

Allowing key changes would need some more thought, because if I simply change my key there is no information about my new address (the one derived from my new key) in the Addresss -> ID map. So this would require having at least an AddMapping(from Address, to ID) call to the InitActor (or the place that manages this mapping)

Yeah, i'm in support of IDs being used as the primary mapping.

dignifiedquire · 2019-03-05T18:50:32Z

actors.md

+    actorID := self.NextActorID
+    self.NextActorID++
+
+    if code == InitActorCodeCid {


I think this should be a white list for now, given there are many actors we don't want other to launch

I actually don't mind people launching their own storage market with its own miners. I think that would be interesting. There could be other storage markets for users who don't want to participate in block production, or something.

If we allow that, then we should have a way to ask the chain directly for the official versions, to reduce chances that anyone is scammed/or accidentally uses a wrong version

yeah... maybe it makes the most sense to not allow other storage markets for now then too. Can always re-enable it later

anorth · 2019-03-10T19:05:43Z

actors.md

@@ -38,7 +74,7 @@ func CreateStorageMiner(pubkey PublicKey, pledge BytesAmount, pid PeerID) Addres
        Fatal("not enough funds to cover required collateral")
    }

-    newminer := VM.CreateActor(MinerActor, msg.Value, pubkey, pledge, pid)
+    newminer := InitActor.Exec(MinerActorCodeCid, EncodeParams(pubkey, pledge, pid))


How about retaining the descriptive InitActor.CreateActor name? Exec is opaque and also limits future change.

We really wanted to stick to unix terminology here, where exec is the accepted terminology for taking a given piece of code and instantiating a new process running it.

dignifiedquire · 2019-03-18T13:10:47Z

actors.md

+- code Cid
+- params []byte
+
+TODO: the type of params should probably just be variadic set of arguments. Making it be a `[]byte` here kinda sucks


@whyrusleeping depends on the level this spec is written, on a low level this has to be []byte serialized in some form to pass through variadic arguments. But if this is meant to describe a high level I would agree

Yeah, I think this can be a little higher level, how things get encoded precisely is discussed elsewhere. But this document should match closely the actual actors that get written.

…on the vmcontext

whyrusleeping · 2019-03-20T17:58:53Z

actors.md

+```go
+func Exec(code Cid, params []byte) Address {
+    // Get the actor ID for this actor.
+    actorID = len(self.AddressMap)


I think relying on the size of this map to keep track of the next ID may end up being error prone, i'm worried that if we allow actors to be deleted, this will result in unexpected behavior

You are not wrong, it just felt nicer to use this given the fact that we don't have deletion atm, but I am fine reverting to keeping track of the id manually

whyrusleeping · 2019-03-20T18:15:43Z

actors.md

+
+    // Ensure that singeltons can be only launched once.
+    // TODO: do we want to enforce this? If so how should actors be marked as such?
+    if IsSingletonActor(code) && self.hasInstance(code) {


tricky. I see what youre doing here, making it so that we can use the init actor itself to launch the other protected actors at network launch. I think maybe we can fake that all for now, and just have a simple whitelist here.

I just realized something, do we have a way to get the code Cid from the actor currently? If not we should have this, such that I can check if a certain actor is spawned from this particular code cid

we should definitely have that, i don't think its exposed right now in an easy to use way

whyrusleeping · 2019-03-20T23:06:58Z

@dignifiedquire thanks for the help! I expanded on the CreateActor call (broke it into the individual steps to make it clearer, since thats not documented anywhere else), and switched back to using the the explicitly tracked actor ID in the state.

whyrusleeping requested review from dignifiedquire and frrist March 5, 2019 01:09

This was referenced Mar 5, 2019

Actor creation #140

Closed

Actor ID Issuance #170

Closed

dignifiedquire reviewed Mar 5, 2019

View reviewed changes

whyrusleeping mentioned this pull request Mar 8, 2019

Design Init actor filecoin-project/venus#509

Closed

anorth reviewed Mar 10, 2019

View reviewed changes

dignifiedquire reviewed Mar 18, 2019

View reviewed changes

whyrusleeping and others added 4 commits March 20, 2019 15:37

first sketch of init actor

f82f0f1

with the init actor we no longer need the ability to create an actor …

ccc0cec

…on the vmcontext

update init actor with new mappings and other cr feedback

f61ca5a

cleanup

f844d70

dignifiedquire force-pushed the feat/init-actor branch from adf9a59 to f844d70 Compare March 20, 2019 14:42

whyrusleeping commented Mar 20, 2019

View reviewed changes

frrist mentioned this pull request Mar 20, 2019

Dump state on BLS signatures progress filecoin-project/venus#2330

Closed

expand on the actor creation logic a bit

bc63438

whyrusleeping merged commit 9ffcbb4 into master Mar 20, 2019

whyrusleeping deleted the feat/init-actor branch March 20, 2019 23:07

nicola mentioned this pull request Mar 21, 2019

Captain log filecoin-project/research#103

Closed

frrist mentioned this pull request Apr 2, 2019

Signature Aggregation #210

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init actor sketch #169

Init actor sketch #169

whyrusleeping commented Mar 5, 2019

dignifiedquire Mar 5, 2019

whyrusleeping Mar 5, 2019

dignifiedquire Mar 5, 2019

whyrusleeping Mar 5, 2019

dignifiedquire Mar 5, 2019

whyrusleeping Mar 5, 2019

dignifiedquire Mar 18, 2019

whyrusleeping Mar 18, 2019

dignifiedquire Mar 5, 2019

whyrusleeping Mar 5, 2019

whyrusleeping Mar 5, 2019

frrist Mar 9, 2019

dignifiedquire Mar 18, 2019

whyrusleeping Mar 18, 2019

dignifiedquire Mar 20, 2019

dignifiedquire Mar 5, 2019

whyrusleeping Mar 5, 2019

dignifiedquire Mar 18, 2019

whyrusleeping Mar 18, 2019

dignifiedquire Mar 20, 2019

anorth Mar 10, 2019

whyrusleeping Mar 13, 2019

dignifiedquire Mar 18, 2019

whyrusleeping Mar 18, 2019

dignifiedquire Mar 20, 2019

whyrusleeping Mar 20, 2019

dignifiedquire Mar 20, 2019

whyrusleeping Mar 20, 2019

dignifiedquire Mar 20, 2019

whyrusleeping Mar 20, 2019

whyrusleeping commented Mar 20, 2019


		addr := VM.CreateActor(code, msg.Value, DecodeParams(params))

		// TODO: where is this mapping stored?

Init actor sketch #169

Init actor sketch #169

Conversation

whyrusleeping commented Mar 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whyrusleeping commented Mar 20, 2019