Experiments on v0.11 #285

zoedberg · 2025-01-15T15:57:15Z

This PR is part of a v0.11 fixes epic, see RGB-WG/rgb-tests#30 for an overview.

zoedberg · 2025-01-15T16:11:39Z

CI fails because this needs other repos in the epic to be patched

dr-orlovsky

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,

zoedberg · 2025-01-16T11:06:06Z

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.

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.

zoedberg · 2025-01-16T16:32:34Z

@dr-orlovsky added layer1 to genesis, here and in related PRs

dr-orlovsky · 2025-01-16T17:52:53Z

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

Consensus doesn't require genesis to have at least one assignment. For instance, in RGB30, genesis doesn't have any.

dr-orlovsky

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 XChain needs to be removed - and you will see how many remnants are still haven't dealt with.

src/validation/validator.rs

zoedberg · 2025-01-17T09:34:46Z

I do not see how 3030a3f handles reorgs; a new enum field is not used at all

@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 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 XChain needs to be removed - and you will see how many remnants are still haven't dealt with.

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).

dr-orlovsky · 2025-01-21T14:41:51Z

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.

dr-orlovsky · 2025-02-11T16:41:44Z

All CI are failing.

Also, I'd like to ask you to provide the detailed description of what specific changes this PR introduce

Finally, can you please use the same style of the commitment messages as used everywhere in the history, based on https://www.conventionalcommits.org/en/v1.0.0/, with two differences:

the detailed body is optional
if it is a feature, it should start with the module (directory) name it is related to (like schema: ... etc).

dr-orlovsky

