diff --git a/qa/rpc-tests/remove_sprout_shielding.py b/qa/rpc-tests/remove_sprout_shielding.py index 4d9504cb9..959f9f85f 100755 --- a/qa/rpc-tests/remove_sprout_shielding.py +++ b/qa/rpc-tests/remove_sprout_shielding.py @@ -34,64 +34,88 @@ HAS_CANOPY = [ ] class RemoveSproutShieldingTest (BitcoinTestFramework): + def __init__(self): + super().__init__() + self.num_nodes = 4 + self.cache_behavior = 'sprout' def setup_nodes(self): return start_nodes(self.num_nodes, self.options.tmpdir, extra_args=[HAS_CANOPY] * self.num_nodes) - def run_test (self): - + def run_test(self): # Generate blocks up to Heartwood activation logging.info("Generating initial blocks. Current height is 200, advance to 210 (activate Heartwood but not Canopy)") self.nodes[0].generate(10) self.sync_all() - # Shield coinbase to Sprout on node 0. Should pass - sprout_addr = self.nodes[0].z_getnewaddress('sprout') - sprout_addr_node2 = self.nodes[2].z_getnewaddress('sprout') - myopid = self.nodes[0].z_shieldcoinbase(get_coinbase_address(self.nodes[0]), sprout_addr, 0)['opid'] - wait_and_assert_operationid_status(self.nodes[0], myopid) - print("taddr -> Sprout z_shieldcoinbase tx accepted before Canopy on node 0") + n0_sprout_addr0 = self.nodes[0].listaddresses()[0]['sprout']['addresses'][0] + n2_sprout_addr = self.nodes[2].listaddresses()[0]['sprout']['addresses'][0] + + # Attempt to shield coinbase to Sprout on node 0. Should fail; + # transfers to Sprout are no longer supported + try: + self.nodes[0].z_shieldcoinbase(get_coinbase_address(self.nodes[0]), n0_sprout_addr0, 0)['opid'] + except JSONRPCException as e: + errorString = e.error['message']; + unsupported_sprout_msg = "Sending funds into the Sprout pool is no longer supported." + assert_equal(unsupported_sprout_msg, errorString) self.nodes[0].generate(1) self.sync_all() - assert_equal(self.nodes[0].z_getbalance(sprout_addr), Decimal('10')) - # Fund taddr_0 from shielded coinbase on node 0 - taddr_0 = self.nodes[0].getnewaddress() + # Check that we have the expected balance from the cached setup + assert_equal(self.nodes[0].z_getbalance(n0_sprout_addr0), Decimal('50')) + + # Fund n0_taddr0 from previously existing Sprout funds on node 0 + n0_taddr0 = self.nodes[0].getnewaddress() for _ in range(3): - recipients = [{"address": taddr_0, "amount": Decimal('1')}] - myopid = self.nodes[0].z_sendmany(sprout_addr, recipients, 1, 0, 'AllowRevealedRecipients') + recipients = [{"address": n0_taddr0, "amount": Decimal('1')}] + myopid = self.nodes[0].z_sendmany(n0_sprout_addr0, recipients, 1, 0, 'AllowRevealedRecipients') wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() self.nodes[0].generate(1) self.sync_all() + assert_equal(self.nodes[0].z_getbalance(n0_taddr0), Decimal('3')) - # Create mergetoaddress taddr -> Sprout transaction and mine on node 0 before it is Canopy-aware. Should pass - merge_tx_0 = self.nodes[0].z_mergetoaddress(["ANY_TADDR"], self.nodes[1].z_getnewaddress('sprout')) + # Create mergetoaddress taddr -> Sprout transaction and mine on node 0 before it is Canopy-aware. Should pass. This will spend the available funds in taddr0 + n1_sprout_addr0 = self.nodes[1].z_getnewaddress('sprout') + merge_tx_0 = self.nodes[0].z_mergetoaddress(["ANY_TADDR"], n1_sprout_addr0, 0) wait_and_assert_operationid_status(self.nodes[0], merge_tx_0['opid']) print("taddr -> Sprout z_mergetoaddress tx accepted before Canopy on node 0") + self.nodes[0].generate(1) + self.sync_all() + assert_equal(self.nodes[1].z_getbalance(n1_sprout_addr0), Decimal('3')) + + # Send some funds back to n0_taddr0 + recipients = [{"address": n0_taddr0, "amount": Decimal('1')}] + myopid = self.nodes[1].z_sendmany(n1_sprout_addr0, recipients, 1, 0, 'AllowRevealedRecipients') + wait_and_assert_operationid_status(self.nodes[1], myopid) + # Mine to one block before Canopy activation on node 0; adding value # to the Sprout pool will fail now since the transaction must be # included in the next (or later) block, after Canopy has activated. - self.nodes[0].generate(5) + self.sync_all() + self.nodes[0].generate(4) self.sync_all() assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['e9ff75a6']['status'], 'pending') + assert_equal(self.nodes[0].z_getbalance(n0_taddr0), Decimal('1')) # Shield coinbase to Sprout on node 0. Should fail - sprout_addr = self.nodes[0].z_getnewaddress('sprout') + n0_coinbase_taddr = get_coinbase_address(self.nodes[0]) + n0_sprout_addr1 = self.nodes[0].z_getnewaddress('sprout') assert_raises_message( JSONRPCException, - "Sprout shielding is not supported after Canopy", + "Sending funds into the Sprout pool is no longer supported.", self.nodes[0].z_shieldcoinbase, - get_coinbase_address(self.nodes[0]), sprout_addr, 0) + n0_coinbase_taddr, n0_sprout_addr1, 0) print("taddr -> Sprout z_shieldcoinbase tx rejected at Canopy activation on node 0") # Create taddr -> Sprout z_sendmany transaction on node 0. Should fail - sprout_addr = self.nodes[1].z_getnewaddress('sprout') - recipients = [{"address": sprout_addr, "amount": Decimal('1')}] - myopid = self.nodes[0].z_sendmany(taddr_0, recipients, 1, 0) - wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Sending funds into the Sprout pool is no longer supported.") + n1_sprout_addr1 = self.nodes[1].z_getnewaddress('sprout') + recipients = [{"address": n1_sprout_addr1, "amount": Decimal('1')}] + myopid = self.nodes[0].z_sendmany(n0_taddr0, recipients, 1, 0) + wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", unsupported_sprout_msg) print("taddr -> Sprout z_sendmany tx rejected at Canopy activation on node 0") # Create z_mergetoaddress [taddr, Sprout] -> Sprout transaction on node 0. Should fail @@ -130,7 +154,7 @@ class RemoveSproutShieldingTest (BitcoinTestFramework): self.sync_all() # Create z_mergetoaddress Sprout -> Sprout transaction on node 1. Should pass - merge_tx_2 = self.nodes[1].z_mergetoaddress(["ANY_SPROUT"], sprout_addr_node2) + merge_tx_2 = self.nodes[1].z_mergetoaddress(["ANY_SPROUT"], n2_sprout_addr) wait_and_assert_operationid_status(self.nodes[1], merge_tx_2['opid']) print("Sprout -> Sprout z_mergetoaddress tx accepted at NU5 activation on node 1") diff --git a/qa/rpc-tests/wallet_shieldcoinbase.py b/qa/rpc-tests/wallet_shieldcoinbase.py index 61aedd841..c6e88e237 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase.py +++ b/qa/rpc-tests/wallet_shieldcoinbase.py @@ -70,7 +70,7 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr) except JSONRPCException as e: errorString = e.error['message'] - assert_equal("Could not find any coinbase funds to shield" in errorString, True) + assert_equal(errorString, "Invalid from address, no payment source found for address.") # Shielding will fail because fee is negative try: @@ -84,14 +84,14 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): self.nodes[0].z_shieldcoinbase("*", myzaddr, Decimal('21000000.00000001')) except JSONRPCException as e: errorString = e.error['message'] - assert_equal("Amount out of range" in errorString, True) + assert_equal(errorString, "Amount out of range") # Shielding will fail because fee is larger than sum of utxos try: self.nodes[0].z_shieldcoinbase("*", myzaddr, 999) except JSONRPCException as e: errorString = e.error['message'] - assert_equal("Insufficient coinbase funds" in errorString, True) + assert_equal(errorString, "Insufficient funds: have 50.00, need 999.00.") # Shielding will fail because limit parameter must be at least 0 try: @@ -193,4 +193,4 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): self.sync_all() # Note, no "if __name__ == '__main__" and call the test here; it's called from -# pool-specific derived classes in wallet_shieldcoinbase_*.py \ No newline at end of file +# pool-specific derived classes in wallet_shieldcoinbase_*.py diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 41c331b69..ac808659a 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -155,6 +155,7 @@ AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) { return preparedTx .map([&](const TransactionEffects& effects) { + effects.LockSpendable(wallet); try { const auto& spendable = effects.GetSpendable(); const auto& payments = effects.GetPayments(); diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index 201a107f7..8255847d8 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp @@ -18,7 +18,6 @@ #include "proof_verifier.h" #include "rpc/protocol.h" #include "rpc/server.h" -#include "transaction_builder.h" #include "timedata.h" #include "util/system.h" #include "util/moneystr.h" @@ -29,7 +28,6 @@ #include "util/match.h" #include "zcash/IncrementalMerkleTree.hpp" #include "miner.h" -#include "wallet/paymentdisclosuredb.h" #include #include @@ -43,60 +41,34 @@ using namespace libzcash; -static int find_output(UniValue obj, int n) { - UniValue outputMapValue = find_value(obj, "outputmap"); - if (!outputMapValue.isArray()) { - throw JSONRPCError(RPC_WALLET_ERROR, "Missing outputmap for JoinSplit operation"); - } - - UniValue outputMap = outputMapValue.get_array(); - assert(outputMap.size() == ZC_NUM_JS_OUTPUTS); - for (size_t i = 0; i < outputMap.size(); i++) { - if (outputMap[i].get_int() == n) { - return i; - } - } - - throw std::logic_error("n is not present in outputmap"); -} - AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase( - TransactionBuilder builder, - CMutableTransaction contextualTx, - std::vector inputs, + WalletTxBuilder builder, + ZTXOSelector ztxoSelector, PaymentAddress toAddress, + TransactionStrategy strategy, + int nUTXOLimit, CAmount fee, UniValue contextInfo) : - builder_(std::move(builder)), tx_(contextualTx), inputs_(inputs), fee_(fee), contextinfo_(contextInfo) + builder_(std::move(builder)), + ztxoSelector_(ztxoSelector), + toAddress_(toAddress), + strategy_(strategy), + nUTXOLimit_(nUTXOLimit), + fee_(fee), + contextinfo_(contextInfo) { - assert(contextualTx.nVersion >= 2); // transaction format version must support vJoinSplit + assert(MoneyRange(fee)); + assert(ztxoSelector.RequireSpendingKeys()); - if (fee < 0 || fee > MAX_MONEY) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Fee is out of range"); - } - - if (inputs.size() == 0) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Empty inputs"); - } - - // Check the destination address is valid for this network i.e. not testnet being used on mainnet - examine(toAddress, match { - [&](CKeyID addr) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2pkh address."); - }, - [&](CScriptID addr) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2sh address."); - }, - [&](libzcash::SaplingPaymentAddress addr) { - tozaddr_ = addr; - }, - [&](libzcash::SproutPaymentAddress addr) { - tozaddr_ = addr; - }, - [&](libzcash::UnifiedAddress addr) { - tozaddr_ = addr; - } - }); + examine(toAddress_, match { + [](const CKeyID&) { + throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2pkh address."); + }, + [](const CScriptID&) { + throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2sh address."); + }, + [](const auto&) { }, + }); // Log the context info if (LogAcceptCategory("zrpcunsafe")) { @@ -104,34 +76,22 @@ AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase( } else { LogPrint("zrpc", "%s: z_shieldcoinbase initialized\n", getId()); } - - // Lock UTXOs - lock_utxos(); - - // Enable payment disclosure if requested - paymentDisclosureMode = fExperimentalPaymentDisclosure; } AsyncRPCOperation_shieldcoinbase::~AsyncRPCOperation_shieldcoinbase() { } void AsyncRPCOperation_shieldcoinbase::main() { - if (isCancelled()) { - unlock_utxos(); // clean up - return; - } - set_state(OperationStatus::EXECUTING); start_execution_clock(); - bool success = false; - #ifdef ENABLE_MINING GenerateBitcoins(false, 0, Params()); #endif + std::optional txid; try { - success = main_impl(); + txid = main_impl(*pwalletMain); } catch (const UniValue& objError) { int code = find_value(objError, "code").get_int(); std::string message = find_value(objError, "message").get_str(); @@ -157,303 +117,141 @@ void AsyncRPCOperation_shieldcoinbase::main() { stop_execution_clock(); - if (success) { + if (txid.has_value()) { set_state(OperationStatus::SUCCESS); } else { set_state(OperationStatus::FAILED); } std::string s = strprintf("%s: z_shieldcoinbase finished (status=%s", getId(), getStateAsString()); - if (success) { - s += strprintf(", txid=%s)\n", tx_.GetHash().ToString()); + if (txid.has_value()) { + s += strprintf(", txid=%s)\n", txid.value().GetHex()); } else { s += strprintf(", error=%s)\n", getErrorMessage()); } LogPrintf("%s",s); +} - unlock_utxos(); // clean up - // !!! Payment disclosure START - if (success && paymentDisclosureMode && paymentDisclosureData_.size()>0) { - uint256 txidhash = tx_.GetHash(); - std::shared_ptr db = PaymentDisclosureDB::sharedInstance(); - for (PaymentDisclosureKeyInfo p : paymentDisclosureData_) { - p.first.hash = txidhash; - if (!db->Put(p.first, p.second)) { - LogPrint("paymentdisclosure", "%s: Payment Disclosure: Error writing entry to database for key %s\n", getId(), p.first.ToString()); +Remaining AsyncRPCOperation_shieldcoinbase::prepare(CWallet& wallet) { + auto spendable = builder_.FindAllSpendableInputs(wallet, ztxoSelector_, COINBASE_MATURITY); + + // Find unspent coinbase utxos and update estimated size + unsigned int max_tx_size = MAX_TX_SIZE_AFTER_SAPLING; + CAmount shieldingValue = 0; + CAmount remainingValue = 0; + size_t estimatedTxSize = 10000; // per ZIP 401 (https://zips.z.cash/zip-0401#specification) + size_t utxoCounter = 0; + size_t numUtxos = 0; + bool maxedOutFlag = false; + + for (const COutput& out : spendable.utxos) { + auto scriptPubKey = out.tx->vout[out.i].scriptPubKey; + CAmount nValue = out.tx->vout[out.i].nValue; + + CTxDestination address; + if (!ExtractDestination(scriptPubKey, address)) { + continue; + } + + utxoCounter++; + if (!maxedOutFlag) { + size_t increase = + (std::get_if(&address) != nullptr) ? CTXIN_SPEND_P2SH_SIZE : CTXIN_SPEND_P2PKH_SIZE; + + if (estimatedTxSize + increase >= max_tx_size || (0 < nUTXOLimit_ && nUTXOLimit_ < utxoCounter)) { + maxedOutFlag = true; } else { - LogPrint("paymentdisclosure", "%s: Payment Disclosure: Successfully added entry to database for key %s\n", getId(), p.first.ToString()); + estimatedTxSize += increase; + shieldingValue += nValue; + numUtxos++; } } - } - // !!! Payment disclosure END -} -bool AsyncRPCOperation_shieldcoinbase::main_impl() { - - CAmount minersFee = fee_; - - size_t numInputs = inputs_.size(); - - CAmount targetAmount = 0; - for (ShieldCoinbaseUTXO & utxo : inputs_) { - targetAmount += utxo.amount; - } - - if (targetAmount <= minersFee) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, - strprintf("Insufficient coinbase funds, have %s and miners fee is %s", - FormatMoney(targetAmount), FormatMoney(minersFee))); - } - - CAmount sendAmount = targetAmount - minersFee; - LogPrint("zrpc", "%s: spending %s to shield %s with fee %s\n", - getId(), FormatMoney(targetAmount), FormatMoney(sendAmount), FormatMoney(minersFee)); - - return std::visit(ShieldToAddress(this, sendAmount), tozaddr_); -} - -void ShieldToAddress::shieldToAddress(const libzcash::RecipientAddress& recipient, AsyncRPCOperation_shieldcoinbase *m_op) { - m_op->builder_.SetFee(m_op->fee_); - - // Sending from a t-address, which we don't have an ovk for. Instead, - // generate a common one from the HD seed. This ensures the data is - // recoverable, while keeping it logically separate from the ZIP 32 - // Sapling key hierarchy, which the user might not be using. - // FIXME: update to use the ZIP-316 OVK (#5511) - HDSeed seed = pwalletMain->GetHDSeedForRPC(); - uint256 ovk = ovkForShieldingFromTaddr(seed); - - // Add transparent inputs - for (auto t : m_op->inputs_) { - m_op->builder_.AddTransparentInput(COutPoint(t.txid, t.vout), t.scriptPubKey, t.amount); - } - - // Send all value to the target recipient - m_op->builder_.SendChangeTo(recipient, ovk); - - // Build the transaction - m_op->tx_ = m_op->builder_.Build().GetTxOrThrow(); -} - -bool ShieldToAddress::operator()(const CKeyID &addr) const { - return false; -} - -bool ShieldToAddress::operator()(const CScriptID &addr) const { - return false; -} - -bool ShieldToAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { - // update the transaction with these inputs - CMutableTransaction rawTx(m_op->tx_); - for (ShieldCoinbaseUTXO & t : m_op->inputs_) { - CTxIn in(COutPoint(t.txid, t.vout)); - rawTx.vin.push_back(in); - } - m_op->tx_ = CTransaction(rawTx); - - // Prepare raw transaction to handle JoinSplits - CMutableTransaction mtx(m_op->tx_); - ed25519::generate_keypair(m_op->joinSplitPrivKey_, m_op->joinSplitPubKey_); - mtx.joinSplitPubKey = m_op->joinSplitPubKey_; - m_op->tx_ = CTransaction(mtx); - - // Create joinsplit - ShieldCoinbaseJSInfo info; - info.vpub_old = sendAmount; - info.vpub_new = 0; - JSOutput jso = JSOutput(zaddr, sendAmount); - info.vjsout.push_back(jso); - UniValue obj = m_op->perform_joinsplit(info); - - auto txAndResult = SignSendRawTransaction(obj, std::nullopt, m_op->testmode); - m_op->tx_ = txAndResult.first; - m_op->set_result(txAndResult.second); - return true; -} - -bool ShieldToAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) const { - ShieldToAddress::shieldToAddress(zaddr, m_op); - - std::vector recipientMappings; - UniValue sendResult = SendTransaction(m_op->tx_, recipientMappings, std::nullopt, m_op->testmode); - m_op->set_result(sendResult); - - return true; -} - -bool ShieldToAddress::operator()(const libzcash::UnifiedAddress &uaddr) const { - // TODO check if an Orchard address is present, send to it if so. - const auto receiver{uaddr.GetSaplingReceiver()}; - if (receiver.has_value()) { - ShieldToAddress::shieldToAddress(receiver.value(), m_op); - - std::vector recipientMappings = {RecipientMapping(uaddr, receiver.value())}; - UniValue sendResult = SendTransaction(m_op->tx_, recipientMappings, std::nullopt, m_op->testmode); - m_op->set_result(sendResult); - - return true; - } - // This UA must contain a transparent address, which can't be the destination of coinbase shielding. - return false; -} - -UniValue AsyncRPCOperation_shieldcoinbase::perform_joinsplit(ShieldCoinbaseJSInfo & info) { - uint32_t consensusBranchId; - uint256 anchor; - { - LOCK(cs_main); - consensusBranchId = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus()); - anchor = pcoinsTip->GetBestAnchor(SPROUT); - } - - - if (anchor.IsNull()) { - throw std::runtime_error("anchor is null"); - } - - // Make sure there are two inputs and two outputs - while (info.vjsin.size() < ZC_NUM_JS_INPUTS) { - info.vjsin.push_back(JSInput()); - } - - while (info.vjsout.size() < ZC_NUM_JS_OUTPUTS) { - info.vjsout.push_back(JSOutput()); - } - - if (info.vjsout.size() != ZC_NUM_JS_INPUTS || info.vjsin.size() != ZC_NUM_JS_OUTPUTS) { - throw runtime_error("unsupported joinsplit input/output counts"); - } - - CMutableTransaction mtx(tx_); - - LogPrint("zrpcunsafe", "%s: creating joinsplit at index %d (vpub_old=%s, vpub_new=%s, in[0]=%s, in[1]=%s, out[0]=%s, out[1]=%s)\n", - getId(), - tx_.vJoinSplit.size(), - FormatMoney(info.vpub_old), FormatMoney(info.vpub_new), - FormatMoney(info.vjsin[0].note.value()), FormatMoney(info.vjsin[1].note.value()), - FormatMoney(info.vjsout[0].value), FormatMoney(info.vjsout[1].value) - ); - - // Generate the proof, this can take over a minute. - std::array inputs - {info.vjsin[0], info.vjsin[1]}; - std::array outputs - {info.vjsout[0], info.vjsout[1]}; - std::array inputMap; - std::array outputMap; - - uint256 esk; // payment disclosure - secret - - assert(mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)); - JSDescription jsdesc = JSDescriptionInfo( - joinSplitPubKey_, - anchor, - inputs, - outputs, - info.vpub_old, - info.vpub_new - ).BuildRandomized( - inputMap, - outputMap, - !this->testmode, - &esk); // parameter expects pointer to esk, so pass in address - { - auto verifier = ProofVerifier::Strict(); - if (!(verifier.VerifySprout(jsdesc, joinSplitPubKey_))) { - throw std::runtime_error("error verifying joinsplit"); + if (maxedOutFlag) { + remainingValue += nValue; } } - mtx.vJoinSplit.push_back(jsdesc); + spendable.LimitTransparentUtxos(numUtxos); - // Empty output script. - CScript scriptCode; - CTransaction signTx(mtx); - std::vector allPrevOutputs; - for (ShieldCoinbaseUTXO & t : inputs_) { - allPrevOutputs.emplace_back(t.amount, t.scriptPubKey); - } - PrecomputedTransactionData txdata(signTx, allPrevOutputs); - uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL, 0, consensusBranchId, txdata); - - // Add the signature - ed25519::sign( - joinSplitPrivKey_, - {dataToBeSigned.begin(), 32}, - mtx.joinSplitSig); - - // Sanity check - if (!ed25519::verify( - mtx.joinSplitPubKey, - mtx.joinSplitSig, - {dataToBeSigned.begin(), 32})) - { - throw std::runtime_error("ed25519_verify failed"); + // TODO: Don’t check this outside of `PrepareTransaction` + if (shieldingValue < fee_) { + ThrowInputSelectionError( + InvalidFundsError(shieldingValue, InsufficientFundsError(fee_)), + ztxoSelector_, + strategy_); } - CTransaction rawTx(mtx); - tx_ = rawTx; + // FIXME: This should be internal, and shouldn’t be a payment, but more like a change address, + // as we don’t know the amount at this point. + std::vector payments = { Payment(toAddress_, shieldingValue - fee_, std::nullopt) }; - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << rawTx; + auto preparationResult = builder_.PrepareTransaction( + wallet, + ztxoSelector_, + spendable, + payments, + chainActive, + strategy_, + fee_, + nAnchorConfirmations); - std::string encryptedNote1; - std::string encryptedNote2; - { - CDataStream ss2(SER_NETWORK, PROTOCOL_VERSION); - ss2 << ((unsigned char) 0x00); - ss2 << jsdesc.ephemeralKey; - ss2 << jsdesc.ciphertexts[0]; - ss2 << ZCJoinSplit::h_sig(jsdesc.randomSeed, jsdesc.nullifiers, joinSplitPubKey_); + preparationResult + .map_error([&](const InputSelectionError& err) { + ThrowInputSelectionError(err, ztxoSelector_, strategy_); + }) + .map([&](const TransactionEffects& effects) { + effects.LockSpendable(wallet); + effects_ = effects; + }); - encryptedNote1 = HexStr(ss2.begin(), ss2.end()); + return Remaining(utxoCounter, numUtxos, remainingValue, shieldingValue); +} + +uint256 AsyncRPCOperation_shieldcoinbase::main_impl(CWallet& wallet) { + uint256 txid; + + try { + const auto& spendable = effects_->GetSpendable(); + const auto& payments = effects_->GetPayments(); + spendable.LogInputs(getId()); + + LogPrint("zrpcunsafe", "%s: spending %s to send %s with fee %s\n", getId(), + FormatMoney(payments.Total()), + FormatMoney(spendable.Total()), + FormatMoney(effects_->GetFee())); + LogPrint("zrpc", "%s: total transparent input: %s (to choose from)\n", getId(), + FormatMoney(spendable.GetTransparentTotal())); + LogPrint("zrpcunsafe", "%s: total shielded input: %s (to choose from)\n", getId(), + FormatMoney(spendable.GetSaplingTotal() + spendable.GetOrchardTotal())); + LogPrint("zrpc", "%s: total transparent output: %s\n", getId(), + FormatMoney(payments.GetTransparentTotal())); + LogPrint("zrpcunsafe", "%s: total shielded Sapling output: %s\n", getId(), + FormatMoney(payments.GetSaplingTotal())); + LogPrint("zrpcunsafe", "%s: total shielded Orchard output: %s\n", getId(), + FormatMoney(payments.GetOrchardTotal())); + LogPrint("zrpc", "%s: fee: %s\n", getId(), FormatMoney(effects_->GetFee())); + + auto buildResult = effects_->ApproveAndBuild( + Params().GetConsensus(), + wallet, + chainActive, + strategy_); + + auto tx = buildResult.GetTxOrThrow(); + + UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode); + set_result(sendResult); + + txid = tx.GetHash(); + + effects_->UnlockSpendable(wallet); + return txid; + } catch (...) { + effects_->UnlockSpendable(wallet); + throw; } - { - CDataStream ss2(SER_NETWORK, PROTOCOL_VERSION); - ss2 << ((unsigned char) 0x01); - ss2 << jsdesc.ephemeralKey; - ss2 << jsdesc.ciphertexts[1]; - ss2 << ZCJoinSplit::h_sig(jsdesc.randomSeed, jsdesc.nullifiers, joinSplitPubKey_); - - encryptedNote2 = HexStr(ss2.begin(), ss2.end()); - } - - UniValue arrInputMap(UniValue::VARR); - UniValue arrOutputMap(UniValue::VARR); - for (size_t i = 0; i < ZC_NUM_JS_INPUTS; i++) { - arrInputMap.push_back(static_cast(inputMap[i])); - } - for (size_t i = 0; i < ZC_NUM_JS_OUTPUTS; i++) { - arrOutputMap.push_back(static_cast(outputMap[i])); - } - - KeyIO keyIO(Params()); - - // !!! Payment disclosure START - size_t js_index = tx_.vJoinSplit.size() - 1; - uint256 placeholder; - for (int i = 0; i < ZC_NUM_JS_OUTPUTS; i++) { - uint8_t mapped_index = outputMap[i]; - // placeholder for txid will be filled in later when tx has been finalized and signed. - PaymentDisclosureKey pdKey = {placeholder, js_index, mapped_index}; - JSOutput output = outputs[mapped_index]; - libzcash::SproutPaymentAddress zaddr = output.addr; // randomized output - PaymentDisclosureInfo pdInfo = {PAYMENT_DISCLOSURE_VERSION_EXPERIMENTAL, esk, joinSplitPrivKey_, zaddr}; - paymentDisclosureData_.push_back(PaymentDisclosureKeyInfo(pdKey, pdInfo)); - - LogPrint("paymentdisclosure", "%s: Payment Disclosure: js=%d, n=%d, zaddr=%s\n", getId(), js_index, int(mapped_index), keyIO.EncodePaymentAddress(zaddr)); - } - // !!! Payment disclosure END - - UniValue obj(UniValue::VOBJ); - obj.pushKV("encryptednote1", encryptedNote1); - obj.pushKV("encryptednote2", encryptedNote2); - obj.pushKV("rawtxn", HexStr(ss.begin(), ss.end())); - obj.pushKV("inputmap", arrInputMap); - obj.pushKV("outputmap", arrOutputMap); - return obj; } /** @@ -470,25 +268,3 @@ UniValue AsyncRPCOperation_shieldcoinbase::getStatus() const { obj.pushKV("params", contextinfo_ ); return obj; } - -/** - * Lock input utxos - */ - void AsyncRPCOperation_shieldcoinbase::lock_utxos() { - LOCK2(cs_main, pwalletMain->cs_wallet); - for (auto utxo : inputs_) { - COutPoint outpt(utxo.txid, utxo.vout); - pwalletMain->LockCoin(outpt); - } -} - -/** - * Unlock input utxos - */ -void AsyncRPCOperation_shieldcoinbase::unlock_utxos() { - LOCK2(cs_main, pwalletMain->cs_wallet); - for (auto utxo : inputs_) { - COutPoint outpt(utxo.txid, utxo.vout); - pwalletMain->UnlockCoin(outpt); - } -} diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.h b/src/wallet/asyncrpcoperation_shieldcoinbase.h index cc7e48251..bde3890e4 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.h +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.h @@ -12,7 +12,7 @@ #include "zcash/JoinSplit.hpp" #include "zcash/Address.hpp" #include "wallet.h" -#include "wallet/paymentdisclosure.h" +#include "wallet/wallet_tx_builder.h" #include #include @@ -23,29 +23,45 @@ using namespace libzcash; -struct ShieldCoinbaseUTXO { - uint256 txid; - int vout; - CScript scriptPubKey; - CAmount amount; -}; +/** +When estimating the number of coinbase utxos we can shield in a single transaction: +1. An Orchard receiver is 9165 bytes. +2. Transaction overhead ~ 100 bytes +3. Spending a typical P2PKH is >=148 bytes, as defined in CTXIN_SPEND_P2PKH_SIZE. +4. Spending a multi-sig P2SH address can vary greatly: + https://github.com/bitcoin/bitcoin/blob/c3ad56f4e0b587d8d763af03d743fdfc2d180c9b/src/main.cpp#L517 + In real-world coinbase utxos, we consider a 3-of-3 multisig, where the size is roughly: + (3*(33+1))+3 = 105 byte redeem script + 105 + 1 + 3*(73+1) = 328 bytes of scriptSig, rounded up to 400 based on testnet experiments. +*/ +#define CTXIN_SPEND_P2SH_SIZE 400 -// Package of info which is passed to perform_joinsplit methods. -struct ShieldCoinbaseJSInfo -{ - std::vector vjsin; - std::vector vjsout; - CAmount vpub_old = 0; - CAmount vpub_new = 0; +#define SHIELD_COINBASE_DEFAULT_LIMIT 50 + +// transaction.h comment: spending taddr output requires CTxIn >= 148 bytes and +// typical taddr txout is 34 bytes +#define CTXIN_SPEND_P2PKH_SIZE 148 +#define CTXOUT_REGULAR_SIZE 34 + +class Remaining { +public: + Remaining(size_t utxoCounter, size_t numUtxos, CAmount remainingValue, CAmount shieldingValue): + utxoCounter(utxoCounter), numUtxos(numUtxos), remainingValue(remainingValue), shieldingValue(shieldingValue) { } + + size_t utxoCounter; + size_t numUtxos; + CAmount remainingValue; + CAmount shieldingValue; }; class AsyncRPCOperation_shieldcoinbase : public AsyncRPCOperation { public: AsyncRPCOperation_shieldcoinbase( - TransactionBuilder builder, - CMutableTransaction contextualTx, - std::vector inputs, + WalletTxBuilder builder, + ZTXOSelector ztxoSelector, PaymentAddress toAddress, + TransactionStrategy strategy, + int nUTXOLimit, CAmount fee = DEFAULT_FEE, UniValue contextInfo = NullUniValue); virtual ~AsyncRPCOperation_shieldcoinbase(); @@ -56,63 +72,30 @@ public: AsyncRPCOperation_shieldcoinbase& operator=(AsyncRPCOperation_shieldcoinbase const&) = delete; // Copy assign AsyncRPCOperation_shieldcoinbase& operator=(AsyncRPCOperation_shieldcoinbase &&) = delete; // Move assign + Remaining prepare(CWallet& wallet); + virtual void main(); virtual UniValue getStatus() const; - bool testmode = false; // Set to true to disable sending txs and generating proofs - - bool paymentDisclosureMode = false; // Set to true to save esk for encrypted notes in payment disclosure database. + bool testmode{false}; // Set to true to disable sending txs and generating proofs private: - friend class ShieldToAddress; friend class TEST_FRIEND_AsyncRPCOperation_shieldcoinbase; // class for unit testing + WalletTxBuilder builder_; + ZTXOSelector ztxoSelector_; + PaymentAddress toAddress_; + TransactionStrategy strategy_; + int nUTXOLimit_; + CAmount fee_; + std::optional effects_; + UniValue contextinfo_; // optional data to include in return value from getStatus() - CAmount fee_; - PaymentAddress tozaddr_; - - ed25519::VerificationKey joinSplitPubKey_; - ed25519::SigningKey joinSplitPrivKey_; - - std::vector inputs_; - - TransactionBuilder builder_; - CTransaction tx_; - - bool main_impl(); - - // JoinSplit without any input notes to spend - UniValue perform_joinsplit(ShieldCoinbaseJSInfo &); - - void lock_utxos(); - - void unlock_utxos(); - - // payment disclosure! - std::vector paymentDisclosureData_; + uint256 main_impl(CWallet& wallet); }; -class ShieldToAddress -{ -private: - AsyncRPCOperation_shieldcoinbase *m_op; - CAmount sendAmount; - - static void shieldToAddress(const libzcash::RecipientAddress& recipient, AsyncRPCOperation_shieldcoinbase *m_op); -public: - ShieldToAddress(AsyncRPCOperation_shieldcoinbase *op, CAmount sendAmount) : - m_op(op), sendAmount(sendAmount) {} - - bool operator()(const CKeyID &zaddr) const; - bool operator()(const CScriptID &zaddr) const; - bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; - bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - bool operator()(const libzcash::UnifiedAddress &uaddr) const; -}; - - // To test private methods, a friend class can act as a proxy class TEST_FRIEND_AsyncRPCOperation_shieldcoinbase { public: @@ -120,22 +103,10 @@ public: TEST_FRIEND_AsyncRPCOperation_shieldcoinbase(std::shared_ptr ptr) : delegate(ptr) {} - CTransaction getTx() { - return delegate->tx_; - } - - void setTx(CTransaction tx) { - delegate->tx_ = tx; - } - // Delegated methods - bool main_impl() { - return delegate->main_impl(); - } - - UniValue perform_joinsplit(ShieldCoinbaseJSInfo &info) { - return delegate->perform_joinsplit(info); + uint256 main_impl(CWallet& wallet) { + return delegate->main_impl(wallet); } void set_state(OperationStatus state) { @@ -145,4 +116,3 @@ public: #endif // ZCASH_WALLET_ASYNCRPCOPERATION_SHIELDCOINBASE_H - diff --git a/src/wallet/gtest/test_rpc_wallet.cpp b/src/wallet/gtest/test_rpc_wallet.cpp index 5bcef4ad2..3989b29cf 100644 --- a/src/wallet/gtest/test_rpc_wallet.cpp +++ b/src/wallet/gtest/test_rpc_wallet.cpp @@ -21,69 +21,6 @@ bool find_error(const UniValue& objError, const std::string& expected) { return find_value(objError, "message").get_str().find(expected) != string::npos; } -TEST(WalletRPCTests,ZShieldCoinbaseInternals) -{ - LoadProofParameters(); - - SelectParams(CBaseChainParams::TESTNET); - const Consensus::Params& consensusParams = Params().GetConsensus(); - - // we need to use pwalletMain because of AsyncRPCOperation_shieldcoinbase - LoadGlobalWallet(); - { - LOCK2(cs_main, pwalletMain->cs_wallet); - - // Mutable tx containing contextual information we need to build tx - // We removed the ability to create pre-Sapling Sprout proofs, so we can - // only create Sapling-onwards transactions. - int nHeight = consensusParams.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight; - CMutableTransaction mtx = CreateNewContextualCMutableTransaction(consensusParams, nHeight + 1, false); - - // Add keys manually - auto pa = pwalletMain->GenerateNewSproutZKey(); - - // Insufficient funds - { - std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, pa) ); - operation->main(); - EXPECT_TRUE(operation->isFailed()); - std::string msg = operation->getErrorMessage(); - EXPECT_TRUE(msg.find("Insufficient coinbase funds") != string::npos); - } - - // Test the perform_joinsplit methods. - { - // Dummy input so the operation object can be instantiated. - std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,100000} }; - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, pa) ); - std::shared_ptr ptr = std::dynamic_pointer_cast (operation); - TEST_FRIEND_AsyncRPCOperation_shieldcoinbase proxy(ptr); - static_cast(operation.get())->testmode = true; - - ShieldCoinbaseJSInfo info; - info.vjsin.push_back(JSInput()); - info.vjsin.push_back(JSInput()); - info.vjsin.push_back(JSInput()); - try { - proxy.perform_joinsplit(info); - } catch (const std::runtime_error & e) { - EXPECT_TRUE(string(e.what()).find("unsupported joinsplit input") != string::npos); - } - - info.vjsin.clear(); - try { - proxy.perform_joinsplit(info); - } catch (const std::runtime_error & e) { - EXPECT_TRUE(string(e.what()).find("error verifying joinsplit") != string::npos); - } - } - - } - UnloadGlobalWallet(); -} - - // TODO: test private methods TEST(WalletRPCTests, RPCZMergeToAddressInternals) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8da116f34..da3db9c9b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4700,10 +4700,6 @@ UniValue z_getoperationstatus_IMPL(const UniValue& params, bool fRemoveFinishedO return ret; } -// transaction.h comment: spending taddr output requires CTxIn >= 148 bytes and typical taddr txout is 34 bytes -#define CTXIN_SPEND_DUST_SIZE 148 -#define CTXOUT_REGULAR_SIZE 34 - size_t EstimateTxSize( const ZTXOSelector& ztxoSelector, const std::vector& recipients, @@ -4762,7 +4758,7 @@ size_t EstimateTxSize( CTransaction tx(mtx); txsize += GetSerializeSize(tx, SER_NETWORK, tx.nVersion); if (fromTaddr) { - txsize += CTXIN_SPEND_DUST_SIZE; + txsize += CTXIN_SPEND_P2PKH_SIZE; txsize += CTXOUT_REGULAR_SIZE; // There will probably be taddr change } txsize += CTXOUT_REGULAR_SIZE * taddrRecipientCount; @@ -5030,7 +5026,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) // Create operation and add to global queue auto nAnchorDepth = std::min((unsigned int) nMinDepth, nAnchorConfirmations); - WalletTxBuilder builder(Params(), minRelayTxFee); + WalletTxBuilder builder(chainparams, minRelayTxFee); std::shared_ptr q = getAsyncRPCQueue(); std::shared_ptr operation( @@ -5174,29 +5170,14 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { return migrationStatus; } -/** -When estimating the number of coinbase utxos we can shield in a single transaction: -1. Joinsplit description is 1802 bytes. -2. Transaction overhead ~ 100 bytes -3. Spending a typical P2PKH is >=148 bytes, as defined in CTXIN_SPEND_DUST_SIZE. -4. Spending a multi-sig P2SH address can vary greatly: - https://github.com/bitcoin/bitcoin/blob/c3ad56f4e0b587d8d763af03d743fdfc2d180c9b/src/main.cpp#L517 - In real-world coinbase utxos, we consider a 3-of-3 multisig, where the size is roughly: - (3*(33+1))+3 = 105 byte redeem script - 105 + 1 + 3*(73+1) = 328 bytes of scriptSig, rounded up to 400 based on testnet experiments. -*/ -#define CTXIN_SPEND_P2SH_SIZE 400 - -#define SHIELD_COINBASE_DEFAULT_LIMIT 50 - UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - if (fHelp || params.size() < 2 || params.size() > 4) + if (fHelp || params.size() < 2 || params.size() > 5) throw runtime_error( - "z_shieldcoinbase \"fromaddress\" \"tozaddress\" ( fee ) ( limit )\n" + "z_shieldcoinbase \"fromaddress\" \"tozaddress\" ( fee ) ( limit ) ( privacyPolicy )\n" "\nShield transparent coinbase funds by sending to a shielded zaddr. This is an asynchronous operation and utxos" "\nselected for shielding will be locked. If there is an error, they are unlocked. The RPC call `listlockunspent`" "\ncan be used to return a list of locked utxos. The number of coinbase utxos selected for shielding can be limited" @@ -5211,12 +5192,15 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) + strprintf("%s", FormatMoney(DEFAULT_FEE)) + ") The fee amount to attach to this transaction.\n" "4. limit (numeric, optional, default=" + strprintf("%d", SHIELD_COINBASE_DEFAULT_LIMIT) + ") Limit on the maximum number of utxos to shield. Set to 0 to use as many as will fit in the transaction.\n" + "5. privacyPolicy (string, optional, default=\"AllowRevealedSenders\") Policy for what information leakage is acceptable.\n" + " This allows the same values as z_sendmany, but only \"AllowRevealedSenders\" and \"AllowLinkingAccountAddresses\"\n" + " are relevant.\n" "\nResult:\n" "{\n" - " \"remainingUTXOs\": xxx (numeric) Number of coinbase utxos still available for shielding.\n" - " \"remainingValue\": xxx (numeric) Value of coinbase utxos still available for shielding.\n" - " \"shieldingUTXOs\": xxx (numeric) Number of coinbase utxos being shielded.\n" - " \"shieldingValue\": xxx (numeric) Value of coinbase utxos being shielded.\n" + " \"remainingUTXOs\": xxx (numeric) Number of coinbase utxos still available for shielding.\n" + " \"remainingValue\": xxx (numeric) Value of coinbase utxos still available for shielding.\n" + " \"shieldingUTXOs\": xxx (numeric) Number of coinbase utxos being shielded.\n" + " \"shieldingValue\": xxx (numeric) Value of coinbase utxos being shielded.\n" " \"opid\": xxx (string) An operationid to pass to z_getoperationstatus to get the result of the operation.\n" "}\n" "\nExamples:\n" @@ -5227,6 +5211,25 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) LOCK2(cs_main, pwalletMain->cs_wallet); ThrowIfInitialBlockDownload(); + const auto chainparams = Params(); + const auto consensus = chainparams.GetConsensus(); + int nextBlockHeight = chainActive.Height() + 1; + + // This API cannot be used to create coinbase shielding transactions before Sapling + // activation. + if (!consensus.NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_SAPLING)) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, "Cannot create shielded transactions before Sapling has activated"); + } + + auto strategy = + ResolveTransactionStrategy( + ReifyPrivacyPolicy( + PrivacyPolicy::AllowRevealedSenders, + params.size() > 4 ? std::optional(params[4].get_str()) : std::nullopt), + // This has identical behavior to `AllowRevealedSenders` for this operation, but + // technically, this is what “LegacyCompat” means, so just for consistency. + PrivacyPolicy::AllowFullyTransparent); // Validate the from address auto fromaddress = params[0].get_str(); @@ -5235,53 +5238,58 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); // Set of source addresses to filter utxos by - std::set sources = {}; - if (!isFromWildcard) { - CTxDestination taddr = keyIO.DecodeDestination(fromaddress); - if (IsValidDestination(taddr)) { - sources.insert(taddr); + ZTXOSelector ztxoSelector = [&]() { + if (isFromWildcard) { + return CWallet::LegacyTransparentZTXOSelector(true, TransparentCoinbasePolicy::Require); } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or \"*\"."); - } - } + auto decoded = keyIO.DecodePaymentAddress(fromaddress); + if (!decoded.has_value()) { + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + "Invalid from address: should be a taddr, zaddr, UA, or the string 'ANY_TADDR'."); + } - int nextBlockHeight = chainActive.Height() + 1; - const bool canopyActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_CANOPY); + auto ztxoSelectorOpt = pwalletMain->ZTXOSelectorForAddress( + decoded.value(), + true, + TransparentCoinbasePolicy::Require, + strategy.AllowLinkingAccountAddresses()); + + if (!ztxoSelectorOpt.has_value()) { + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + "Invalid from address, no payment source found for address."); + } + + auto selectorAccount = pwalletMain->FindAccountForSelector(ztxoSelectorOpt.value()); + examine(decoded.value(), match { + [&](const libzcash::UnifiedAddress&) { + if (!selectorAccount.has_value() || selectorAccount.value() == ZCASH_LEGACY_ACCOUNT) { + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + "Invalid from address, UA does not correspond to a known account."); + } + }, + [&](const auto&) { + if (selectorAccount.has_value() && selectorAccount.value() != ZCASH_LEGACY_ACCOUNT) { + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + "Invalid from address: is a bare receiver from a Unified Address in this wallet. Provide the UA as returned by z_getaddressforaccount instead."); + } + } + }); + + return ztxoSelectorOpt.value(); + } + }(); // Validate the destination address auto destStr = params[1].get_str(); auto destaddress = keyIO.DecodePaymentAddress(destStr); - if (destaddress.has_value()) { - examine(destaddress.value(), match { - [&](const CKeyID& addr) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2pkh address."); - }, - [&](const CScriptID&) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a p2sh address."); - }, - [&](const libzcash::SaplingPaymentAddress& addr) { - // OK - }, - [&](const libzcash::SproutPaymentAddress& addr) { - if (canopyActive) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy activation"); - } - }, - [&](const libzcash::UnifiedAddress& ua) { - if (!ua.GetSaplingReceiver().has_value()) { - throw JSONRPCError( - RPC_VERIFY_REJECTED, - "Only Sapling shielding is currently supported by z_shieldcoinbase. " - "Use z_sendmany with a transaction amount that results in no change for Orchard shielding."); - } - involvesOrchard = ua.GetOrchardReceiver().has_value(); - } - }); - } else { + if (!destaddress.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destStr); } - // Convert fee from currency format to zatoshis CAmount nFee = DEFAULT_FEE; if (params.size() > 2) { if (params[2].get_real() == 0.0) { @@ -5291,98 +5299,9 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) } } - int nLimit = SHIELD_COINBASE_DEFAULT_LIMIT; - if (params.size() > 3) { - nLimit = params[3].get_int(); - if (nLimit < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Limit on maximum number of utxos cannot be negative"); - } - } - - const bool saplingActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_SAPLING); - - // We cannot create shielded transactions before Sapling activates. - if (!saplingActive) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, "Cannot create shielded transactions before Sapling has activated"); - } - - bool overwinterActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_OVERWINTER); - assert(overwinterActive); - unsigned int max_tx_size = MAX_TX_SIZE_AFTER_SAPLING; - - // Prepare to get coinbase utxos - std::vector inputs; - CAmount shieldedValue = 0; - CAmount remainingValue = 0; - size_t estimatedTxSize = 2000; // 1802 joinsplit description + tx overhead + wiggle room - size_t utxoCounter = 0; - bool maxedOutFlag = false; - const size_t mempoolLimit = nLimit; - - // Get available utxos - vector vecOutputs; - pwalletMain->AvailableCoins(vecOutputs, std::nullopt, true, NULL, false, true); - - // Find unspent coinbase utxos and update estimated size - for (const COutput& out : vecOutputs) { - if (!out.fSpendable) { - continue; - } - - CTxDestination address; - if (!ExtractDestination(out.tx->vout[out.i].scriptPubKey, address)) { - continue; - } - - // If from address was not the wildcard "*", filter utxos - if (sources.size() > 0 && !sources.count(address)) { - continue; - } - - if (!out.tx->IsCoinBase()) { - continue; - } - - utxoCounter++; - auto scriptPubKey = out.tx->vout[out.i].scriptPubKey; - CAmount nValue = out.tx->vout[out.i].nValue; - - if (!maxedOutFlag) { - size_t increase = (std::get_if(&address) != nullptr) ? CTXIN_SPEND_P2SH_SIZE : CTXIN_SPEND_DUST_SIZE; - if (estimatedTxSize + increase >= max_tx_size || - (mempoolLimit > 0 && utxoCounter > mempoolLimit)) - { - maxedOutFlag = true; - } else { - estimatedTxSize += increase; - ShieldCoinbaseUTXO utxo = {out.tx->GetHash(), out.i, scriptPubKey, nValue}; - inputs.push_back(utxo); - shieldedValue += nValue; - } - } - - if (maxedOutFlag) { - remainingValue += nValue; - } - } - - size_t numUtxos = inputs.size(); - - if (numUtxos == 0) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any coinbase funds to shield."); - } - - if (shieldedValue < nFee) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, - strprintf("Insufficient coinbase funds, have %s, which is less than miners fee %s", - FormatMoney(shieldedValue), FormatMoney(nFee))); - } - - // Check that the user specified fee is sane (if too high, it can result in error -25 absurd fee) - CAmount netAmount = shieldedValue - nFee; - if (nFee > netAmount) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee %s is greater than the net amount to be shielded %s", FormatMoney(nFee), FormatMoney(netAmount))); + int nUTXOLimit = params.size() > 3 ? params[3].get_int() : SHIELD_COINBASE_DEFAULT_LIMIT; + if (nUTXOLimit < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Limit on maximum number of utxos cannot be negative"); } // Keep record of parameters in context object @@ -5391,42 +5310,26 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) contextInfo.pushKV("toaddress", params[1]); contextInfo.pushKV("fee", ValueFromAmount(nFee)); - // Builder (used if Sapling addresses are involved) - std::optional orchardAnchor; - if (nAnchorConfirmations > 0 && (involvesOrchard || nPreferredTxVersion >= ZIP225_MIN_TX_VERSION)) { - // Allow Orchard recipients by setting an Orchard anchor. - auto orchardAnchorHeight = nextBlockHeight - nAnchorConfirmations; - if (Params().GetConsensus().NetworkUpgradeActive(orchardAnchorHeight, Consensus::UPGRADE_NU5)) { - auto anchorBlockIndex = chainActive[orchardAnchorHeight]; - assert(anchorBlockIndex != nullptr); - orchardAnchor = anchorBlockIndex->hashFinalOrchardRoot; - } - } - TransactionBuilder builder = TransactionBuilder( - Params().GetConsensus(), nextBlockHeight, orchardAnchor, pwalletMain); + // Create the wallet builder + WalletTxBuilder builder(chainparams, minRelayTxFee); - // Contextual transaction we will build on - // (used if no Sapling addresses are involved) - CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction( - Params().GetConsensus(), nextBlockHeight, - nPreferredTxVersion < ZIP225_MIN_TX_VERSION); - if (contextualTx.nVersion == 1) { - contextualTx.nVersion = 2; // Tx format should support vJoinSplit - } + auto async_shieldcoinbase = + new AsyncRPCOperation_shieldcoinbase( + std::move(builder), ztxoSelector, destaddress.value(), strategy, nUTXOLimit, nFee, contextInfo); + auto results = async_shieldcoinbase->prepare(*pwalletMain); // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase( - std::move(builder), contextualTx, inputs, destaddress.value(), nFee, contextInfo) ); + std::shared_ptr operation(async_shieldcoinbase); q->addOperation(operation); AsyncRPCOperationId operationId = operation->getId(); // Return continuation information UniValue o(UniValue::VOBJ); - o.pushKV("remainingUTXOs", static_cast(utxoCounter - numUtxos)); - o.pushKV("remainingValue", ValueFromAmount(remainingValue)); - o.pushKV("shieldingUTXOs", static_cast(numUtxos)); - o.pushKV("shieldingValue", ValueFromAmount(shieldedValue)); + o.pushKV("remainingUTXOs", static_cast(results.utxoCounter - results.numUtxos)); + o.pushKV("remainingValue", ValueFromAmount(results.remainingValue)); + o.pushKV("shieldingUTXOs", static_cast(results.numUtxos)); + o.pushKV("shieldingValue", ValueFromAmount(results.shieldingValue)); o.pushKV("opid", operationId); return o; } @@ -5707,7 +5610,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) CAmount nValue = out.tx->vout[out.i].nValue; if (!maxedOutUTXOsFlag) { - size_t increase = (std::get_if(&address) != nullptr) ? CTXIN_SPEND_P2SH_SIZE : CTXIN_SPEND_DUST_SIZE; + size_t increase = (std::get_if(&address) != nullptr) ? CTXIN_SPEND_P2SH_SIZE : CTXIN_SPEND_P2PKH_SIZE; if (estimatedTxSize + increase >= max_tx_size || (mempoolLimit > 0 && utxoCounter > mempoolLimit)) { diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 39475c566..c136ab163 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -1521,26 +1521,24 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters) "100 -1" ), runtime_error); - // Mutable tx containing contextual information we need to build tx - UniValue retValue = CallRPC("getblockcount"); - int nHeight = retValue.get_int(); - CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nHeight + 1, false); - if (mtx.nVersion == 1) { - mtx.nVersion = 2; - } - // Test constructor of AsyncRPCOperation_shieldcoinbase KeyIO keyIO(Params()); + UniValue retValue; + BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewaddress")); + auto taddr2 = std::get(keyIO.DecodePaymentAddress(retValue.get_str()).value()); auto testnetzaddr = std::get(keyIO.DecodePaymentAddress("ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP").value()); + auto selector = pwalletMain->ZTXOSelectorForAddress( + taddr2, + true, + // In the real transaction builder we use either Require or Disallow, but here we + // are checking that there are no UTXOs at all, so we allow either to be selected to + // confirm this. + TransparentCoinbasePolicy::Allow, + false).value(); + WalletTxBuilder builder(Params(), minRelayTxFee); try { - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, {}, testnetzaddr, -1 )); - } catch (const UniValue& objError) { - BOOST_CHECK( find_error(objError, "Fee is out of range")); - } - - try { - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, {}, testnetzaddr, 1)); + std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(std::move(builder), selector, testnetzaddr, PrivacyPolicy::AllowRevealedSenders, SHIELD_COINBASE_DEFAULT_LIMIT, 1)); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Empty inputs")); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1304e9335..0c81b48ee 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -7757,6 +7757,13 @@ bool ZTXOSelector::SelectsOrchard() const { }); } +void SpendableInputs::LimitTransparentUtxos(size_t maxUtxoCount) +{ + while (utxos.size() > maxUtxoCount) { + utxos.pop_back(); + } +} + bool SpendableInputs::LimitToAmount( const CAmount amountRequired, const CAmount dustThreshold, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 705c1e701..236ce0419 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -958,6 +958,11 @@ public: std::vector saplingNoteEntries; std::vector orchardNoteMetadata; + /** + * Retain the first `maxUtxoCount` utxos, and discard the rest. + */ + void LimitTransparentUtxos(size_t maxUtxoCount); + /** * Selectively discard notes that are not required to obtain the desired * amount. Returns `false` if the available inputs do not add up to the diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index 6bead7085..cb98426a4 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -157,7 +157,7 @@ WalletTxBuilder::PrepareTransaction( return selected.map([&](const InputSelection& resolvedSelection) { auto ovks = SelectOVKs(wallet, selector, spendable); - auto effects = TransactionEffects( + return TransactionEffects( anchorConfirmations, resolvedSelection.GetInputs(), resolvedSelection.GetPayments(), @@ -166,8 +166,6 @@ WalletTxBuilder::PrepareTransaction( ovks.first, ovks.second, anchorHeight); - effects.LockSpendable(wallet); - return effects; }); }