Add hierarchical consensus spec

[adlrocha]

Updated version of hierarchical consensus spec: Rendered version

[vukolic]

left few minor comments - amazing job

[matejpavlovic]

First part of my review, covering everything up to and including "Naming".
Reads pretty well so far, the vast majority of comments are just minor fixes.

General comment: The document seems to be mixing the "smart contract / transaction" terminology with the "actor / message" terminology. I think it would be better to pick one (probably the latter) and stick with it.

And of course treat all those comments as suggestions and feel free to ignore (even resolve) them at your own discretion. :)

[matejpavlovic]

Part 2: Up to and including "Cross-net messages".

[jsoares]

I'm about 30% of the way but thought it best to commit this now as I'll have a multi-hour break in my review.

[aakoshh]

I would try to remove this from the interface. The consensus should be described by its methods, it should not have to be known by an ID for the host system, ideally, or if it does, we should elaborate on what those are and why this is important.

As I understand the SubnetActor should be the only one that understands how to validate a Checkpoint that comes from this consensus. If the correct subnet actor has been registered for a subnet, it should not need a type.

Now, perhaps the reference implementation of the actor does need a type, because it can handle multiple ones, but in that case the IDs it an handle should be specific to the reference subnet actor, just an example, and not be part of the common hierarchical namespace.

I think this is how it works in Rust, although not in Go. I'm just saying so the spec isn't misleading people into thinking this is a common type they can't extend without buy-in from the whole community.

[jsoares]

I think I understand your point (you shouldn't need a IANA-style central registry for protocol IDs) but not necessarily your solution. Since the protocols tie into a local (i.e. ex-actor) implementation, how do nodes match the right subnet protocol? Maybe use some UUID-like probabilistically unique ID?

[adlrocha]

I agree with @aakoshh, we shouldn't need Type() in the interface (I am actually going to remove it from the spec for now). It is a consequence of using Go and having a single reference actor for every consensus algorithm.

Since the protocols tie into a local (i.e. ex-actor) implementation, how do nodes match the right subnet protocol? Maybe use some UUID-like probabilistically unique ID?

But also, @jsoares is right here. We'll need to revisit how we do this. My dream would be a "plug-in system" where you provide the WASM bytecode for your consensus implementation (following the right interface) and that is enough for the node to be able to run the protocol. It may be an over-simplification and having that level of abstraction may be impossible, but I feel it is something worth exploring.

[aakoshh]

I thought that we run:

1. A different node software that implements SuperDuperConsensus in the child subnets. It only communicates with the parent subnet nodes via libp2p gossipsub topics. Nobody knows what SuperDuperConsensus is.
2. Except that the validators deploy an SA actor code into the parent subnet. This actor conforms to the expected SA interface in terms of message handling, but it can also receive SuperDuperCheckpoints and knows how to verify them and send them on to the SCA of the parent subnet.

[adlrocha]

You are totally right, actually we used a similar approach to 1. to integrate Tendermint in the MVP. That being said, we want to ship the node with a built-in catalogue of basic consensus implementations (to lower the barriers for users to join/spawn their subnets), but we should definitely support the approach you suggest.

[aakoshh]

Curious: this check has not been mentioned in the subnet actor interface, it just says "sanity checks". From what I saw, first there's the actor state transition in a transaction, and then there's the send. What if somebody implements an actor which doesn't adhere to this rule, kills their subnet actor, and then sends the message to the SCA, but the SCA refuses to kill it?

[adlrocha]

Great question. If I am not mistaken, in the current implementation of the VM side-effects are atomic, meaning that when Kill() is called in the subnet actor, the set of checks are run, and if they pass a side-effect message is sent to SCA::Kill. If the latter fails, the whole message will fail, so there's no way for the subnet actor kill to succeed without the SCA Kill succeeding.

That being said, I don't know if this will change for FVM-M2 (I'll try to find where this is being discussed). I agree that if this is the case we'll need to be careful and revisit how we kill subnets.

[aakoshh]

Interesting. Yeah the semantics seem to be left the the actor coder, because Runtime is way too powerful. It is handed to the actor as-is, and you are expected to use transaction and not call send while doing so. But you are not forced to do so - you can retrieve your state with runtime.state(), do your thing and use send, then use transaction only to set the state to the value you updated earlier.

I agree that it probably won't commit if send fails, but I also didn't understand what send does, whether it executes immediately, or just lines up a message to be done later. Looked like the latter.

I suppose using transactions and not sending messages in the middle prevents some reentrancy bugs, but because it's not enforced and it looks like you have to have discipline to stick to it and code it right, it would be good to hear the full story of where this is heading.

