CORE: Ensure renewal is working properly

There were some regression/issues with renewal of accounts that expired in phase 3 when trying to renew in phase4

src/Gulden/rpcgulden.cpp

@@ -2664,7 +2664,7 @@ static UniValue renewwitnessaccount(const JSONRPCRequest& request)
 
     if (request.fHelp || request.params.size() != 2)
         throw std::runtime_error(
-            "renewwitnessaccount \"witness_account\" \n"
+            "renewwitnessaccount \"funding_account\" \"witness_account\" \n"
             "\nRenew an expired witness account. \n"
             "1. \"funding_account\"        (required) The unique UUID or label for the account.\n"
             "2. \"witness_account\"        (required) The unique UUID or label for the account.\n"
@@ -2700,20 +2700,23 @@ static UniValue renewwitnessaccount(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_MISC_ERROR, "Cannot split a witness-only account as spend key is required to do this.");
     }
 
+    bool shouldUpgrade = false;
+
     //fixme: (PHASE5) - Share common code with GUI::requestRenewWitness
     std::string strError;
     CMutableTransaction tx(CURRENT_TX_VERSION_POW2);
     CReserveKeyOrScript changeReserveKey(pactiveWallet, fundingAccount, KEYCHAIN_EXTERNAL);
     CAmount transactionFee;
-    if (!pactiveWallet->PrepareRenewWitnessAccountTransaction(fundingAccount, witnessAccount, changeReserveKey, tx, transactionFee, strError))
+    if (!pactiveWallet->PrepareRenewWitnessAccountTransaction(fundingAccount, witnessAccount, changeReserveKey, tx, transactionFee, strError, nullptr, nullptr, &shouldUpgrade))
     {
         throw std::runtime_error(strprintf("Failed to create renew transaction [%s]", strError.c_str()));
     }
 
