-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix blank transition type #6
Conversation
@dr-orlovsky this is needed in a 0.8 release, could you please give it a look? |
Sure. IDNK how I missed it - I was going thought all PRs in all repos - and still missed this one somehow... Pls let me know if anything else is missed too |
I looked into the issue details. Impressed by the level of investigation you had to do. Thank you for the analysis. I am considering several options of fixing the issue; I hope by Monday I will get it fixed. Unfortunately I think that just changing a constant will be a very temporarily one, so I will try to work out something more reliable. |
@dr-orlovsky It may be a dumb question, but having a dedicated transition type for blank transfers wouldn't decrease privacy? My reasoning is that when inspecting the consignment it would be possible to distinguish past blank transitions, and therefore it would be possible to associate some UTXOs to the same owner. Probably this is not an issue and I'm missing what is the advantage of having a different constant for blank transitions. I would really appreciate if you could explain it to me |
I am not sure I understand the question. For me a concept of "blank state transition" looks unavoidable: if assets under two (or more) different contracts are assigned to the same UTXO, and we need to move assets under one of the contracts, the RGB Core still have to move the other assets as well - while it was not provided with any information from the wallet (which may not even know how that different contract behaves and what "moving assets" means). Thus, a concept of "blank state transition" had appeared, which is how RGB Node can construct a state transition moving everything under any RGB contract from one UTXO to a new one (using change as a new one). I agree that this concept provides privacy leak (not an on-chain though), but over the years I have not found any better alternative: the only "alternative" is to make user loose all the assets under other contracts which are on the same UTXO. The final design for constructing the blank state transition will be the following: schema developers will provide AluVM scripts which do a construction of the state transition, and these scripts will be imported into RGB Node and used by it to do them in any arbitrary way the schema devs want. At that stage schema devs may create untrivial things, and the privacy leak risk will be subsequently reduced. This additional AluVM scripts are not, and will not be a part of the consensus data, so the scripts can be upgraded to the new version. They will be a part of the upgradable "metadata package" supplied with the contracts/schemata (like asset images etc); they will be validated by the RGB Node (not RGB Core!) by checking signatures (the scripts must be signed by the same developers which created schema/contract). Before that is implemented (still a lot of work) we default to a hardcoded "embedded blank transition constructor", which is yes, a bit privacy-leaking (but we can't do anything better). One thing which may help now is getting rid of the fixed field ids and transition ids, which I already figured out how can be accomplished: RGB-WG/rgb-core#116 |
Now I was able to recall that this was actually indented: TRANSITION_TYPE_RIGHTS_SPLIT is a form of BLANK_TRANSITION_TYPE! So the bug must be in something else.
This will not work as expected, since TRANSITION_TYPE_VALUE_TRANSFER is not a blank transition: it does not allow transfer of all exiting owned rights to a new UTXO (while TRANSITION_TYPE_RIGHTS_SPLIT allows that). The rule for the blank transition is to be able to transfer all types of state to the new UTXO. That is exactly what TRANSITION_TYPE_RIGHTS_SPLIT does - and not what TRANSITION_TYPE_VALUE_TRANSFER does. |
@zoedberg can you please re-articulate what is not working with blank transitions specifically such that I can come with a fix? |
Looks like the solutions you propose are backwards-incompatible and quite expensive to implement, so couldn't they be a better fit for the 0.9 branch? As detailed in the PR , the current code for BLANK_TRANSITION_TYPE (0x8000) matches by chance TRANSITION_TYPE_RIGHTS_SPLIT in rgb-core (https://github.com/RGB-WG/rgb-core/blob/master/src/vm/embedded.rs). The current implementation has an issue but the code still works for rgb20, meaning the BLANK_TRANSITION_TYPE constant is not enforced and the change in PR #6 is not breaking anything. PR #6 allows rgb21 to work while staying compatible with version 0.8, so probably the best thing would be to merge it and then design a better solution for 0.9 |
@fedsten by no means I propose any breaking changes for v0.8, since a minor update can't have breaking changes! However this PR "fixes" bug by introducing a different one: it will lead to the loss of the non-fungible rights (secondary issuance, renomination) if were merged. That is why I am asking in the last comment @zoedberg to provide details of her investigation so I can figure a fix for the bug without any breaking new features. |
If this was intended then the only bug I see is that costant, The reason I thought I'm not sure I completely understood what you said, but I think I'm close to understanding the reason we need I always assumed that it was possible to create more @dr-orlovsky Could you please tell me if this is the reason and, if so, could you explain why this limitation is there? |
@zoedberg let me try to answer different questions and points from your message in some order:
I think this PR should be closed, since it does not fixes the bug and instead introduces a new one. |
What I am failing to understand (and to reproduce) is the exact workflow which leads to the buggy behavior. @zoedberg can you please provide once again that explanation - and also give any details and suggestions which are coming from your investigation of the bug? |
This I understand and already knew.
I don't understand why you say "Thus", to me (1) doesn't explain why we require the definition of a rights split transition.
I understand this, but I'm asking: wouldn't it be possible to move rights with dedicated transitions (a transition per right) to the same change UTXO? Example: I have a
It depends on your definition of bug, to me it could easily cause a bug so it's kinda a bug, IMO a comment on top of
The bug I've reported:
is not a bug that you can see with rgb20. In rgb21 schema we didn't define any
@dr-orlovsky You can close it (but I highly recommend to at least use the costant from rgb-core or add a comment) and in rgb-lib I can write a custom |
Hm, looking at the NFT schema on the link, I have to say that I am not sure it will work in the long run: it mixes very different concepts and uses them in a non-standard way. In fact, it has nothing to do with NFTs: it is a fungible asset schema (since you can issue any amount of the assets using it) plus some data you named "engraving", which can be transferred separately from the assets and have no validation rules attached. Finally, lack of the rights split will lead to the situation when an engraving and asset are assigned to the same UTXO, the assets or engraving will be lost - and RGB Node will get stuck (the situation will require a manual state transition creation with user deciding which state to loose) I am not trying to convince you in anything, I am just describing how RGB validation works, and that is a fact written in the code. If you do not believe me on how the code works, than you can always check it directly :) But I clearly see that the lack of established terminology leads to bugs, and I will work on writing out definitions of the core RGB terms to make sure we all speak the same languague, for instance we call "rights" the same thing (currently we do not). |
I do not know how to explain without drawing. I will try once more. State transition has many-to-many relations with a set of owned rights (owned right is a right over part of the RGB contract state, like certain amount of assets, owned by a party of the contract, The party owning the state is exactly the same party which can spend/owns bitcoins on the UTXO the state is assigned to). So the state transition is the update to multiple owned rights (multiple parts of the state). At the same time state transition CAN touch only the rights which are allowed by the schema (the "closes" part of the transition definition). For instance, the "Engraving" state transition in your schema is allowed to close single-use-seals related to one or more of asset ownership rights and any amount of "engraving" ownership rights, whatever that engraving is (right now it is any file) - and create more rights of that types in any proportion: So "rights split"/"blank transition" is the special state transition which is allowed to update ownership of any rights defined in the schema. The "engraving" state transition in your schema (presumably by occasion) does exactly that. Unfortunately, It has a non-standard state transition id, so RGB Node can't guess that this is the type of the transition it should use to do the blank transition - and there is no proper state transition matching the required id. So that is the bug we are looking for, and it is in the NFT schema, not in the RGB std lib nor RGB core lib. Now, going back to why we need that "blank transition" or "rights split": lets assume schema defines rights of two types - |
Regarding this PR: you can change it to use the constant from the RGB Core - or do a new PR closing this. Than that can be merged. |
Tried to start defining main terms which I propose to stick: https://github.com/orgs/RGB-WG/discussions/218 Please feel free to ask questions there if some of the terms are not clear |
@zoedberg I wrote RGB terms glossary where also the situation with blank transitions/rights split, and the meaning of "rights" term in RGB is explained: https://github.com/orgs/RGB-WG/discussions/219 |
Yeah, we know this is not an NFT, we wanted collectibles so using fungibles schema with some little adjustments (added engraving and removed most other features) seemed to us the better approach, this way validation didn't need to be changed and is already ok. About engraving, probably we are imagining different features, but for us this way works good. We don't think engraving requires validation.
As I told you before, we voluntarirly burn engraving right to prevent the consignment from growing unecessarily. Since this and since the only other right we defined is the ownership one, our schema can work even without rights split.
Of course I'll check ;)
I actually think that this design, in which rgb std lib and rgb core lib need to know constants of schemas, is not very good. This easily leads to invalid schemas (as we are seeing now with rgb21). Moreover I don't think that the tool that will help writing schemas is the best way to solve this, since it will still be possible to manually create an invalid schema.
This doesn't answer my question. Please, read this again:
@dr-orlovsky In other words, I'm asking: is it possible to commit multiple transitions for the same contract in the same bitcoin transaction? Please I would appreciate a yes/no answer and also an explanation. |
Only a single state transiton is allowed per bitcoin witness transaction input. If the transaction has multiple inputs (spends multiple outputs with seals) than it can have one state transition per input (or less). |
Superseded due to v0.10 and new contract interfaces In #23 and RGB-WG/rgb-core#145 blank transitions have become built-in at the consensus level and do not require anymore manual coding. |
While testing RGB with a draft RGB21 schema we discovered a bug with blank transitions that does not happen on RGB20 (only for a coincidence).
In the
blank
rgb-std method the constructedTransition
gets assigned with aBLANK_TRANSITION_TYPE
transition type. This constant (value0x8000
) is defined in the same file of the method but is not imported in any other RGB project.Since the hunted bug was that blank transitions (only of RGB21 contracts) weren't used as RGB "inputs" in the consignment created with the
consign
rgb-node API, I started debugging that. Inspecting the consignment construction code I've noticed thatCONTRACT_TRANSITIONS
is retrieved by looping on the schema transition keys:These keys are defined at a schema-level, you can find the ones used for RGB20 here.
Looking at those
TransitionType
values I've noticedRightsSplit = TRANSITION_TYPE_RIGHTS_SPLIT
.TRANSITION_TYPE_RIGHTS_SPLIT
comes from rgb-core and has value0x8000
, the same ofBLANK_TRANSITION_TYPE
.To prove the bug was there I just changed the value of the
BLANK_TRANSITION_TYPE
to0x8002
(any number outside RGB20TransitionType
values should do the trick) and all tests on RGB20 blank transitions stop working.I believe that
BLANK_TRANSITION_TYPE
andTRANSITION_TYPE_RIGHTS_SPLIT
having the same value is just a coincidence and we need a proper fix.Currently I've fixed the issue by changing
BLANK_TRANSITION_TYPE
with aTRANSITION_TYPE_VALUE_TRANSFER
. Not sure this is the desired fix since I can see some other solutions, but this seems the "correct" one to me.