From bc5aef9264eebdacf7dd15b375a364e14e9bd29d Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 21 Nov 2021 10:18:49 -0500 Subject: [PATCH 1/4] Add contractchangetoinputaddress setting to Args manager --- src/init.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 76ec2895a0..e343e10886 100755 --- a/src/init.cpp +++ b/src/init.cpp @@ -458,7 +458,9 @@ void SetupServerArgs() ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-maxsigcachesize=", "Set maximum size for signature cache (default: 50000)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - + argsman.AddArg("-contractchangetoinputaddress", "Change from a contract transaction is returned to an input address " + "rather than creating a new change address (default: 0)", + ArgsManager::ALLOW_ANY | ArgsManager::IMMEDIATE_EFFECT, OptionsCategory::WALLET); // Connections argsman.AddArg("-timeout=", "Specify connection timeout in milliseconds (default: 5000)", From 79de6fd1ce3aecd1044e3d70a86a846172b64f69 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 21 Nov 2021 10:19:21 -0500 Subject: [PATCH 2/4] Add change_back_to_input_address boolean to CreateTransaction This supports specifying a transaction pick a change address from one of the inputs rather than creating a new change address for change when this boolean is true. It defaults to false, which is the original behavior. --- src/wallet/wallet.cpp | 73 ++++++++++++++++++++++++++++--------------- src/wallet/wallet.h | 7 +++-- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8c47a52e50..1fec49f513 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1770,7 +1770,8 @@ bool CWallet::SelectCoinsForStaking(unsigned int nSpendTime, std::vector >& vecSend, set>& setCoins_in, - CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl* coinControl) + CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl* coinControl, + bool change_back_to_input_address) { int64_t nValueOut = 0; @@ -1938,31 +1939,50 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CScript scriptChange; // coin control: send change to custom address - if (coinControl && !std::get_if(&coinControl->destChange)) - scriptChange.SetDestination(coinControl->destChange); + if (coinControl && !std::get_if(&coinControl->destChange)) { + LogPrintf("INFO: %s: Setting custom change address: %s", __func__, + CBitcoinAddress(coinControl->destChange).ToString()); - // no coin control: send change to newly generated address - else - { - // Note: We use a new key here to keep it from being obvious which side is the change. - // The drawback is that by not reusing a previous key, the change may be lost if a - // backup is restored, if the backup doesn't have the new private key for the change. - // If we reused the old key, it would be possible to add code to look for and - // rediscover unknown transactions that were written with keys of ours to recover - // post-backup change. - - // Reserve a new key pair from key pool - CPubKey vchPubKey; - if (!reservekey.GetReservedKey(vchPubKey)) - { - LogPrintf("Keypool ran out, please call keypoolrefill first"); - return false; + scriptChange.SetDestination(coinControl->destChange); + } else { // no coin control + if (change_back_to_input_address) { // send change back to an existing input address + CTxDestination change_address; + + if (!setCoins_out.empty()) { + // Select the first input with a valid address as the change address. This seems as good + // a choice as any, and is the fastest. + for (const auto& input : setCoins_out) { + if (ExtractDestination(input.first->vout[input.second].scriptPubKey, change_address)) { + scriptChange.SetDestination(change_address); + + break; + } + } + + LogPrintf("INFO: %s: Sending change to input address %s", __func__, + CBitcoinAddress(change_address).ToString()); + } + } else { // send change to newly generated address + // Note: We use a new key here to keep it from being obvious which side is the change. + // The drawback is that by not reusing a previous key, the change may be lost if a + // backup is restored, if the backup doesn't have the new private key for the change. + // If we reused the old key, it would be possible to add code to look for and + // rediscover unknown transactions that were written with keys of ours to recover + // post-backup change. + + // Reserve a new key pair from key pool + CPubKey vchPubKey; + if (!reservekey.GetReservedKey(vchPubKey)) + { + LogPrintf("Keypool ran out, please call keypoolrefill first"); + return false; + } + + scriptChange.SetDestination(vchPubKey.GetID()); } - - scriptChange.SetDestination(vchPubKey.GetID()); } - // Insert change txn at random position: + // Insert change output at random position in the transaction: vector::iterator position = wtxNew.vout.begin()+GetRandInt(wtxNew.vout.size()); wtxNew.vout.insert(position, CTxOut(nChange, scriptChange)); } @@ -2054,22 +2074,23 @@ bool CWallet::CreateTransaction(const vector >& vecSend, } bool CWallet::CreateTransaction(const vector >& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, - int64_t& nFeeRet, const CCoinControl* coinControl) + int64_t& nFeeRet, const CCoinControl* coinControl, bool change_back_to_input_address) { // Initialize setCoins empty to let CreateTransaction choose via SelectCoins... set> setCoins; - return CreateTransaction(vecSend, setCoins, wtxNew, reservekey, nFeeRet, coinControl); + return CreateTransaction(vecSend, setCoins, wtxNew, reservekey, nFeeRet, coinControl, change_back_to_input_address); } -bool CWallet::CreateTransaction(CScript scriptPubKey, int64_t nValue, CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl* coinControl) +bool CWallet::CreateTransaction(CScript scriptPubKey, int64_t nValue, CWalletTx& wtxNew, CReserveKey& reservekey, + int64_t& nFeeRet, const CCoinControl* coinControl, bool change_back_to_input_address) { vector< pair > vecSend; vecSend.push_back(make_pair(scriptPubKey, nValue)); - return CreateTransaction(vecSend, wtxNew, reservekey, nFeeRet, coinControl); + return CreateTransaction(vecSend, wtxNew, reservekey, nFeeRet, coinControl, change_back_to_input_address); } // Call after CreateTransaction unless you want to abort diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bcde29000b..5794980edf 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -270,11 +270,12 @@ class CWallet : public CCryptoKeyStore int64_t GetStake() const; int64_t GetNewMint() const; bool CreateTransaction(const std::vector>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, - int64_t& nFeeRet, const CCoinControl* coinControl = nullptr); + int64_t& nFeeRet, const CCoinControl* coinControl = nullptr, bool change_back_to_input_address = false); bool CreateTransaction(const std::vector>& vecSend, std::set>& setCoins, - CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl* coinControl = nullptr); + CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl* coinControl = nullptr, + bool change_back_to_input_address = false); bool CreateTransaction(CScript scriptPubKey, int64_t nValue, CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, - const CCoinControl* coinControl = nullptr); + const CCoinControl* coinControl = nullptr, bool change_back_to_input_address = false); bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey); std::string SendMoney(CScript scriptPubKey, int64_t nValue, CWalletTx& wtxNew, bool fAskFee=false); From fdffe1d72a45757f63b5079527e09dae188102b8 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 21 Nov 2021 10:20:28 -0500 Subject: [PATCH 3/4] Modify CreateContractTx to respect the contractchangetoinputaddress setting --- src/gridcoin/contract/message.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/gridcoin/contract/message.cpp b/src/gridcoin/contract/message.cpp index 8f7d8cfc96..0bc72ee058 100644 --- a/src/gridcoin/contract/message.cpp +++ b/src/gridcoin/contract/message.cpp @@ -66,12 +66,26 @@ bool CreateContractTx(CWalletTx& wtx_out, CReserveKey reserve_key, CAmount burn_ CCoinControl coin_control_out; CAmount applied_fee_out; // Unused bool admin = false; + bool contract_change_to_input_address = gArgs.GetBoolArg("-contractchangetoinputaddress", false); + CTxDestination out_address {CNoDestination()}; // If the input transaction already selected some inputs, ensure that we // pick those inputs again when creating the final transaction: - // for (auto& txin : wtx_out.vin) { coin_control_out.Select(txin.prevout); + + // If contract_change_to_input_address is false or once the first already selected input is encountered + // that has a valid address, select that for change and then skip over further address selection for change. + if (!contract_change_to_input_address + || !std::get_if(&out_address) + || !ExtractDestination(txin.scriptSig, out_address)) { + continue; + } + + coin_control_out.destChange = out_address; + + LogPrintf("INFO: %s: Change sent to %s for contract transaction per contractchangetoinputaddress setting.", + __func__, CBitcoinAddress(coin_control_out.destChange).ToString()); } for (const auto& contract : wtx_out.vContracts) { @@ -82,7 +96,6 @@ bool CreateContractTx(CWalletTx& wtx_out, CReserveKey reserve_key, CAmount burn_ // Nodes validate administrative contracts by checking that the containing // transactions include an input signed by the master key, so select coins // from the master address and send any change back to it: - // if (admin && !SelectMasterInputOutput(coin_control_out)) { return false; } @@ -100,7 +113,7 @@ bool CreateContractTx(CWalletTx& wtx_out, CReserveKey reserve_key, CAmount burn_ wtx_out, reserve_key, applied_fee_out, - &coin_control_out); + &coin_control_out, contract_change_to_input_address); } //! From 9b0e95914096420ef5c32d7b79e931986a401f64 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 21 Nov 2021 12:43:11 -0500 Subject: [PATCH 4/4] Add checkbox for sending contract tx change back to an input address --- src/qt/forms/optionsdialog.ui | 7 +++++++ src/qt/optionsdialog.cpp | 1 + src/qt/optionsmodel.cpp | 10 ++++++++++ src/qt/optionsmodel.h | 1 + 4 files changed, 19 insertions(+) diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 8409391723..ee3632e12c 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -110,6 +110,13 @@ + + + + Return change to an input address for contract transactions + + + diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index bce4859699..3cafe6122e 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -162,6 +162,7 @@ void OptionsDialog::setMapper() mapper->addMapping(ui->gridcoinAtStartup, OptionsModel::StartAtStartup); mapper->addMapping(ui->gridcoinAtStartupMinimised, OptionsModel::StartMin); mapper->addMapping(ui->disableUpdateCheck, OptionsModel::DisableUpdateCheck); + mapper->addMapping(ui->returnChangeToInputAddressForContracts, OptionsModel::ContractChangeToInput); /* Network */ mapper->addMapping(ui->mapPortUpnp, OptionsModel::MapPortUPnP); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 9f0520c4f5..6c0e4d1a58 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -151,6 +151,9 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const case MinStakeSplitValue: // This comes from the core and is a read-write setting (see below). return QVariant((qint64) gArgs.GetArg("-minstakesplitvalue", MIN_STAKE_SPLIT_VALUE_GRC)); + case ContractChangeToInput: + // This comes from the core and is a read-write setting (see below). + return QVariant(gArgs.GetBoolArg("-contractchangetoinputaddress", false)); default: return QVariant(); } @@ -303,6 +306,13 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in //config file. gArgs.ForceSetArg("-minstakesplitvalue", value.toString().toStdString()); updateRwSetting("minstakesplitvalue", gArgs.GetArg("-minstakesplitvalue", MIN_STAKE_SPLIT_VALUE_GRC)); + break; + case ContractChangeToInput: + // This is a core setting stored in the read-write settings file and once set will override the read-only + //config file. + gArgs.ForceSetArg("-contractchangetoinputaddress", value.toBool() ? "1" : "0"); + updateRwSetting("contractchangetoinputaddress", gArgs.GetBoolArg("contractchangetoinputaddress")); + break; default: break; } diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index b2a4f3de86..dc86acee63 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -43,6 +43,7 @@ class OptionsModel : public QAbstractListModel EnableStakeSplit, // bool StakingEfficiency, // double MinStakeSplitValue, // int + ContractChangeToInput, // bool OptionIDRowCount };