Fix bip69 vs change position issue (#3063)

* Fix bip69 vs change position issue

* Drop `setbip69enabled` rpc

Author: UdjinM6

src/rpc/client.cpp

@@ -148,7 +148,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "prioritisetransaction", 1, "fee_delta" },
     { "setban", 2, "bantime" },
     { "setban", 3, "absolute" },
-    { "setbip69enabled", 0, "enabled" },
     { "setnetworkactive", 0, "state" },
     { "setprivatesendrounds", 0, "rounds" },
     { "setprivatesendamount", 0, "amount" },

src/wallet/rpcwallet.cpp

@@ -3064,27 +3064,6 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
     return result;
 }
 
-UniValue setbip69enabled(const JSONRPCRequest& request)
-{
-    CWallet* const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
-        return NullUniValue;
-
-    if (request.fHelp || request.params.size() != 1)
-        throw std::runtime_error(
-            "setbip69enabled enable\n"
-                "\nEnable/Disable BIP69 input/output sorting (-regtest only)\n"
-                "\nArguments:\n"
-                "1. enable  (bool, required) true or false"
-        );
-    if (Params().NetworkIDString() != CBaseChainParams::REGTEST)
-        throw std::runtime_error("setbip69enabled for regression testing (-regtest mode) only");
-
-    bBIP69Enabled = request.params[0].get_bool();
-
-    return NullUniValue;
-}
-
 #if ENABLE_MINER
 UniValue generate(const JSONRPCRequest& request)
 {
@@ -3208,8 +3187,6 @@ static const CRPCCommand commands[] =
     { "hidden",             "instantsendtoaddress",     &instantsendtoaddress,     false,  {"address","amount","comment","comment_to","subtractfeefromamount"} },
     { "wallet",             "dumphdinfo",               &dumphdinfo,               true,   {} },
     { "wallet",             "importelectrumwallet",     &importelectrumwallet,     true,   {"filename", "index"} },
-
-    { "hidden",             "setbip69enabled",          &setbip69enabled,          true,   {} },
 };
 
 void RegisterWalletRPCCommands(CRPCTable &t)

src/wallet/wallet.cpp

@@ -50,7 +50,6 @@ std::vector<CWalletRef> vpwallets;
 CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE);
 unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET;
 bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE;
-bool bBIP69Enabled = true;
 
 const char * DEFAULT_WALLET_DAT = "wallet.dat";
 
@@ -3760,7 +3759,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                     txNew.vin.push_back(txin);
                 }
 
-                if (bBIP69Enabled) {
+                // If no specific change position was requested, apply BIP69
+                if (nChangePosRequest == -1) {
                     std::sort(txNew.vin.begin(), txNew.vin.end(), CompareInputBIP69());
                     std::sort(vecTxDSInTmp.begin(), vecTxDSInTmp.end(), CompareInputBIP69());
                     std::sort(txNew.vout.begin(), txNew.vout.end(), CompareOutputBIP69());

src/wallet/wallet.h

@@ -43,7 +43,6 @@ extern std::vector<CWalletRef> vpwallets;
 extern CFeeRate payTxFee;
 extern unsigned int nTxConfirmTarget;
 extern bool bSpendZeroConfChange;
-extern bool bBIP69Enabled;
 
 static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000;
 //! -paytxfee default

test/functional/fundrawtransaction.py

@@ -667,9 +667,6 @@ def run_test(self):
         # Make sure there is exactly one input so coin selection can't skew the result
         assert_equal(len(self.nodes[3].listunspent(1)), 1)
 
-        # Disable BIP69 sorting of inputs and outputs
-        self.nodes[3].setbip69enabled(False)
-
         inputs = []
         outputs = {self.nodes[2].getnewaddress(): 1}
         rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
@@ -732,8 +729,5 @@ def run_test(self):
         # the total subtracted from the outputs is equal to the fee
         assert_equal(share[0] + share[2] + share[3], result[0]['fee'])
 
-        # Reenable BIP69 sorting of inputs and outputs
-        self.nodes[3].setbip69enabled(True)
-
 if __name__ == '__main__':
     RawTransactionsTest().main()