diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index d68c9a98a..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 \ @@ -17,7 +18,9 @@ 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_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 new file mode 100644 index 000000000..61f81c331 --- /dev/null +++ b/src/gtest/test_random.cpp @@ -0,0 +1,27 @@ +#include + +#include "random.h" + +extern int GenZero(int n); +extern int GenMax(int n); + +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/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 1cedd6f27..5fc2a39ab 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -41,6 +41,30 @@ 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, + 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, gen); + MappedShuffle(outputs.begin(), outputMap.begin(), ZC_NUM_JS_OUTPUTS, gen); + + 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..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" @@ -78,6 +79,20 @@ 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 + std::function gen = GetRandInt + ); + // Verifies that the JoinSplit proof is correct. bool Verify(ZCJoinSplit& params, const uint256& pubKeyHash) const; diff --git a/src/random.h b/src/random.h index 1a2d3e8ee..1f6f7fef5 100644 --- a/src/random.h +++ b/src/random.h @@ -8,6 +8,7 @@ #include "uint256.h" +#include #include /** @@ -24,6 +25,31 @@ 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); + assert(r >= 0); + assert(r <= i); + 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 diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c2b0a7017..25efbd292 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -367,8 +367,6 @@ BOOST_AUTO_TEST_CASE(test_basic_joinsplit_verification) test.anchor = GetRandHash(); BOOST_CHECK(!test.Verify(*p, pubKeyHash)); } - - delete p; } BOOST_AUTO_TEST_CASE(test_simple_joinsplit_invalidity) diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index e4dfef5a0..d6f412192 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -29,6 +29,23 @@ 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(); + assert(outputMap.size() == ZC_NUM_JS_OUTPUTS); + for (size_t i = 0; i < outputMap.size(); i++) { + if (outputMap[i] == n) { + return i; + } + } + + throw std::logic_error("n is not present in outputmap"); +} + AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( std::string fromAddress, std::vector tOutputs, @@ -372,6 +389,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 +447,10 @@ bool AsyncRPCOperation_sendmany::main_impl() { } obj = perform_joinsplit(info, outPoints); + + if (jsChange > 0) { + changeOutputIndex = find_output(obj, 1); + } } } @@ -442,9 +464,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; @@ -483,7 +502,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); @@ -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; } 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");