Merge #836: Backports + version bump for v0.18.1.6

17f8e74 Bump version to 0.18.1.6 (Steven Roose) 8de91ec Reduce mintxfee to 0.1 sat/vbyte and fallback fee to 2 sat/vb (Steven Roose) 356f25b Some readability improvements in blind.cpp (Steven Roose) a72d9c6 Switch default bits of hiding to 52 (Gregory Sanders) 32792e3 initpegoutwallet: check for hardened derivation steps early (Gregory Sanders) ed88521 Create witness outputs for (re)issueasset RPC (Gregory Sanders) 214789b Place the mainchain RPC warmup message in a better place (Steven Roose) 34784a6 Fix claimpegin and createrawpegin support for multiwallet. (Glenn Willen) Pull request description: v0.18.1.6 changelog: - Reduce mintxfee to 0.1 sat/vbyte and fallback fee to 2 sat/vb - Switch default bits of hiding to 52 - check for hardened derivation steps early in initpegoutwallet - Create witness outputs for (re)issueasset RPC - Fix claimpegin and createrawpegin support for multiwallet - Place the mainchain RPC warmup message in a better place Backported commits: ``` commit bfb77ad (origin/pr/835, mine/mintxfee, mintxfee) Author: Steven Roose <[email protected]> Date: Tue Mar 17 12:47:03 2020 +0000 Reduce mintxfee to 0.1 sat/vbyte and fallback fee to 2 sat/vb commit 26e5490 Author: Gregory Sanders <[email protected]> Date: Tue Dec 3 16:14:13 2019 -0500 Switch default bits of hiding to 52 commit a7b41e1 (upstream/pr/829, origin/pr/829, instagibbs/check_hardened) Author: Gregory Sanders <[email protected]> Date: Tue Mar 3 09:52:57 2020 -0500 initpegoutwallet: check for hardened derivation steps early commit 91583b6 (upstream/pr/825, origin/pr/825, instagibbs/witness_issue) Author: Gregory Sanders <[email protected]> Date: Mon Feb 24 09:32:21 2020 -0500 Create witness outputs for (re)issueasset RPC commit c5cb714 (upstream/pr/818, origin/pr/818, mine/msg-warmup, msg-warmup) Author: Steven Roose <[email protected]> Date: Tue Feb 18 15:15:52 2020 +0000 Place the mainchain RPC warmup message in a better place commit fc841c8 (upstream/pr/813, origin/pr/813) Author: Glenn Willen <[email protected]> Date: Thu Feb 6 23:08:56 2020 -0800 Fix claimpegin and createrawpegin support for multiwallet. ``` Tree-SHA512: 8ad9f22a26e7ea174f8a001651c2f36b5799578d18187231fb170b000c5d6489cd20f2aea776506e6e604a7d6c981a6b32edd532629ec21851ff5f9e7f95a0ed
ElementsProject · Mar 18, 2020 · 49d3c1c · 49d3c1c
2 parents 4683c94 + 17f8e74
commit 49d3c1c
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 39 deletions.
diff --git a/configure.ac b/configure.ac
@@ -3,7 +3,7 @@ AC_PREREQ([2.60])
 define(_CLIENT_VERSION_MAJOR, 0)
 define(_CLIENT_VERSION_MINOR, 18)
 define(_CLIENT_VERSION_REVISION, 1)
-define(_CLIENT_VERSION_BUILD, 5)
+define(_CLIENT_VERSION_BUILD, 6)
 define(_CLIENT_VERSION_RC, 0)
 define(_CLIENT_VERSION_IS_RELEASE, true)
 define(_COPYRIGHT_YEAR, 2019)

