From 0382417fee52bd0b66eceff61180caf05d51367b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Oct 2016 00:20:47 -0500 Subject: [PATCH 1/6] Implement MappedShuffle for tracking the permutation of an array --- src/Makefile.gtest.include | 1 + src/gtest/test_random.cpp | 34 ++++++++++++++++++++++++++++++++++ src/random.h | 24 ++++++++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 src/gtest/test_random.cpp diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index d68c9a98a..d247e3e96 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -17,6 +17,7 @@ zcash_gtest_SOURCES = \ gtest/test_noteencryption.cpp \ gtest/test_merkletree.cpp \ gtest/test_pow.cpp \ + gtest/test_random.cpp \ gtest/test_rpc.cpp \ gtest/test_circuit.cpp \ gtest/test_txid.cpp \ diff --git a/src/gtest/test_random.cpp b/src/gtest/test_random.cpp new file mode 100644 index 000000000..e4a2f5c41 --- /dev/null +++ b/src/gtest/test_random.cpp @@ -0,0 +1,34 @@ +#include + +#include "random.h" + +int GenZero(int n) +{ + return 0; +} + +int GenMax(int n) +{ + return n-1; +} + +TEST(Random, MappedShuffle) { + std::vector a {8, 4, 6, 3, 5}; + std::vector m {0, 1, 2, 3, 4}; + + auto a1 = a; + auto m1 = m; + MappedShuffle(a1.begin(), m1.begin(), a1.size(), GenZero); + std::vector ea1 {4, 6, 3, 5, 8}; + std::vector em1 {1, 2, 3, 4, 0}; + EXPECT_EQ(ea1, a1); + EXPECT_EQ(em1, m1); + + auto a2 = a; + auto m2 = m; + MappedShuffle(a2.begin(), m2.begin(), a2.size(), GenMax); + std::vector ea2 {8, 4, 6, 3, 5}; + std::vector em2 {0, 1, 2, 3, 4}; + EXPECT_EQ(ea2, a2); + EXPECT_EQ(em2, m2); +} diff --git a/src/random.h b/src/random.h index 1a2d3e8ee..11be2ee37 100644 --- a/src/random.h +++ b/src/random.h @@ -8,6 +8,7 @@ #include "uint256.h" +#include #include /** @@ -24,6 +25,29 @@ uint64_t GetRand(uint64_t nMax); int GetRandInt(int nMax); uint256 GetRandHash(); +/** + * Rearranges the elements in the range [first,first+len) randomly, assuming + * that gen is a uniform random number generator. Follows the same algorithm as + * std::shuffle in C++11 (a Durstenfeld shuffle). + * + * The elements in the range [mapFirst,mapFirst+len) are rearranged according to + * the same permutation, enabling the permutation to be tracked by the caller. + * + * gen takes an integer n and produces a uniform random output in [0,n). + */ +template +void MappedShuffle(RandomAccessIterator first, + MapRandomAccessIterator mapFirst, + size_t len, + std::function gen) +{ + for (size_t i = len-1; i > 0; --i) { + auto r = gen(i+1); + std::swap(first[i], first[r]); + std::swap(mapFirst[i], mapFirst[r]); + } +} + /** * Seed insecure_rand using the random pool. * @param Deterministic Use a deterministic seed From 7f0aa74666acf962bc37dca21339bb438a2195b1 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Oct 2016 00:22:58 -0500 Subject: [PATCH 2/6] Implement static method for creating a randomized JSDescription --- src/primitives/transaction.cpp | 24 ++++++++++++++++++++++++ src/primitives/transaction.h | 13 +++++++++++++ src/test/transaction_tests.cpp | 26 ++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 1cedd6f27..704b10a5c 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -6,6 +6,7 @@ #include "primitives/transaction.h" #include "hash.h" +#include "random.h" #include "tinyformat.h" #include "utilstrencodings.h" @@ -41,6 +42,29 @@ JSDescription::JSDescription(ZCJoinSplit& params, ); } +JSDescription JSDescription::Randomized( + ZCJoinSplit& params, + const uint256& pubKeyHash, + const uint256& anchor, + boost::array& inputs, + boost::array& outputs, + boost::array& inputMap, + boost::array& outputMap, + CAmount vpub_old, + CAmount vpub_new, + bool computeProof) +{ + // Randomize the order of the inputs and outputs + inputMap = {0, 1}; + outputMap = {0, 1}; + MappedShuffle(inputs.begin(), inputMap.begin(), ZC_NUM_JS_INPUTS, GetRandInt); + MappedShuffle(outputs.begin(), outputMap.begin(), ZC_NUM_JS_OUTPUTS, GetRandInt); + + return JSDescription( + params, pubKeyHash, anchor, inputs, outputs, + vpub_old, vpub_new, computeProof); +} + bool JSDescription::Verify( ZCJoinSplit& params, const uint256& pubKeyHash diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 2e83773c4..b665033d8 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -78,6 +78,19 @@ public: bool computeProof = true // Set to false in some tests ); + static JSDescription Randomized( + ZCJoinSplit& params, + const uint256& pubKeyHash, + const uint256& rt, + boost::array& inputs, + boost::array& outputs, + boost::array& inputMap, + boost::array& outputMap, + CAmount vpub_old, + CAmount vpub_new, + bool computeProof = true // Set to false in some tests + ); + // Verifies that the JoinSplit proof is correct. bool Verify(ZCJoinSplit& params, const uint256& pubKeyHash) const; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c2b0a7017..e7a932a4f 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -368,6 +368,32 @@ BOOST_AUTO_TEST_CASE(test_basic_joinsplit_verification) BOOST_CHECK(!test.Verify(*p, pubKeyHash)); } + { + boost::array inputMap; + boost::array outputMap; + auto jsdesc = JSDescription::Randomized( + *p, pubKeyHash, rt, + inputs, outputs, + inputMap, outputMap, + 0, 0); + BOOST_CHECK(jsdesc.Verify(*p, pubKeyHash)); + + std::set inputSet; + for (size_t i = 0; i < ZC_NUM_JS_INPUTS; i++) { + inputSet.insert(inputMap[i]); + } + std::set expectedInputSet {0, 1}; + BOOST_CHECK(expectedInputSet == inputSet); + + std::set outputSet; + for (size_t i = 0; i < ZC_NUM_JS_OUTPUTS; i++) { + outputSet.insert(outputMap[i]); + } + std::set expectedOutputSet {0, 1}; + BOOST_CHECK(expectedOutputSet == outputSet); + } + + delete p; } From 2eeb6bebdea684903233b23e9ae8b4794e2b3e0a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Oct 2016 10:41:30 -0500 Subject: [PATCH 3/6] Randomize JoinSplits in z_sendmany --- src/wallet/asyncrpcoperation_sendmany.cpp | 54 ++++++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index e4dfef5a0..ded24e66b 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -29,6 +29,22 @@ using namespace libzcash; +int find_output(Object obj, int n) { + Value outputMapValue = find_value(obj, "outputmap"); + if (outputMapValue.type() != array_type) { + throw JSONRPCError(RPC_WALLET_ERROR, "Missing outputmap for JoinSplit operation"); + } + + Array outputMap = outputMapValue.get_array(); + for (size_t i = 0; i < outputMap.size(); i++) { + if (outputMap[i] == n) { + return i; + } + } + + return -1; +} + AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( std::string fromAddress, std::vector tOutputs, @@ -372,6 +388,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { */ Object obj; CAmount jsChange = 0; // this is updated after each joinsplit + int changeOutputIndex = -1; // this is updated after each joinsplit if jsChange > 0 bool minersFeeProcessed = false; if (t_outputs_total > 0) { @@ -429,6 +446,10 @@ bool AsyncRPCOperation_sendmany::main_impl() { } obj = perform_joinsplit(info, outPoints); + + if (jsChange > 0) { + changeOutputIndex = find_output(obj, 1); + } } } @@ -442,9 +463,6 @@ bool AsyncRPCOperation_sendmany::main_impl() { // Keep track of treestate within this transaction boost::unordered_map intermediates; std::vector previousCommitments; - - // NOTE: Randomization of input and output order could break this in future - const int changeOutputIndex = 1; while (zOutputsDeque.size() > 0) { AsyncJoinSplitInfo info; @@ -645,6 +663,10 @@ bool AsyncRPCOperation_sendmany::main_impl() { } obj = perform_joinsplit(info, witnesses, jsAnchor); + + if (jsChange > 0) { + changeOutputIndex = find_output(obj, 1); + } } } @@ -845,11 +867,20 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( ); // Generate the proof, this can take over a minute. - JSDescription jsdesc(*pzcashParams, + boost::array inputs + {info.vjsin[0], info.vjsin[1]}; + boost::array outputs + {info.vjsout[0], info.vjsout[1]}; + boost::array inputMap; + boost::array outputMap; + JSDescription jsdesc = JSDescription::Randomized( + *pzcashParams, joinSplitPubKey_, anchor, - {info.vjsin[0], info.vjsin[1]}, - {info.vjsout[0], info.vjsout[1]}, + inputs, + outputs, + inputMap, + outputMap, info.vpub_old, info.vpub_new, !this->testmode); @@ -910,10 +941,21 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( encryptedNote2 = HexStr(ss2.begin(), ss2.end()); } + Array arrInputMap; + Array arrOutputMap; + for (size_t i = 0; i < ZC_NUM_JS_INPUTS; i++) { + arrInputMap.push_back(inputMap[i]); + } + for (size_t i = 0; i < ZC_NUM_JS_OUTPUTS; i++) { + arrOutputMap.push_back(outputMap[i]); + } + Object obj; obj.push_back(Pair("encryptednote1", encryptedNote1)); obj.push_back(Pair("encryptednote2", encryptedNote2)); obj.push_back(Pair("rawtxn", HexStr(ss.begin(), ss.end()))); + obj.push_back(Pair("inputmap", arrInputMap)); + obj.push_back(Pair("outputmap", arrOutputMap)); return obj; } From 3774c944f849be5c5b828c27c1c3375520ea7aa4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Oct 2016 12:44:56 -0500 Subject: [PATCH 4/6] Refactor test code to better test JSDescription::Randomized() --- src/Makefile.gtest.include | 2 + src/gtest/test_random.cpp | 11 +---- src/gtest/test_transaction.cpp | 85 ++++++++++++++++++++++++++++++++ src/gtest/utils.cpp | 13 +++++ src/primitives/transaction.cpp | 8 +-- src/primitives/transaction.h | 4 +- src/test/transaction_tests.cpp | 28 ----------- src/wallet/gtest/test_wallet.cpp | 2 +- 8 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 src/gtest/test_transaction.cpp create mode 100644 src/gtest/utils.cpp diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index d247e3e96..d635c5b94 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -4,6 +4,7 @@ bin_PROGRAMS += zcash-gtest # tool for generating our public parameters zcash_gtest_SOURCES = \ gtest/main.cpp \ + gtest/utils.cpp \ gtest/test_checktransaction.cpp \ gtest/json_test_vectors.cpp \ gtest/json_test_vectors.h \ @@ -19,6 +20,7 @@ zcash_gtest_SOURCES = \ gtest/test_pow.cpp \ gtest/test_random.cpp \ gtest/test_rpc.cpp \ + gtest/test_transaction.cpp \ gtest/test_circuit.cpp \ gtest/test_txid.cpp \ gtest/test_libzcash_utils.cpp \ diff --git a/src/gtest/test_random.cpp b/src/gtest/test_random.cpp index e4a2f5c41..61f81c331 100644 --- a/src/gtest/test_random.cpp +++ b/src/gtest/test_random.cpp @@ -2,15 +2,8 @@ #include "random.h" -int GenZero(int n) -{ - return 0; -} - -int GenMax(int n) -{ - return n-1; -} +extern int GenZero(int n); +extern int GenMax(int n); TEST(Random, MappedShuffle) { std::vector a {8, 4, 6, 3, 5}; diff --git a/src/gtest/test_transaction.cpp b/src/gtest/test_transaction.cpp new file mode 100644 index 000000000..a339f7652 --- /dev/null +++ b/src/gtest/test_transaction.cpp @@ -0,0 +1,85 @@ +#include + +#include "primitives/transaction.h" +#include "zcash/Note.hpp" +#include "zcash/Address.hpp" + +extern ZCJoinSplit* params; +extern int GenZero(int n); +extern int GenMax(int n); + +TEST(Transaction, JSDescriptionRandomized) { + // construct a merkle tree + ZCIncrementalMerkleTree merkleTree; + + libzcash::SpendingKey k = libzcash::SpendingKey::random(); + libzcash::PaymentAddress addr = k.address(); + + libzcash::Note note(addr.a_pk, 100, uint256(), uint256()); + + // commitment from coin + uint256 commitment = note.cm(); + + // insert commitment into the merkle tree + merkleTree.append(commitment); + + // compute the merkle root we will be working with + uint256 rt = merkleTree.root(); + + auto witness = merkleTree.witness(); + + // create JSDescription + uint256 pubKeyHash; + boost::array inputs = { + libzcash::JSInput(witness, note, k), + libzcash::JSInput() // dummy input of zero value + }; + boost::array outputs = { + libzcash::JSOutput(addr, 50), + libzcash::JSOutput(addr, 50) + }; + boost::array inputMap; + boost::array outputMap; + + { + auto jsdesc = JSDescription::Randomized( + *params, pubKeyHash, rt, + inputs, outputs, + inputMap, outputMap, + 0, 0, false); + + std::set inputSet(inputMap.begin(), inputMap.end()); + std::set expectedInputSet {0, 1}; + EXPECT_EQ(expectedInputSet, inputSet); + + std::set outputSet(outputMap.begin(), outputMap.end()); + std::set expectedOutputSet {0, 1}; + EXPECT_EQ(expectedOutputSet, outputSet); + } + + { + auto jsdesc = JSDescription::Randomized( + *params, pubKeyHash, rt, + inputs, outputs, + inputMap, outputMap, + 0, 0, false, GenZero); + + boost::array expectedInputMap {1, 0}; + boost::array expectedOutputMap {1, 0}; + EXPECT_EQ(expectedInputMap, inputMap); + EXPECT_EQ(expectedOutputMap, outputMap); + } + + { + auto jsdesc = JSDescription::Randomized( + *params, pubKeyHash, rt, + inputs, outputs, + inputMap, outputMap, + 0, 0, false, GenMax); + + boost::array expectedInputMap {0, 1}; + boost::array expectedOutputMap {0, 1}; + EXPECT_EQ(expectedInputMap, inputMap); + EXPECT_EQ(expectedOutputMap, outputMap); + } +} diff --git a/src/gtest/utils.cpp b/src/gtest/utils.cpp new file mode 100644 index 000000000..cf025162c --- /dev/null +++ b/src/gtest/utils.cpp @@ -0,0 +1,13 @@ +#include "zcash/JoinSplit.hpp" + +ZCJoinSplit* params = ZCJoinSplit::Unopened(); + +int GenZero(int n) +{ + return 0; +} + +int GenMax(int n) +{ + return n-1; +} diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 704b10a5c..5fc2a39ab 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -6,7 +6,6 @@ #include "primitives/transaction.h" #include "hash.h" -#include "random.h" #include "tinyformat.h" #include "utilstrencodings.h" @@ -52,13 +51,14 @@ JSDescription JSDescription::Randomized( boost::array& outputMap, CAmount vpub_old, CAmount vpub_new, - bool computeProof) + bool computeProof, + std::function gen) { // Randomize the order of the inputs and outputs inputMap = {0, 1}; outputMap = {0, 1}; - MappedShuffle(inputs.begin(), inputMap.begin(), ZC_NUM_JS_INPUTS, GetRandInt); - MappedShuffle(outputs.begin(), outputMap.begin(), ZC_NUM_JS_OUTPUTS, GetRandInt); + MappedShuffle(inputs.begin(), inputMap.begin(), ZC_NUM_JS_INPUTS, gen); + MappedShuffle(outputs.begin(), outputMap.begin(), ZC_NUM_JS_OUTPUTS, gen); return JSDescription( params, pubKeyHash, anchor, inputs, outputs, diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index b665033d8..9b78436c5 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -7,6 +7,7 @@ #define BITCOIN_PRIMITIVES_TRANSACTION_H #include "amount.h" +#include "random.h" #include "script/script.h" #include "serialize.h" #include "uint256.h" @@ -88,7 +89,8 @@ public: boost::array& outputMap, CAmount vpub_old, CAmount vpub_new, - bool computeProof = true // Set to false in some tests + bool computeProof = true, // Set to false in some tests + std::function gen = GetRandInt ); // Verifies that the JoinSplit proof is correct. diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index e7a932a4f..25efbd292 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -367,34 +367,6 @@ BOOST_AUTO_TEST_CASE(test_basic_joinsplit_verification) test.anchor = GetRandHash(); BOOST_CHECK(!test.Verify(*p, pubKeyHash)); } - - { - boost::array inputMap; - boost::array outputMap; - auto jsdesc = JSDescription::Randomized( - *p, pubKeyHash, rt, - inputs, outputs, - inputMap, outputMap, - 0, 0); - BOOST_CHECK(jsdesc.Verify(*p, pubKeyHash)); - - std::set inputSet; - for (size_t i = 0; i < ZC_NUM_JS_INPUTS; i++) { - inputSet.insert(inputMap[i]); - } - std::set expectedInputSet {0, 1}; - BOOST_CHECK(expectedInputSet == inputSet); - - std::set outputSet; - for (size_t i = 0; i < ZC_NUM_JS_OUTPUTS; i++) { - outputSet.insert(outputMap[i]); - } - std::set expectedOutputSet {0, 1}; - BOOST_CHECK(expectedOutputSet == outputSet); - } - - - delete p; } BOOST_AUTO_TEST_CASE(test_simple_joinsplit_invalidity) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index c1bf5d34c..808aff6c4 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -13,7 +13,7 @@ using ::testing::Return; -ZCJoinSplit* params = ZCJoinSplit::Unopened(); +extern ZCJoinSplit* params; ACTION(ThrowLogicError) { throw std::logic_error("Boom"); From 328d39d29c009a97c763c28f8ce94184dea287b6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Oct 2016 13:10:20 -0500 Subject: [PATCH 5/6] Remove stale comment --- src/wallet/asyncrpcoperation_sendmany.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index ded24e66b..4742e2235 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -501,7 +501,6 @@ bool AsyncRPCOperation_sendmany::main_impl() { throw JSONRPCError(RPC_WALLET_ERROR, "Could not find previous JoinSplit anchor"); } - // NOTE: We assume the last commitment, output 1, is the change we want for (const uint256& commitment : prevJoinSplit.commitments) { tree.append(commitment); previousCommitments.push_back(commitment); From aa36398b274c8967232c16c9b241a08dd78f64b6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 19 Oct 2016 08:49:08 -0500 Subject: [PATCH 6/6] Add more assertions, throw if find_output params are invalid --- src/random.h | 2 ++ src/wallet/asyncrpcoperation_sendmany.cpp | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/random.h b/src/random.h index 11be2ee37..1f6f7fef5 100644 --- a/src/random.h +++ b/src/random.h @@ -43,6 +43,8 @@ void MappedShuffle(RandomAccessIterator first, { for (size_t i = len-1; i > 0; --i) { auto r = gen(i+1); + assert(r >= 0); + assert(r <= i); std::swap(first[i], first[r]); std::swap(mapFirst[i], mapFirst[r]); } diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 4742e2235..d6f412192 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -36,13 +36,14 @@ int find_output(Object obj, int n) { } Array outputMap = outputMapValue.get_array(); + assert(outputMap.size() == ZC_NUM_JS_OUTPUTS); for (size_t i = 0; i < outputMap.size(); i++) { if (outputMap[i] == n) { return i; } } - return -1; + throw std::logic_error("n is not present in outputmap"); } AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(