Merge pull request #6569 from sellout/wallet_tx_builder-z_mergetoaddress

Have z_mergetoaddress use WalletTxBuilder
This commit is contained in:
str4d 2023-04-20 22:50:09 +01:00 committed by GitHub
commit 555e085539
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 407 additions and 1362 deletions

View File

@ -21,14 +21,17 @@ RPC Changes
also available for testnet and regtest nodes, in which case it does not
return end-of-service halt information (as testnet and regtest nodes do not
have an end-of-service halt feature.)
- Several `z_sendmany` failures have moved from synchronous to asynchronous, so
while you should already be checking the async operation status, there are now
more cases that may trigger failure at that stage.
- Several `z_sendmany`, `z_shieldcoinbase` and `z_mergetoaddress` failures have
moved from synchronous to asynchronous, so while you should already be
checking the async operation status, there are now more cases that may trigger
failure at that stage.
- The `AllowRevealedRecipients` privacy policy is now required in order to choose a
transparent change address for a transaction. This will only occur when the wallet
is unable to construct the transaction without selecting funds from the transparent
pool, so the impact of this change is that for such transactions, the user must specify
`AllowFullyTransparent`.
- The `z_shieldcoinbase` and `z_mergetoaddress` RPC methods now support an
optional privacy policy.
- The `estimatepriority` RPC call has been removed.
- The `priority_delta` argument to the `prioritisetransaction` RPC call now has
no effect and must be set to a dummy value (0 or null).

View File

@ -24,7 +24,7 @@ def assert_mergetoaddress_exception(expected_error_msg, merge_to_address_lambda)
except Exception as e:
fail("Expected JSONRPCException. Found %s" % repr(e))
else:
fail("Expected exception: %s" % expected_error_msg)
fail("Expected exception: %s”, but didnt fail" % expected_error_msg)
class MergeToAddressHelper:
@ -129,9 +129,9 @@ class MergeToAddressHelper:
"Amount out of range",
lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('21000000.00000001')))
# Merging will fail because fee is larger than sum of UTXOs
# Merging will fail because fee is larger than `-maxtxfee`
assert_mergetoaddress_exception(
"Insufficient funds, have 50.00, which is less than miners fee 999.00",
"Fee (999.00 ZEC) is greater than the maximum fee allowed by this instance (0.10 ZEC). Run zcashd with `-maxtxfee` to adjust this limit.",
lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, 999))
# Merging will fail because transparent limit parameter must be at least 0
@ -174,7 +174,7 @@ class MergeToAddressHelper:
# Confirm balances and that do_not_shield_taddr containing funds of 10 was left alone
assert_equal(test.nodes[0].getbalance(), Decimal('10'))
assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('10'))
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('40') - DEFAULT_FEE)
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('40') - conventional_fee(4))
assert_equal(test.nodes[1].getbalance(), Decimal('40'))
assert_equal(test.nodes[2].getbalance(), Decimal('30'))
@ -191,7 +191,7 @@ class MergeToAddressHelper:
test.sync_all()
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('0'))
assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('40') - DEFAULT_FEE)
assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('40') - conventional_fee(4))
# Shield coinbase UTXOs from any node 2 taddr, and set fee to 0
result = test.nodes[2].z_shieldcoinbase("*", myzaddr, 0)
@ -202,7 +202,7 @@ class MergeToAddressHelper:
assert_equal(test.nodes[0].getbalance(), Decimal('10'))
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('30'))
assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('40') - DEFAULT_FEE)
assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('40') - conventional_fee(4))
assert_equal(test.nodes[1].getbalance(), Decimal('60'))
assert_equal(test.nodes[2].getbalance(), Decimal('0'))
@ -213,9 +213,9 @@ class MergeToAddressHelper:
generate_and_check(test.nodes[1], 2)
test.sync_all()
assert_equal(test.nodes[0].getbalance(), Decimal('80') - DEFAULT_FEE)
assert_equal(test.nodes[0].getbalance(), Decimal('80') - conventional_fee(4))
assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('10'))
assert_equal(test.nodes[0].z_getbalance(mytaddr), Decimal('70') - DEFAULT_FEE)
assert_equal(test.nodes[0].z_getbalance(mytaddr), Decimal('70') - conventional_fee(4))
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('0'))
assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('0'))
assert_equal(test.nodes[1].getbalance(), Decimal('70'))
@ -235,8 +235,8 @@ class MergeToAddressHelper:
assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('0'))
assert_equal(test.nodes[0].z_getbalance(mytaddr), Decimal('0'))
assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('0'))
assert_equal(test.nodes[1].getbalance(), Decimal('160') - DEFAULT_FEE)
assert_equal(test.nodes[1].z_getbalance(n1taddr), Decimal('80') - DEFAULT_FEE)
assert_equal(test.nodes[1].getbalance(), Decimal('160') - conventional_fee(4))
assert_equal(test.nodes[1].z_getbalance(n1taddr), Decimal('80') - conventional_fee(4))
assert_equal(test.nodes[2].getbalance(), Decimal('0'))
# Generate 5 regular UTXOs on node 0, and 20 regular UTXOs on node 2.

View File

@ -21,6 +21,7 @@ class MergeToAddressMixedNotes(BitcoinTestFramework):
'-minrelaytxfee=0',
'-anchorconfirmations=1',
'-allowdeprecated=getnewaddress',
'-allowdeprecated=legacy_privacy',
'-allowdeprecated=z_getnewaddress',
'-allowdeprecated=z_getbalance',
'-allowdeprecated=z_gettotalbalance',

View File

@ -14,7 +14,10 @@ class MergeToAddressSapling (BitcoinTestFramework):
self.helper.setup_chain(self)
def setup_network(self, split=False):
self.helper.setup_network(self, ['-anchorconfirmations=1'])
self.helper.setup_network(self, [
'-anchorconfirmations=1',
'-allowdeprecated=legacy_privacy',
])
def run_test(self):
self.helper.run_test(self)

View File