diff --git a/src/blind.cpp b/src/blind.cpp
@@ -180,8 +180,11 @@ bool GenerateRangeproof(std::vector<unsigned char>& rangeproof, const std::vecto
     memcpy(asset_message+32, asset_blindptrs[asset_blindptrs.size()-1], 32);
 
     // Sign rangeproof
+    int ct_exponent = std::min(std::max((int)gArgs.GetArg("-ct_exponent", 0), -1), 18);
+    int ct_bits = (int)gArgs.GetArg("-ct_bits", 52);
     // If min_value is 0, scriptPubKey must be unspendable
-    int res = secp256k1_rangeproof_sign(secp256k1_blind_context, rangeproof.data(), &nRangeProofLen, scriptPubKey.IsUnspendable() ? 0 : 1, &value_commit, value_blindptrs.back(), nonce.begin(), std::min(std::max((int)gArgs.GetArg("-ct_exponent", 0), -1),18), std::min(std::max((int)gArgs.GetArg("-ct_bits", 36), 1), 51), amount, asset_message, sizeof(asset_message), scriptPubKey.size() ? &scriptPubKey.front() : NULL, scriptPubKey.size(), &gen);
+    uint64_t min_value = scriptPubKey.IsUnspendable() ? 0 : 1;
+    int res = secp256k1_rangeproof_sign(secp256k1_blind_context, rangeproof.data(), &nRangeProofLen, min_value, &value_commit, value_blindptrs.back(), nonce.begin(), ct_exponent, ct_bits, amount, asset_message, sizeof(asset_message), scriptPubKey.size() ? &scriptPubKey.front() : NULL, scriptPubKey.size(), &gen);
     rangeproof.resize(nRangeProofLen);
     return (res == 1);
 }

diff --git a/src/blind.h b/src/blind.h
@@ -14,8 +14,8 @@
 #include <secp256k1_rangeproof.h>
 #include <secp256k1_surjectionproof.h>
 
