Merge #588: CreateTransaction: Place blinding pubkey into nNonce in case of fundraw

7fd6b88 claimpegins should arrive in witness destinations (Gregory Sanders)
a4a051e Add tests for fundrawtransaction placing back blinding keys properly (Gregory Sanders)
48958a6 CreateTransaction: Place blinding pubkey into nNonce in case of fundraw (Gregory Sanders)
2e6191d QA: Hack around no issuance an non-witness tx bug issue#473 (Gregory Sanders)

Pull request description:

  Currently any fundrawtransaction call will result in unblinded change output. Automated flow isn't effected by this bug.

Tree-SHA512: ee09a58bd8d5ac35f8d090a0e8928efebb7f9d8e886105bff7188ad4983406d53e4693b46b1e4f70ba76d3e06a420bd778b4c879de183d02d7ec3c7ccbac3f8e

Author: stevenroose

src/wallet/rpcwallet.cpp

@@ -5189,16 +5189,16 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef
     CPubKey newKey;
     if (!pwallet->GetKeyFromPool(newKey))
         throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
-    PKHash pkhash(newKey);
+    WitnessV0KeyHash wpkhash(newKey.GetID());
 
-    pwallet->SetAddressBook(pkhash, "", "receive");
+    pwallet->SetAddressBook(wpkhash, "", "receive");
 
     // One peg-in input, one wallet output and one fee output
     CMutableTransaction mtx;
     mtx.vin.push_back(CTxIn(COutPoint(txHashes[0], nOut), CScript(), ~(uint32_t)0));
     // mark as peg-in input
     mtx.vin[0].m_is_pegin = true;
-    mtx.vout.push_back(CTxOut(Params().GetConsensus().pegged_asset, value, GetScriptForDestination(pkhash)));
+    mtx.vout.push_back(CTxOut(Params().GetConsensus().pegged_asset, value, GetScriptForDestination(wpkhash)));
     mtx.vout.push_back(CTxOut(Params().GetConsensus().pegged_asset, 0, CScript()));
 
     // Construct pegin proof

src/wallet/wallet.cpp

@@ -3166,15 +3166,17 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
                         }
 
                         std::vector<CTxOut>::iterator position = txNew.vout.begin()+vChangePosInOut[assetChange.first];
-                        txNew.vout.insert(position, newTxOut);
-                        CPubKey blind_pub = GetBlindingPubKey(itScript->second.second);
                         if (blind_details) {
+                            CPubKey blind_pub = GetBlindingPubKey(itScript->second.second);
                             blind_details->o_pubkeys.insert(blind_details->o_pubkeys.begin() + vChangePosInOut[assetChange.first], blind_pub);
                             assert(blind_pub.IsFullyValid());
                             blind_details->num_to_blind++;
                             blind_details->change_to_blind++;
                             blind_details->only_change_pos = vChangePosInOut[assetChange.first];
+                            // Place the blinding pubkey here in case of fundraw calls
+                            newTxOut.nNonce.vchCommitment = std::vector<unsigned char>(blind_pub.begin(), blind_pub.end());
                         }
+                        txNew.vout.insert(position, newTxOut);
                     }
                 }
                 // Set the correct nChangePosInOut for output.  Should be policyAsset's position.

test/functional/feature_confidential_transactions.py

@@ -281,6 +281,29 @@ def run_test(self):
         signed = self.nodes[0].signrawtransactionwithwallet(blinded2)
         self.nodes[0].sendrawtransaction(signed["hex"])
 
+        # Aside: Check all outputs after fundraw are properly marked for blinding
+        fund_decode = self.nodes[0].decoderawtransaction(funded["hex"])
+        for output in fund_decode["vout"][:-1]:
+            assert "asset" in output
+            assert "value" in output
+            assert output["scriptPubKey"]["type"] != "fee"
+            assert output["commitmentnonce_fully_valid"]
+        assert fund_decode["vout"][-1]["scriptPubKey"]["type"] == "fee"
+        assert not fund_decode["vout"][-1]["commitmentnonce_fully_valid"]
+
+        # Also check that all fundraw outputs marked for blinding are blinded later
+        for blind_tx in [blinded, blinded2]:
+            blind_decode = self.nodes[0].decoderawtransaction(blind_tx)
+            for output in blind_decode["vout"][:-1]:
+                assert "asset" not in output
+                assert "value" not in output
+                assert output["scriptPubKey"]["type"] != "fee"
+                assert output["commitmentnonce_fully_valid"]
+            assert blind_decode["vout"][-1]["scriptPubKey"]["type"] == "fee"
+            assert "asset" in blind_decode["vout"][-1]
+            assert "value" in blind_decode["vout"][-1]
+            assert not blind_decode["vout"][-1]["commitmentnonce_fully_valid"]
+
         # Check createblindedaddress functionality
         blinded_addr = self.nodes[0].getnewaddress()
         validated_addr = self.nodes[0].validateaddress(blinded_addr)
@@ -379,6 +402,9 @@ def run_test(self):
         self.sync_all()
 
         # Check for value accounting when asset issuance is null but token not, ie unblinded
+        # HACK: Self-send to sweep up bitcoin inputs into blinded output.
+        # We were hitting https://github.com/ElementsProject/elements/issues/473 for the following issuance
+        self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), self.nodes[0].getwalletinfo()["balance"]["bitcoin"], "", "", True)
         issued = self.nodes[0].issueasset(0, 1, False)
         walletinfo = self.nodes[0].getwalletinfo()
         assert(issued["asset"] not in walletinfo["balance"])
@@ -559,7 +585,6 @@ def run_test(self):
         self.nodes[0].sendtoaddress(blinded_addr, 1)
         self.nodes[0].sendtoaddress(blinded_addr, 3)
         unspent = self.nodes[0].listunspent(0, 0)
-        assert_equal(len(unspent), 4)
         rawtx = self.nodes[0].createrawtransaction([{"txid":unspent[0]["txid"], "vout":unspent[0]["vout"]}, {"txid":unspent[1]["txid"], "vout":unspent[1]["vout"]}], {addr:unspent[0]["amount"]+unspent[1]["amount"]-Decimal("0.2"), "fee":Decimal("0.2")})
         # Blinding will fail with 2 blinded inputs and 0 blinded outputs
         # since it has no notion of a wallet to fill in a 0-value OP_RETURN output