@ -49,7 +49,6 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
self.sync_all()
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
@ -77,20 +76,21 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
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. This will spend the available funds in taddr0
# Create mergetoaddress taddr -> Sprout transaction, should fail
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")
assert_raises_message(
JSONRPCException,
"Sending funds into the Sprout pool is no longer supported.",
self.nodes[0].z_mergetoaddress,
["ANY_TADDR"], n1_sprout_addr0, 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)
myopid = self.nodes[0].z_sendmany(n0_sprout_addr0, recipients, 1, 0, 'AllowRevealedRecipients')
wait_and_assert_operationid_status(self.nodes[0], 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
@ -99,7 +99,7 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
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'))
assert_equal(self.nodes[0].z_getbalance(n0_taddr0), Decimal('4'))
# Shield coinbase to Sprout on node 0. Should fail
n0_coinbase_taddr = get_coinbase_address(self.nodes[0])
@ -121,15 +121,9 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
# Create z_mergetoaddress [taddr, Sprout] -> Sprout transaction on node 0. Should fail
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_mergetoaddress,
["ANY_TADDR", "ANY_SPROUT"], self.nodes[1].z_getnewaddress('sprout'))
print("[taddr, Sprout] -> Sprout z_mergetoaddress tx rejected at Canopy activation on node 0")
# Create z_mergetoaddress Sprout -> Sprout transaction on node 0. Should pass
merge_tx_1 = self.nodes[0].z_mergetoaddress(["ANY_SPROUT"], self.nodes[1].z_getnewaddress('sprout'))
wait_and_assert_operationid_status(self.nodes[0], merge_tx_1['opid'])
print("Sprout -> Sprout z_mergetoaddress tx accepted at Canopy activation on node 0")
# Activate Canopy
self.nodes[0].generate(1)
@ -149,17 +143,5 @@ class RemoveSproutShieldingTest (BitcoinTestFramework):
wait_and_assert_operationid_status(self.nodes[0], myopid)
print("taddr -> Sapling z_shieldcoinbase tx accepted after Canopy on node 0")
# Mine to one block before NU5 activation.
self.nodes[0].generate(4)
self.sync_all()
# Create z_mergetoaddress Sprout -> Sprout transaction on node 1. Should pass
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")
self.nodes[1].generate(1)
self.sync_all()
if __name__ == '__main__':
RemoveSproutShieldingTest().main()

View File

@ -317,18 +317,18 @@ TEST(TransactionBuilder, FailsWithNegativeChange)
// 0.00005 z-ZEC out, default fee
auto builder = TransactionBuilder(consensusParams, 1, std::nullopt);
builder.AddSaplingOutput(fvk.ovk, pa, 5000, {});
EXPECT_EQ("Change cannot be negative", builder.Build().GetError());
EXPECT_EQ("Change cannot be negative: -0.00006 ZEC", builder.Build().GetError());
// Fail if there is only a transparent output
// 0.00005 t-ZEC out, default fee
builder = TransactionBuilder(consensusParams, 1, std::nullopt, &keystore);
builder.AddTransparentOutput(taddr, 5000);
EXPECT_EQ("Change cannot be negative", builder.Build().GetError());
EXPECT_EQ("Change cannot be negative: -0.00006 ZEC", builder.Build().GetError());
// Fails if there is insufficient input
// 0.00005 t-ZEC out, default fee, 0.00005999 z-ZEC in
builder.AddSaplingSpend(expsk, testNote.note, testNote.tree.root(), testNote.tree.witness());
EXPECT_EQ("Change cannot be negative", builder.Build().GetError());
EXPECT_EQ("Change cannot be negative: -0.00000001 ZEC", builder.Build().GetError());
// Succeeds if there is sufficient input
builder.AddTransparentInput(COutPoint(), scriptPubKey, 1);

View File

@ -501,7 +501,8 @@ TransactionBuilderResult TransactionBuilder::Build()
change -= tOut.nValue;
}
if (change < 0) {
return TransactionBuilderResult("Change cannot be negative");
return TransactionBuilderResult(
strprintf("Change cannot be negative: %s", DisplayMoney(change)));
}
//

View File

@ -167,6 +167,15 @@ void ThrowInputSelectionError(
"The proposed transaction would result in %s in change.",
FormatMoney(err.available - err.required)));
},
[](const InvalidFeeError& err) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
strprintf(
"The provided fee, %s, is invalid. Fees must be non-negative, and no greater "
"than the total amount of ZEC that will ever be available.",
DisplayMoney(err.fixedFee),
DisplayMoney(MAX_MONEY)));
},
[](const AbsurdFeeError& err) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,

File diff suppressed because it is too large Load Diff

View File

@ -9,8 +9,10 @@
#include "asyncrpcoperation.h"
#include "primitives/transaction.h"
#include "transaction_builder.h"
#include "uint256.h"
#include "wallet.h"
#include "wallet/paymentdisclosure.h"
#include "wallet/wallet_tx_builder.h"
#include "zcash/Address.hpp"
#include "zcash/JoinSplit.hpp"
@ -25,17 +27,6 @@
using namespace libzcash;
// Input UTXO is a tuple of txid, vout, amount, script
typedef std::tuple<COutPoint, CAmount, CScript> MergeToAddressInputUTXO;
// Input JSOP is a tuple of JSOutpoint, note, amount, spending key
typedef std::tuple<JSOutPoint, SproutNote, CAmount, SproutSpendingKey> MergeToAddressInputSproutNote;
typedef std::tuple<SaplingOutPoint, SaplingNote, CAmount, SaplingExpandedSpendingKey> MergeToAddressInputSaplingNote;
// A recipient is a tuple of address, memo (optional if zaddr)
typedef std::pair<libzcash::PaymentAddress, std::string> MergeToAddressRecipient;
// Package of info which is passed to perform_joinsplit methods.
struct MergeToAddressJSInfo {
std::vector<JSInput> vjsin;
@ -56,14 +47,10 @@ class AsyncRPCOperation_mergetoaddress : public AsyncRPCOperation
{
public:
AsyncRPCOperation_mergetoaddress(
std::optional<TransactionBuilder> builder,
CMutableTransaction contextualTx,
std::vector<MergeToAddressInputUTXO> utxoInputs,
std::vector<MergeToAddressInputSproutNote> sproutNoteInputs,
std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs,
MergeToAddressRecipient recipient,
CAmount fee = DEFAULT_FEE,
UniValue contextInfo = NullUniValue);
CWallet& wallet,
TransactionStrategy strategy,
TransactionEffects effects,
UniValue contextInfo = NullUniValue);
virtual ~AsyncRPCOperation_mergetoaddress();
// We don't want to be copied or moved around
@ -76,119 +63,16 @@ public:
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.
/// Set to true to disable sending txs and generating proofs
bool testmode = false;
private:
friend class TEST_FRIEND_AsyncRPCOperation_mergetoaddress; // class for unit testing
const TransactionStrategy strategy_;
UniValue contextinfo_; // optional data to include in return value from getStatus()
const TransactionEffects effects_;
bool isUsingBuilder_; // Indicates that no Sprout addresses are involved
uint32_t consensusBranchId_;
CAmount fee_;
int mindepth_;
bool isToTaddr_;
bool isToZaddr_;
CTxDestination toTaddr_;
PaymentAddress toPaymentAddress_;
std::string memo_;
ed25519::VerificationKey joinSplitPubKey_;
ed25519::SigningKey joinSplitPrivKey_;
// The key is the result string from calling JSOutPoint::ToString()
std::unordered_map<std::string, MergeToAddressWitnessAnchorData> jsopWitnessAnchorMap;
std::vector<MergeToAddressInputUTXO> utxoInputs_;
std::vector<MergeToAddressInputSproutNote> sproutNoteInputs_;
std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs_;
TransactionBuilder builder_;
CTransaction tx_;
std::array<unsigned char, ZC_MEMO_SIZE> get_memo_from_hex_string(std::string s);
bool main_impl();
// JoinSplit without any input notes to spend
UniValue perform_joinsplit(MergeToAddressJSInfo&);
// JoinSplit with input notes to spend (JSOutPoints))
UniValue perform_joinsplit(MergeToAddressJSInfo&, std::vector<JSOutPoint>&);
// JoinSplit where you have the witnesses and anchor
UniValue perform_joinsplit(
MergeToAddressJSInfo& info,
std::vector<std::optional<SproutWitness>> witnesses,
uint256 anchor);
void lock_utxos();
void unlock_utxos();
void lock_notes();
void unlock_notes();
// payment disclosure!
std::vector<PaymentDisclosureKeyInfo> paymentDisclosureData_;
/// optional data to include in return value from getStatus()
UniValue contextinfo_;
};
// To test private methods, a friend class can act as a proxy
class TEST_FRIEND_AsyncRPCOperation_mergetoaddress
{
public:
std::shared_ptr<AsyncRPCOperation_mergetoaddress> delegate;
TEST_FRIEND_AsyncRPCOperation_mergetoaddress(std::shared_ptr<AsyncRPCOperation_mergetoaddress> ptr) : delegate(ptr) {}
CTransaction getTx()
{
return delegate->tx_;
}
void setTx(CTransaction tx)
{
delegate->tx_ = tx;
}
// Delegated methods
std::array<unsigned char, ZC_MEMO_SIZE> get_memo_from_hex_string(std::string s)
{
return delegate->get_memo_from_hex_string(s);
}
bool main_impl()
{
return delegate->main_impl();
}
UniValue perform_joinsplit(MergeToAddressJSInfo& info)
{
return delegate->perform_joinsplit(info);
}
UniValue perform_joinsplit(MergeToAddressJSInfo& info, std::vector<JSOutPoint>& v)
{
return delegate->perform_joinsplit(info, v);
}
UniValue perform_joinsplit(
MergeToAddressJSInfo& info,
std::vector<std::optional<SproutWitness>> witnesses,
uint256 anchor)
{
return delegate->perform_joinsplit(info, witnesses, anchor);
}
void set_state(OperationStatus state)
{
delegate->state_.store(state);
}
};
#endif // ZCASH_WALLET_ASYNCRPCOPERATION_MERGETOADDRESS_H

