Skip to content
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

Add new operators to avoid pedersen commitments in amount validation #276

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

St333p
Copy link
Contributor

@St333p St333p commented Sep 30, 2024

Validation for NIA contracts currently involves pedersen commitments to verify that the sum of inputs equals the sum of outputs. Since bulletproofs are still not an option, amounts are included in clear within the consignment. Hence, given pedersen commitments are constructed by the receiver during validation, this currently brings no advantages over a simple sum verification, but we still want to keep them so that bulletproof can be added in a backwards compatible update.

This PR introduces 3 new AluVM operators that behave like their pedersen commitment counterparts but only do simple sums. This should bring the following advantages:

  • no cryptographic operations where they're not necessary, which should simplify audit
  • no dependency on secp256k1-zkp in case we put pedersen commitment operations behind a feature flag
  • performance improvement, even though this doesn't seem to be a bottleneck for now

The new operators can be used by schema developers to create schemas corresponding to NIA and CFA that only work with revealed state.

@St333p St333p changed the title add new operators to avoid pedersen commitments in amount validation Add new operators to avoid pedersen commitments in amount validation Sep 30, 2024
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 52 lines in your changes missing coverage. Please review.

Project coverage is 13.0%. Comparing base (7ae9760) to head (0f1b25e).

Files with missing lines Patch % Lines
src/vm/op_contract.rs 0.0% 52 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #276     +/-   ##
========================================
- Coverage    13.2%   13.0%   -0.2%     
========================================
  Files          29      29             
  Lines        3883    3932     +49     
========================================
  Hits          513     513             
- Misses       3370    3419     +49     
Flag Coverage Δ
rust 13.0% <0.0%> (-0.2%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RGB-WG RGB-WG locked and limited conversation to collaborators Sep 30, 2024
@RGB-WG RGB-WG unlocked this conversation Sep 30, 2024
@dr-orlovsky
Copy link
Member

dr-orlovsky commented Oct 3, 2024

Well, I don't think this is going to work in the real-world cases: once we add bulletproofs and just a single user apply them, he will lose all his assets. This is a footgun.

That's why I was saying it is not that easy: to do a proper support for non-pedersen fungible state you need to introduce a completely new type of fungible state into Core, re-write state management in RGB Std and re-write the whole body of RGB20 interfaces

@fedsten
Copy link
Member

fedsten commented Oct 4, 2024

once we add bulletproofs and just a single user apply them, he will lose all his assets.

Why would a user apply bulletproof to an asset using a schema that cannot support bulletproof? He would need a wallet that supports this. If we are concerned about evil/stupid wallet developers implementing buttons in the UI that lead to the destruction of the user's assets there are already plenty of simpler ways to do it.

@dr-orlovsky
Copy link
Member

Why would a user apply bulletproof to an asset using a schema that cannot support bulletproof?

This PR doesn't introduce such schema, so there is no such thing as a "schema that does not support bulletproof"

@fedsten
Copy link
Member

fedsten commented Oct 7, 2024

This PR doesn't introduce such schema

But it introduces new AluVM operators that would make it possible for schema developers to create such schemas, i.e. schemas that don't rely on pedersen commitment to verify that the sum of inputs equals the sum of outputs and as a consequence would not be able to support bulletproof whenever it becomes available.

@dr-orlovsky
Copy link
Member

Yes, and this converts additions of bulletproofs in the future from fast-forward into a consensus-breaking push-back. Basically, we would be able to add bullteproofs with this only with a new RGB v2, backward-incompatible to RGBv1

@St333p
Copy link
Contributor Author

St333p commented Oct 8, 2024

I honestly don't understand how this bulletproof addition will work then. Is it an RGB-wide update that applies to all schemas regardless if they need it or not? What about existing schemas (like UDA) that have no pedersen commitments in validation, will all holders lose those assets when we'll add bulletproofs?

I also tried creating a new NIA-like schema with these new operators and everything seems to work correctly, so I'd say the one issue we should address is how to selectively apply the bulletproof update only to schemas that require it. This would allow both UDA and a new schema using the operators from this PR to be unaffected by a bulletproof addition.

@dr-orlovsky
Copy link
Member

Is it an RGB-wide update that applies to all schemas regardless if they need it or not?

Please forget about schemas in context of bulletproofs; they are unrelated! Bulletproofs apply at the level of RGB basic data structures, much before any schema do exist. Also, UDA has nothing to do with bulletproofs - this is a specific schema, existing outside of rgb-core on the application level.

I also tried creating a new NIA-like schema with these new operators and everything seems to work correctly, so I'd say the one issue we should address is how to selectively apply the bulletproof update only to schemas that require it.

Yes, as I said it will work until we activate bulletproofs :) Thus, with this PR we would either be unable to ever activate them - or their activation would mean loss of the assets using scripts with the opcodes introduced in this PRs (or there would be two RGB versions, incompatible with each other).

