[Refactoring] Initialize txbuilder inside sapling_operation

Save a wallet reference when initializing sapling_operation, and pass it
to the txbuilder (so we are sure that the keystore used to sign the keys
belongs to the wallet used to retrieve the inputs).

src/qt/walletmodel.cpp

@@ -607,8 +607,7 @@ OperationResult WalletModel::PrepareShieldedTransaction(WalletModelTransaction*
     if (!opResult) return opResult;
 
     // Create the operation
-    TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, wallet);
-    SaplingOperation operation(txBuilder);
+    SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, wallet);
     auto operationResult = operation.setRecipients(recipients)
              ->setTransparentKeyChange(modelTransaction->getPossibleKeyChange())
              ->setSelectTransparentCoins(fromTransparent)

src/sapling/sapling_operation.cpp

@@ -19,6 +19,18 @@ struct TxValues
     CAmount target{0};
 };
 
+SaplingOperation::SaplingOperation(const Consensus::Params& consensusParams, int nHeight, CWallet* _wallet) :
+    wallet(_wallet),
+    txBuilder(consensusParams, nHeight, _wallet)
+{
+    assert (wallet != nullptr);
+};
+
+SaplingOperation::~SaplingOperation()
+{
+    delete tkeyChange;
+}
+
 OperationResult SaplingOperation::checkTxValues(TxValues& txValues, bool isFromtAddress, bool isFromShielded)
 {
     assert(!isFromtAddress || txValues.shieldedInTotal == 0);

src/sapling/sapling_operation.h

@@ -81,10 +81,8 @@ class FromAddress {
 
 class SaplingOperation {
 public:
-    explicit SaplingOperation(const Consensus::Params& consensusParams, int chainHeight) : txBuilder(consensusParams, chainHeight) {};
-    explicit SaplingOperation(TransactionBuilder& _builder) : txBuilder(_builder) {};
-
-    ~SaplingOperation() { delete tkeyChange; }
+    explicit SaplingOperation(const Consensus::Params& consensusParams, int nHeight, CWallet* _wallet);
+    ~SaplingOperation();
 
     OperationResult build();
     OperationResult send(std::string& retTxHash);
@@ -107,6 +105,13 @@ class SaplingOperation {
     CTransactionRef getFinalTxRef() { return finalTx; }
 
 private:
+    /*
+     * Cannot be nullptr. A pointer to the wallet, used to retrieve the inputs to spend, the keys to create the outputs,
+     * sapling notes and nullifiers, as well as to commit transactions.
+     * The same keystore is passed to the transaction builder in order to produce the required signatures.
+     */
+    CWallet* wallet;
+
     FromAddress fromAddress;
     // In case of no addressFrom filter selected, it will accept any utxo in the wallet as input.
     bool selectFromtaddrs{false};

src/test/librust/sapling_rpc_wallet_tests.cpp

@@ -333,7 +333,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
     // there are no utxos to spend
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF") };
-        SaplingOperation operation(consensusParams, 1);
+        SaplingOperation operation(consensusParams, 1, pwalletMain);
         operation.setFromAddress(taddr1);
         auto res = operation.setRecipients(recipients)->buildAndSend(ret);
         BOOST_CHECK(!res);
@@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
     // minconf cannot be zero when sending from zaddr
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF") };
-        SaplingOperation operation(consensusParams, 1);
+        SaplingOperation operation(consensusParams, 1, pwalletMain);
         operation.setFromAddress(zaddr1);
         auto res = operation.setRecipients(recipients)->setMinDepth(0)->buildAndSend(ret);
         BOOST_CHECK(!res);
@@ -353,7 +353,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
     // there are no unspent notes to spend
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(taddr1, COIN) };
-        SaplingOperation operation(consensusParams, 1);
+        SaplingOperation operation(consensusParams, 1, pwalletMain);
         operation.setFromAddress(zaddr1);
         auto res = operation.setRecipients(recipients)->buildAndSend(ret);
         BOOST_CHECK(!res);
@@ -438,19 +438,16 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
     pwalletMain->BlockConnected(std::make_shared<CBlock>(block), mi->second, vtxConflicted);
     BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed");
 
-    // Context that shieldsendmany requires
-    auto builder = TransactionBuilder(consensusParams, nextBlockHeight, pwalletMain);
-
     std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") };
-    SaplingOperation operation(builder);
+    SaplingOperation operation(consensusParams, nextBlockHeight, pwalletMain);
     operation.setFromAddress(taddr);
     BOOST_CHECK(operation.setRecipients(recipients)
                          ->setMinDepth(0)
                          ->build());
 
     // try from auto-selected transparent address
     std::vector<SendManyRecipient> recipients2 = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") };
-    SaplingOperation operation2(builder);
+    SaplingOperation operation2(consensusParams, nextBlockHeight, pwalletMain);
     BOOST_CHECK(operation2.setSelectTransparentCoins(true)
                           ->setRecipients(recipients2)
                           ->setMinDepth(0)

src/wallet/rpcwallet.cpp

@@ -1168,8 +1168,7 @@ UniValue CreateColdStakeDelegation(const UniValue& params, CTransactionRef& txNe
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling not active yet");
         }
         std::vector<SendManyRecipient> recipients = {SendManyRecipient(ownerKey, *stakeKey, nValue)};
-        TransactionBuilder txBuilder = TransactionBuilder(consensus, nextBlockHeight, pwalletMain);
-        SaplingOperation operation(txBuilder);
+        SaplingOperation operation(consensus, nextBlockHeight, pwalletMain);
         OperationResult res = operation.setSelectShieldedCoins(true)
                                        ->setRecipients(recipients)
                                        ->build();
@@ -1520,8 +1519,7 @@ static SaplingOperation CreateShieldedTransaction(const JSONRPCRequest& request)
     EnsureWalletIsUnlocked();
     LOCK2(cs_main, pwalletMain->cs_wallet);
     int nextBlockHeight = chainActive.Height() + 1;
-    TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain);
-    SaplingOperation operation(txBuilder);
+    SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, pwalletMain);
 
     // Param 0: source of funds. Can either be a valid address, sapling address,
     // or the string "from_transparent"|"from_trans_cold"|"from_shield"

src/wallet/test/wallet_sapling_transactions_validations_tests.cpp

@@ -43,8 +43,7 @@ SaplingOperation createOperationAndBuildTx(std::vector<SendManyRecipient> recipi
                                            bool selectTransparentCoins)
 {
     // Create the operation
-    TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain);
-    SaplingOperation operation(txBuilder);
+    SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, pwalletMain);
     auto operationResult = operation.setRecipients(recipients)
             ->setSelectTransparentCoins(selectTransparentCoins)
             ->setSelectShieldedCoins(!selectTransparentCoins)