+
     uint256 finalTransactionHash;
     {
         LOCK2(cs_main, pactiveWallet->cs_wallet);
-        if (!pactiveWallet->SignAndSubmitTransaction(changeReserveKey, tx, strError, &finalTransactionHash))
+        if (!pactiveWallet->SignAndSubmitTransaction(changeReserveKey, tx, strError, &finalTransactionHash, shouldUpgrade?SignType::WitnessUpdate:SignType::Spend))
         {
             throw std::runtime_error(strprintf("Failed to sign renew transaction [%s]", strError.c_str()));
         }

src/policy/policy.cpp

@@ -294,7 +294,7 @@ bool IsSegregatedSignatureDataStandard(const CTransaction& tx, const CCoinsViewC
 
     for (uint64_t i=0; i<tx.vin.size(); ++i)
     {
-        size_t sizeSegregatedSignatureStack = tx.vin[i].segregatedSignatureData.stack.size() - 1;
+        size_t sizeSegregatedSignatureStack = tx.vin[i].segregatedSignatureData.stack.size();
         if (sizeSegregatedSignatureStack > MAX_STANDARD_SEGREGATED_SIGNATURE_STACK_ITEMS)
             return false;
 

src/qt/_Gulden/witnessdialog.cpp

@@ -708,8 +708,9 @@ bool WitnessDialog::doUpdate(bool forceUpdate, WitnessStatus* pWitnessStatus)
                 case WitnessStatus::Expired:
                 {
                     computedWidgetIndex = WitnessDialogStates::EXPIRED;
-                    stateRenewWitnessButton = !accountStatus.hasUnconfirmedWittnessTx;
-                    stateExtendButton = IsSegSigEnabled(chainActive.TipPrev()) && accountStatus.hasUnconfirmedWittnessTx;
+                    stateUpgradeButton = IsSegSigEnabled(chainActive.TipPrev()) && accountStatus.hasScriptLegacyOutput && !accountStatus.hasUnconfirmedWittnessTx;
+                    stateRenewWitnessButton = !stateUpgradeButton && !accountStatus.hasUnconfirmedWittnessTx;
+                    stateExtendButton = !stateUpgradeButton && IsSegSigEnabled(chainActive.TipPrev()) && accountStatus.hasUnconfirmedWittnessTx;
                     break;
                 }
                 case WitnessStatus::Emptying:

src/script/sign.cpp

@@ -184,6 +184,11 @@ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptP
                 }
                 break;
             }
+            case WitnessUpdate:
+            {
+                return true;
+                break;
+            }
             case Witness:
             {
                 if (!Sign1(witnessKeyID, creator, scriptPubKey, ret, sigversion))
@@ -232,6 +237,10 @@ static bool SignStep(const BaseSignatureCreator& creator, const CTxOutPoW2Witnes
                 return false;
             return true;
         }
+        case WitnessUpdate:
+        {
+            return false;
+        }
         case Witness:
         {
             if (!Sign1(pow2Witness.witnessKeyID, creator, scriptSignatureDataPlaceholder, ret, SIGVERSION_SEGSIG))

src/script/sign.h

@@ -77,7 +77,8 @@ struct SignatureData {
 enum SignType
 {
     Spend,
-    Witness
+    Witness, 
+    WitnessUpdate
 };
 
 /** Get the CKeyID of the pubkey for the key that should be used to sign an output */

src/wallet/wallet.h

@@ -771,7 +771,7 @@ class CWallet : public CGuldenWallet
     bool SignTransaction(CAccount* fromAccount, CMutableTransaction& tx, SignType type);
 
     //! Create a transaction that renews an expired witness account
-    bool PrepareRenewWitnessAccountTransaction(CAccount* funderAccount, CAccount* targetWitnessAccount, CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, CAmount& nFeeOut, std::string& strError, uint64_t* skipPastTransaction=nullptr, CCoinControl* coinControl=nullptr);
+    bool PrepareRenewWitnessAccountTransaction(CAccount* funderAccount, CAccount* targetWitnessAccount, CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, CAmount& nFeeOut, std::string& strError, uint64_t* skipPastTransaction=nullptr, CCoinControl* coinControl=nullptr, bool* shouldUpgrade=nullptr);
 
     //! Create a transaction upgrades an old ScriptLegacyOutput witness to a new PoW2WitnessOutput
     void PrepareUpgradeWitnessAccountTransaction(CAccount* funderAccount, CAccount* targetWitnessAccount, CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, CAmount& nFeeOut);
@@ -799,7 +799,7 @@ class CWallet : public CGuldenWallet
     /**
      * Sign and submit a transaction (that has not yet been signed) to the network, add to wallet as appropriate etc.
      */
-    bool SignAndSubmitTransaction(CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, std::string& strError, uint256* pTransactionHashOut=nullptr);
+    bool SignAndSubmitTransaction(CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, std::string& strError, uint256* pTransactionHashOut=nullptr, SignType type=SignType::Spend);
 
     bool CommitTransaction(CWalletTx& wtxNew, CReserveKeyOrScript& reservekey, CConnman* connman, CValidationState& state);
 

src/wallet/wallet_transaction.cpp

@@ -776,9 +776,16 @@ bool CWallet::AddFeeForTransaction(CAccount* forAccount, CMutableTransaction& tx
                 CTransaction txNewConst(txNew);
 
                 // Remove scriptSigs to eliminate the fee calculation dummy signatures
-                for (auto& vin : txNew.vin) {
-                    vin.scriptSig = CScript();
-                    vin.segregatedSignatureData.SetNull();
+                {
+                    LOCK2(cs_main, cs_wallet);
+                    for (auto& txin : txNew.vin)
+                    {
+                        txin.scriptSig = CScript();
+                        txin.segregatedSignatureData.SetNull();
+
+                        // Prevent input being spent twice
+                        LockCoin(txin.prevout);
+                    }
                 }
 
                 // Allow to override the default confirmation target over the CoinControl instance
@@ -888,7 +895,7 @@ bool CWallet::AddFeeForTransaction(CAccount* forAccount, CMutableTransaction& tx
 }
 
 
-bool CWallet::PrepareRenewWitnessAccountTransaction(CAccount* funderAccount, CAccount* targetWitnessAccount, CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, CAmount& nFeeOut, std::string& strError, uint64_t* skipPastTransaction, CCoinControl* coinControl)
+bool CWallet::PrepareRenewWitnessAccountTransaction(CAccount* funderAccount, CAccount* targetWitnessAccount, CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, CAmount& nFeeOut, std::string& strError, uint64_t* skipPastTransaction, CCoinControl* coinControl, bool* shouldUpgrade)
 {
     LOCK2(cs_main, cs_wallet); // cs_main required for ReadBlockFromDisk.
 
@@ -920,6 +927,9 @@ bool CWallet::PrepareRenewWitnessAccountTransaction(CAccount* funderAccount, CAc
 
                 addedAny = true;
 
+                if (shouldUpgrade && witCoin.coin.out.GetType() == CTxOutType::ScriptLegacyOutput)
+                    *shouldUpgrade = true;
+
                 // Add witness input
                 AddTxInput(tx, CInputCoin(witCoin.outpoint, witCoin.coin.out, allowIndexBased, witCoin.coin.nHeight, witCoin.coin.nTxIndex), false);
 
@@ -1050,9 +1060,9 @@ void CWallet::PrepareUpgradeWitnessAccountTransaction(CAccount* funderAccount, C
     throw std::runtime_error("Could not find suitable witness to upgrade");
 }
 
-bool CWallet::SignAndSubmitTransaction(CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, std::string& strError, uint256* pTransactionHashOut)
+bool CWallet::SignAndSubmitTransaction(CReserveKeyOrScript& changeReserveKey, CMutableTransaction& tx, std::string& strError, uint256* pTransactionHashOut, SignType type)
 {
-    if (!SignTransaction(nullptr, tx, SignType::Spend))
+    if (!SignTransaction(nullptr, tx, type))
     {
         strError = "Unable to sign transaction";
         return false;

src/wallet/witness_operations.cpp

@@ -146,18 +146,18 @@ void extendwitnessaddresshelper(CAccount* fundingAccount, witnessOutputsInfoVect
         }
     }
 
-    uint256 finalTransactionHash;
+    uint256 extendTransactionHash;
     {
         LOCK2(cs_main, pwallet->cs_wallet);
-        if (!pwallet->SignAndSubmitTransaction(reservekey, extendWitnessTransaction, reasonForFail, &finalTransactionHash))
+        if (!pwallet->SignAndSubmitTransaction(reservekey, extendWitnessTransaction, reasonForFail, &extendTransactionHash))
         {
             throw witness_error(witness::RPC_MISC_ERROR, strprintf("Failed to commit transaction [%s]", reasonForFail.c_str()));
         }
     }
 
     // Set result parameters
     if (pTxid != nullptr)
-        *pTxid = finalTransactionHash.GetHex();
+        *pTxid = extendTransactionHash.GetHex();
     if (pFee != nullptr)
         *pFee = transactionFee;
 }
@@ -324,15 +324,15 @@ void upgradewitnessaccount(CWallet* pwallet, CAccount* fundingAccount, CAccount*
     CAmount transactionFee;
     pwallet->PrepareUpgradeWitnessAccountTransaction(fundingAccount, witnessAccount, changeReserveKey, tx, transactionFee);
 
-    uint256 finalTransactionHash;
-    if (!pwallet->SignAndSubmitTransaction(changeReserveKey, tx, strError, &finalTransactionHash))
+    uint256 upgradeTransactionHash;
+    if (!pwallet->SignAndSubmitTransaction(changeReserveKey, tx, strError, &upgradeTransactionHash, SignType::WitnessUpdate))
     {
         throw std::runtime_error(strprintf("Failed to sign transaction [%s]", strError.c_str()));
     }
 
     // Set result parameters
     if (pTxid != nullptr)
-        *pTxid = finalTransactionHash.GetHex();
+        *pTxid = upgradeTransactionHash.GetHex();
     if (pFee != nullptr)
         *pFee = transactionFee;
 }
@@ -442,18 +442,18 @@ void rotatewitnessaddresshelper(CAccount* fundingAccount, witnessOutputsInfoVect
         throw witness_error(witness::RPC_MISC_ERROR, strprintf("Failed to fund transaction [%s]", reasonForFail.c_str()));
     }
 
-    uint256 finalTransactionHash;
+    uint256 rotateTransactionHash;
     {
         LOCK2(cs_main, pactiveWallet->cs_wallet);
-        if (!pwallet->SignAndSubmitTransaction(reservekey, rotateWitnessTransaction, reasonForFail, &finalTransactionHash))
+        if (!pwallet->SignAndSubmitTransaction(reservekey, rotateWitnessTransaction, reasonForFail, &rotateTransactionHash))
         {
             throw witness_error(witness::RPC_MISC_ERROR, strprintf("Failed to commit transaction [%s]", reasonForFail.c_str()));
         }
     }
 
     // Set result parameters
     if (pTxid != nullptr)
-        *pTxid = finalTransactionHash.GetHex();
+        *pTxid = rotateTransactionHash.GetHex();
     if (pFee != nullptr)
         *pFee = transactionFee;
 }