View File

@ -82,6 +82,11 @@ AsyncRPCOperation_shieldcoinbase::~AsyncRPCOperation_shieldcoinbase() {
}
void AsyncRPCOperation_shieldcoinbase::main() {
if (isCancelled()) {
effects_.value().UnlockSpendable(*pwalletMain);
return;
}
set_state(OperationStatus::EXECUTING);
start_execution_clock();

View File

@ -8,7 +8,6 @@
#include "transaction_builder.h"
#include "util/test.h"
#include "gtest/utils.h"
#include "wallet/asyncrpcoperation_mergetoaddress.h"
#include "wallet/asyncrpcoperation_shieldcoinbase.h"
#include "wallet/asyncrpcoperation_sendmany.h"
#include "wallet/memo.h"
@ -17,10 +16,21 @@
#include <librustzcash.h>
#include <rust/ed25519.h>
namespace {
bool find_error(const UniValue& objError, const std::string& expected) {
return find_value(objError, "message").get_str().find(expected) != string::npos;
}
CWalletTx FakeWalletTx() {
CMutableTransaction mtx;
mtx.vout.resize(1);
mtx.vout[0].nValue = 1;
return CWalletTx(nullptr, mtx);
}
}
// TODO: test private methods
TEST(WalletRPCTests, RPCZMergeToAddressInternals)
{
@ -46,129 +56,62 @@ TEST(WalletRPCTests, RPCZMergeToAddressInternals)
auto taddr = pwalletMain->GenerateNewKey(true).GetID();
std::string taddr_string = keyIO.EncodeDestination(taddr);
MergeToAddressRecipient taddr1(keyIO.DecodePaymentAddress(taddr_string).value(), "");
auto pa = pwalletMain->GenerateNewSproutZKey();
MergeToAddressRecipient zaddr1(pa, "DEADBEEF");
NetAmountRecipient taddr1(keyIO.DecodePaymentAddress(taddr_string).value(), Memo());
auto sproutKey = pwalletMain->GenerateNewSproutZKey();
NetAmountRecipient zaddr1(sproutKey, Memo());
auto saplingKey = pwalletMain->GenerateNewLegacySaplingZKey();
NetAmountRecipient zaddr2(saplingKey, Memo());
WalletTxBuilder builder(Params(), minRelayTxFee);
auto selector = CWallet::LegacyTransparentZTXOSelector(
true,
TransparentCoinbasePolicy::Disallow);
TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders);
SpendableInputs inputs;
auto wtx = FakeWalletTx();
inputs.utxos.emplace_back(COutput(&wtx, 0, 100, true));
// Cant send to Sprout
builder.PrepareTransaction(
*pwalletMain,
selector,
inputs,
zaddr1,
chainActive,
strategy,
0,
1)
.map_error([](const auto& err) {
EXPECT_TRUE(examine(err, match {
[](const AddressResolutionError& are) {
return are == AddressResolutionError::SproutRecipientsNotSupported;
},
[](const auto&) { return false; },
}));
})
.map([](const auto&) { EXPECT_TRUE(false); });
// Insufficient funds
{
std::vector<MergeToAddressInputUTXO> inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},0, CScript()} };
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, inputs, {}, {}, zaddr1) );
operation->main();
EXPECT_TRUE(operation->isFailed());
std::string msg = operation->getErrorMessage();
EXPECT_TRUE(msg.find("Insufficient funds, have 0.00 and miners fee is 0.00001") != string::npos);
}
// get_memo_from_hex_string())
{
std::vector<MergeToAddressInputUTXO> inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000, CScript()} };
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, inputs, {}, {}, zaddr1) );
std::shared_ptr<AsyncRPCOperation_mergetoaddress> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_mergetoaddress> (operation);
TEST_FRIEND_AsyncRPCOperation_mergetoaddress proxy(ptr);
std::string memo = "DEADBEEF";
std::array<unsigned char, ZC_MEMO_SIZE> array = proxy.get_memo_from_hex_string(memo);
EXPECT_EQ(array[0], 0xDE);
EXPECT_EQ(array[1], 0xAD);
EXPECT_EQ(array[2], 0xBE);
EXPECT_EQ(array[3], 0xEF);
for (int i=4; i<ZC_MEMO_SIZE; i++) {
EXPECT_EQ(array[i], 0x00); // zero padding
}
// memo is longer than allowed
std::vector<char> 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);
FAIL() << "Should have caused an error";
} catch (const UniValue& objError) {
EXPECT_TRUE(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);
FAIL() << "Should have caused an error";
} catch (const UniValue& objError) {
EXPECT_TRUE(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);
FAIL() << "Should have caused an error";
} catch (const UniValue& objError) {
EXPECT_TRUE(find_error(objError, "hexadecimal format"));
}
}
// Test the perform_joinsplit methods.
{
// Dummy input so the operation object can be instantiated.
std::vector<MergeToAddressInputUTXO> inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000, CScript()} };
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, inputs, {}, {}, zaddr1) );
std::shared_ptr<AsyncRPCOperation_mergetoaddress> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_mergetoaddress> (operation);
TEST_FRIEND_AsyncRPCOperation_mergetoaddress proxy(ptr);
// Enable test mode so tx is not sent and proofs are not generated
static_cast<AsyncRPCOperation_mergetoaddress *>(operation.get())->testmode = true;
MergeToAddressJSInfo info;
std::vector<std::optional < SproutWitness>> witnesses;
uint256 anchor;
try {
proxy.perform_joinsplit(info, witnesses, anchor);
FAIL() << "Should have caused an error";
} catch (const std::runtime_error & e) {
EXPECT_TRUE(string(e.what()).find("anchor is null") != string::npos);
}
try {
std::vector<JSOutPoint> v;
proxy.perform_joinsplit(info, v);
FAIL() << "Should have caused an error";
} catch (const std::runtime_error & e) {
EXPECT_TRUE(string(e.what()).find("anchor is null") != string::npos);
}
info.notes.push_back(SproutNote());
try {
proxy.perform_joinsplit(info);
FAIL() << "Should have caused an error";
} catch (const std::runtime_error & e) {
EXPECT_TRUE(string(e.what()).find("number of notes") != string::npos);
}
info.notes.clear();
info.vjsin.push_back(JSInput());
info.vjsin.push_back(JSInput());
info.vjsin.push_back(JSInput());
try {
proxy.perform_joinsplit(info);
FAIL() << "Should have caused an error";
} 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);
FAIL() << "Should have caused an error";
} catch (const std::runtime_error & e) {
EXPECT_TRUE(string(e.what()).find("error verifying joinsplit") != string::npos);
}
}
builder.PrepareTransaction(
*pwalletMain,
selector,
inputs,
zaddr2,
chainActive,
strategy,
std::nullopt,
1)
.map_error([](const auto& err) {
EXPECT_TRUE(examine(err, match {
[](const InvalidFundsError& ife) {
return std::holds_alternative<InsufficientFundsError>(ife.reason);
},
[](const auto&) { return false; },
}));
})
.map([](const auto&) { EXPECT_TRUE(false); });
}
UnloadGlobalWallet();
}