I honestly don't understand how this bulletproof addition will work then.

Not just bulletproof: if you add something to RGB (or any client-side validation) which was valid before, but becomes invalid later, than this is called a push-back, which means loss of assets affected by those features - or a protocol split into two incompatible versions.

If you introduce opcodes from this PR, they will be working only when fungible state is not stored as Pedersen commitments (i.e. bulletproofs are not active). Once they are active, the state got concealed and this opcodes will fail for the users - meaning a push-back ("something was valid, becomes invalid").

The only right way to allow non-Pedersen fungible state is to add a new data type to schema and operation modules; and re-write a lot of code in validation etc. Basically we are talking about few thousands lines of consensus code changes...

@dr-orlovsky
Copy link
Member

This is how the removal of bulleptroofs means: #279

@St333p
Copy link
Contributor Author

St333p commented Oct 14, 2024

Please forget about schemas in context of bulletproofs; they are unrelated! Bulletproofs apply at the level of RGB basic data structures, much before any schema do exist. Also, UDA has nothing to do with bulletproofs - this is a specific schema, existing outside of rgb-core on the application level.

I brought up UDA just an example of a usecase that requires some state that should not be concealed. For instance, RGB21 supports fractioning non-fungible assets, which seems to require some sort of fungible-like state. Would that be concealed in a future bulletproof update as well?

The only right way to allow non-Pedersen fungible state is to add a new data type to schema and operation modules; and re-write a lot of code in validation etc. Basically we are talking about few thousands lines of consensus code changes...

There is a need for schemas that only work with revealed state regardless of any future updates enabling concealed fungible state. At the very least, it would be pretty weird if a system that "was designed to allow everything that is possible with blockchain-based smart contracts" could not support simple fungible assets without computationally intensive privacy features.

@dr-orlovsky
Copy link
Member

dr-orlovsky commented Oct 16, 2024

For instance, RGB21 supports fractioning non-fungible assets, which seems to require some sort of fungible-like state. Would that be concealed in a future bulletproof update as well?

With this PR is no more concealements of the state at the consensus level, as well as no more difference between fungible and non1fungible state. It all becomes implementation detail up to specific schemata. They way it would work will depend on how you will write the schemata logic; it is Turing-complete and you can do whatever you like.

There is a need for schemas that only work with revealed state regardless of any future updates enabling concealed fungible state.

See the answer in previous paragraph.

At the very least, it would be pretty weird if a system that "was designed to allow everything that is possible with blockchain-based smart contracts" could not support simple fungible assets without computationally intensive privacy features.

The design goal for RGB was always agreed between all stakeholders as "we need system where privacy is non-optional, even at the cost of poor UX". But now, taking into account the combination of the following facts

  1. privacy is still not working since we wait for bulletroofs to be completed for 5 years and they are still not there;
  2. history compression with zk-STARKs is much more supreme both in terms of privacy and UX;
  3. Pedersen commitments and confidential state is incompatible (or poorly compatible) with zk-STARKs history compression

we have no other reasonable option than to remove Pedersen commitments and state concealment altogether, which will not make privacy worse and will fix the UX. This PR does exactly that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants