diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 07372d80a..5a14319fc 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -152,6 +152,11 @@ class WalletZSendmanyTest(BitcoinTestFramework): # The following assertion might fail nondeterministically # assert_equal(node2balance, Decimal('16.99799000')) + # try sending with a memo to a taddr, which should fail + recipients = [{"address":self.nodes[0].getnewaddress(), "amount":1, "memo":"DEADBEEF"}] + opid = self.nodes[2].z_sendmany(myzaddr, recipients, 1, DEFAULT_FEE, 'AllowRevealedRecipients') + wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', 'Failed to build transaction: Memos cannot be sent to transparent addresses.') + recipients = [] recipients.append({"address":self.nodes[0].getnewaddress(), "amount":1}) recipients.append({"address":self.nodes[2].getnewaddress(), "amount":1.0}) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f2156ee93..3bf0ca44f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4736,7 +4736,8 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) " [{\n" " \"address\":address (string, required) The address is a taddr, zaddr, or Unified Address\n" " \"amount\":amount (numeric, required) The numeric amount in " + CURRENCY_UNIT + " is the value\n" - " \"memo\":memo (string, optional) If the address is a zaddr, raw data represented in hexadecimal string format\n" + " \"memo\":memo (string, optional) If the address is a zaddr, raw data represented in hexadecimal string format. If\n" + " the output is being sent to a transparent address, it’s an error to include this field.\n" " }, ... ]\n" "3. minconf (numeric, optional, default=" + strprintf("%u", DEFAULT_NOTE_CONFIRMATIONS) + ") Only use funds confirmed at least this many times.\n" "4. fee (numeric, optional, default=" + strprintf("%s", FormatMoney(DEFAULT_FEE)) + ") The fee amount to attach to this transaction.\n" @@ -4841,12 +4842,6 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) std::optional memo; if (!memoValue.isNull()) { auto memoHex = memoValue.get_str(); - if (!std::visit(libzcash::HasShieldedRecipient(), addr.value())) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - "Invalid parameter, memos cannot be sent to transparent addresses."); - } - std::visit(match { [&](MemoError err) { switch (err) { diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index 65b0b4334..1f448e3a3 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -548,25 +548,39 @@ TransactionBuilderResult TransactionEffects::ApproveAndBuild( } // Add outputs - for (const auto& r : payments.GetResolvedPayments()) { - std::visit(match { - [&](const CKeyID& keyId) { - builder.AddTransparentOutput(keyId, r.amount); - }, - [&](const CScriptID& scriptId) { - builder.AddTransparentOutput(scriptId, r.amount); - }, - [&](const libzcash::SaplingPaymentAddress& addr) { - builder.AddSaplingOutput( - r.isInternal ? internalOVK : externalOVK, addr, r.amount, - r.memo.has_value() ? r.memo.value().ToBytes() : Memo::NoMemo().ToBytes()); - }, - [&](const libzcash::OrchardRawAddress& addr) { - builder.AddOrchardOutput( - r.isInternal ? internalOVK : externalOVK, addr, r.amount, - r.memo.has_value() ? std::optional(r.memo.value().ToBytes()) : std::nullopt); - } - }, r.address); + try { + for (const auto& r : payments.GetResolvedPayments()) { + std::visit(match { + [&](const CKeyID& keyId) { + if (r.memo.has_value()) { + throw TransactionBuilderResult( + "Memos cannot be sent to transparent addresses."); + } else { + builder.AddTransparentOutput(keyId, r.amount); + } + }, + [&](const CScriptID& scriptId) { + if (r.memo.has_value()) { + throw TransactionBuilderResult( + "Memos cannot be sent to transparent addresses."); + } else { + builder.AddTransparentOutput(scriptId, r.amount); + } + }, + [&](const libzcash::SaplingPaymentAddress& addr) { + builder.AddSaplingOutput( + r.isInternal ? internalOVK : externalOVK, addr, r.amount, + r.memo.has_value() ? r.memo.value().ToBytes() : Memo::NoMemo().ToBytes()); + }, + [&](const libzcash::OrchardRawAddress& addr) { + builder.AddOrchardOutput( + r.isInternal ? internalOVK : externalOVK, addr, r.amount, + r.memo.has_value() ? std::optional(r.memo.value().ToBytes()) : std::nullopt); + } + }, r.address); + } + } catch (const TransactionBuilderResult& result) { + return result; } // Add transparent utxos