View File

@ -38,6 +38,7 @@
#include "util/time.h"
#include "asyncrpcoperation.h"
#include "asyncrpcqueue.h"
#include "wallet/asyncrpcoperation_common.h"
#include "wallet/asyncrpcoperation_mergetoaddress.h"
#include "wallet/asyncrpcoperation_saplingmigration.h"
#include "wallet/asyncrpcoperation_sendmany.h"
@ -5342,9 +5343,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
if (!EnsureWalletIsAvailable(fHelp))
return NullUniValue;
if (fHelp || params.size() < 2 || params.size() > 6)
if (fHelp || params.size() < 2 || params.size() > 7)
throw runtime_error(
"z_mergetoaddress [\"fromaddress\", ... ] \"toaddress\" ( fee ) ( transparent_limit ) ( shielded_limit ) ( memo )\n"
"z_mergetoaddress [\"fromaddress\", ... ] \"toaddress\" ( fee ) ( transparent_limit ) ( shielded_limit ) ( memo ) ( privacyPolicy )\n"
"\nMerge multiple UTXOs and notes into a single UTXO or note. Coinbase UTXOs are ignored; use `z_shieldcoinbase`"
"\nto combine those into a single note."
"\n\nThis is an asynchronous operation, and UTXOs selected for merging will be locked. If there is an error, they"
@ -5369,13 +5370,33 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
" ,...\n"
" ]\n"
"2. \"toaddress\" (string, required) The taddr or zaddr to send the funds to.\n"
"3. fee (numeric, optional, default="
+ strprintf("%s", FormatMoney(DEFAULT_FEE)) + ") The fee amount to attach to this transaction.\n"
"3. fee (numeric, optional, default=null) The fee amount in " + CURRENCY_UNIT + " to attach to this transaction. The default behavior\n"
" is to use a fee calculated according to ZIP 317.\n"
"4. transparent_limit (numeric, optional, default="
+ strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT) + ") Limit on the maximum number of UTXOs to merge. Set to 0 to use as many as will fit in the transaction.\n"
"5. shielded_limit (numeric, optional, default="
+ strprintf("%d Sprout or %d Sapling Notes", MERGE_TO_ADDRESS_DEFAULT_SPROUT_LIMIT, MERGE_TO_ADDRESS_DEFAULT_SAPLING_LIMIT) + ") Limit on the maximum number of notes to merge. Set to 0 to merge as many as will fit in the transaction.\n"
"6. \"memo\" (string, optional) Encoded as hex. When toaddress is a zaddr, this will be stored in the memo field of the new note.\n"
"7. privacyPolicy (string, optional, default=\"LegacyCompat\") Policy for what information leakage is acceptable.\n"
" One of the following strings:\n"
" - \"FullPrivacy\": Only allow fully-shielded transactions (involving a single shielded value pool).\n"
" - \"LegacyCompat\": If the transaction involves any Unified Addresses, this is equivalent to\n"
" \"FullPrivacy\". Otherwise, this is equivalent to \"AllowFullyTransparent\".\n"
" - \"AllowRevealedAmounts\": Allow funds to cross between shielded value pools, revealing the amount\n"
" that crosses pools.\n"
" - \"AllowRevealedRecipients\": Allow transparent recipients. This also implies revealing\n"
" information described under \"AllowRevealedAmounts\".\n"
" - \"AllowRevealedSenders\": Allow transparent funds to be spent, revealing the sending\n"
" addresses and amounts. This implies revealing information described under \"AllowRevealedAmounts\".\n"
" - \"AllowFullyTransparent\": Allow transaction to both spend transparent funds and have\n"
" transparent recipients. This implies revealing information described under \"AllowRevealedSenders\"\n"
" and \"AllowRevealedRecipients\".\n"
" - \"AllowLinkingAccountAddresses\": Allow selecting transparent coins from the full account,\n"
" rather than just the funds sent to the transparent receiver in the provided Unified Address.\n"
" This implies revealing information described under \"AllowRevealedSenders\".\n"
" - \"NoPrivacy\": Allow the transaction to reveal any information necessary to create it.\n"
" This implies revealing information described under \"AllowFullyTransparent\" and\n"
" \"AllowLinkingAccountAddresses\".\n"
"\nResult:\n"
"{\n"
" \"remainingUTXOs\": xxx (numeric) Number of UTXOs still available for merging.\n"
@ -5409,10 +5430,10 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
// Keep track of addresses to spot duplicates
std::set<std::string> setAddress;
std::set<ReceiverType> receiverTypes;
bool isFromNonSprout = false;
KeyIO keyIO(Params());
const auto chainparams = Params();
KeyIO keyIO(chainparams);
// Sources
for (const UniValue& o : addresses.getValues()) {
if (!o.isStr())
@ -5421,28 +5442,27 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
std::string address = o.get_str();
if (address == "ANY_TADDR") {
receiverTypes.insert({ReceiverType::P2PKH, ReceiverType::P2SH});
useAnyUTXO = true;
isFromNonSprout = true;
} else if (address == "ANY_SPROUT") {
// TODO: How can we add sprout addresses?
// receiverTypes.insert(ReceiverType::Sprout);
useAnySprout = true;
} else if (address == "ANY_SAPLING") {
receiverTypes.insert(ReceiverType::Sapling);
useAnySapling = true;
isFromNonSprout = true;
} else {
auto addr = keyIO.DecodePaymentAddress(address);
if (addr.has_value()) {
examine(addr.value(), match {
[&](const CKeyID& taddr) {
taddrs.insert(taddr);
isFromNonSprout = true;
},
[&](const CScriptID& taddr) {
taddrs.insert(taddr);
isFromNonSprout = true;
},
[&](const libzcash::SaplingPaymentAddress& zaddr) {
zaddrs.push_back(zaddr);
isFromNonSprout = true;
},
[&](const libzcash::SproutPaymentAddress& zaddr) {
zaddrs.push_back(zaddr);
@ -5450,7 +5470,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
[&](libzcash::UnifiedAddress) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Unified addresses are not supported in z_mergetoaddress");
"Funds belonging to unified addresses can not be merged in z_mergetoaddress");
}
});
} else {
@ -5471,16 +5491,16 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
}
const int nextBlockHeight = chainActive.Height() + 1;
const bool overwinterActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_OVERWINTER);
const bool saplingActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_SAPLING);
const bool canopyActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_CANOPY);
const bool overwinterActive = chainparams.GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_OVERWINTER);
const bool saplingActive = chainparams.GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_SAPLING);
const bool canopyActive = chainparams.GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_CANOPY);
// Validate the destination address
auto destStr = params[1].get_str();
auto destaddress = keyIO.DecodePaymentAddress(destStr);
bool isToTaddr = false;
bool isToSproutZaddr = false;
bool isToSaplingZaddr = false;
if (destaddress.has_value()) {
examine(destaddress.value(), match {
[&](CKeyID addr) {
@ -5496,10 +5516,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated");
}
},
[&](libzcash::SproutPaymentAddress addr) {
isToSproutZaddr = true;
},
[&](libzcash::UnifiedAddress) {
[](libzcash::SproutPaymentAddress) { },
[](libzcash::UnifiedAddress) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Invalid parameter, unified addresses are not yet supported.");
@ -5511,19 +5529,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
string("Invalid parameter, unknown address format: ") + destStr);
}
if (canopyActive && isFromNonSprout && isToSproutZaddr) {
// Value can be moved within Sprout, but not into Sprout.
throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy");
}
// Convert fee from currency format to zatoshis
CAmount nFee = DEFAULT_FEE;
if (params.size() > 2) {
if (params[2].get_real() == 0.0) {
nFee = 0;
} else {
nFee = AmountFromValue( params[2] );
}
std::optional<CAmount> nFee;
if (params.size() > 2 && !params[2].isNull()) {
nFee = AmountFromValue( params[2] );
}
int nUTXOLimit = MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT;
@ -5545,25 +5553,15 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
saplingNoteLimit = nNoteLimit;
}
std::string memo;
std::optional<Memo> memo;
if (params.size() > 5) {
memo = params[5].get_str();
if (!(isToSproutZaddr || isToSaplingZaddr)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Memo can not be used with a taddr. It can only be used with a zaddr.");
} else if (!IsHex(memo)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected memo data in hexadecimal format.");
}
if (memo.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 ));
}
memo = Memo::FromHexOrThrow(params[5].get_str());
}
MergeToAddressRecipient recipient(destaddress.value(), memo);
NetAmountRecipient recipient(destaddress.value(), memo);
// Prepare to get UTXOs and notes
std::vector<MergeToAddressInputUTXO> utxoInputs;
std::vector<MergeToAddressInputSproutNote> sproutNoteInputs;
std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs;
SpendableInputs allInputs;
CAmount mergedUTXOValue = 0;
CAmount mergedNoteValue = 0;
CAmount remainingUTXOValue = 0;
@ -5576,9 +5574,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
unsigned int max_tx_size = saplingActive ? MAX_TX_SIZE_AFTER_SAPLING : MAX_TX_SIZE_BEFORE_SAPLING;
size_t estimatedTxSize = 200; // tx overhead + wiggle room
if (isToSproutZaddr) {
estimatedTxSize += JOINSPLIT_SIZE(SAPLING_TX_VERSION); // We assume that sapling has activated
} else if (isToSaplingZaddr) {
if (isToSaplingZaddr) {
estimatedTxSize += OUTPUTDESCRIPTION_SIZE;
}
@ -5615,8 +5611,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
maxedOutUTXOsFlag = true;
} else {
estimatedTxSize += increase;
COutPoint utxo(out.tx->GetHash(), out.i);
utxoInputs.emplace_back(utxo, nValue, scriptPubKey);
allInputs.utxos.push_back(out);
mergedUTXOValue += nValue;
}
}
@ -5629,48 +5624,55 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
if (useAnySprout || useAnySapling || zaddrs.size() > 0) {
// Get available notes
std::vector<SproutNoteEntry> sproutEntries;
std::vector<SaplingNoteEntry> saplingEntries;
std::vector<OrchardNoteMetadata> orchardEntries;
std::optional<NoteFilter> noteFilter =
useAnySprout || useAnySapling ?
std::nullopt :
std::optional(NoteFilter::ForPaymentAddresses(zaddrs));
pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, orchardEntries, noteFilter, std::nullopt, nAnchorConfirmations);
std::vector<SproutNoteEntry> sproutCandidateNotes;
std::vector<SaplingNoteEntry> saplingCandidateNotes;
std::vector<OrchardNoteMetadata> orchardCandidateNotes;
pwalletMain->GetFilteredNotes(
sproutCandidateNotes,
saplingCandidateNotes,
orchardCandidateNotes,
noteFilter,
std::nullopt,
nAnchorConfirmations);
// If Sapling is not active, do not allow sending from a sapling addresses.
if (!saplingActive && saplingEntries.size() > 0) {
if (!saplingActive && saplingCandidateNotes.size() > 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated");
}
// Do not include Sprout/Sapling notes if using "ANY_SAPLING"/"ANY_SPROUT" respectively
if (useAnySprout) {
saplingEntries.clear();
saplingCandidateNotes.clear();
}
if (useAnySapling) {
sproutEntries.clear();
sproutCandidateNotes.clear();
}
// Sending from both Sprout and Sapling is currently unsupported using z_mergetoaddress
if ((sproutEntries.size() > 0 && saplingEntries.size() > 0) || (useAnySprout && useAnySapling)) {
if ((sproutCandidateNotes.size() > 0 && saplingCandidateNotes.size() > 0) || (useAnySprout && useAnySapling)) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Cannot send from both Sprout and Sapling addresses using z_mergetoaddress");
}
// If sending between shielded addresses, they must be within the same value pool
if ((saplingEntries.size() > 0 && isToSproutZaddr) || (sproutEntries.size() > 0 && isToSaplingZaddr)) {
if (sproutCandidateNotes.size() > 0 && isToSaplingZaddr) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Cannot send between Sprout and Sapling addresses using z_mergetoaddress");
}
// Find unspent notes and update estimated size
for (const SproutNoteEntry& entry : sproutEntries) {
for (const SproutNoteEntry& entry : sproutCandidateNotes) {
noteCounter++;
CAmount nValue = entry.note.value();
if (!maxedOutNotesFlag) {
// If we haven't added any notes yet and the merge is to a
// z-address, we have already accounted for the first JoinSplit.
size_t increase = (sproutNoteInputs.empty() && !isToSproutZaddr) || (sproutNoteInputs.size() % 2 == 0) ?
size_t increase = allInputs.sproutNoteEntries.empty() || allInputs.sproutNoteEntries.size() % 2 == 0 ?
JOINSPLIT_SIZE(SAPLING_TX_VERSION) : 0;
if (estimatedTxSize + increase >= max_tx_size ||
(sproutNoteLimit > 0 && noteCounter > sproutNoteLimit))
@ -5681,7 +5683,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
auto zaddr = entry.address;
SproutSpendingKey zkey;
pwalletMain->GetSproutSpendingKey(zaddr, zkey);
sproutNoteInputs.emplace_back(entry.jsop, entry.note, nValue, zkey);
allInputs.sproutNoteEntries.push_back(entry);
mergedNoteValue += nValue;
}
}
@ -5691,7 +5693,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
}
}
for (const SaplingNoteEntry& entry : saplingEntries) {
for (const SaplingNoteEntry& entry : saplingCandidateNotes) {
noteCounter++;
CAmount nValue = entry.note.value();
if (!maxedOutNotesFlag) {
@ -5706,7 +5708,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
if (!pwalletMain->GetSaplingExtendedSpendingKey(entry.address, extsk)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find spending key for payment address.");
}
saplingNoteInputs.emplace_back(entry.op, entry.note, nValue, extsk.expsk);
allInputs.saplingNoteEntries.push_back(entry);
mergedNoteValue += nValue;
}
}
@ -5717,8 +5719,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
}
}
size_t numUtxos = utxoInputs.size();
size_t numNotes = sproutNoteInputs.size() + saplingNoteInputs.size();
size_t numUtxos = allInputs.utxos.size();
size_t numNotes = allInputs.sproutNoteEntries.size() + allInputs.saplingNoteEntries.size();
if (numUtxos == 0 && numNotes == 0) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any funds to merge.");
@ -5728,68 +5730,73 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
// - We only have one from address
// - It's equal to toaddress
// - The address only contains a single UTXO or note
// TODO: Move this to WalletTxBuilder
if (setAddress.size() == 1 && setAddress.count(destStr) && (numUtxos + numNotes) == 1) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Destination address is also the only source address, and all its funds are already merged.");
}
CAmount mergedValue = mergedUTXOValue + mergedNoteValue;
if (mergedValue < nFee) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS,
strprintf("Insufficient funds, have %s, which is less than miners fee %s",
FormatMoney(mergedValue), FormatMoney(nFee)));
}
// Check that the user specified fee is sane (if too high, it can result in error -25 absurd fee)
CAmount netAmount = mergedValue - 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)));
}
// Keep record of parameters in context object
UniValue contextInfo(UniValue::VOBJ);
contextInfo.pushKV("fromaddresses", params[0]);
contextInfo.pushKV("toaddress", params[1]);
contextInfo.pushKV("fee", ValueFromAmount(nFee));
if (!sproutNoteInputs.empty() || !saplingNoteInputs.empty() || !isToTaddr) {
// We have shielded inputs or the recipient is a shielded address, and
// therefore we cannot create transactions before Sapling activates.
if (!saplingActive) {
throw JSONRPCError(
RPC_INVALID_PARAMETER, "Cannot create shielded transactions before Sapling has activated");
}
if (nFee.has_value()) {
contextInfo.pushKV("fee", ValueFromAmount(nFee.value()));
}
bool isSproutShielded = sproutNoteInputs.size() > 0 || isToSproutZaddr;
// Contextual transaction we will build on
CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(
Params().GetConsensus(),
nextBlockHeight,
isSproutShielded || nPreferredTxVersion < ZIP225_MIN_TX_VERSION);
if (contextualTx.nVersion == 1 && isSproutShielded) {
contextualTx.nVersion = 2; // Tx format should support vJoinSplit
// The privacy policy is determined early so as to be able to use it
// for selector construction.
auto strategy =
ResolveTransactionStrategy(
ReifyPrivacyPolicy(
std::nullopt,
params.size() > 6 ? std::optional(params[6].get_str()) : std::nullopt),
InterpretLegacyCompat(std::nullopt, {recipient.first}));
WalletTxBuilder builder(Params(), minRelayTxFee);
// The ZTXOSelector here is only used to determine whether or not Sprout can be selected. It
// should not be used for other purposes (e.g., note selection or finding a change address).
// TODO: Add a ZTXOSelector that can support the full range of `z_mergetoaddress` behavior and
// use it instead of `GetFilteredNotes`.
std::optional<ZTXOSelector> ztxoSelector;
if (allInputs.sproutNoteEntries.size() > 0) {
ztxoSelector = pwalletMain->ZTXOSelectorForAddress(
allInputs.sproutNoteEntries[0].address,
true,
TransparentCoinbasePolicy::Disallow,
strategy.AllowLinkingAccountAddresses());
} else if (allInputs.saplingNoteEntries.size() > 0) {
ztxoSelector = pwalletMain->ZTXOSelectorForAddress(
allInputs.saplingNoteEntries[0].address,
true,
TransparentCoinbasePolicy::Disallow,
strategy.AllowLinkingAccountAddresses());
} else {
ztxoSelector = CWallet::LegacyTransparentZTXOSelector(true, TransparentCoinbasePolicy::Disallow);
}
// Builder (used if Sapling addresses are involved)
std::optional<TransactionBuilder> builder;
if (isToSaplingZaddr || saplingNoteInputs.size() > 0) {
std::optional<uint256> orchardAnchor;
if (!isSproutShielded && nPreferredTxVersion >= ZIP225_MIN_TX_VERSION && nAnchorConfirmations > 0) {
// 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;
}
}
builder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, orchardAnchor, pwalletMain);
if (!ztxoSelector.has_value()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing spending key for an address to be merged.");
}
// Create operation and add to global queue
auto effects = builder.PrepareTransaction(
*pwalletMain,
ztxoSelector.value(),
allInputs,
recipient,
chainActive,
strategy,
nFee,
nAnchorConfirmations)
.map_error([&](const auto& err) {
ThrowInputSelectionError(err, ztxoSelector.value(), strategy);
})
.value();
std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
std::shared_ptr<AsyncRPCOperation> operation(
new AsyncRPCOperation_mergetoaddress(
std::move(builder), contextualTx, utxoInputs, sproutNoteInputs, saplingNoteInputs, recipient, nFee, contextInfo) );
new AsyncRPCOperation_mergetoaddress(*pwalletMain, strategy, effects, contextInfo));
q->addOperation(operation);
AsyncRPCOperationId operationId = operation->getId();