First review of some parts (haven't got to seals and validation yet)

doc/Commitments.md

dr-orlovsky · 2025-02-12T12:48:48Z

src/operation/layer1.rs

+)]
+#[repr(u8)]
+#[derive(Default)]
+pub enum ChainNet {


Regarding the ChainNet: I do believe that we need to keep testnet flag in genesis alongside Chainnet value. The reason is simple: while there might be multiple testnets per blockchain (like in case of Bitcoin, there is testnet3, 4, signet etc); the contract is either has a value (i.e. mainnet) or doesn't (testnet) and can operate any testnet with no further clarification. Otherwise each time a new testned is made we would have to change the consensus by updating ChainNet type.

Thus, my proposal is:

Make enum ChainNet { Bitcoin, Liquid } and that's it.

Add testnet: bool to the Genesis back.

We think testnet is not precise enough. Treating test networks differently from mainnet makes some tests possible only using mainnet, while the purpose of test networks is to simulate the behavior of mainnet without actually using it. Moreover we added a check on the resolver to detect if the resolver chain-network pair is different from the contract one, making some errors impossible (see the issue_on_different_layers test in rgb-tests). We already simulated what happens when a new ChainNet variant is added and old contracts still work so we don't see any issue in that regard.

There is a clear issue I have described: "each time a new testned is made we would have to change the consensus by updating ChainNet type.".

Both in RGB v0.11.0 and in v0.12 testnet was a flag separate from blockchain. If we intend to change that, this should be discussed as a separate proposal and implemented once a common decision is achieved - like with each one past consensus changes we had.

I don't see an issue with changing rgb-core each time a new testnet needs to be added, for the following reasons:

this is a consensus change because it affects rgb-core, but it doesn't mean there would be any consensus-breaking when it comes to contracts, as existing contracts will continue to work fine and only new contracts using the new testnet will require an updated RGB stack

we're talking about rare events (e.g. testnet5) that take time to settle, so the time-frame of the update process is reasonable

this allows testing conditions that the testnet bool alone prevents us from

ChainNet already existed (in rgb-std) and has just been moved to rgb-core, so it's not an addition of new untested code but just a small change that is IMO a very good tradeoff

consensus code (i.e. rgb-core) changes will always be needed, e.g. when addressing bugs or updating dependencies

To me, 1 to 3 are the main points, others are there to make it clearer that it's also a low-impact change.

src/operation/layer1.rs

src/operation/commit.rs

dr-orlovsky · 2025-02-12T12:52:19Z

src/operation/fungible.rs

+    /// Original value in smallest indivisible units
+    pub value: FungibleState,
+    /// Field necessary only to avoid clash with RevealedValue during yaml deserialization
+    pub concealed_dummy: (),


Nope, we can't add something because of "YAML" to the consensus level. It's like making Linux kernel depend on JSON specifics

This is temporary, it's there because the removal of pedersen commitments was already causing a lot of changes and we wanted to keep the diff small (also easing the review). We're going to refactor this on the next batch of PRs.

Why do we need "temporary" things in consensus layer? It will be easy to forget to remove it later, so it will stay forever.

Can you explain the problem with YAML; I am quite sure there can be a simple solution (since I had a lot of YAML problems in the past and know some gotchas how to overcome them).

Why do we need "temporary" things in consensus layer? It will be easy to forget to remove it later, so it will stay forever.

Because we agreed that this approach was ok on a different communication channel. We also agreed there's no reason to worry about details since this version is experimental for now. We can open a tracking issue if you are worried that we may forget about things. Moreover, we decided that you should review the changes only when we'll have finished everything, so I think we should just discuss these matters at that point and avoid adding any friction right now.

Can you explain the problem with YAML; I am quite sure there can be a simple solution (since I had a lot of YAML problems in the past and know some gotchas how to overcome them).

The code comment "Field necessary only to avoid clash with RevealedValue during yaml deserialization" means that since RevealedValue and ConcealedValue would be identical without adding the concealed_dummy field, the deserialization wouldn't know if it should deserialize the value into one struct or the other. This was the fastest temporary fix we found.

Anyway, I've just pushed the removal of the ConcealedValue so that there's no need for this dummy field anymore.

src/operation/fungible.rs

zoedberg · 2025-02-12T14:18:50Z

All CI are failing.

CI is failing because we needed to update to the new bp-core first (which has been released yesterday). I've added a commit to fix that and now the CI should pass.

Also, I'd like to ask you to provide the detailed description of what specific changes this PR introduce

The detailed description of the changes is RGB-WG/rgb-tests#30, as the description of this PR says. Anyway, ignoring chore commits like lint and those commits that have been superseded by others, this PR:

removes support for alternative layer1s and makes the contract commit to a chain-network pair
fixes RGB state when a reorg happens
fixes an undetected double spend in consignment validation
removes pedersen commitments
implements the unified seals by removing anchor dichotomy

Finally, can you please use the same style of the commitment messages as used everywhere in the history, based on https://www.conventionalcommits.org/en/v1.0.0/, with two differences:
* the detailed body is optional

* if it is a feature, it should start with the module (directory) name it is related to (like `schema: ...` etc).

Done. On this matter, I haven't seen this requirement in the contributing instructions, it would be nice IMO if this was documented there along with the set of supported tags.

dr-orlovsky · 2025-02-19T10:13:41Z

Ok, merging this with no review and without approval from my side.

NB: this is alpha-release, and merge into an experimental branch, and using of the new v0.11.1-alpha.1 is discouraged for anyone

dr-orlovsky requested changes Jan 15, 2025

View reviewed changes

zoedberg mentioned this pull request Jan 16, 2025

Tracking: v0.11 fixes RGB-WG/rgb#277

Open

dr-orlovsky requested changes Jan 16, 2025

View reviewed changes

src/validation/validator.rs Outdated Show resolved Hide resolved

src/validation/validator.rs Outdated Show resolved Hide resolved

zoedberg mentioned this pull request Jan 17, 2025

v0.11 fixes BP-WG/bp-core#111

Merged

dr-orlovsky changed the base branch from master to v0.11.1 February 11, 2025 16:37

zoedberg added 12 commits February 12, 2025 11:39

chore: lint code with rust 1.84.0

91a270c

schema: remove support for alt layers

e1f0481

schema: make contract commit to a close method

6bcf339

vm: handle RGB reorgs

952e9e2

fix(validation): detect double spend

b4544bd

operation: add layer1 to Genesis

b461511

schema: remove pedersen commitments

5d07b15

vm: add WitnessOrd::Ignored

f1e1d07

operation: remove XChain

cca8df4

validation: unified seals

1954130

operation: chain net in genesis + resolver check

4265485

chore: remove bp-std

5111760

dr-orlovsky requested changes Feb 12, 2025

View reviewed changes

zoedberg added 2 commits February 12, 2025 14:26

doc: fix Genesis fields

a2013df

chore: update bp-core to 0.11.1-alpha.1

5883d22

zoedberg force-pushed the v0.11_fixes branch from cefaa1b to 5883d22 Compare February 12, 2025 14:07

zoedberg requested a review from dr-orlovsky February 12, 2025 14:17

operation: remove concealed states

af35bcd

dr-orlovsky merged commit 261f655 into RGB-WG:v0.11.1 Feb 19, 2025

dr-orlovsky changed the title ~~v0.11 fixes~~ Experiments on v0.11 Feb 19, 2025

This was referenced Feb 19, 2025

Experiments with v0.11 RGB-WG/rgb-std#295

Merged

Experiments on v0.11 RGB-WG/rgb#276

Merged

Experiments with v0.11 RGB-WG/rgb-interfaces#16

Merged

Experiments with v0.11 RGB-WG/rgb-schemata#51

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experiments on v0.11 #285

Experiments on v0.11 #285

zoedberg commented Jan 15, 2025

zoedberg commented Jan 15, 2025

dr-orlovsky left a comment •

edited

Loading

zoedberg commented Jan 16, 2025

zoedberg commented Jan 16, 2025

dr-orlovsky commented Jan 16, 2025 •

edited

Loading

dr-orlovsky left a comment •

edited

Loading

zoedberg commented Jan 17, 2025

dr-orlovsky commented Jan 21, 2025

dr-orlovsky commented Feb 11, 2025

dr-orlovsky left a comment

dr-orlovsky Feb 12, 2025

zoedberg Feb 12, 2025

dr-orlovsky Feb 14, 2025 •

edited

Loading

nicbus Feb 17, 2025

dr-orlovsky Feb 12, 2025

zoedberg Feb 12, 2025

dr-orlovsky Feb 14, 2025

zoedberg Feb 17, 2025

zoedberg commented Feb 12, 2025

dr-orlovsky commented Feb 19, 2025

Experiments on v0.11 #285

Experiments on v0.11 #285

Conversation

zoedberg commented Jan 15, 2025

zoedberg commented Jan 15, 2025

dr-orlovsky left a comment • edited Loading

Choose a reason for hiding this comment

zoedberg commented Jan 16, 2025

zoedberg commented Jan 16, 2025

dr-orlovsky commented Jan 16, 2025 • edited Loading

dr-orlovsky left a comment • edited Loading

Choose a reason for hiding this comment

zoedberg commented Jan 17, 2025

dr-orlovsky commented Jan 21, 2025

dr-orlovsky commented Feb 11, 2025

dr-orlovsky left a comment

Choose a reason for hiding this comment

dr-orlovsky Feb 12, 2025

Choose a reason for hiding this comment

zoedberg Feb 12, 2025

Choose a reason for hiding this comment

dr-orlovsky Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

nicbus Feb 17, 2025

Choose a reason for hiding this comment

dr-orlovsky Feb 12, 2025

Choose a reason for hiding this comment

zoedberg Feb 12, 2025

Choose a reason for hiding this comment

dr-orlovsky Feb 14, 2025

Choose a reason for hiding this comment

zoedberg Feb 17, 2025

Choose a reason for hiding this comment

zoedberg commented Feb 12, 2025

dr-orlovsky commented Feb 19, 2025

dr-orlovsky left a comment •

edited

Loading

dr-orlovsky commented Jan 16, 2025 •

edited

Loading

dr-orlovsky left a comment •

edited

Loading

dr-orlovsky Feb 14, 2025 •

edited

Loading