From 7c463780cf5339cd04a671b649305e834939070c Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 4 Nov 2016 23:23:48 -0700 Subject: [PATCH 1/2] Fixes #1779 so that sending to multiple zaddrs no longer fails. Commit 2eeb6b randomized the order of input and output notes, but this is now known to prevent the chaining of multiple joinsplits in a single transaction. The root cause has yet to be determined. This patch is a temporary fix and disables the shuffling of input and output notes. It also adds a chained joinsplit test to the python qa test suite. --- qa/rpc-tests/wallet_protectcoinbase.py | 17 +++++++++++++++++ src/primitives/transaction.cpp | 6 ++++-- src/wallet/asyncrpcoperation_sendmany.cpp | 5 ++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index b3d554889..eabe26bf4 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -148,5 +148,22 @@ class Wallet2Test (BitcoinTestFramework): # check balance assert_equal(self.nodes[2].getbalance(), 9) + # Check that chained joinsplits in a single tx are created successfully. + recipients = [] + num_recipients = 3 + amount_per_recipient = Decimal('0.002') + for i in xrange(0,num_recipients): + newzaddr = self.nodes[2].z_getnewaddress() + recipients.append({"address":newzaddr, "amount":amount_per_recipient}) + myopid = self.nodes[0].z_sendmany(myzaddr, recipients) + self.wait_for_operationd_success(myopid) + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + + # check balances + resp = self.nodes[2].z_gettotalbalance() + assert_equal(Decimal(resp["private"]), num_recipients * amount_per_recipient) + if __name__ == '__main__': Wallet2Test ().main () diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 399cd665a..de722eb92 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -57,8 +57,10 @@ JSDescription JSDescription::Randomized( // 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); + if (gen) { + 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/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index f72badc60..fa359206f 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -875,6 +875,7 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( {info.vjsout[0], info.vjsout[1]}; boost::array inputMap; boost::array outputMap; + std::function emptyFunc; JSDescription jsdesc = JSDescription::Randomized( *pzcashParams, joinSplitPubKey_, @@ -885,7 +886,9 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( outputMap, info.vpub_old, info.vpub_new, - !this->testmode); + !this->testmode, + // Temporary fix for #1779 is to disable shuffling of inputs and outputs. + emptyFunc); if (!(jsdesc.Verify(*pzcashParams, joinSplitPubKey_))) { throw std::runtime_error("error verifying joinsplit"); From 38276c6ba2a38c6aa200438bfb3cb1aadff747e5 Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 5 Nov 2016 14:12:41 -0700 Subject: [PATCH 2/2] Add GenIdentity, an identity function for MappedShuffle. We use this function in z_sendmany as part of the fix for #1779. --- src/gtest/test_random.cpp | 8 ++++++++ src/primitives/transaction.cpp | 9 +++++---- src/random.cpp | 5 +++++ src/random.h | 5 +++++ src/wallet/asyncrpcoperation_sendmany.cpp | 3 +-- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/gtest/test_random.cpp b/src/gtest/test_random.cpp index 61f81c331..d89702bcd 100644 --- a/src/gtest/test_random.cpp +++ b/src/gtest/test_random.cpp @@ -24,4 +24,12 @@ TEST(Random, MappedShuffle) { std::vector em2 {0, 1, 2, 3, 4}; EXPECT_EQ(ea2, a2); EXPECT_EQ(em2, m2); + + auto a3 = a; + auto m3 = m; + MappedShuffle(a3.begin(), m3.begin(), a3.size(), GenIdentity); + std::vector ea3 {8, 4, 6, 3, 5}; + std::vector em3 {0, 1, 2, 3, 4}; + EXPECT_EQ(ea3, a3); + EXPECT_EQ(em3, m3); } diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index de722eb92..d1e969360 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -57,10 +57,11 @@ JSDescription JSDescription::Randomized( // Randomize the order of the inputs and outputs inputMap = {0, 1}; outputMap = {0, 1}; - if (gen) { - MappedShuffle(inputs.begin(), inputMap.begin(), ZC_NUM_JS_INPUTS, gen); - MappedShuffle(outputs.begin(), outputMap.begin(), ZC_NUM_JS_OUTPUTS, gen); - } + + assert(gen); + + 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/random.cpp b/src/random.cpp index 0ba0de908..4f197fcac 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -137,3 +137,8 @@ void seed_insecure_rand(bool fDeterministic) insecure_rand_Rw = tmp; } } + +int GenIdentity(int n) +{ + return n-1; +} diff --git a/src/random.h b/src/random.h index 1f6f7fef5..5bc8b5480 100644 --- a/src/random.h +++ b/src/random.h @@ -25,6 +25,11 @@ uint64_t GetRand(uint64_t nMax); int GetRandInt(int nMax); uint256 GetRandHash(); +/** + * Identity function for MappedShuffle, so that elements retain their original order. + */ + int GenIdentity(int n); + /** * Rearranges the elements in the range [first,first+len) randomly, assuming * that gen is a uniform random number generator. Follows the same algorithm as diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index fa359206f..7f9549b74 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -875,7 +875,6 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( {info.vjsout[0], info.vjsout[1]}; boost::array inputMap; boost::array outputMap; - std::function emptyFunc; JSDescription jsdesc = JSDescription::Randomized( *pzcashParams, joinSplitPubKey_, @@ -888,7 +887,7 @@ Object AsyncRPCOperation_sendmany::perform_joinsplit( info.vpub_new, !this->testmode, // Temporary fix for #1779 is to disable shuffling of inputs and outputs. - emptyFunc); + GenIdentity); if (!(jsdesc.Verify(*pzcashParams, joinSplitPubKey_))) { throw std::runtime_error("error verifying joinsplit");