View File

@ -17,7 +17,6 @@
#include "asyncrpcqueue.h"
#include "asyncrpcoperation.h"
#include "wallet/asyncrpcoperation_common.h"
#include "wallet/asyncrpcoperation_mergetoaddress.h"
#include "wallet/asyncrpcoperation_sendmany.h"
#include "wallet/asyncrpcoperation_shieldcoinbase.h"
#include "wallet/memo.h"
@ -69,6 +68,13 @@ private:
fs::path old_cwd;
};
CWalletTx FakeWalletTx() {
CMutableTransaction mtx;
mtx.vout.resize(1);
mtx.vout[0].nValue = 1;
return CWalletTx(nullptr, mtx);
}
}
static UniValue ValueFromString(const std::string &str)
@ -1219,6 +1225,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
SelectParams(CBaseChainParams::TESTNET);
const Consensus::Params& consensusParams = Params().GetConsensus();
KeyIO keyIO(Params());
WalletTxBuilder builder(Params(), minRelayTxFee);
LOCK2(cs_main, pwalletMain->cs_wallet);
@ -1248,7 +1255,6 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
// confirm this.
TransparentCoinbasePolicy::Allow,
false).value();
WalletTxBuilder builder(Params(), minRelayTxFee);
std::vector<Payment> recipients = { Payment(zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders);
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt));
@ -1265,7 +1271,6 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
true,
TransparentCoinbasePolicy::Disallow,
false).value();
WalletTxBuilder builder(Params(), minRelayTxFee);
std::vector<Payment> recipients = { Payment(taddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) };
TransactionStrategy strategy(PrivacyPolicy::AllowRevealedRecipients);
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt));
@ -1523,6 +1528,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
// Test constructor of AsyncRPCOperation_shieldcoinbase
KeyIO keyIO(Params());
WalletTxBuilder builder(Params(), minRelayTxFee);
UniValue retValue;
BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewaddress"));
auto taddr2 = std::get<CKeyID>(keyIO.DecodePaymentAddress(retValue.get_str()).value());
@ -1535,7 +1541,6 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
// confirm this.
TransparentCoinbasePolicy::Allow,
false).value();
WalletTxBuilder builder(Params(), minRelayTxFee);
try {
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase(std::move(builder), selector, testnetzaddr, PrivacyPolicy::AllowRevealedSenders, SHIELD_COINBASE_DEFAULT_LIMIT, 1));
@ -1620,54 +1625,44 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters)
std::fill(v.begin(),v.end(), 'A');
std::string badmemo(v.begin(), v.end());
CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + aSproutAddr + " 0.00001 100 100 " + badmemo,
"Invalid parameter, size of memo is larger than maximum allowed 512");
strprintf("Invalid parameter, memo is longer than the maximum allowed %d characters.", ZC_MEMO_SIZE));
// 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);
// Test constructor of AsyncRPCOperation_mergetoaddress
KeyIO keyIO(Params());
MergeToAddressRecipient testnetzaddr(
keyIO.DecodePaymentAddress("ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP").value(),
"testnet memo");
WalletTxBuilder builder(Params(), minRelayTxFee);
auto saplingKey = pwalletMain->GenerateNewLegacySaplingZKey();
NetAmountRecipient testnetzaddr(saplingKey, Memo());
auto selector = CWallet::LegacyTransparentZTXOSelector(
true,
TransparentCoinbasePolicy::Disallow);
TransactionStrategy strategy(PrivacyPolicy::AllowRevealedRecipients);
try {
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, {}, {}, {}, testnetzaddr, -1 ));
BOOST_FAIL("Should have caused an error");
} catch (const UniValue& objError) {
BOOST_CHECK( find_error(objError, "Fee is out of range"));
}
builder.PrepareTransaction(*pwalletMain, selector, {}, testnetzaddr, chainActive, strategy, -1, 1)
.map_error([&](const auto& err) {
// TODO: Provide `operator==` on `InputSelectionError` and use that here.
BOOST_CHECK(std::holds_alternative<InvalidFeeError>(err));
})
.map([](const auto&) {
BOOST_FAIL("Fee value of -1 expected to be out of the valid range of values.");
});
try {
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, {}, {}, {}, testnetzaddr, 1));
BOOST_FAIL("Should have caused an error");
} catch (const UniValue& objError) {
BOOST_CHECK( find_error(objError, "No inputs"));
}
std::vector<MergeToAddressInputUTXO> inputs = { MergeToAddressInputUTXO{ COutPoint{uint256(), 0}, 0, CScript()} };
std::vector<MergeToAddressInputSproutNote> sproutNoteInputs =
{MergeToAddressInputSproutNote{JSOutPoint(), SproutNote(), 0, SproutSpendingKey()}};
std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs =
{MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote({}, uint256(), 0, uint256(), Zip212Enabled::BeforeZip212), 0, SaplingExpandedSpendingKey()}};
// Sprout and Sapling inputs -> throw
try {
auto operation = new AsyncRPCOperation_mergetoaddress(std::nullopt, mtx, inputs, sproutNoteInputs, saplingNoteInputs, testnetzaddr, 1);
BOOST_FAIL("Should have caused an error");
} catch (const UniValue& objError) {
BOOST_CHECK(find_error(objError, "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress"));
}
// Sprout inputs and TransactionBuilder -> throw
try {
auto operation = new AsyncRPCOperation_mergetoaddress(TransactionBuilder(), mtx, inputs, sproutNoteInputs, {}, testnetzaddr, 1);
BOOST_FAIL("Should have caused an error");
} catch (const UniValue& objError) {
BOOST_CHECK(find_error(objError, "Sprout notes are not supported by the TransactionBuilder"));
}
builder.PrepareTransaction(*pwalletMain, selector, {}, testnetzaddr, chainActive, strategy, 1, 1)
.map_error([&](const auto& err) {
// TODO: Provide `operator==` on `InputSelectionError` and use that here.
BOOST_CHECK(examine(err, match {
[](const InvalidFundsError& ife) {
return std::holds_alternative<InsufficientFundsError>(ife.reason);
},
[](const auto&) { return false; },
}));
})
.map([](const auto&) {
BOOST_FAIL("Expected an error.");
});
}
void TestWTxStatus(const Consensus::Params consensusParams, const int delta) {

View File

@ -11,7 +11,7 @@ using namespace libzcash;
int GetAnchorHeight(const CChain& chain, uint32_t anchorConfirmations)
{
int nextBlockHeight = chain.Height() + 1;
return nextBlockHeight - anchorConfirmations;
return std::max(0, nextBlockHeight - (int) anchorConfirmations);
}
static size_t PadCount(size_t n)
@ -260,7 +260,7 @@ tl::expected<ChangeAddress, AddressResolutionError>
WalletTxBuilder::GetChangeAddress(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendable,
const SpendableInputs& spendable,
const Payments& resolvedPayments,
const TransactionStrategy& strategy,
bool afterNU5) const
@ -385,15 +385,20 @@ tl::expected<TransactionEffects, InputSelectionError>
WalletTxBuilder::PrepareTransaction(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendable,
const SpendableInputs& spendable,
const Recipients& payments,
const CChain& chain,
TransactionStrategy strategy,
std::optional<CAmount> fee,
const TransactionStrategy& strategy,
const std::optional<CAmount>& fee,
uint32_t anchorConfirmations) const
{
if (fee.has_value() && maxTxFee < fee.value()) {
return tl::make_unexpected(MaxFeeError(fee.value()));
if (fee.has_value()) {
if (maxTxFee < fee.value()) {
return tl::make_unexpected(MaxFeeError(fee.value()));
} else if (!MoneyRange(fee.value())) {
// TODO: This check will be obviated by #6579.
return tl::make_unexpected(InvalidFeeError(fee.value()));
}
}
int anchorHeight = GetAnchorHeight(chain, anchorConfirmations);
@ -521,7 +526,7 @@ tl::expected<
WalletTxBuilder::IterateLimit(
CWallet& wallet,
const ZTXOSelector& selector,
const TransactionStrategy strategy,
const TransactionStrategy& strategy,
CAmount sendAmount,
CAmount dustThreshold,
const SpendableInputs& spendable,
@ -615,11 +620,11 @@ tl::expected<InputSelection, InputSelectionError>
WalletTxBuilder::ResolveInputsAndPayments(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendableMut,
const SpendableInputs& spendable,
const std::vector<Payment>& payments,
const CChain& chain,
const TransactionStrategy& strategy,
std::optional<CAmount> fee,
const std::optional<CAmount>& fee,
bool afterNU5) const
{
LOCK2(cs_main, wallet.cs_wallet);
@ -629,8 +634,8 @@ WalletTxBuilder::ResolveInputsAndPayments(
// as possible. This will also perform opportunistic shielding if the
// transaction strategy permits.
CAmount maxSaplingAvailable = spendableMut.GetSaplingTotal();
CAmount maxOrchardAvailable = spendableMut.GetOrchardTotal();
CAmount maxSaplingAvailable = spendable.GetSaplingTotal();
CAmount maxOrchardAvailable = spendable.GetOrchardTotal();
uint32_t orchardOutputs{0};
// we can only select Orchard addresses if were not sending from Sprout, since there is no tx
@ -665,10 +670,12 @@ WalletTxBuilder::ResolveInputsAndPayments(
sendAmount += payment.GetAmount();
}
SpendableInputs spendableMut;
CAmount finalFee;
CAmount targetAmount;
std::optional<ChangeAddress> changeAddr;
if (fee.has_value()) {
spendableMut = spendable;
finalFee = fee.value();
targetAmount = sendAmount + finalFee;
// TODO: the set of recipient pools is not quite sufficient information here; we should
@ -726,7 +733,7 @@ WalletTxBuilder::ResolveInputsAndPayments(
}
}
} else {
auto limitResult = IterateLimit(wallet, selector, strategy, sendAmount, dustThreshold, spendableMut, resolved, afterNU5);
auto limitResult = IterateLimit(wallet, selector, strategy, sendAmount, dustThreshold, spendable, resolved, afterNU5);
if (limitResult.has_value()) {
std::tie(spendableMut, finalFee, changeAddr) = limitResult.value();
targetAmount = sendAmount + finalFee;

View File

@ -49,7 +49,7 @@ public:
CAmount amount,
std::optional<Memo> memo) :
address(address), amount(amount), memo(memo) {
assert(amount >= 0);
assert(MoneyRange(amount));
}
const PaymentAddress& GetAddress() const {
@ -300,6 +300,15 @@ public:
available(available), required(required) { }
};
/// Error when a fee is outside `MoneyRange`
class InvalidFeeError {
public:
CAmount fixedFee;
InvalidFeeError(CAmount fixedFee):
fixedFee(fixedFee) { }
};
/// Error when a fee is higher than can be useful. This reduces the chance of accidentally
/// overpaying with explicit fees.
class AbsurdFeeError {
@ -340,6 +349,7 @@ typedef std::variant<
AddressResolutionError,
InvalidFundsError,
ChangeNotAllowedError,
InvalidFeeError,
AbsurdFeeError,
MaxFeeError,
ExcessOrchardActionsError> InputSelectionError;
@ -376,7 +386,7 @@ private:
GetChangeAddress(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendable,
const SpendableInputs& spendable,
const Payments& resolvedPayments,
const TransactionStrategy& strategy,
bool afterNU5) const;
@ -387,7 +397,7 @@ private:
IterateLimit(
CWallet& wallet,
const ZTXOSelector& selector,
const TransactionStrategy strategy,
const TransactionStrategy& strategy,
CAmount sendAmount,
CAmount dustThreshold,
const SpendableInputs& spendable,
@ -403,11 +413,11 @@ private:
ResolveInputsAndPayments(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendable,
const SpendableInputs& spendable,
const std::vector<Payment>& payments,
const CChain& chain,
const TransactionStrategy& strategy,
std::optional<CAmount> fee,
const std::optional<CAmount>& fee,
bool afterNU5) const;
/**
* Compute the internal and external OVKs to use in transaction construction, given
@ -431,12 +441,12 @@ public:
PrepareTransaction(
CWallet& wallet,
const ZTXOSelector& selector,
SpendableInputs& spendable,
const SpendableInputs& spendable,
const Recipients& payments,
const CChain& chain,
TransactionStrategy strategy,
const TransactionStrategy& strategy,
/// A fixed fee is used if provided, otherwise it is calculated based on ZIP 317.
std::optional<CAmount> fee,
const std::optional<CAmount>& fee,
uint32_t anchorConfirmations) const;
};