search

mimblewimble / grin

Minimal implementation of the Mimblewimble protocol.
https://grin.mw/
Apache License 2.0
5.04k stars 989 forks source link

Add multisig bulletproof functions #3645

Closed GeneFerneau closed 3 years ago

GeneFerneau commented 3 years ago

Add functions to create and verify a multisignature bulletproof. Callers must go through multiple steps to create the full bulletproof

Each party must complete each step in the protocol, and the initiator finalizes the multisignature bulletproof

quentinlesceller commented 3 years ago

Looking good at first glance. Do you think adding tests for the ser/deser would be useful?

GeneFerneau commented 3 years ago

Looking good at first glance. Do you think adding tests for the ser/deser would be useful?

I did run into issues with V5 slate changes in grin-wallet, but it was because of how serde interprets enums (took forever to figure that out).

Can take a look at what the ser/deser tests would look like for the node. Are you talking about the intermediate states? The final bulletproof looks the same AFAIU.

In the wallet, I think only the tau*, common_nonce, and partial_commit members need to be serialized. I've added those to the V5 slate format. Still working on multiparty transaction building, so maybe more is needed.

Serialization tests for multiparty bulletproofs are probably better suited in the wallet, IMHO.

DavidBurkett commented 3 years ago

@GeneFerneau If I understand this correctly, the multi-party bulletproofs will be seen as invalid if we try to use the existing verify bulletproof APIs, which is why we're exposing verify_bullet_proof_multi. Is that correct? If so, then we're talking about needing a hardfork to support multiparty bulletproofs?

GeneFerneau commented 3 years ago

@GeneFerneau If I understand this correctly, the multi-party bulletproofs will be seen as invalid if we try to use the existing verify bulletproof APIs, which is why we're exposing verify_bullet_proof_multi. Is that correct? If so, then we're talking about needing a hardfork to support multiparty bulletproofs?

As I understand it, the verify_bullet_proof_multi just allows to verify multiple bulletproofs in one call (instead of multiple calls to verify_bullet_proof). The multi-party bulletproofs are the same as single-party bulletproofs when finalized. The main difference is in how they are built.

No hardfork necessary, AFAIU

DavidBurkett commented 3 years ago

Yep, you're correct. I don't think you even need to use the batch api here though. You're only verifying a single bulletproof by the looks of it.

GeneFerneau commented 3 years ago

Definitely, I can remove it if you want. Just thought it might be useful for if/when the node wanted to do batch verify, maybe during IBD?

DavidBurkett commented 3 years ago

We already use the batch api during IBD. It's called from TransactionBody::validate()

GeneFerneau commented 3 years ago

We already use the batch api during IBD. It's called from TransactionBody::validate()

I'll remove verify_bullet_proof_multi from this PR, then. Seems easy enough to add back, if another use comes up.

GeneFerneau commented 3 years ago

Merged these changes into #3643