[jsoares]

Review 3/3

I left a few suggestion on the cryptoecon WIP parts, even knowing these are rough and likely to be replaced; I think we anyhow benefit from having a bit more polish there, lest someone start reading from the end :)

[aakoshh]

I understand how Fund and Release work because it's clear who will get tokens in which subnet.

My question is, what happens if Alice in /root/subnet-1/subnet-11 wants to send 10 FIL to Bob in /root/subnet-2/subnet-21? Do cross messages have a notion of the Value included in token transfers between users at all? Or is it only the HC related messages where Value is tracked?

Let's say that HC was able to cater for this use case.

If Alice wanted to send from /root to /root/subnet-2/subnet-21, she would spend 10 in root, get 10 in subnet-2, then she would I assume have to pay the fees for the funds to go into subnet-21. If she doesn't have an account in subnet-2, maybe she'd only be able to forward 10-fees. Bob would get whatever's left.

Going up from /root/subnet-1/subnet-11 to /root would mean the release of her funds, for which she pays the fee in subnet-11, she gets 10 in subnet-1. Then if she didn't have any funds there, she has to pay fees for it to go further up.

Do I get the fee mechanism right? What if it's not a transfer, when we don't now what kind of message we are sending? Does Alice have to have an account in each subnet along the path to pay for the next leg?

[aakoshh]

Or actually if the message goes from /root/subnet-1/subnet-11 to /root then subnet-1 only adjusts the Value, but doesn't actually execute anything, it just passes on the meta, recursively. So it costs nothing for Alice to pass through this layer, right?

Only in the final destination, subnet-21 will her message be delivered, and that is where she will have to be able to foot the bill for the fees, right?

This also means that Value cannot go like this, only via Fund and Release, which makes sense.

[aakoshh]

Or a third variant is that if the child subnet does understand what the message that is encoded in the cross-msg means in terms of Value being transferred, then the parent SCA can trust that the child validators are correctly setting the Value and limit its exposure to the CircSupply.

It releases the exact same amount Alice, if the destination is the current subnet, before delivering the message, or if it's not the current subnet, it just passes on the message without giving Alice anything, or charging fees.

[jsoares]

I think what you're asking is tentatively explained at the end of the document:

With this information, cross-net message will include an array of gas_limit amounts that the message is willing to allocate to each subnet in the hop. Subnets in the path won't be able to charge over the specified gas_limit for the subnet. When a cross-net reaches a subnet for which gas_limit is insufficient, it fails and an error message is propagated back to the source, along with the unspent gas.

I don't think Value is affected at all.

[adlrocha]

Great points, @aakoshh. The design for fee handling hasn't been fully finalized. The current proposal is that the Value doesn't change in-route, and the source address in the source subnet needs to provision in the message value + sum(gas_limit_subnet). In each subnet in the path to the destination at most gas_limit_subnet will be paid (if not the message fails and is not routed further). At destination, the remaining tokens not used for paying fees is deposited in the originators address in the destination subnet.

Actually, I'll update that section with our current thinking so we can start gathering feedback for it.

[aakoshh]

Oh, thanks @jsoares I completely missed that part on my second read. I did not expect it to be at the bottom of the page, rather than colocated with the explanation of bottom-up and top-down message handling.

Thanks @adlrocha, I think I get it. This means (and I think this would be worth pointing out in the text), in my example this does not mean that Alice has to have an account every step of the way; she is charged the full amount in the source subnet, then in the higher level subnets the CircSupply is reduced by Value + remaining-fees, and the validator proposing the block in the given subnet chips away at the value, up to the limit of the fees set for their level, before sending the transaction on with an adjusted value. Not sure if Alice can get any refunds for unspent gas.

[adlrocha]

Oh, thanks @jsoares I completely missed that part on my second read. I did not expect it to be at the bottom of the page, rather than colocated with the explanation of bottom-up and top-down message handling.

Fair point. I will move the section up. The reason why is there is because we are exploring these mechanics as part of our collaboration with CEL, and for completeness we kept everything in the crytpo-econ seection.

Alice has to have an account every step of the way; she is charged the full amount in the source subnet, then in the higher level subnets the CircSupply is reduced by Value + remaining-fees, and the validator proposing the block in the given subnet chips away at the value, up to the limit of the fees set for their level, before sending the transaction on with an adjusted value.

Correct

Not sure if Alice can get any refunds for unspent gas.

She can. The remaining fees are routes with the message to the destination subnet. Once the message executed in the destination subnet, if there are any remaining funds in the message, they are deposited in Alice's address in the destination subnet. Makes sense?

[aakoshh]

