v0.11 fixes #285
v0.11 fixes #285
Conversation
|
CI fails because this needs other repos in the epic to be patched |
There was a problem hiding this comment.
Reviewed first two commits, up to cf40921
Concept ACK.
Approach in cf40921 is NACK: a contract must commit to the Layer1. Yes, it shouldn't commit to AltLayers, but it must now commit to a specific layer1.
Before, all contracts were multi-layer; and thus supported bitcoin + maybe some other layers.
Now contracts are single-layer, and there is no way how to find whether it is bitcoin or liquid or something. It must be specified as a strong commitment in the contract Genesis, like we have that in v0.12,
I thought that since the genesis always has some assignments it would be enough to check that those assignments are all defined on the same layer 1. But I'll add a layer 1 to the genesis to make this more explicit. |
|
@dr-orlovsky added layer1 to genesis, here and in related PRs |
Consensus doesn't require genesis to have at least one assignment. For instance, in RGB30, genesis doesn't have any. |
There was a problem hiding this comment.
Three other general notes:
- I do not see how 3030a3f handles reorgs; a new enum field is not used at all
- I believe we do not need bundles to commit to the close method, that is excessive
- The support of multi-blockchain contracts is still there. Overall, the type
XChainneeds to be removed - and you will see how many remnants are still haven't dealt with.
| )); | ||
| continue; | ||
| } | ||
| if self.close_method != bundle.close_method { |
There was a problem hiding this comment.
Since contracts are monomorphic over seal type, we do not need close_method in bundles anymore
| @@ -581,10 +600,10 @@ impl< | |||
| }); | |||
| continue; | |||
| } | |||
| if !self.layers1.contains(&seal.layer1()) { | |||
| if self.layer1 != seal.layer1() { | |||
There was a problem hiding this comment.
Wait, seals always belong to the same layer1, the contract belongs to.
@dr-orlovsky Please start reviewing fixes from RGB-WG/rgb-tests#30. Many fixes required changes to multiple repos. For example the "handle RGB reorgs" fix required changes to both rgb-core and rgb-std, you can see this in RGB-WG/rgb-tests@2d00297
I'm aware of this but removing the close method from the bundle and the XChain from multiple objects requires a lot of refactor so we decided that for know we accept to have some redundant bytes in the consignment and some extra unneeded code to avoid making too difficult reviewing the PRs. Especially if no one proves that keeping them could cause a bug/attack. We'll do some refactor PRs after we agree these ones are fixing known bugs or adding limitations that we agreed on during Lugano (i.e. contract committing to single chain and close method). |
|
Sorry, for the last four days I was sick and kinda braindead... I see your point. Let's discuss it during the call today - there are some risks I see with it. |
This PR is part of a v0.11 fixes epic, see RGB-WG/rgb-tests#30 for an overview.