-//! ELEMENTS: 36-bit rangeproof size
-static const size_t DEFAULT_RANGEPROOF_SIZE = 2893;
+//! ELEMENTS: 52-bit rangeproof size
+static const size_t DEFAULT_RANGEPROOF_SIZE = 4174;
 
 /*
  * Unblind a pair of confidential asset and value.

diff --git a/src/init.cpp b/src/init.cpp
@@ -591,7 +591,7 @@ void SetupServerArgs()
     gArgs.AddArg("-feeasset=<hex>", strprintf("Asset ID (hex) for mempool/relay fees (default: %s)", defaultChainParams->GetConsensus().pegged_asset.GetHex()), false, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-subsidyasset=<hex>", strprintf("Asset ID (hex) for the block subsidy (default: %s)", defaultChainParams->GetConsensus().pegged_asset.GetHex()), false, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-initialreissuancetokens=<n>", "The amount of reissuance tokens created in the genesis block. (default: 0)", false, OptionsCategory::CHAINPARAMS);
-    gArgs.AddArg("-ct_bits", strprintf("The default number of hiding bits in a rangeproof. Will be exceeded to cover amounts exceeding the maximum hiding value. (default: %d)", 36), false, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-ct_bits", strprintf("The default number of hiding bits in a rangeproof. Will be exceeded to cover amounts exceeding the maximum hiding value. (default: %d)", 52), false, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-ct_exponent", strprintf("The hiding exponent. (default: %s)", 0), false, OptionsCategory::CHAINPARAMS);
 
     // Add the hidden options
@@ -1877,7 +1877,6 @@ bool AppInitMain(InitInterfaces& interfaces)
     SetRPCWarmupFinished();
 
     // ELEMENTS:
-    uiInterface.InitMessage(_("Awaiting mainchain RPC warmup"));
     if (!MainchainRPCCheck(true)) { //Initial check only
         const std::string err_msg = "ERROR: elements is set to verify pegins but cannot get valid response from the mainchain daemon. Please check debug.log for more information.\n\nIf you haven't setup a bitcoind please get the latest stable version from https://bitcoincore.org/en/download/ or if you do not need to validate pegins set in your elements configuration validatepegin=0";
         // We fail immediately if this node has RPC server enabled

diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp
@@ -265,17 +265,17 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test)
 
         // Check wallet borromean-based rangeproof results against expected args
         size_t proof_size = DEFAULT_RANGEPROOF_SIZE;
-        BOOST_CHECK(tx4.witness.vtxoutwit[2].vchRangeproof.size() == proof_size);
+        BOOST_CHECK_EQUAL(tx4.witness.vtxoutwit[2].vchRangeproof.size(), proof_size);
         secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
         int exp = 0;
         int mantissa = 0;
         uint64_t min_value = 0;
         uint64_t max_value = 0;
         BOOST_CHECK(secp256k1_rangeproof_info(ctx, &exp, &mantissa, &min_value, &max_value, tx4.witness.vtxoutwit[2].vchRangeproof.data(), proof_size) == 1);
-        BOOST_CHECK(exp == 0);
-        BOOST_CHECK(mantissa == 36); // 36 bit default
-        BOOST_CHECK(min_value == 1);
-        BOOST_CHECK(max_value == 68719476736);
+        BOOST_CHECK_EQUAL(exp, 0);
+        BOOST_CHECK_EQUAL(mantissa, 52); // 36 bit default
+        BOOST_CHECK_EQUAL(min_value, 1);
+        BOOST_CHECK_EQUAL(max_value, 4503599627370496);
     }
     {
         inputs.clear();

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -5288,6 +5288,7 @@ bool MainchainRPCCheck(const bool init)
 
     // Next, check for working and valid rpc
     if (gArgs.GetBoolArg("-validatepegin", Params().GetConsensus().has_parent_chain)) {
+        uiInterface.InitMessage(_("Awaiting mainchain RPC warmup"));
         // During init try until a non-RPC_IN_WARMUP result
         while (true) {
             try {

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -4984,6 +4984,19 @@ UniValue initpegoutwallet(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_INVALID_PARAMETER, "bitcoin_descriptor must be a ranged descriptor.");
     }
 
+    // Check if we can actually generate addresses(catches hardened derivation steps etc) before
+    // writing to cache
+    UniValue address_list(UniValue::VARR);
+    for (int i = counter; i < counter+3; i++) {
+        std::vector<CScript> scripts;
+        if (!desc->Expand(i, provider, scripts, provider)) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "Could not generate lookahead addresses with descriptor. Are there hardened derivations after the xpub?");
+        }
+        CTxDestination destination;
+        ExtractDestination(scripts[0], destination);
+        address_list.push_back(EncodeParentDestination(destination));
+    }
+
     // For our manual pattern matching, we don't want the checksum part.
     auto checksum_char = bitcoin_desc.find('#');
     if (checksum_char != std::string::npos) {
@@ -5052,16 +5065,6 @@ UniValue initpegoutwallet(const JSONRPCRequest& request)
     assert(len == 33);
     assert(negatedpubkeybytes.size() == 33);
 
-    UniValue address_list(UniValue::VARR);
-    for (int i = counter; i < counter+3; i++) {
-        std::vector<CScript> scripts;
-        if (!desc->Expand(i, provider, scripts, provider)) {
-            throw JSONRPCError(RPC_WALLET_ERROR, "Could not generate lookahead addresses with descriptor. This is a bug.");
-        }
-        CTxDestination destination;
-        ExtractDestination(scripts[0], destination);
-        address_list.push_back(EncodeParentDestination(destination));
-    }
     UniValue pak(UniValue::VOBJ);
     pak.pushKV("pakentry", "pak=" + HexStr(negatedpubkeybytes) + ":" + HexStr(online_pubkey));
     pak.pushKV("liquid_pak", HexStr(online_pubkey));
@@ -5418,6 +5421,12 @@ unsigned int GetPeginTxnOutputIndex(const T_tx& txn, const CScript& witnessProgr
 template<typename T_tx_ref, typename T_tx, typename T_merkle_block>
 static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef, T_tx& tx_aux, T_merkle_block& merkleBlock)
 {
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    CWallet* const pwallet = wallet.get();
+
+    if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
+        return NullUniValue;
+
     if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
         throw std::runtime_error(
             RPCHelpMan{"createrawpegin",
@@ -5441,9 +5450,6 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef
                 },
             }.ToString());
 
-    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
-    CWallet* const pwallet = wallet.get();
-
     auto locked_chain = pwallet->chain().lock();
     LOCK(pwallet->cs_wallet);
 
@@ -5640,6 +5646,11 @@ UniValue createrawpegin(const JSONRPCRequest& request)
 
 UniValue claimpegin(const JSONRPCRequest& request)
 {
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    CWallet* const pwallet = wallet.get();
+
+    if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
+        return NullUniValue;
 
     if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
         throw std::runtime_error(
@@ -5661,8 +5672,6 @@ UniValue claimpegin(const JSONRPCRequest& request)
                 },
             }.ToString());
 
-    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
-    CWallet* const pwallet = wallet.get();
     CTransactionRef tx_ref;
     CMutableTransaction mtx;
 
@@ -5673,20 +5682,30 @@ UniValue claimpegin(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_WALLET_ERROR, "Peg-ins cannot be completed during initial sync or reindexing.");
     }
 
+    // NOTE: Making an RPC from within another RPC is not generally a good idea. In particular, it
+    //   is necessary to copy the URI, which contains the wallet if one was given; otherwise
+    //   multi-wallet support will silently break. The resulting request object is still missing a
+    //   bunch of other fields, although they are usually not used by RPC handlers. This is a
+    //   brittle hack, and further examples of this pattern should not be introduced.
+
     // Get raw peg-in transaction
-    UniValue ret(createrawpegin(request));
+    JSONRPCRequest req;
+    req.URI = request.URI;
+    req.params = request.params;
+    UniValue ret(createrawpegin(req));  // See the note above, on why this is a bad idea.
 
     // Make sure it can be propagated and confirmed
     if (!ret["mature"].isNull() && ret["mature"].get_bool() == false) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Peg-in Bitcoin transaction needs more confirmations to be sent.");
     }
 
     // Sign it
-    JSONRPCRequest request2;
+    JSONRPCRequest req2;
+    req2.URI = request.URI;
     UniValue varr(UniValue::VARR);
     varr.push_back(ret["hex"]);
-    request2.params = varr;
-    UniValue result = signrawtransactionwithwallet(request2);
+    req2.params = varr;
+    UniValue result = signrawtransactionwithwallet(req2);  // See the note above, on why this is a bad idea.
 
     if (!DecodeHexTx(mtx, result["hex"].get_str(), false, true)) {
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
@@ -6168,15 +6187,15 @@ UniValue issueasset(const JSONRPCRequest& request)
         if (!pwallet->GetKeyFromPool(newKey)) {
             throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
         }
-        asset_dest = PKHash(newKey.GetID());
+        asset_dest = WitnessV0KeyHash(newKey.GetID());
         pwallet->SetAddressBook(asset_dest, "", "receive");
         asset_dest_blindpub = pwallet->GetBlindingPubKey(GetScriptForDestination(asset_dest));
     }
     if (nTokens > 0) {
         if (!pwallet->GetKeyFromPool(newKey)) {
             throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
         }
-        token_dest = PKHash(newKey.GetID());
+        token_dest = WitnessV0KeyHash(newKey.GetID());
         pwallet->SetAddressBook(token_dest, "", "receive");
         token_dest_blindpub = pwallet->GetBlindingPubKey(GetScriptForDestination(token_dest));
     }
@@ -6275,7 +6294,7 @@ UniValue reissueasset(const JSONRPCRequest& request)
     if (!pwallet->GetKeyFromPool(newAssetKey)) {
         throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
     }
-    CTxDestination asset_dest = PKHash(newAssetKey.GetID());
+    CTxDestination asset_dest = WitnessV0KeyHash(newAssetKey.GetID());
     pwallet->SetAddressBook(asset_dest, "", "receive");
     CPubKey asset_dest_blindpub = pwallet->GetBlindingPubKey(GetScriptForDestination(asset_dest));
 
@@ -6284,7 +6303,7 @@ UniValue reissueasset(const JSONRPCRequest& request)
     if (!pwallet->GetKeyFromPool(newTokenKey)) {
         throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
     }
-    CTxDestination token_dest = PKHash(newTokenKey.GetID());
+    CTxDestination token_dest = WitnessV0KeyHash(newTokenKey.GetID());
     pwallet->SetAddressBook(token_dest, "", "receive");
     CPubKey token_dest_blindpub = pwallet->GetBlindingPubKey(GetScriptForDestination(token_dest));
 

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -74,11 +74,11 @@ static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000;
 //! -paytxfee default
 constexpr CAmount DEFAULT_PAY_TX_FEE = 0;
 //! -fallbackfee default
-static const CAmount DEFAULT_FALLBACK_FEE = 20000;
+static const CAmount DEFAULT_FALLBACK_FEE = 2000;
 //! -discardfee default
 static const CAmount DEFAULT_DISCARD_FEE = 10000;
 //! -mintxfee default
-static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000;
+static const CAmount DEFAULT_TRANSACTION_MINFEE = 100;
 //! minimum recommended increment for BIP 125 replacement txs
 static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000;
 //! Default for -spendzeroconfchange

diff --git a/test/functional/feature_confidential_transactions.py b/test/functional/feature_confidential_transactions.py
@@ -361,7 +361,9 @@ def run_test(self):
 
         # Send some bitcoin and other assets over as well to fund wallet
         addr = self.nodes[2].getnewaddress()
-        self.nodes[0].sendtoaddress(addr, 5)
+        txid = self.nodes[0].sendtoaddress(addr, 5)
+        # Make sure we're doing 52 bits of hiding which covers 21M BTC worth
+        assert_equal(self.nodes[0].getrawtransaction(txid, 1)["vout"][0]["ct-bits"], 52)
         self.nodes[0].sendmany("", {addr: 1, self.nodes[2].getnewaddress(): 13}, 0, "", [], False, 1, "UNSET", {addr: test_asset})
 
         self.sync_all()

diff --git a/test/functional/feature_fedpeg.py b/test/functional/feature_fedpeg.py
@@ -225,6 +225,14 @@ def run_test(self):
 
         raw = parent.gettransaction(txid1)["hex"]
 
+        # Create a wallet in order to test that multi-wallet support works correctly for claimpegin
+        #   (Regression test for https://github.com/ElementsProject/elements/issues/812 .)
+        sidechain.createwallet("throwaway")
+        # Set up our sidechain RPCs to use the first wallet (with empty name). We do this by
+        #   overriding the RPC object in a hacky way, to avoid breaking a different hack on TestNode
+        #   that enables generate() to work despite the deprecation of the generate RPC.
+        sidechain.rpc = sidechain.get_wallet_rpc("")
+
         print("Attempting peg-ins")
         # First attempt fails the consensus check but gives useful result
         try:

diff --git a/test/functional/feature_issuance.py b/test/functional/feature_issuance.py
@@ -16,7 +16,7 @@ def process_raw_issuance(node, issuance_list):
         raise Exception('Issuance list too long')
     # Make enough outputs for any subsequent spend
     next_destinations = {}
-    output_values = (node.getbalance()['bitcoin']-1)/5
+    output_values = Decimal('%.8f' % ((node.getbalance()['bitcoin']-1)/5))
     for i in range(5):
         next_destinations[node.getnewaddress()] = output_values
 
@@ -382,7 +382,7 @@ def run_test(self):
                 assert_equal(blinded_multisig, self.nodes[0].getaddressinfo(utxo_info["address"])["confidential"])
                 break
         assert(utxo_info is not None)
-        assert(utxo_info["amountblinder"] is not "0000000000000000000000000000000000000000000000000000000000000000")
+        assert(utxo_info["amountblinder"] != "0000000000000000000000000000000000000000000000000000000000000000")
 
         # Now make transaction spending that input
         raw_tx = self.nodes[0].createrawtransaction([], {issued_address:1}, 0, False, {issued_address:issued_asset["token"]})