[zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature #936

random-zebra · 2019-06-28T16:02:29Z

This PR proposes (as hard fork) a new PublicCoinSpend protocol suitable to be used also with zerocoins version 1. Closes #937

Motivation and overview in the following document:
https://github.com/random-zebra/PIVX-Wiki/blob/master/Developer-Documentation/Specs/zPIV/CoinRandomnessSchnorrSignature.mediawiki

Enforcement height uses placeholder values (one million blocks after the activation of spends v3 for main net and testnet, and 100 blocks for regnet).

Changes

replaces many instances of CBigNum(0), CBigNum(1), CBigNum(2) throughout the code with fixed constants defined in bignum.h.
defines the CoinRandomnessSchnorrSignature using the algorithm described in the wiki.
defines the temporary activation height and also defines an helper function Zerocoin_PublicSpendVersion which returns the version active at a given block height.
defines a convenient method to retrieve the coin version from a PublicCoinSpend class (now that coin version and spend version are different).
defines PublicCoinSpend behaviour for v4.
enforces the PublicCoinSpend version in ContextualCheckZerocoinSpendNoSerialCheck.
provides unit tests for the new v4 PublicCoinSpend class.

furszy · 2019-06-28T16:49:02Z

There we go 👌 , concept ACK.

src/zpiv/zpivmodule.cpp

random-zebra · 2019-06-30T00:08:50Z

Thinking more about it, we can also remove the coin serial number from the coinSpend serialization of v2 coins.
We already have the pubkey. With that, the verifier can compute independently the serial number. This saves other 32 bytes of space and is even more practical (the verifier doesn't need to check that the hashed pubkey matches the serial).

cc2a3ab implements the new spend serialization. A new ExtractSerialFromPubKey method is provided.
The coin version is added (just one byte) to the serialization in order to differentiate v1 coins (I think this is preferable over try-catching for missing fields).
The wiki document has been updated to reflect these changes.

Unit tests show:

Spend v3 size: 218 bytes
Spend v4 (coin v2) size: 221 bytes
Spend v4 (coin v1) size: 148 bytes

Fuzzbawls · 2019-06-30T08:19:54Z

the p2p_pos_doublespend.py regression test needs to be adjusted for this.

random-zebra · 2019-06-30T10:26:32Z

@Fuzzbawls I don't think it's because of this, since that particular test doesn't involve zerocoins and even ends at block 330 (while CoinSpend activation is 350 for v3 and 450 for v4).

Restarting the Travis job seems to have fixed it.

On a related matter, functional tests on regnet for this change will be provided soon.

Fuzzbawls · 2019-07-27T00:28:31Z

Needs Rebase

random-zebra · 2019-07-27T05:41:49Z

Rebased.

Warrows

Concept and code ACK with a few minor nits.
As discussed, keeping the signature for v2 mints in v4 spends is not strictly necessary but given the space available in our blocks and the huge improvements public spends are it is not a problem either.
Unit tests are passing nicely.

src/libzerocoin/ParamGeneration.cpp

src/zpiv/zpivmodule.h

Mrs-X

utACK 764f4d6 with 2 nits:

the indenting changes make it unnecessarily hard to review this PR
why no const CBigNum BN_THREE = CBigNum(3); ?

Is there a 'real-life' way to test this or is all we can do to rely on the tests?

random-zebra · 2019-08-05T20:08:07Z

the indenting changes make it unnecessarily hard to review this PR

It's easier for review to check the commits individually. The indenting change is only in the first commit (which is tagged with [Refactor]).

why no const CBigNum BN_THREE = CBigNum(3); ?

There weren't as many instances. There are still 8 of them tough (in AccumulatorProofOfKnowledge and ParamGeneration) and, since we are at it, it makes sense to do this here too.

Is there a 'real-life' way to test this or is all we can do to rely on the tests?

RegTest has no v1 functionality unfortunately (if anyone wants to add it is more than welcome to do so). I'm updating the publicSpend test to include v4 spends at least for v2 coins.

Other than that, we'll soon protobump a "segregated" testnet to do more functional testing there.

random-zebra · 2019-08-06T01:02:19Z

Rebased on top of #976 to be able to add the functional test.

sets temporarily the activation height for zerocoin public spends v4. Also defines an helper function Zerocoin_PublicSpendVersion which returns the version active at a given block height

now that coinSpend has not the same version of the PublicCoin (extracted from the serial) we need a convenient way to retrieve the latter. This has to be used in HasValidSignature.

Instead of rejecting v1 serials, enforce the proper spend version. v1 serial will fail spend version 3 validation.

furszy

We can merge this already, ACK 6cf4922 .

Mrs-X

utACK 6cf4922 (unfortunately cannot really test it)

…ture 6cf4922 [Bug][Wallet] Guard chainActive access in GetMintMaturityHeight (random-zebra) 95c1532 [Build] Fix CMake linking order (random-zebra) 46f1e53 [Build] Add CoinRandomnessSchnorrSignature to CMakeLists.txt (random-zebra) 183bce4 [zPIV] Add schnorr pk to challenge hash (random-zebra) 0178bbc [Tests] Additional cases for zc public spends v4 (random-zebra) 64efc69 [Tests] Add v4 Spends to zerocoin_valid_public_spend functional test (random-zebra) 3398090 [Refactor] add BN_THREE hardcoded constant for CBigNum(3) (random-zebra) 95952f1 [Cleanup] Fix non UTF-8 chars in ParamGeneration comments (random-zebra) 6288bb7 [Tests] PublicCoinSpend v4 / CRSchnorrSig unit tests (random-zebra) 7b81e54 [zPIV] Remove v2 serials from serialized v4 PublicCoinSpend (random-zebra) a9b8aa0 [Consensus] PublicCoinSpend v4 enforcement (random-zebra) ecd9bcd [zPIV] PublicCoinSpend v4 definition (random-zebra) 0f1f881 [zPIV] CoinSpend: define getCoinVersion() (random-zebra) 5a2ab57 [Consensus] set placeholder for nPublicZCSpendsV4 (random-zebra) e346308 [zPIV] CoinRandomnessSchnorrSignature class definition (random-zebra) 5f9de6d [Refactor] Use hardcoded constants for CBigNum 0, 1, 2 (random-zebra) Pull request description: This PR proposes (as hard fork) a new PublicCoinSpend protocol suitable to be used also with zerocoins version 1. Closes #937 Motivation and overview in the following document: https://github.com/random-zebra/PIVX-Wiki/blob/master/Developer-Documentation/Specs/zPIV/CoinRandomnessSchnorrSignature.mediawiki Enforcement height uses placeholder values (one million blocks after the activation of spends v3 for main net and testnet, and 100 blocks for regnet). Changes - replaces many instances of `CBigNum(0)`, `CBigNum(1)`, `CBigNum(2)` throughout the code with fixed constants defined in bignum.h. - defines the `CoinRandomnessSchnorrSignature` using the algorithm described in the [wiki](https://github.com/random-zebra/PIVX-Wiki/blob/master/Developer-Documentation/Specs/zPIV/CoinRandomnessSchnorrSignature.mediawiki). - defines the temporary activation height and also defines an helper function `Zerocoin_PublicSpendVersion` which returns the version active at a given block height. - defines a convenient method to retrieve the coin version from a `PublicCoinSpend` class (now that coin version and spend version are different). - defines `PublicCoinSpend` behaviour for v4. - enforces the PublicCoinSpend version in `ContextualCheckZerocoinSpendNoSerialCheck`. - provides unit tests for the new v4 `PublicCoinSpend` class. ACKs for top commit: furszy: We can merge this already, ACK 6cf4922 . Mrs-X: utACK 6cf4922 (unfortunately cannot really test it) Tree-SHA512: 56a4e796323862453e593035d2ee9c1148e67db85bd90af015916c5e2be71e586aad289ebcfa63dc076bd4a6be9a5430ae01b5d42c8f9df7027bfee908dfdb99

e06f7b6 [Tests] Minor optimizations in zerocoin_valid_public_spend test (random-zebra) cdc18ea [RPC] fix parameters check for spendzerocoin (missing ispublicspend) (random-zebra) 72bc39a [Tests] Add zerocoin_valid_public_spend to testRunner (random-zebra) 26045d3 [Consensus] Set zc PublicSpend version v3/v4 via SPORK (random-zebra) Pull request description: This follows #936 It sets the required version for zerocoin PublicSpends via SPORK instead of having it height-based. When `SPORK_18_ZEROCOIN_PUBLICSPEND_V4` is not active (default), the required version is v3 (where old version 1 serials cannot be spent. ref: #891 ). It also fixes the relative functional test `zerocoin_valid_public_spend.py` and adds it to the test_runner. ACKs for top commit: furszy: utACK e06f7b6 Warrows: utACK e06f7b6 Tree-SHA512: 07f18e77a91503f5d48dafad32364cc6292ffd1f9a3f9286a78de72dafc98b07342e6abd96eaabe55d20193f10be383c8d5bbac1b33c45712ade8c522e457b3f

random-zebra added Protocol-Update Tests Wallet zPiv labels Jun 28, 2019

furszy self-requested a review June 28, 2019 16:49

random-zebra force-pushed the 2019_randomness-schnorr branch from 7676093 to 8a70a55 Compare June 28, 2019 16:50

random-zebra self-assigned this Jun 28, 2019

furszy reviewed Jun 28, 2019

View reviewed changes

src/zpiv/zpivmodule.cpp Outdated Show resolved Hide resolved

src/zpiv/zpivmodule.cpp Show resolved Hide resolved

Fuzzbawls mentioned this pull request Jun 30, 2019

Fix Version 1 zPIV public Spends #937

Closed

Fuzzbawls added this to the 4.0.0 milestone Jul 14, 2019

random-zebra force-pushed the 2019_randomness-schnorr branch 3 times, most recently from a534e47 to 45415f4 Compare July 27, 2019 05:37

Fuzzbawls added the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Jul 28, 2019

Warrows reviewed Jul 31, 2019

View reviewed changes

src/libzerocoin/ParamGeneration.cpp Outdated Show resolved Hide resolved

src/libzerocoin/ParamGeneration.cpp Outdated Show resolved Hide resolved

src/zpiv/zpivmodule.h Show resolved Hide resolved

random-zebra changed the title ~~[WIP][zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature~~ [zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature Aug 5, 2019

Mrs-X previously approved these changes Aug 5, 2019

View reviewed changes

random-zebra dismissed Mrs-X’s stale review via afce1cc August 5, 2019 20:46

random-zebra force-pushed the 2019_randomness-schnorr branch from afce1cc to 63b5616 Compare August 6, 2019 01:00

random-zebra force-pushed the 2019_randomness-schnorr branch 2 times, most recently from 3dcdf3a to 3bd8974 Compare August 6, 2019 19:06

furszy mentioned this pull request Aug 9, 2019

[Refactor] Fix stake age checks for regtest #976

Merged

random-zebra force-pushed the 2019_randomness-schnorr branch 4 times, most recently from cfb829e to 01086b5 Compare October 9, 2019 22:38

random-zebra force-pushed the 2019_randomness-schnorr branch 3 times, most recently from e8b39bc to c97c8d1 Compare October 18, 2019 17:41

random-zebra added 16 commits October 21, 2019 06:39

[Refactor] Use hardcoded constants for CBigNum 0, 1, 2

5f9de6d

[zPIV] CoinRandomnessSchnorrSignature class definition

e346308

[Consensus] set placeholder for nPublicZCSpendsV4

5a2ab57

sets temporarily the activation height for zerocoin public spends v4. Also defines an helper function Zerocoin_PublicSpendVersion which returns the version active at a given block height

[zPIV] CoinSpend: define getCoinVersion()

0f1f881

now that coinSpend has not the same version of the PublicCoin (extracted from the serial) we need a convenient way to retrieve the latter. This has to be used in HasValidSignature.

[zPIV] PublicCoinSpend v4 definition

ecd9bcd

[Consensus] PublicCoinSpend v4 enforcement

a9b8aa0

Instead of rejecting v1 serials, enforce the proper spend version. v1 serial will fail spend version 3 validation.

[zPIV] Remove v2 serials from serialized v4 PublicCoinSpend

7b81e54

[Tests] PublicCoinSpend v4 / CRSchnorrSig unit tests

6288bb7

[Cleanup] Fix non UTF-8 chars in ParamGeneration comments

95952f1

[Refactor] add BN_THREE hardcoded constant for CBigNum(3)

3398090

[Tests] Add v4 Spends to zerocoin_valid_public_spend functional test

64efc69

[Tests] Additional cases for zc public spends v4

0178bbc

[zPIV] Add schnorr pk to challenge hash

183bce4

[Build] Add CoinRandomnessSchnorrSignature to CMakeLists.txt

46f1e53

[Build] Fix CMake linking order

95c1532

[Bug][Wallet] Guard chainActive access in GetMintMaturityHeight

6cf4922

random-zebra force-pushed the 2019_randomness-schnorr branch from c97c8d1 to 6cf4922 Compare October 21, 2019 05:23

furszy approved these changes Oct 22, 2019

View reviewed changes

Mrs-X approved these changes Oct 22, 2019

View reviewed changes

random-zebra merged commit 6cf4922 into PIVX-Project:master Oct 22, 2019

Fuzzbawls removed the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Dec 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature #936

[zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature #936

random-zebra commented Jun 28, 2019 •

edited

furszy commented Jun 28, 2019

random-zebra commented Jun 30, 2019 •

edited

Fuzzbawls commented Jun 30, 2019

random-zebra commented Jun 30, 2019

Fuzzbawls commented Jul 27, 2019

random-zebra commented Jul 27, 2019

Warrows left a comment

Mrs-X left a comment •

edited

random-zebra commented Aug 5, 2019

random-zebra commented Aug 6, 2019

furszy left a comment

Mrs-X left a comment

[zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature #936

[zPIV] PublicCoinSpend v4 - Coin Randomness Schnorr Signature #936

Conversation

random-zebra commented Jun 28, 2019 • edited

furszy commented Jun 28, 2019

random-zebra commented Jun 30, 2019 • edited

Fuzzbawls commented Jun 30, 2019

random-zebra commented Jun 30, 2019

Fuzzbawls commented Jul 27, 2019

random-zebra commented Jul 27, 2019

Warrows left a comment

Choose a reason for hiding this comment

Mrs-X left a comment • edited

Choose a reason for hiding this comment

random-zebra commented Aug 5, 2019

random-zebra commented Aug 6, 2019

furszy left a comment

Choose a reason for hiding this comment

Mrs-X left a comment

Choose a reason for hiding this comment

random-zebra commented Jun 28, 2019 •

edited

random-zebra commented Jun 30, 2019 •

edited

Mrs-X left a comment •

edited