Yes, that makes sense.

Although I would not be surprised if users wanted their funds returned to where they came from, so their applications are simplified and don't have to track funds scattered around all the subnets they ever sent messages to. This could happen any time, whenever there is bandwidth, offered as a free service, included in the price of the original hops. In a way it happens anyway if there is an error, to return the unspent fees and provide feedback.

Perhaps the feedback mechanism could be applied in the successful case as well, so a sent message will always get an echo with the remaining fees and the outcome of whether it could be delivered and is stable. The stability could be indicated by the applied nonces being included in a checkpoint. That might not exist to day, but may be worth doing.

I think the IBC protocol also contains acknowledgements going back.

[aakoshh]

Do you think it's possible to charge a flat fee for pass-through messages, ones that only need their meta-data appended to the top/bottom message list? I forget if they need message resolution, I think they do. But they don't get executed as smart contracts.

Or are fees expected to fluctuate by demand? Perhaps have a flat rate in gas, but cost various amount of FIL based on prevalent gas prices?

I can see how it would be a nightmare for users to keep track of where their message bounced, or even provision up front, because the Value is in FIL, while the gas_limit I assume is not, so in the end the final value delivered may be less than they intended.

They might also expect that the network should delay the inclusion of their messages to a time when fees are more reasonable.

[aakoshh]

I suppose if we add price to the fees, then delaying is out of the question as it would necessarily have to hold up all other cross net messages as well, because of the nonces, to avoid out-of-order delivery.

[aakoshh]

Just noting that the mechanism to signal errors and return gas is not clearly reflected in the SCA interface (or SA). If it's some kind of cross message that source actors will receive, it should perhaps be specified so those actors know what they have to handle.

[adlrocha]

I can see how it would be a nightmare for users to keep track of where their message bounced, or even provision up front, because the Value is in FIL, while the gas_limit I assume is not, so in the end the final value delivered may be less than they intended.

Definitely, this really concerns me. We've had a lot of back-and-forth with the CEL team to see what is the best way to have this designed. As mentioned above, I think this deserves its own thread, so let me open a new PR to discuss this design. I'll ping you there.

[aakoshh]

provision in the message value + sum(gas_limit_subnet)

I'm not familiar with the Filecoin gas model, but in Ethereum you have a gas_limit and also a gas_price (in gwei), with the fees being the product of the two.

That suggests you need not just an array of gas_limit but potentially an array of gas_price as well, and you would provision value + sum(gas_limit * gas_price for (gas_limit, gas_price) in subnet_fee_limits) in the source.

[aakoshh]

It would be worth mentioning what happens to over-provisioned gas in the happy path, not just when a message fails. Does the sender get it back in the final destination, or do they trickle back to the source? If the latter, at what cost?

[adlrocha]

That suggests you need not just an array of gas_limit but potentially an array of gas_price as well, and you would provision value + sum(gas_limit * gas_price for (gas_limit, gas_price) in subnet_fee_limits) in the source.

You are right, actually, my proposal was to include an array of gas_fee_limit to implicitly include the gas_limit and gas_price for each subnet you are willing to spend. The naming of the variable (and the description) may not have been the best. This would work, right?

It would be worth mentioning what happens to over-provisioned gas in the happy path, not just when a message fails. Does the sender get it back in the final destination, or do they trickle back to the source? If the latter, at what cost?

That's a great question, we have a first approach implemented in the MVP but I am afraid it not be the best. Actually, I am going to merge this PR and do a follow-up to discuss this design. I think it can be useful.

[adlrocha]

Question to all involved. Do you think that the spec would be easier to read and review if we split it in different files? Should we consider this for a follow-up PR once we get this merged?

I want to address the last few comments from @aakoshh regarding cross-net fees (and move the description earlier in the text), and if no one opposes I was planning to get the current version merged. Let me know if someone needs more time to review 🙏

[jsoares]

Question to all involved. Do you think that the spec would be easier to read and review if we split it in different files? Should we consider this for a follow-up PR once we get this merged?

I don't think so. Change tracking is also easier in a single file. If things get really out of hand (e.g. Filecoin spec), we could have a sidebar + collapsible sections, but I don't think this is big enough to justify that.

I want to address the last few comments from @aakoshh regarding cross-net fees (and move the description earlier in the text), and if no one opposes I was planning to get the current version merged. Let me know if someone needs more time to review 🙏

No objections from my end.

[adlrocha]

Thank you all for the amazing job reviewing the spec 🙏 🎉. I am merging this PR and opening follow-ups to discuss the parts of the spec that need more work. Feel free to do the same if you feel there's anything that needs further discussion.