From 9c600a558267c028aba95b99e214573baed47f91 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 24 May 2022 16:50:53 -0600 Subject: [PATCH 1/2] Factor out memo parsing from asyncrpcoperation_sendmany --- src/Makefile.am | 2 + src/transaction_builder.cpp | 4 +- src/transaction_builder.h | 4 +- src/wallet/asyncrpcoperation_sendmany.cpp | 44 +++-------- src/wallet/asyncrpcoperation_sendmany.h | 20 +---- src/wallet/gtest/test_rpc_wallet.cpp | 3 +- src/wallet/memo.h | 92 +++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 41 +++++++--- src/wallet/test/rpc_wallet_tests.cpp | 84 ++++++++------------- src/wallet/wallet.cpp | 2 +- src/wallet/wallet.h | 30 +++++++- src/wallet/wallet_tx_builder.h | 28 +++++++ 12 files changed, 228 insertions(+), 126 deletions(-) create mode 100644 src/wallet/memo.h create mode 100644 src/wallet/wallet_tx_builder.h diff --git a/src/Makefile.am b/src/Makefile.am index 72cff51c9..8b14284ca 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -323,12 +323,14 @@ BITCOIN_CORE_H = \ wallet/asyncrpcoperation_shieldcoinbase.h \ wallet/crypter.h \ wallet/db.h \ + wallet/memo.h \ wallet/orchard.h \ wallet/paymentdisclosure.h \ wallet/paymentdisclosuredb.h \ wallet/rpcwallet.h \ wallet/wallet.h \ wallet/walletdb.h \ + wallet/wallet_tx_builder.h \ warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index ae898dec9..58d3dafc4 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -254,8 +254,8 @@ TransactionBuilder::TransactionBuilder( const Consensus::Params& consensusParams, int nHeight, std::optional orchardAnchor, - CKeyStore* keystore, - CCoinsViewCache* coinsView, + const CKeyStore* keystore, + const CCoinsViewCache* coinsView, CCriticalSection* cs_coinsView) : consensusParams(consensusParams), nHeight(nHeight), diff --git a/src/transaction_builder.h b/src/transaction_builder.h index e2f96846f..cba865634 100644 --- a/src/transaction_builder.h +++ b/src/transaction_builder.h @@ -290,8 +290,8 @@ public: const Consensus::Params& consensusParams, int nHeight, std::optional orchardAnchor, - CKeyStore* keyStore = nullptr, - CCoinsViewCache* coinsView = nullptr, + const CKeyStore* keyStore = nullptr, + const CCoinsViewCache* coinsView = nullptr, CCriticalSection* cs_coinsView = nullptr); // TransactionBuilder should never be copied diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 31dcc114f..87e6ae10e 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -30,6 +30,7 @@ #include "zcash/IncrementalMerkleTree.hpp" #include "miner.h" #include "wallet/paymentdisclosuredb.h" +#include "wallet/wallet_tx_builder.h" #include #include @@ -46,7 +47,7 @@ using namespace libzcash; AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( TransactionBuilder builder, ZTXOSelector ztxoSelector, - std::vector recipients, + std::vector recipients, int minDepth, unsigned int anchorDepth, TransactionStrategy strategy, @@ -64,7 +65,7 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( sendFromAccount_ = pwalletMain->FindAccountForSelector(ztxoSelector_).value_or(ZCASH_LEGACY_ACCOUNT); // Determine the target totals and recipient pools - for (const SendManyRecipient& recipient : recipients_) { + for (const ResolvedPayment& recipient : recipients_) { std::visit(match { [&](const CKeyID& addr) { transparentRecipients_ += 1; @@ -559,15 +560,14 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { builder_.AddTransparentOutput(scriptId, r.amount); }, [&](const libzcash::SaplingPaymentAddress& addr) { - auto value = r.amount; - auto memo = get_memo_from_hex_string(r.memo.value_or("")); - - builder_.AddSaplingOutput(ovks.second, addr, value, memo); + builder_.AddSaplingOutput( + ovks.second, addr, r.amount, + r.memo.has_value() ? r.memo.value().ToBytes() : Memo::NoMemo().ToBytes()); }, [&](const libzcash::OrchardRawAddress& addr) { - auto value = r.amount; - auto memo = r.memo.has_value() ? std::optional(get_memo_from_hex_string(r.memo.value())) : std::nullopt; - builder_.AddOrchardOutput(ovks.second, addr, value, memo); + builder_.AddOrchardOutput( + ovks.second, addr, r.amount, + r.memo.has_value() ? std::optional(r.memo.value().ToBytes()) : std::nullopt); } }, r.address); } @@ -770,32 +770,6 @@ CAmount AsyncRPCOperation_sendmany::DefaultDustThreshold() { return txout.GetDustThreshold(minRelayTxFee); } -std::array AsyncRPCOperation_sendmany::get_memo_from_hex_string(std::string s) { - // initialize to default memo (no_memo), see section 5.5 of the protocol spec - std::array memo = {{0xF6}}; - - std::vector rawMemo = ParseHex(s.c_str()); - - // If ParseHex comes across a non-hex char, it will stop but still return results so far. - size_t slen = s.length(); - if (slen % 2 !=0 || (slen>0 && rawMemo.size()!=slen/2)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Memo must be in hexadecimal format"); - } - - if (rawMemo.size() > ZC_MEMO_SIZE) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - strprintf("Memo size of %d bytes is too big, maximum allowed is %d bytes", rawMemo.size(), ZC_MEMO_SIZE)); - } - - // copy vector into boost array - int lenMemo = rawMemo.size(); - for (int i = 0; i < ZC_MEMO_SIZE && i < lenMemo; i++) { - memo[i] = rawMemo[i]; - } - return memo; -} - /** * Override getStatus() to append the operation's input parameters to the default status object. */ diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index 0fdcf7c11..e6297e2fe 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -13,6 +13,7 @@ #include "zcash/Address.hpp" #include "wallet.h" #include "wallet/paymentdisclosure.h" +#include "wallet/wallet_tx_builder.h" #include #include @@ -25,15 +26,6 @@ using namespace libzcash; -class SendManyRecipient : public RecipientMapping { -public: - CAmount amount; - std::optional memo; - - SendManyRecipient(std::optional ua_, libzcash::RecipientAddress address_, CAmount amount_, std::optional memo_) : - RecipientMapping(ua_, address_), amount(amount_), memo(memo_) {} -}; - class TxOutputAmounts { public: CAmount t_outputs_total{0}; @@ -46,7 +38,7 @@ public: AsyncRPCOperation_sendmany( TransactionBuilder builder, ZTXOSelector ztxoSelector, - std::vector recipients, + std::vector recipients, int minDepth, unsigned int anchorDepth, TransactionStrategy strategy, @@ -71,7 +63,7 @@ private: TransactionBuilder builder_; ZTXOSelector ztxoSelector_; - std::vector recipients_; + std::vector recipients_; int mindepth_{1}; unsigned int anchordepth_{nAnchorConfirmations}; CAmount fee_; @@ -93,8 +85,6 @@ private: static CAmount DefaultDustThreshold(); - static std::array get_memo_from_hex_string(std::string s); - uint256 main_impl(); }; @@ -105,10 +95,6 @@ public: TEST_FRIEND_AsyncRPCOperation_sendmany(std::shared_ptr ptr) : delegate(ptr) {} - std::array get_memo_from_hex_string(std::string s) { - return delegate->get_memo_from_hex_string(s); - } - uint256 main_impl() { return delegate->main_impl(); } diff --git a/src/wallet/gtest/test_rpc_wallet.cpp b/src/wallet/gtest/test_rpc_wallet.cpp index 1b74f9138..3e29112c5 100644 --- a/src/wallet/gtest/test_rpc_wallet.cpp +++ b/src/wallet/gtest/test_rpc_wallet.cpp @@ -11,6 +11,7 @@ #include "wallet/asyncrpcoperation_mergetoaddress.h" #include "wallet/asyncrpcoperation_shieldcoinbase.h" #include "wallet/asyncrpcoperation_sendmany.h" +#include "wallet/memo.h" #include "zcash/JoinSplit.hpp" #include @@ -287,7 +288,7 @@ TEST(WalletRPCTests, RPCZsendmanyTaddrToSapling) mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight, false); auto selector = pwalletMain->ZTXOSelectorForAddress(taddr, true, false).value(); - std::vector recipients = { SendManyRecipient(std::nullopt, pa, 1*COIN, "ABCD") }; + std::vector recipients = { ResolvedPayment(std::nullopt, pa, 1*COIN, Memo::FromHexOrThrow("ABCD")) }; TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders); std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 0, 0, strategy)); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); diff --git a/src/wallet/memo.h b/src/wallet/memo.h new file mode 100644 index 000000000..165cdadee --- /dev/null +++ b/src/wallet/memo.h @@ -0,0 +1,92 @@ +// Copyright (c) 2022 The Zcash developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#ifndef ZCASH_WALLET_MEMO_H +#define ZCASH_WALLET_MEMO_H + +#include "util/strencodings.h" +#include "zcash/Zcash.h" +#include "tinyformat.h" + + +enum class MemoError { + HexDecodeError, + MemoTooLong +}; + +typedef std::array MemoBytes; + +class Memo { +private: + MemoBytes value; +public: + // initialize to default memo (no_memo), see section 5.5 of the protocol spec + Memo(): value({{0xF6}}) { } + Memo(MemoBytes value): value(value) {} + + static Memo NoMemo() { + return Memo(); + } + + static std::variant FromHex(const std::string& memoHex) { + Memo result; + std::vector rawMemo = ParseHex(memoHex.c_str()); + + // If ParseHex comes across a non-hex char, it will stop but still return results so far. + size_t slen = memoHex.length(); + if (slen % 2 != 0 || (slen > 0 && rawMemo.size() != slen / 2)) { + return MemoError::HexDecodeError; + } + + if (rawMemo.size() > ZC_MEMO_SIZE) { + return MemoError::MemoTooLong; + } + + for (int i = 0; i < ZC_MEMO_SIZE; i++) { + if (i < rawMemo.size()) { + result.value[i] = rawMemo[i]; + } else { + result.value[i] = 0x0; + } + } + + return result; + } + + static Memo FromHexOrThrow(const std::string& memoHex) { + return std::visit(match { + [&](Memo memo) { + return memo; + }, + [&](MemoError err) { + switch (err) { + case MemoError::HexDecodeError: + throw std::runtime_error( + "Invalid parameter, expected memo data in hexadecimal format."); + case MemoError::MemoTooLong: + throw std::runtime_error(strprintf( + "Invalid parameter, memo is longer than the maximum allowed %d characters.", + ZC_MEMO_SIZE)); + default: + assert(false); + } + // unreachable, but the compiler can't tell + return Memo::NoMemo(); + } + }, Memo::FromHex(memoHex)); + } + + // This copies, because if it returns a reference to the underlying value, + // using an idiom like `Memo::FromHexOrThrow("abcd").ToBytes()` is unsafe + // and can result in pointing to memory that has been deallocated. + MemoBytes ToBytes() const { + return value; + } + + std::string ToHex() const { + return HexStr(value); + } +}; + +#endif diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 81ce755b9..2460eabcf 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -41,6 +41,7 @@ #include "wallet/asyncrpcoperation_saplingmigration.h" #include "wallet/asyncrpcoperation_sendmany.h" #include "wallet/asyncrpcoperation_shieldcoinbase.h" +#include "wallet/wallet_tx_builder.h" #include @@ -4988,7 +4989,7 @@ UniValue z_getoperationstatus_IMPL(const UniValue& params, bool fRemoveFinishedO size_t EstimateTxSize( const ZTXOSelector& ztxoSelector, - const std::vector& recipients, + const std::vector& recipients, int nextBlockHeight) { CMutableTransaction mtx; mtx.fOverwintered = true; @@ -5002,7 +5003,7 @@ size_t EstimateTxSize( size_t txsize = 0; size_t taddrRecipientCount = 0; size_t orchardRecipientCount = 0; - for (const SendManyRecipient& recipient : recipients) { + for (const ResolvedPayment& recipient : recipients) { std::visit(match { [&](const CKeyID&) { taddrRecipientCount += 1; @@ -5207,7 +5208,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) } std::set recipientAddrs; - std::vector recipients; + std::vector recipients; CAmount nTotalOut = 0; size_t nOrchardOutputs = 0; for (const UniValue& o : outputs.getValues()) { @@ -5249,18 +5250,34 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) } UniValue memoValue = find_value(o, "memo"); - std::optional memo; + std::optional memo; if (!memoValue.isNull()) { - memo = memoValue.get_str(); + auto memoHex = memoValue.get_str(); if (!std::visit(libzcash::IsShieldedRecipient(), addr.value())) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, memos cannot be sent to transparent addresses."); - } else if (!IsHex(memo.value())) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected memo data in hexadecimal format."); + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Invalid parameter, memos cannot be sent to transparent addresses."); } - if (memo.value().length() > ZC_MEMO_SIZE*2) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, size of memo is larger than maximum allowed %d", ZC_MEMO_SIZE )); - } + std::visit(match { + [&](MemoError err) { + switch (err) { + case MemoError::HexDecodeError: + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Invalid parameter, expected memo data in hexadecimal format."); + case MemoError::MemoTooLong: + throw JSONRPCError( + RPC_INVALID_PARAMETER, + strprintf("Invalid parameter, size of memo is larger than maximum allowed %d", ZC_MEMO_SIZE )); + default: + assert(false); + } + }, + [&](Memo result) { + memo = result; + } + }, Memo::FromHex(memoHex)); } UniValue av = find_value(o, "amount"); @@ -5292,7 +5309,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) } } - recipients.push_back(SendManyRecipient(ua, addr.value(), nAmount, memo)); + recipients.push_back(ResolvedPayment(ua, addr.value(), nAmount, memo)); nTotalOut += nAmount; } if (recipients.empty()) { diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 029d00d52..c05794e00 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -20,6 +20,7 @@ #include "wallet/asyncrpcoperation_mergetoaddress.h" #include "wallet/asyncrpcoperation_sendmany.h" #include "wallet/asyncrpcoperation_shieldcoinbase.h" +#include "wallet/memo.h" #include "init.h" #include "util/test.h" @@ -1237,7 +1238,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) { auto selector = pwalletMain->ZTXOSelectorForAddress(taddr1, true, false).value(); TransactionBuilder builder(consensusParams, nHeight + 1, std::nullopt, pwalletMain); - std::vector recipients = { SendManyRecipient(std::nullopt, zaddr1, 100*COIN, "DEADBEEF") }; + std::vector recipients = { ResolvedPayment(std::nullopt, zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) }; TransactionStrategy strategy; std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy)); operation->main(); @@ -1250,7 +1251,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) { auto selector = pwalletMain->ZTXOSelectorForAddress(zaddr1, true, false).value(); TransactionBuilder builder(consensusParams, nHeight + 1, std::nullopt, pwalletMain); - std::vector recipients = { SendManyRecipient(std::nullopt, taddr1, 100*COIN, "DEADBEEF") }; + std::vector recipients = { ResolvedPayment(std::nullopt, taddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) }; TransactionStrategy strategy(PrivacyPolicy::AllowRevealedRecipients); std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy)); operation->main(); @@ -1258,59 +1259,36 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) std::string msg = operation->getErrorMessage(); BOOST_CHECK(msg.find("Insufficient funds: have 0.00") != string::npos); } +} - // get_memo_from_hex_string()) - { - auto selector = pwalletMain->ZTXOSelectorForAddress(zaddr1, true, false).value(); - TransactionBuilder builder(consensusParams, nHeight + 1, std::nullopt, pwalletMain); - std::vector recipients = { SendManyRecipient(std::nullopt, zaddr1, 100*COIN, "DEADBEEF") }; - TransactionStrategy strategy; - std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy)); - std::shared_ptr ptr = std::dynamic_pointer_cast (operation); - TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr); - - std::string memo = "DEADBEEF"; - std::array array = proxy.get_memo_from_hex_string(memo); - BOOST_CHECK_EQUAL(array[0], 0xDE); - BOOST_CHECK_EQUAL(array[1], 0xAD); - BOOST_CHECK_EQUAL(array[2], 0xBE); - BOOST_CHECK_EQUAL(array[3], 0xEF); - for (int i=4; i v (2 * (ZC_MEMO_SIZE+1)); - std::fill(v.begin(),v.end(), 'A'); - std::string bigmemo(v.begin(), v.end()); - - try { - proxy.get_memo_from_hex_string(bigmemo); - } catch (const UniValue& objError) { - BOOST_CHECK( find_error(objError, "too big")); - } - - // invalid hexadecimal string - std::fill(v.begin(),v.end(), '@'); // not a hex character - std::string badmemo(v.begin(), v.end()); - - try { - proxy.get_memo_from_hex_string(badmemo); - } catch (const UniValue& objError) { - BOOST_CHECK( find_error(objError, "hexadecimal format")); - } - - // odd length hexadecimal string - std::fill(v.begin(),v.end(), 'A'); - v.resize(v.size() - 1); - assert(v.size() %2 == 1); // odd length - std::string oddmemo(v.begin(), v.end()); - try { - proxy.get_memo_from_hex_string(oddmemo); - } catch (const UniValue& objError) { - BOOST_CHECK( find_error(objError, "hexadecimal format")); - } +BOOST_AUTO_TEST_CASE(memo_hex_parsing) { + std::string memo = "DEADBEEF"; + MemoBytes memoBytes = Memo::FromHexOrThrow(memo).ToBytes(); + BOOST_CHECK_EQUAL(memoBytes[0], 0xDE); + BOOST_CHECK_EQUAL(memoBytes[1], 0xAD); + BOOST_CHECK_EQUAL(memoBytes[2], 0xBE); + BOOST_CHECK_EQUAL(memoBytes[3], 0xEF); + for (int i=4; i v (2 * (ZC_MEMO_SIZE+1)); + std::fill(v.begin(),v.end(), 'A'); + std::string bigmemo(v.begin(), v.end()); + BOOST_CHECK(std::get(Memo::FromHex(bigmemo)) == MemoError::MemoTooLong); + + // invalid hexadecimal string + std::fill(v.begin(),v.end(), '@'); // not a hex character + std::string badmemo(v.begin(), v.end()); + BOOST_CHECK(std::get(Memo::FromHex(badmemo)) == MemoError::HexDecodeError); + + // odd length hexadecimal string + std::fill(v.begin(),v.end(), 'A'); + v.resize(v.size() - 1); + assert(v.size() %2 == 1); // odd length + std::string oddmemo(v.begin(), v.end()); + BOOST_CHECK(std::get(Memo::FromHex(oddmemo)) == MemoError::HexDecodeError); } /* diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 898cf37dc..57e50387d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -7851,7 +7851,7 @@ bool ZTXOSelector::SelectsOrchard() const { bool SpendableInputs::LimitToAmount( const CAmount amountRequired, const CAmount dustThreshold, - std::set recipientPools) + const std::set& recipientPools) { assert(amountRequired >= 0 && dustThreshold > 0); // Calling this method twice is a programming error. diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 39ecc59c0..467ea5553 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -906,24 +906,48 @@ public: * This method must only be called once. */ bool LimitToAmount( - CAmount amount, - CAmount dustThreshold, - std::set recipientPools); + const CAmount amount, + const CAmount dustThreshold, + const std::set& recipientPools); /** * Compute the total ZEC amount of spendable inputs. */ CAmount Total() const { + CAmount result = 0; + result += GetTransparentBalance(); + result += GetSproutBalance(); + result += GetSaplingBalance(); + result += GetOrchardBalance(); + return result; + } + + CAmount GetTransparentBalance() const { CAmount result = 0; for (const auto& t : utxos) { result += t.Value(); } + return result; + } + + CAmount GetSproutBalance() const { + CAmount result = 0; for (const auto& t : sproutNoteEntries) { result += t.note.value(); } + return result; + } + + CAmount GetSaplingBalance() const { + CAmount result = 0; for (const auto& t : saplingNoteEntries) { result += t.note.value(); } + return result; + } + + CAmount GetOrchardBalance() const { + CAmount result = 0; for (const auto& t : orchardNoteMetadata) { result += t.GetNoteValue(); } diff --git a/src/wallet/wallet_tx_builder.h b/src/wallet/wallet_tx_builder.h new file mode 100644 index 000000000..75bb38a27 --- /dev/null +++ b/src/wallet/wallet_tx_builder.h @@ -0,0 +1,28 @@ +// Copyright (c) 2022 The Zcash developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#ifndef ZCASH_WALLET_WALLET_TX_BUILDER_H +#define ZCASH_WALLET_WALLET_TX_BUILDER_H + +#include "wallet/memo.h" + +using namespace libzcash; + +/** + * A payment that has been resolved to send to a specific + * recipient address in a single pool. + */ +class ResolvedPayment : public RecipientMapping { +public: + CAmount amount; + std::optional memo; + + ResolvedPayment( + std::optional ua, + libzcash::RecipientAddress address, + CAmount amount, + std::optional memo) : + RecipientMapping(ua, address), amount(amount), memo(memo) {} +}; +#endif From 4cff6f80c65272c8d3ebba9b94f0a31344b243e5 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Tue, 6 Dec 2022 09:22:52 -0700 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Daira Hopwood --- src/wallet/memo.h | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/wallet/memo.h b/src/wallet/memo.h index 165cdadee..a8f182a34 100644 --- a/src/wallet/memo.h +++ b/src/wallet/memo.h @@ -35,7 +35,7 @@ public: // If ParseHex comes across a non-hex char, it will stop but still return results so far. size_t slen = memoHex.length(); - if (slen % 2 != 0 || (slen > 0 && rawMemo.size() != slen / 2)) { + if (slen != rawMemo.size() * 2) { return MemoError::HexDecodeError; } @@ -43,13 +43,8 @@ public: return MemoError::MemoTooLong; } - for (int i = 0; i < ZC_MEMO_SIZE; i++) { - if (i < rawMemo.size()) { - result.value[i] = rawMemo[i]; - } else { - result.value[i] = 0x0; - } - } + auto rest = std::copy(rawMemo.begin(), rawMemo.end(), result.value.begin()); + std::fill(rest, result.value.end(), 0); return result; }