diff --git a/qa/rpc-tests/mempool_limit.py b/qa/rpc-tests/mempool_limit.py index 9a3c2d5c2..466444006 100755 --- a/qa/rpc-tests/mempool_limit.py +++ b/qa/rpc-tests/mempool_limit.py @@ -13,6 +13,7 @@ from test_framework.util import ( wait_and_assert_operationid_status, DEFAULT_FEE ) +from test_framework.zip317 import conventional_fee from decimal import Decimal from time import sleep @@ -105,9 +106,9 @@ class MempoolLimit(BitcoinTestFramework): print("Checking mempool size reset after block mined...") self.check_mempool_sizes(0) zaddr4 = self.nodes[0].z_getnewaddress('sapling') - opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*DEFAULT_FEE}], 1) + opid4 = self.nodes[0].z_sendmany(zaddr1, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1) wait_and_assert_operationid_status(self.nodes[0], opid4) - opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*DEFAULT_FEE}], 1) + opid5 = self.nodes[0].z_sendmany(zaddr2, [{"address": zaddr4, "amount": Decimal('10.0') - 2*conventional_fee(2)}], 1) wait_and_assert_operationid_status(self.nodes[0], opid5) self.sync_all() diff --git a/qa/rpc-tests/mempool_tx_expiry.py b/qa/rpc-tests/mempool_tx_expiry.py index 526328768..a5d77ef99 100755 --- a/qa/rpc-tests/mempool_tx_expiry.py +++ b/qa/rpc-tests/mempool_tx_expiry.py @@ -12,6 +12,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, \ connect_nodes_bi, sync_blocks, start_nodes, \ wait_and_assert_operationid_status, DEFAULT_FEE +from test_framework.zip317 import conventional_fee from decimal import Decimal @@ -93,7 +94,7 @@ class MempoolTxExpiryTest(BitcoinTestFramework): # Create transactions blockheight = self.nodes[0].getblockchaininfo()['blocks'] - zsendamount = Decimal('1.0') - DEFAULT_FEE + zsendamount = Decimal('1.0') - conventional_fee(2) recipients = [] recipients.append({"address": z_bob, "amount": zsendamount}) myopid = self.nodes[0].z_sendmany(z_alice, recipients, 1) diff --git a/qa/rpc-tests/test_framework/zip317.py b/qa/rpc-tests/test_framework/zip317.py new file mode 100644 index 000000000..dbe8f7e51 --- /dev/null +++ b/qa/rpc-tests/test_framework/zip317.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php . + +# +# zip317.py +# +# Utilities for ZIP 317 conventional fee specification, as defined in https://zips.z.cash/zip-0317. +# + +from test_framework.mininode import COIN +from decimal import Decimal + +# The fee per logical action, in zatoshis. See https://zips.z.cash/zip-0317#fee-calculation. +MARGINAL_FEE = 5000 + +# The lower bound on the number of logical actions in a tx, for purposes of fee calculation. See +# https://zips.z.cash/zip-0317#fee-calculation. +GRACE_ACTIONS = 2 + +# Limits the relative probability of picking a given transaction to be at most `WEIGHT_RATIO_CAP` +# times greater than a transaction that pays exactly the conventional fee. See +# https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction +WEIGHT_RATIO_CAP = 4 + +# The zcashd RPC sentinel value to indicate the conventional_fee when a positional argument is +# required. +ZIP_317_FEE = -1 + +def conventional_fee_zats(logical_actions): + return MARGINAL_FEE * max(GRACE_ACTIONS, logical_actions) + +def conventional_fee(logical_actions): + return Decimal(conventional_fee_zats(logical_actions)) / COIN diff --git a/qa/rpc-tests/wallet_listreceived.py b/qa/rpc-tests/wallet_listreceived.py index 96eea5aa6..22fcbb63e 100755 --- a/qa/rpc-tests/wallet_listreceived.py +++ b/qa/rpc-tests/wallet_listreceived.py @@ -13,10 +13,10 @@ from test_framework.util import ( connect_nodes_bi, nuparams, DEFAULT_FEE, - DEFAULT_FEE_ZATS, NU5_BRANCH_ID, ) from test_framework.util import wait_and_assert_operationid_status, start_nodes +from test_framework.zip317 import conventional_fee, conventional_fee_zats from decimal import Decimal my_memo_str = 'c0ffee' # stay awake @@ -157,8 +157,8 @@ class ListReceivedTest (BitcoinTestFramework): outputs = sorted(pt['outputs'], key=lambda x: x['valueZat']) assert_equal(outputs[0]['pool'], 'sapling') assert_equal(outputs[0]['address'], zaddr1) - assert_equal(outputs[0]['value'], Decimal('0.4') - DEFAULT_FEE) - assert_equal(outputs[0]['valueZat'], 40000000 - DEFAULT_FEE_ZATS) + assert_equal(outputs[0]['value'], Decimal('0.4') - conventional_fee(2)) + assert_equal(outputs[0]['valueZat'], 40000000 - conventional_fee_zats(2)) assert_equal(outputs[0]['output'], 1) assert_equal(outputs[0]['outgoing'], False) assert_equal(outputs[0]['memo'], no_memo) @@ -178,8 +178,8 @@ class ListReceivedTest (BitcoinTestFramework): assert_equal(2, len(r), "zaddr1 Should have received 2 notes") r = sorted(r, key = lambda received: received['amount']) assert_equal(txid, r[0]['txid']) - assert_equal(Decimal('0.4')-DEFAULT_FEE, r[0]['amount']) - assert_equal(40000000-DEFAULT_FEE_ZATS, r[0]['amountZat']) + assert_equal(Decimal('0.4')-conventional_fee(2), r[0]['amount']) + assert_equal(40000000-conventional_fee_zats(2), r[0]['amountZat']) assert_equal(r[0]['change'], True, "Note valued at (0.4-"+str(DEFAULT_FEE)+") should be change") assert_equal(no_memo, r[0]['memo']) @@ -382,8 +382,8 @@ class ListReceivedTest (BitcoinTestFramework): # Verify that we observe the change output assert_equal(outputs[2]['pool'], 'orchard') - assert_equal(outputs[2]['value'], Decimal('0.49999')) - assert_equal(outputs[2]['valueZat'], 49999000) + assert_equal(outputs[2]['value'], Decimal('0.5') - conventional_fee(3)) + assert_equal(outputs[2]['valueZat'], 50000000 - conventional_fee_zats(3)) assert_equal(outputs[2]['outgoing'], False) assert_equal(outputs[2]['walletInternal'], True) assert_equal(outputs[2]['memo'], no_memo) diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 938945bc6..1a16df993 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -7,6 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_true, bitcoind_processes, \ connect_nodes_bi, start_node, start_nodes, wait_and_assert_operationid_status, \ get_coinbase_address, DEFAULT_FEE +from test_framework.zip317 import conventional_fee from decimal import Decimal @@ -92,7 +93,7 @@ class WalletNullifiersTest (BitcoinTestFramework): # check zaddr balance zsendmany2notevalue = Decimal('2.0') - zsendmanyfee = DEFAULT_FEE + zsendmanyfee = conventional_fee(2) zaddrremaining = zsendmanynotevalue - zsendmany2notevalue - zsendmanyfee assert_equal(self.nodes[3].z_getbalance(myzaddr3), zsendmany2notevalue) assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining) @@ -117,7 +118,7 @@ class WalletNullifiersTest (BitcoinTestFramework): # check zaddr balance zsendmany3notevalue = Decimal('1.0') - zaddrremaining2 = zaddrremaining - zsendmany3notevalue - zsendmanyfee + zaddrremaining2 = zaddrremaining - zsendmany3notevalue - DEFAULT_FEE assert_equal(self.nodes[1].z_getbalance(myzaddr), zaddrremaining2) assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining2) diff --git a/qa/rpc-tests/wallet_parsing_amounts.py b/qa/rpc-tests/wallet_parsing_amounts.py index bdfdf42c3..ff792b934 100755 --- a/qa/rpc-tests/wallet_parsing_amounts.py +++ b/qa/rpc-tests/wallet_parsing_amounts.py @@ -70,19 +70,6 @@ class WalletAmountParsingTest(BitcoinTestFramework): print(errorString) assert(False) - # This fee is larger than the default fee and since amount=0 - # it should trigger error - fee = Decimal('0.1') - recipients = [ {"address": myzaddr, "amount": Decimal('0.0') } ] - minconf = 1 - errorString = '' - - try: - myopid = self.nodes[0].z_sendmany(myzaddr, recipients, minconf, fee) - except JSONRPCException as e: - errorString = e.error['message'] - assert('Small transaction amount' in errorString) - # This fee is less than default and greater than amount, but still valid fee = Decimal('0.0000001') recipients = [ {"address": myzaddr, "amount": Decimal('0.00000001') } ] diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py index 7e1ccc42d..f3b9b15be 100755 --- a/qa/rpc-tests/wallet_shieldingcoinbase.py +++ b/qa/rpc-tests/wallet_shieldingcoinbase.py @@ -10,6 +10,7 @@ from test_framework.util import assert_equal, initialize_chain_clean, \ start_nodes, connect_nodes_bi, wait_and_assert_operationid_status, \ wait_and_assert_operationid_status_result, get_coinbase_address, \ check_node_log, DEFAULT_FEE +from test_framework.zip317 import conventional_fee, WEIGHT_RATIO_CAP, ZIP_317_FEE import sys import timeit @@ -104,7 +105,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): recipients = [] recipients.append({"address":myzaddr, "amount":Decimal('1.23456789')}) - myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 10, DEFAULT_FEE, 'AllowRevealedSenders') + myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 10, DEFAULT_FEE, 'AllowFullyTransparent') error_result = wait_and_assert_operationid_status_result( self.nodes[0], myopid, "failed", @@ -188,7 +189,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): # A custom fee of 0 is okay. Here the node will send the note value back to itself. recipients = [] - recipients.append({"address":myzaddr, "amount": Decimal('20.0') - DEFAULT_FEE}) + recipients.append({"address":myzaddr, "amount": Decimal('20.0') - conventional_fee(2)}) myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, Decimal('0.0')) mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() @@ -230,7 +231,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): # UTXO selection in z_sendmany sorts in ascending order, so smallest utxos are consumed first. # At this point in time, unspent notes all have a value of 10.0. recipients = [] - amount = Decimal('10.0') - DEFAULT_FEE - Decimal('0.00000001') # this leaves change at 1 zatoshi less than dust threshold + amount = Decimal('10.0') - conventional_fee(2) - Decimal('0.00000001') # this leaves change at 1 zatoshi less than dust threshold recipients.append({"address":self.nodes[0].getnewaddress(), "amount":amount }) myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 1) wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 0.00000053 more to avoid creating invalid change output 0.00000001 (dust threshold is 0.00000054); note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.") @@ -247,7 +248,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): recipients = [] recipients.append({"address":self.nodes[1].getnewaddress(), "amount":Decimal('10000.0')}) myopid = self.nodes[0].z_sendmany(mytaddr, recipients, 1) - wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.") + wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 10.00, need 10000.0001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.") myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, DEFAULT_FEE, 'AllowRevealedRecipients') wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Insufficient funds: have 9.99998, need 10000.00001; note that coinbase outputs will not be selected if you specify ANY_TADDR, any transparent recipients are included, or if the `privacyPolicy` parameter is not set to `AllowRevealedSenders` or weaker.") @@ -264,7 +265,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): # given the tx size, resulting in mempool rejection. errorString = '' recipients = [] - num_t_recipients = 2500 + num_t_recipients = 1998 # stay just under the absurdly-high-fee error amount_per_recipient = Decimal('0.00000546') # dust threshold # Note that regtest chainparams does not require standard tx, so setting the amount to be # less than the dust threshold, e.g. 0.00000001 will not result in mempool rejection. @@ -284,7 +285,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): self.nodes[0].getinfo() # Issue #2263 Workaround END - myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, DEFAULT_FEE, 'AllowRevealedRecipients') + myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, ZIP_317_FEE, 'AllowRevealedRecipients') try: wait_and_assert_operationid_status(self.nodes[0], myopid) except JSONRPCException as e: @@ -298,15 +299,18 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): self.nodes[1].generate(1) self.sync_all() + many_recipient_fee = conventional_fee(2+num_t_recipients) # 2+ for padded Sapling input/change + # check balance node2balance = amount_per_recipient * num_t_recipients - saplingvalue -= node2balance + DEFAULT_FEE + saplingvalue -= node2balance + many_recipient_fee assert_equal(self.nodes[2].getbalance(), node2balance) check_value_pool(self.nodes[0], 'sapling', saplingvalue) # Send will fail because fee is negative try: - self.nodes[0].z_sendmany(myzaddr, recipients, 1, -1) + # NB: Using -2 as the fee because -1 is the sentinel for using the conventional fee. + self.nodes[0].z_sendmany(myzaddr, recipients, 1, -2) except JSONRPCException as e: errorString = e.error['message'] assert_equal("Amount out of range" in errorString, True) @@ -318,12 +322,9 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): errorString = e.error['message'] assert_equal("Amount out of range" in errorString, True) - # Send will fail because fee is larger than sum of outputs - try: - self.nodes[0].z_sendmany(myzaddr, recipients, 1, (amount_per_recipient * num_t_recipients) + Decimal('0.00000001')) - except JSONRPCException as e: - errorString = e.error['message'] - assert_equal("is greater than the sum of outputs" in errorString, True) + # Send will fail because fee is more than `WEIGHT_RATIO_CAP * conventional_fee` + myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, (WEIGHT_RATIO_CAP * many_recipient_fee) + Decimal('0.00000001')) + wait_and_assert_operationid_status(self.nodes[0], myopid, 'failed', 'Fee 0.40000001 is greater than 4 times the conventional fee for this tx (which is 0.10). There is no prioritisation benefit to a fee this large (see https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), and it likely indicates a mistake in setting the fee.') # Send will succeed because the balance of non-coinbase utxos is 10.0 try: diff --git a/qa/rpc-tests/wallet_treestate.py b/qa/rpc-tests/wallet_treestate.py index e1afb60c5..4fa1a245c 100755 --- a/qa/rpc-tests/wallet_treestate.py +++ b/qa/rpc-tests/wallet_treestate.py @@ -7,6 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, initialize_chain_clean, \ start_nodes, connect_nodes_bi, wait_and_assert_operationid_status, \ get_coinbase_address, DEFAULT_FEE +from test_framework.zip317 import conventional_fee import time from decimal import Decimal @@ -79,7 +80,7 @@ class WalletTreeStateTest (BitcoinTestFramework): # the z_sendmany implementation because there are only two inputs per joinsplit. recipients = [] recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('18.0')}) - recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('12.0') - 4*DEFAULT_FEE}) + recipients.append({"address": self.nodes[2].z_getnewaddress(), "amount": Decimal('12.0') - 4 * conventional_fee(3)}) myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1) # Wait for Tx 2 to begin executing... diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 10734465e..7dc57757c 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -22,6 +22,10 @@ from decimal import Decimal # Test wallet address behaviour across network upgrades class WalletZSendmanyTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.cache_behavior = 'sprout' + def setup_network(self, split=False): self.nodes = start_nodes(3, self.options.tmpdir, [[ nuparams(NU5_BRANCH_ID, 238), @@ -110,12 +114,13 @@ class WalletZSendmanyTest(BitcoinTestFramework): # check balances zsendmanynotevalue = Decimal('7.0') zsendmanyfee = DEFAULT_FEE - node2utxobalance = Decimal('260.00000000') - zsendmanynotevalue - zsendmanyfee + node2sproutbalance = Decimal('50.00000000') + node2utxobalance = Decimal('210.00000000') - zsendmanynotevalue - zsendmanyfee # check shielded balance status with getwalletinfo wallet_info = self.nodes[2].getwalletinfo() assert_equal(Decimal(wallet_info["shielded_unconfirmed_balance"]), zsendmanynotevalue) - assert_equal(Decimal(wallet_info["shielded_balance"]), Decimal('0.0')) + assert_equal(Decimal(wallet_info["shielded_balance"]), node2sproutbalance) self.nodes[2].generate(10) self.sync_all() @@ -130,13 +135,13 @@ class WalletZSendmanyTest(BitcoinTestFramework): # check via z_gettotalbalance resp = self.nodes[2].z_gettotalbalance() assert_equal(Decimal(resp["transparent"]), node2utxobalance) - assert_equal(Decimal(resp["private"]), zbalance) - assert_equal(Decimal(resp["total"]), node2utxobalance + zbalance) + assert_equal(Decimal(resp["private"]), node2sproutbalance + zbalance) + assert_equal(Decimal(resp["total"]), node2utxobalance + node2sproutbalance + zbalance) # check confirmed shielded balance with getwalletinfo wallet_info = self.nodes[2].getwalletinfo() assert_equal(Decimal(wallet_info["shielded_unconfirmed_balance"]), Decimal('0.0')) - assert_equal(Decimal(wallet_info["shielded_balance"]), zsendmanynotevalue) + assert_equal(Decimal(wallet_info["shielded_balance"]), node2sproutbalance + zsendmanynotevalue) # there should be at least one Sapling output mytxdetails = self.nodes[2].getrawtransaction(mytxid, 1) @@ -377,7 +382,7 @@ class WalletZSendmanyTest(BitcoinTestFramework): for (policy, msg) in [ ('FullPrivacy', 'Could not send to a shielded receiver of a unified address without spending funds from a different pool, which would reveal transaction amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedAmounts` or weaker if you wish to allow this transaction to proceed anyway.'), ('AllowRevealedAmounts', 'This transaction would send to a transparent receiver of a unified address, which is not enabled by default because it will publicly reveal transaction recipients and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedRecipients` or weaker if you wish to allow this transaction to proceed anyway.'), - ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), or NU5 has not been activated yet.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver despite a lax privacy policy, because NU5 has not been activated yet.'), ]: opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) @@ -416,17 +421,18 @@ class WalletZSendmanyTest(BitcoinTestFramework): self.sync_all() # - # Test Orchard-only UA with insufficient non-Sprout funds + # Test sending Sprout funds to Orchard-only UA # + sproutAddr = self.nodes[2].listaddresses()[0]['sprout']['addresses'][0] recipients = [{"address":n0orchard_only, "amount":100}] for (policy, msg) in [ ('FullPrivacy', 'Could not send to a shielded receiver of a unified address without spending funds from a different pool, which would reveal transaction amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedAmounts` or weaker if you wish to allow this transaction to proceed anyway.'), ('AllowRevealedAmounts', 'This transaction would send to a transparent receiver of a unified address, which is not enabled by default because it will publicly reveal transaction recipients and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedRecipients` or weaker if you wish to allow this transaction to proceed anyway.'), - ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver, despite a lax privacy policy. Either there are insufficient non-Sprout funds (there is no transaction version that supports both Sprout and Orchard), or NU5 has not been activated yet.'), + ('AllowRevealedRecipients', 'Could not send to an Orchard-only receiver despite a lax privacy policy, because you are sending from the Sprout pool and there is no transaction version that supports both Sprout and Orchard.'), ]: - opid = self.nodes[1].z_sendmany(n1ua0, recipients, 1, 0, policy) - wait_and_assert_operationid_status(self.nodes[1], opid, 'failed', msg) + opid = self.nodes[2].z_sendmany(sproutAddr, recipients, 1, 0, policy) + wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', msg) # # Test AllowRevealedAmounts policy @@ -488,5 +494,24 @@ class WalletZSendmanyTest(BitcoinTestFramework): {'pools': {'orchard': {'valueZat': 200000000}}, 'minimum_confirmations': 1}, self.nodes[0].z_getbalanceforaccount(n0account0)) + + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + + # + # Test transparent change + # + + recipients = [{"address":n0ua1, "amount": 4}] + # Should fail because this generates transparent change, but we don’t have + # `AllowRevealedRecipients` + opid = self.nodes[2].z_sendmany(mytaddr, recipients, 1, 0, 'AllowRevealedSenders') + wait_and_assert_operationid_status(self.nodes[2], opid, 'failed', "This transaction would have transparent change, which is not enabled by default because it will publicly reveal the change address and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit with the `privacyPolicy` parameter set to `AllowRevealedRecipients` or weaker if you wish to allow this transaction to proceed anyway.") + + # Should succeed once we include `AllowRevealedRecipients` + opid = self.nodes[2].z_sendmany(mytaddr, recipients, 1, 0, 'AllowFullyTransparent') + wait_and_assert_operationid_status(self.nodes[2], opid) + if __name__ == '__main__': WalletZSendmanyTest().main() diff --git a/src/wallet/asyncrpcoperation_common.cpp b/src/wallet/asyncrpcoperation_common.cpp index f03dad03a..0bd6e6037 100644 --- a/src/wallet/asyncrpcoperation_common.cpp +++ b/src/wallet/asyncrpcoperation_common.cpp @@ -4,6 +4,7 @@ #include "init.h" #include "rpc/protocol.h" #include "util/moneystr.h" +#include "zip317.h" extern UniValue signrawtransaction(const UniValue& params, bool fHelp); @@ -48,7 +49,7 @@ void ThrowInputSelectionError( const TransactionStrategy& strategy) { examine(err, match { - [](const AddressResolutionError& err) { + [&](const AddressResolutionError& err) { switch (err) { case AddressResolutionError::SproutRecipientsNotSupported: throw JSONRPCError( @@ -62,6 +63,14 @@ void ThrowInputSelectionError( "recipients and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit " "with the `privacyPolicy` parameter set to `AllowRevealedRecipients` " "or weaker if you wish to allow this transaction to proceed anyway."); + case AddressResolutionError::TransparentChangeNotAllowed: + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "This transaction would have transparent change, which is not " + "enabled by default because it will publicly reveal the change " + "address and amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit " + "with the `privacyPolicy` parameter set to `AllowRevealedRecipients` " + "or weaker if you wish to allow this transaction to proceed anyway."); case AddressResolutionError::RevealingSaplingAmountNotAllowed: throw JSONRPCError( RPC_INVALID_PARAMETER, @@ -73,10 +82,19 @@ void ThrowInputSelectionError( case AddressResolutionError::CouldNotResolveReceiver: throw JSONRPCError( RPC_INVALID_PARAMETER, - "Could not send to an Orchard-only receiver, despite a lax privacy policy. " - "Either there are insufficient non-Sprout funds (there is no transaction " - "version that supports both Sprout and Orchard), or NU5 has not been " - "activated yet."); + strprintf("Could not send to an Orchard-only receiver %s.", + strategy.AllowRevealedAmounts() + ? strprintf("despite a lax privacy policy, because %s", + selector.SelectsSprout() + ? "you are sending from the Sprout pool and there is " + "no transaction version that supports both Sprout " + "and Orchard" + : "NU5 has not been activated yet") + : "without spending non-Orchard funds, which would reveal " + "transaction amounts. THIS MAY AFFECT YOUR PRIVACY. Resubmit " + "with the `privacyPolicy` parameter set to " + "`AllowRevealedAmounts` or weaker if you wish to allow this " + "transaction to proceed anyway")); case AddressResolutionError::TransparentReceiverNotAllowed: throw JSONRPCError( RPC_INVALID_PARAMETER, @@ -106,9 +124,20 @@ void ThrowInputSelectionError( "Insufficient funds: have %s, %s", FormatMoney(err.available), examine(err.reason, match { + [](const PhantomChangeError& qce) { + return strprintf( + "need %s more to surpass the dust threshold and avoid being " + "forced to over-pay the fee. Alternatively, you could specify " + "a fee of %s to allow overpayment of the conventional fee and " + "have this transaction proceed.", + FormatMoney(qce.dustThreshold), + FormatMoney(qce.finalFee)); + }, [](const InsufficientFundsError& ife) { return strprintf("need %s", FormatMoney(ife.required)); }, + // TODO: Add the fee here, so we can suggest specifying an explicit fee (see + // `PhantomChangeError`). [](const DustThresholdError& dte) { return strprintf( "need %s more to avoid creating invalid change output %s (dust threshold is %s)", @@ -138,6 +167,18 @@ void ThrowInputSelectionError( "The proposed transaction would result in %s in change.", FormatMoney(err.available - err.required))); }, + [](const AbsurdFeeError& err) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + strprintf( + "Fee %s is greater than %d times the conventional fee for this tx (which is " + "%s). There is no prioritisation benefit to a fee this large (see " + "https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), " + "and it likely indicates a mistake in setting the fee.", + FormatMoney(err.fixedFee), + WEIGHT_RATIO_CAP, + FormatMoney(err.conventionalFee))); + }, [](const ExcessOrchardActionsError& err) { std::string side; switch (err.side) { diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 1bf4bf81d..41c331b69 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -47,13 +47,13 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( int minDepth, unsigned int anchorDepth, TransactionStrategy strategy, - CAmount fee, + std::optional fee, UniValue contextInfo) : builder_(std::move(builder)), ztxoSelector_(ztxoSelector), recipients_(recipients), mindepth_(minDepth), anchordepth_(anchorDepth), strategy_(strategy), fee_(fee), contextinfo_(contextInfo) { - assert(fee_ >= 0); + assert(!fee_.has_value() || fee_.value() >= 0); assert(mindepth_ >= 0); assert(!recipients_.empty()); assert(ztxoSelector.RequireSpendingKeys()); @@ -82,7 +82,11 @@ void AsyncRPCOperation_sendmany::main() { std::optional txid; try { - txid = main_impl(*pwalletMain); + txid = main_impl(*pwalletMain) + .map_error([&](const InputSelectionError& err) { + ThrowInputSelectionError(err, ztxoSelector_, strategy_); + }) + .value(); } catch (const UniValue& objError) { int code = find_value(objError, "code").get_int(); std::string message = find_value(objError, "message").get_str(); @@ -135,7 +139,8 @@ void AsyncRPCOperation_sendmany::main() { // 4. #3615 There is no padding of inputs or outputs, which may leak information. // // At least #4 differs from the Rust transaction builder. -uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) { +tl::expected +AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) { auto spendable = builder_.FindAllSpendableInputs(wallet, ztxoSelector_, mindepth_); auto preparedTx = builder_.PrepareTransaction( @@ -148,12 +153,8 @@ uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) { fee_, anchordepth_); - uint256 txid; - examine(preparedTx, match { - [&](const InputSelectionError& err) { - ThrowInputSelectionError(err, ztxoSelector_, strategy_); - }, - [&](const TransactionEffects& effects) { + return preparedTx + .map([&](const TransactionEffects& effects) { try { const auto& spendable = effects.GetSpendable(); const auto& payments = effects.GetPayments(); @@ -185,16 +186,13 @@ uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) { UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode); set_result(sendResult); - txid = tx.GetHash(); effects.UnlockSpendable(wallet); + return tx.GetHash(); } catch (...) { effects.UnlockSpendable(wallet); throw; } - } - }); - - return txid; + }); } /** diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index ad37f2de6..409839064 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -33,7 +33,7 @@ public: int minDepth, unsigned int anchorDepth, TransactionStrategy strategy, - CAmount fee = DEFAULT_FEE, + std::optional fee, UniValue contextInfo = NullUniValue); virtual ~AsyncRPCOperation_sendmany(); @@ -59,10 +59,10 @@ private: TransactionStrategy strategy_; int mindepth_{1}; unsigned int anchordepth_{nAnchorConfirmations}; - CAmount fee_; + std::optional fee_; UniValue contextinfo_; // optional data to include in return value from getStatus() - uint256 main_impl(CWallet& wallet); + tl::expected main_impl(CWallet& wallet); }; // To test private methods, a friend class can act as a proxy @@ -72,7 +72,7 @@ public: TEST_FRIEND_AsyncRPCOperation_sendmany(std::shared_ptr ptr) : delegate(ptr) {} - uint256 main_impl(CWallet& wallet) { + tl::expected main_impl(CWallet& wallet) { return delegate->main_impl(wallet); } diff --git a/src/wallet/gtest/test_rpc_wallet.cpp b/src/wallet/gtest/test_rpc_wallet.cpp index 716654c31..5bcef4ad2 100644 --- a/src/wallet/gtest/test_rpc_wallet.cpp +++ b/src/wallet/gtest/test_rpc_wallet.cpp @@ -296,7 +296,7 @@ TEST(WalletRPCTests, RPCZsendmanyTaddrToSapling) TransparentCoinbasePolicy::Disallow, false).value(); std::vector recipients = { Payment(pa, 1*COIN, Memo::FromHexOrThrow("ABCD")) }; - std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 0, 0, strategy)); + std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 0, 0, strategy, std::nullopt)); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); // Enable test mode so tx is not sent diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0d2e91a6c..d75ce25ca 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4808,7 +4808,8 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) " 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" + "4. fee (numeric, optional, default=-1) 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" "5. 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" @@ -5010,32 +5011,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) // Minimum confirmations int nMinDepth = parseMinconf(DEFAULT_NOTE_CONFIRMATIONS, params, 2, std::nullopt); - // Fee in Zatoshis, not currency format) - CAmount nFee = DEFAULT_FEE; - if (params.size() > 3) { - if (params[3].get_real() == 0.0) { - nFee = 0; - } else { - nFee = AmountFromValue( params[3] ); - } - - // Check that the user specified fee is not absurd. - // This allows amount=0 (and all amount < DEFAULT_FEE) transactions to use the default network fee - // or anything less than DEFAULT_FEE instead of being forced to use a custom fee and leak metadata - if (nTotalOut < DEFAULT_FEE) { - if (nFee > DEFAULT_FEE) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - strprintf("Small transaction amount %s has fee %s that is greater than the default fee %s", FormatMoney(nTotalOut), FormatMoney(nFee), FormatMoney(DEFAULT_FEE))); - } - } else { - // Check that the user specified fee is not absurd. - if (nFee > nTotalOut) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - strprintf("Fee %s is greater than the sum of outputs %s and also greater than the default fee", FormatMoney(nFee), FormatMoney(nTotalOut))); - } - } + std::optional nFee; + if (params.size() > 3 && params[3].get_real() != -1.0) { + nFee = AmountFromValue( params[3] ); } // Use input parameters as the optional context info to be returned by z_getoperationstatus and z_getoperationresult. @@ -5043,7 +5021,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) o.pushKV("fromaddress", params[0]); o.pushKV("amounts", params[1]); o.pushKV("minconf", nMinDepth); - o.pushKV("fee", std::stod(FormatMoney(nFee))); + if (nFee.has_value()) { + o.pushKV("fee", ValueFromAmount(nFee.value())); + } UniValue contextInfo = o; // Create operation and add to global queue diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 58583c556..cd4b4454a 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -1251,7 +1251,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) WalletTxBuilder builder(Params(), minRelayTxFee); std::vector recipients = { Payment(zaddr1, 100*COIN, Memo::FromHexOrThrow("DEADBEEF")) }; TransactionStrategy strategy(PrivacyPolicy::AllowRevealedSenders); - std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy)); + std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt)); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); @@ -1268,7 +1268,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) WalletTxBuilder builder(Params(), minRelayTxFee); std::vector recipients = { Payment(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)); + std::shared_ptr operation(new AsyncRPCOperation_sendmany(std::move(builder), selector, recipients, 1, 1, strategy, std::nullopt)); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index 28605b137..1576d1ba2 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -3,6 +3,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php . #include "wallet/wallet_tx_builder.h" +#include "zip317.h" using namespace libzcash; @@ -12,153 +13,178 @@ int GetAnchorHeight(const CChain& chain, uint32_t anchorConfirmations) return nextBlockHeight - anchorConfirmations; } -PrepareTransactionResult WalletTxBuilder::PrepareTransaction( +tl::expected +WalletTxBuilder::GetChangeAddress( + CWallet& wallet, + const ZTXOSelector& selector, + SpendableInputs& spendable, + const Payments& resolvedPayments, + const TransactionStrategy& strategy, + bool afterNU5) const +{ + // Determine the account we're sending from. + auto sendFromAccount = wallet.FindAccountForSelector(selector).value_or(ZCASH_LEGACY_ACCOUNT); + + auto getAllowedChangePools = [&](const std::set& receiverTypes) { + std::set result{resolvedPayments.GetRecipientPools()}; + // We always allow shielded change when not sending from the legacy account. + if (sendFromAccount != ZCASH_LEGACY_ACCOUNT) { + result.insert(OutputPool::Sapling); + } + for (ReceiverType rtype : receiverTypes) { + switch (rtype) { + case ReceiverType::P2PKH: + case ReceiverType::P2SH: + if (strategy.AllowRevealedRecipients()) { + result.insert(OutputPool::Transparent); + } + break; + case ReceiverType::Sapling: + if (!spendable.saplingNoteEntries.empty() || strategy.AllowRevealedAmounts()) { + result.insert(OutputPool::Sapling); + } + break; + case ReceiverType::Orchard: + if (afterNU5 + && (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts())) { + result.insert(OutputPool::Orchard); + } + break; + } + } + return result; + }; + + auto changeAddressForTransparentSelector = [&](const std::set& receiverTypes) + -> tl::expected { + auto addr = wallet.GenerateChangeAddressForAccount( + sendFromAccount, + getAllowedChangePools(receiverTypes)); + if (addr.has_value()) { + return {addr.value()}; + } else { + return tl::make_unexpected(AddressResolutionError::TransparentChangeNotAllowed); + } + }; + + auto changeAddressForSaplingAddress = [&](const libzcash::SaplingPaymentAddress& addr) + -> RecipientAddress { + // for Sapling, if using a legacy address, return change to the + // originating address; otherwise return it to the Sapling internal + // address corresponding to the UFVK. + if (sendFromAccount == ZCASH_LEGACY_ACCOUNT) { + return addr; + } else { + auto addr = wallet.GenerateChangeAddressForAccount( + sendFromAccount, + getAllowedChangePools({ReceiverType::Sapling})); + assert(addr.has_value()); + return addr.value(); + } + }; + + auto changeAddressForZUFVK = [&]( + const ZcashdUnifiedFullViewingKey& zufvk, + const std::set& receiverTypes) { + auto addr = zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes)); + assert(addr.has_value()); + return addr.value(); + }; + + return examine(selector.GetPattern(), match { + [&](const CKeyID&) { + return changeAddressForTransparentSelector({ReceiverType::P2PKH}); + }, + [&](const CScriptID&) { + return changeAddressForTransparentSelector({ReceiverType::P2SH}); + }, + [](const libzcash::SproutPaymentAddress& addr) + -> tl::expected { + // for Sprout, we return change to the originating address using the tx builder. + return addr; + }, + [](const libzcash::SproutViewingKey& vk) + -> tl::expected { + // for Sprout, we return change to the originating address using the tx builder. + return vk.address(); + }, + [&](const libzcash::SaplingPaymentAddress& addr) + -> tl::expected { + return changeAddressForSaplingAddress(addr); + }, + [&](const libzcash::SaplingExtendedFullViewingKey& fvk) + -> tl::expected { + return changeAddressForSaplingAddress(fvk.DefaultAddress()); + }, + [&](const libzcash::UnifiedAddress& ua) + -> tl::expected { + auto zufvk = wallet.GetUFVKForAddress(ua); + assert(zufvk.has_value()); + return changeAddressForZUFVK(zufvk.value(), ua.GetKnownReceiverTypes()); + }, + [&](const libzcash::UnifiedFullViewingKey& fvk) + -> tl::expected { + return changeAddressForZUFVK( + ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk), + fvk.GetKnownReceiverTypes()); + }, + [&](const AccountZTXOPattern& acct) -> tl::expected { + auto addr = wallet.GenerateChangeAddressForAccount( + acct.GetAccountId(), + getAllowedChangePools(acct.GetReceiverTypes())); + assert(addr.has_value()); + return addr.value(); + } + }); +} + +tl::expected +WalletTxBuilder::PrepareTransaction( CWallet& wallet, const ZTXOSelector& selector, SpendableInputs& spendable, const std::vector& payments, const CChain& chain, TransactionStrategy strategy, - CAmount fee, + std::optional fee, uint32_t anchorConfirmations) const { assert(fee < MAX_MONEY); int anchorHeight = GetAnchorHeight(chain, anchorConfirmations); - auto selected = ResolveInputsAndPayments(wallet, selector, spendable, payments, chain, strategy, fee, anchorHeight); - if (std::holds_alternative(selected)) { - return std::get(selected); - } + bool afterNU5 = params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5); + auto selected = ResolveInputsAndPayments(wallet, selector, spendable, payments, chain, strategy, fee, afterNU5); + return selected.map([&](const InputSelection& resolvedSelection) { + auto ovks = SelectOVKs(wallet, selector, spendable); - auto resolvedSelection = std::get(selected); - auto resolvedPayments = resolvedSelection.GetPayments(); - - // We do not set a change address if there is no change. - std::optional changeAddr; - auto changeAmount = spendable.Total() - resolvedPayments.Total() - fee; - if (changeAmount > 0) { - // Determine the account we're sending from. - auto sendFromAccount = wallet.FindAccountForSelector(selector).value_or(ZCASH_LEGACY_ACCOUNT); - - auto getAllowedChangePools = [&](const std::set& receiverTypes) { - std::set result{resolvedPayments.GetRecipientPools()}; - // We always allow shielded change when not sending from the legacy account. - if (sendFromAccount != ZCASH_LEGACY_ACCOUNT) { - result.insert(OutputPool::Sapling); - } - for (ReceiverType rtype : receiverTypes) { - switch (rtype) { - case ReceiverType::P2PKH: - case ReceiverType::P2SH: - if (strategy.AllowRevealedRecipients()) { - result.insert(OutputPool::Transparent); - } - break; - case ReceiverType::Sapling: - if (!spendable.saplingNoteEntries.empty() || strategy.AllowRevealedAmounts()) { - result.insert(OutputPool::Sapling); - } - break; - case ReceiverType::Orchard: - if (params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5) - && (!spendable.orchardNoteMetadata.empty() || strategy.AllowRevealedAmounts())) { - result.insert(OutputPool::Orchard); - } - break; - } - } - return result; - }; - - auto addChangePayment = [&](const std::optional& sendTo) { - assert(sendTo.has_value()); - resolvedPayments.AddPayment( - ResolvedPayment(std::nullopt, sendTo.value(), changeAmount, std::nullopt, true)); - return sendTo.value(); - }; - - auto changeAddressForTransparentSelector = [&](const std::set& receiverTypes) { - return addChangePayment( - wallet.GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools(receiverTypes))); - }; - - auto changeAddressForSaplingAddress = [&](const libzcash::SaplingPaymentAddress& addr) { - // for Sapling, if using a legacy address, return change to the - // originating address; otherwise return it to the Sapling internal - // address corresponding to the UFVK. - return addChangePayment( - sendFromAccount == ZCASH_LEGACY_ACCOUNT - ? addr - : wallet.GenerateChangeAddressForAccount( - sendFromAccount, - getAllowedChangePools({ReceiverType::Sapling}))); - }; - - auto changeAddressForZUFVK = [&]( - const ZcashdUnifiedFullViewingKey& zufvk, - const std::set& receiverTypes) { - return addChangePayment(zufvk.GetChangeAddress(getAllowedChangePools(receiverTypes))); - }; - - changeAddr = examine(selector.GetPattern(), match { - [&](const CKeyID&) -> ChangeAddress { - return changeAddressForTransparentSelector({ReceiverType::P2PKH}); - }, - [&](const CScriptID&) -> ChangeAddress { - return changeAddressForTransparentSelector({ReceiverType::P2SH}); - }, - [](const libzcash::SproutPaymentAddress& addr) -> ChangeAddress { - // for Sprout, we return change to the originating address using the tx builder. - return addr; - }, - [](const libzcash::SproutViewingKey& vk) -> ChangeAddress { - // for Sprout, we return change to the originating address using the tx builder. - return vk.address(); - }, - [&](const libzcash::SaplingPaymentAddress& addr) -> ChangeAddress { - return changeAddressForSaplingAddress(addr); - }, - [&](const libzcash::SaplingExtendedFullViewingKey& fvk) -> ChangeAddress { - return changeAddressForSaplingAddress(fvk.DefaultAddress()); - }, - [&](const libzcash::UnifiedAddress& ua) -> ChangeAddress { - auto zufvk = wallet.GetUFVKForAddress(ua); - assert(zufvk.has_value()); - return changeAddressForZUFVK(zufvk.value(), ua.GetKnownReceiverTypes()); - }, - [&](const libzcash::UnifiedFullViewingKey& fvk) -> ChangeAddress { - return changeAddressForZUFVK( - ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(params, fvk), - fvk.GetKnownReceiverTypes()); - }, - [&](const AccountZTXOPattern& acct) -> ChangeAddress { - return addChangePayment( - wallet.GenerateChangeAddressForAccount( - acct.GetAccountId(), - getAllowedChangePools(acct.GetReceiverTypes()))); - } - }); - } - - auto ovks = SelectOVKs(wallet, selector, spendable); - - auto effects = TransactionEffects( - anchorConfirmations, - spendable, - resolvedPayments, - changeAddr, - fee, - ovks.first, - ovks.second, - anchorHeight); - effects.LockSpendable(wallet); - return effects; + auto effects = TransactionEffects( + anchorConfirmations, + resolvedSelection.GetInputs(), + resolvedSelection.GetPayments(), + resolvedSelection.GetChangeAddress(), + resolvedSelection.GetFee(), + ovks.first, + ovks.second, + anchorHeight); + effects.LockSpendable(wallet); + return effects; + }); } -Payments InputSelection::GetPayments() const { - return this->payments; +const SpendableInputs& InputSelection::GetInputs() const { + return inputs; +} + +const Payments& InputSelection::GetPayments() const { + return payments; +} + +CAmount InputSelection::GetFee() const { + return fee; +} + +const std::optional InputSelection::GetChangeAddress() const { + return changeAddr; } CAmount WalletTxBuilder::DefaultDustThreshold() const { @@ -177,25 +203,248 @@ SpendableInputs WalletTxBuilder::FindAllSpendableInputs( return wallet.FindSpendableInputs(selector, minDepth, std::nullopt); } -InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments( - const CWallet& wallet, +static size_t PadCount(size_t n) +{ + return n == 1 ? 2 : n; +} + +static CAmount +CalcZIP317Fee( + const std::optional& inputs, + const std::vector& payments, + const std::optional& changeAddr) +{ + std::vector vout{}; + size_t sproutOutputCount{}, saplingOutputCount{}, orchardOutputCount{}; + for (const auto& payment : payments) { + std::visit(match { + [&](const CKeyID& addr) { + vout.emplace_back(payment.amount, GetScriptForDestination(addr)); + }, + [&](const CScriptID& addr) { + vout.emplace_back(payment.amount, GetScriptForDestination(addr)); + }, + [&](const libzcash::SaplingPaymentAddress&) { + ++saplingOutputCount; + }, + [&](const libzcash::OrchardRawAddress&) { + ++orchardOutputCount; + } + }, payment.address); + } + + if (changeAddr.has_value()) { + examine(changeAddr.value(), match { + [&](const SproutPaymentAddress&) { ++sproutOutputCount; }, + [&](const RecipientAddress& addr) { + examine(addr, match { + [&](const CKeyID& taddr) { + vout.emplace_back(0, GetScriptForDestination(taddr)); + }, + [&](const CScriptID taddr) { + vout.emplace_back(0, GetScriptForDestination(taddr)); + }, + [&](const libzcash::SaplingPaymentAddress&) { ++saplingOutputCount; }, + [&](const libzcash::OrchardRawAddress&) { ++orchardOutputCount; } + }); + } + }); + } + + std::vector vin{}; + size_t sproutInputCount = 0; + size_t saplingInputCount = 0; + size_t orchardInputCount = 0; + if (inputs.has_value()) { + for (const auto& utxo : inputs.value().utxos) { + vin.emplace_back( + COutPoint(utxo.tx->GetHash(), utxo.i), + utxo.tx->vout[utxo.i].scriptPubKey); + } + sproutInputCount = inputs.value().sproutNoteEntries.size(); + saplingInputCount = inputs.value().saplingNoteEntries.size(); + orchardInputCount = inputs.value().orchardNoteMetadata.size(); + } + size_t logicalActionCount = CalculateLogicalActionCount( + vin, + vout, + std::max(sproutInputCount, sproutOutputCount), + saplingInputCount, + PadCount(saplingOutputCount), + PadCount(std::max(orchardInputCount, orchardOutputCount))); + + return CalculateConventionalFee(logicalActionCount); +} + +InvalidFundsError ReportInvalidFunds( + const SpendableInputs& spendable, + bool hasPhantomChange, + CAmount fee, + CAmount dustThreshold, + CAmount targetAmount, + CAmount changeAmount) +{ + return InvalidFundsError( + spendable.Total(), + hasPhantomChange + // TODO: NEED TESTS TO EXERCISE THIS + ? InvalidFundsReason(PhantomChangeError(fee, dustThreshold)) + : (changeAmount > 0 && changeAmount < dustThreshold + // TODO: we should provide the option for the caller to explicitly forego change + // (definitionally an amount below the dust amount) and send the extra to the + // recipient or the miner fee to avoid creating dust change, rather than prohibit + // them from sending entirely in this circumstance. (Daira disagrees, as this could + // leak information to the recipient or publicly in the fee.) + ? InvalidFundsReason(DustThresholdError(dustThreshold, changeAmount)) + : InvalidFundsReason(InsufficientFundsError(targetAmount)))); +} + +static tl::expected +AddChangePayment( + const SpendableInputs& spendable, + Payments& resolvedPayments, + const ChangeAddress& changeAddr, + CAmount changeAmount, + CAmount targetAmount) +{ + assert(changeAmount > 0); + + // When spending transparent coinbase outputs, all inputs must be fully consumed. + if (spendable.HasTransparentCoinbase()) { + return tl::make_unexpected(ChangeNotAllowedError(spendable.Total(), targetAmount)); + } + + examine(changeAddr, match { + // TODO: Once we can add Sprout change to `resolvedPayments`, we don’t need to pass + // `changeAddr` around the rest of these functions. + [](const libzcash::SproutPaymentAddress&) {}, + [](const libzcash::SproutViewingKey&) {}, + [&](const auto& sendTo) { + resolvedPayments.AddPayment( + ResolvedPayment(std::nullopt, sendTo, changeAmount, std::nullopt, true)); + } + }); + + return {}; +} + +/// On the initial call, we haven’t yet selected inputs, so we assume the outputs dominate the +/// actions. +/// +/// 1. calc fee using only resolvedPayments to set a lower bound on the actual fee +/// • this also needs to know which pool change is going to, so it can determine what the fee is +/// with change _if_ there is change +/// 2. iterate over LimitToAmount until the updated fee (now including spends) matches the expected +/// fee +tl::expected< + std::tuple>, + InputSelectionError> +WalletTxBuilder::IterateLimit( + CWallet& wallet, + const ZTXOSelector& selector, + const TransactionStrategy strategy, + CAmount sendAmount, + CAmount dustThreshold, + const SpendableInputs& spendable, + Payments& resolved, + bool afterNU5) const +{ + SpendableInputs spendableMut; + + auto previousFee = MINIMUM_FEE; + auto updatedFee = CalcZIP317Fee(std::nullopt, resolved.GetResolvedPayments(), std::nullopt); + // This is used to increase the target amount just enough (generally by 0 or 1) to force + // selection of additional notes. + CAmount bumpTargetAmount{0}; + std::optional changeAddr; + CAmount changeAmount{0}; + CAmount targetAmount{0}; + + do { + // NB: This makes a fresh copy so that we start from the full set of notes when we re-limit. + spendableMut = spendable; + + targetAmount = sendAmount + updatedFee; + + // TODO: the set of recipient pools is not quite sufficient information here; we should + // probably perform note selection at the same time as we're performing resolved payment + // construction above. + bool foundSufficientFunds = + spendableMut.LimitToAmount( + targetAmount + bumpTargetAmount, + dustThreshold, + resolved.GetRecipientPools()); + changeAmount = spendableMut.Total() - targetAmount; + if (foundSufficientFunds) { + // Don’t want to generate a change address if we don’t need one (because it could be + // fresh) and once we generate it, hold onto it. But we still don’t have a guarantee + // that we won’t end up discarding it. + if (changeAmount > 0 && !changeAddr.has_value()) { + auto maybeChangeAddr = GetChangeAddress( + wallet, + selector, + spendableMut, + resolved, + strategy, + afterNU5); + + if (maybeChangeAddr.has_value()) { + changeAddr = maybeChangeAddr.value(); + } else { + return tl::make_unexpected(maybeChangeAddr.error()); + } + } + previousFee = updatedFee; + updatedFee = CalcZIP317Fee( + spendableMut, + resolved.GetResolvedPayments(), + changeAmount > 0 ? changeAddr : std::nullopt); + } else { + return tl::make_unexpected( + ReportInvalidFunds( + spendableMut, + bumpTargetAmount != 0, + previousFee, + dustThreshold, + targetAmount, + changeAmount)); + } + // This happens when we have exactly `MARGINAL_FEE` change, then add a change output that + // causes the conventional fee to consume that change, leaving us with no change, which then + // lowers the fee. + if (updatedFee < previousFee) { + // Bump the updated fee so that we don’t exit the loop, but should force us to take an + // extra note (or fail) in the next `LimitToAmount`. + bumpTargetAmount = 1; + } + } while (updatedFee != previousFee); + + if (changeAmount > 0) { + assert(changeAddr.has_value()); + + auto changeRes = + AddChangePayment(spendableMut, resolved, changeAddr.value(), changeAmount, targetAmount); + if (!changeRes.has_value()) { + return tl::make_unexpected(changeRes.error()); + } + } + + return std::make_tuple(spendableMut, updatedFee, changeAddr); +} + +tl::expected +WalletTxBuilder::ResolveInputsAndPayments( + CWallet& wallet, const ZTXOSelector& selector, SpendableInputs& spendableMut, const std::vector& payments, const CChain& chain, - TransactionStrategy strategy, - CAmount fee, - int anchorHeight) const + const TransactionStrategy& strategy, + std::optional fee, + bool afterNU5) const { LOCK2(cs_main, wallet.cs_wallet); - // Determine the target totals - CAmount sendAmount{0}; - for (const auto& payment : payments) { - sendAmount += payment.GetAmount(); - } - CAmount targetAmount = sendAmount + fee; - // This is a simple greedy algorithm to attempt to preserve requested // transactional privacy while moving as much value to the most recent pool // as possible. This will also perform opportunistic shielding if the @@ -205,11 +454,9 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments( CAmount maxOrchardAvailable = spendableMut.GetOrchardTotal(); uint32_t orchardOutputs{0}; - // we can only select Orchard addresses if there are sufficient non-Sprout - // funds to cover the total payments + fee. - bool canResolveOrchard = - params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5) - && spendableMut.Total() - spendableMut.GetSproutTotal() >= targetAmount; + // we can only select Orchard addresses if we’re not sending from Sprout, since there is no tx + // version where both Sprout and Orchard are valid. + bool canResolveOrchard = afterNU5 && !selector.SelectsSprout(); std::vector resolvedPayments; std::optional resolutionError; for (const auto& payment : payments) { @@ -289,59 +536,113 @@ InputSelectionResult WalletTxBuilder::ResolveInputsAndPayments( }); if (resolutionError.has_value()) { - return resolutionError.value(); + return tl::make_unexpected(resolutionError.value()); } } auto resolved = Payments(resolvedPayments); if (orchardOutputs > this->maxOrchardActions) { - return ExcessOrchardActionsError( - ActionSide::Output, - orchardOutputs, - this->maxOrchardActions); + return tl::make_unexpected( + ExcessOrchardActionsError( + ActionSide::Output, + orchardOutputs, + this->maxOrchardActions)); } // Set the dust threshold so that we can select enough inputs to avoid // creating dust change amounts. CAmount dustThreshold{this->DefaultDustThreshold()}; - // TODO: the set of recipient pools is not quite sufficient information here; we should - // probably perform note selection at the same time as we're performing resolved payment - // construction above. - if (!spendableMut.LimitToAmount(targetAmount, dustThreshold, resolved.GetRecipientPools())) { - CAmount changeAmount{spendableMut.Total() - targetAmount}; - return InvalidFundsError( - spendableMut.Total(), - changeAmount > 0 && changeAmount < dustThreshold - // TODO: we should provide the option for the caller to explicitly - // forego change (definitionally an amount below the dust amount) - // and send the extra to the recipient or the miner fee to avoid - // creating dust change, rather than prohibit them from sending - // entirely in this circumstance. - // (Daira disagrees, as this could leak information to the recipient - // or publicly in the fee.) - ? InvalidFundsReason(DustThresholdError(dustThreshold, changeAmount)) - : InvalidFundsReason(InsufficientFundsError(targetAmount))); + // Determine the target totals + CAmount sendAmount{0}; + for (const auto& payment : payments) { + sendAmount += payment.GetAmount(); } - // When spending transparent coinbase outputs, all inputs must be fully - // consumed, and they may only be sent to shielded recipients. - if (spendableMut.HasTransparentCoinbase()) { - if (spendableMut.Total() != targetAmount) { - return ChangeNotAllowedError(spendableMut.Total(), targetAmount); - } else if (resolved.HasTransparentRecipient()) { - return AddressResolutionError::TransparentRecipientNotAllowed; + CAmount finalFee; + CAmount targetAmount; + std::optional changeAddr; + if (fee.has_value()) { + finalFee = fee.value(); + targetAmount = sendAmount + finalFee; + // TODO: the set of recipient pools is not quite sufficient information here; we should + // probably perform note selection at the same time as we're performing resolved payment + // construction above. + bool foundSufficientFunds = spendableMut.LimitToAmount( + targetAmount, + dustThreshold, + resolved.GetRecipientPools()); + CAmount changeAmount{spendableMut.Total() - targetAmount}; + if (!foundSufficientFunds) { + return tl::make_unexpected( + ReportInvalidFunds( + spendableMut, + false, + finalFee, + dustThreshold, + targetAmount, + changeAmount)); + } + if (changeAmount > 0) { + auto maybeChangeAddr = GetChangeAddress( + wallet, + selector, + spendableMut, + resolved, + strategy, + afterNU5); + + if (maybeChangeAddr.has_value()) { + changeAddr = maybeChangeAddr.value(); + } else { + return tl::make_unexpected(maybeChangeAddr.error()); + } + + // TODO: This duplicates the check in the `else` branch of the containing `if`. Until we + // can add Sprout change to `Payments` (#5660), we need to check this before + // adding the change payment. We can remove this check and make the later one + // unconditional once that’s fixed. + auto conventionalFee = + CalcZIP317Fee(spendableMut, resolved.GetResolvedPayments(), changeAddr); + if (finalFee > WEIGHT_RATIO_CAP * conventionalFee) { + return tl::make_unexpected(AbsurdFeeError(conventionalFee, finalFee)); + } + auto changeRes = + AddChangePayment(spendableMut, resolved, changeAddr.value(), changeAmount, targetAmount); + if (!changeRes.has_value()) { + return tl::make_unexpected(changeRes.error()); + } + } else { + auto conventionalFee = + CalcZIP317Fee(spendableMut, resolved.GetResolvedPayments(), std::nullopt); + if (finalFee > WEIGHT_RATIO_CAP * conventionalFee) { + return tl::make_unexpected(AbsurdFeeError(resolved.Total(), finalFee)); + } + } + } else { + auto limitResult = IterateLimit(wallet, selector, strategy, sendAmount, dustThreshold, spendableMut, resolved, afterNU5); + if (limitResult.has_value()) { + std::tie(spendableMut, finalFee, changeAddr) = limitResult.value(); + targetAmount = sendAmount + finalFee; + } else { + return tl::make_unexpected(limitResult.error()); } } - if (spendableMut.orchardNoteMetadata.size() > this->maxOrchardActions) { - return ExcessOrchardActionsError( - ActionSide::Input, - spendableMut.orchardNoteMetadata.size(), - this->maxOrchardActions); + // When spending transparent coinbase outputs they may only be sent to shielded recipients. + if (spendableMut.HasTransparentCoinbase() && resolved.HasTransparentRecipient()) { + return tl::make_unexpected(AddressResolutionError::TransparentRecipientNotAllowed); } - return InputSelection(resolved, anchorHeight); + if (spendableMut.orchardNoteMetadata.size() > this->maxOrchardActions) { + return tl::make_unexpected( + ExcessOrchardActionsError( + ActionSide::Input, + spendableMut.orchardNoteMetadata.size(), + this->maxOrchardActions)); + } + + return InputSelection(spendableMut, resolved, finalFee, changeAddr); } std::pair @@ -625,7 +926,7 @@ TransactionBuilderResult TransactionEffects::ApproveAndBuild( // TODO: We currently can’t store Sprout change in `Payments`, so we only validate the // spend/output balance in the case that `TransactionBuilder` doesn’t need to // (re)calculate the change. In future, we shouldn’t rely on `TransactionBuilder` ever - // calculating change. + // calculating change. (#5660) if (changeAddr.has_value()) { examine(changeAddr.value(), match { [&](const SproutPaymentAddress& addr) { diff --git a/src/wallet/wallet_tx_builder.h b/src/wallet/wallet_tx_builder.h index cb1b5441c..d44b665bc 100644 --- a/src/wallet/wallet_tx_builder.h +++ b/src/wallet/wallet_tx_builder.h @@ -225,6 +225,8 @@ enum class AddressResolutionError { SproutRecipientsNotSupported, //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients` TransparentRecipientNotAllowed, + //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedRecipients` + TransparentChangeNotAllowed, //! Requested `PrivacyPolicy` doesn’t include `AllowRevealedAmounts`, but we don’t have enough //! Sapling funds to avoid revealing amounts RevealingSaplingAmountNotAllowed, @@ -239,6 +241,19 @@ enum class AddressResolutionError { RevealingReceiverAmountsNotAllowed, }; +/// Phantom change is change that appears to exist until we add the output for it, at which point it +/// is consumed by the increase to the conventional fee. When we are at the limit of selectable +/// notes, this makes it impossible to create the transaction without either creating a 0-valued +/// output or overpaying the fee. +class PhantomChangeError { +public: + CAmount finalFee; + CAmount dustThreshold; + + PhantomChangeError(CAmount finalFee, CAmount dustThreshold): + finalFee(finalFee), dustThreshold(dustThreshold) { } +}; + class InsufficientFundsError { public: CAmount required; @@ -257,6 +272,7 @@ public: }; typedef std::variant< + PhantomChangeError, InsufficientFundsError, DustThresholdError> InvalidFundsReason; @@ -278,6 +294,17 @@ public: available(available), required(required) { } }; +/// Error when a fee is higher than can be useful. This reduces the chance of accidentally +/// overpaying with explicit fees. +class AbsurdFeeError { +public: + CAmount conventionalFee; + CAmount fixedFee; + + AbsurdFeeError(CAmount conventionalFee, CAmount fixedFee): + conventionalFee(conventionalFee), fixedFee(fixedFee) { } +}; + enum ActionSide { Input, Output, @@ -298,28 +325,26 @@ typedef std::variant< AddressResolutionError, InvalidFundsError, ChangeNotAllowedError, + AbsurdFeeError, ExcessOrchardActionsError> InputSelectionError; class InputSelection { private: + SpendableInputs inputs; Payments payments; - int orchardAnchorHeight; + CAmount fee; + std::optional changeAddr; public: - InputSelection(Payments payments, int orchardAnchorHeight): - payments(payments), orchardAnchorHeight(orchardAnchorHeight) {} + InputSelection(SpendableInputs inputs, Payments payments, CAmount fee, std::optional changeAddr): + inputs(inputs), payments(payments), fee(fee), changeAddr(changeAddr) {} - Payments GetPayments() const; + const SpendableInputs& GetInputs() const; + const Payments& GetPayments() const; + CAmount GetFee() const; + const std::optional GetChangeAddress() const; }; -typedef std::variant< - InputSelectionError, - InputSelection> InputSelectionResult; - -typedef std::variant< - InputSelectionError, - TransactionEffects> PrepareTransactionResult; - class WalletTxBuilder { private: const CChainParams& params; @@ -331,20 +356,43 @@ private: */ CAmount DefaultDustThreshold() const; + tl::expected + GetChangeAddress( + CWallet& wallet, + const ZTXOSelector& selector, + SpendableInputs& spendable, + const Payments& resolvedPayments, + const TransactionStrategy& strategy, + bool afterNU5) const; + + tl::expected< + std::tuple>, + InputSelectionError> + IterateLimit( + CWallet& wallet, + const ZTXOSelector& selector, + const TransactionStrategy strategy, + CAmount sendAmount, + CAmount dustThreshold, + const SpendableInputs& spendable, + Payments& resolved, + bool afterNU5) const; + /** * Select inputs sufficient to fulfill the specified requested payments, * and choose unified address receivers based upon the available inputs * and the requested transaction strategy. */ - InputSelectionResult ResolveInputsAndPayments( - const CWallet& wallet, + tl::expected + ResolveInputsAndPayments( + CWallet& wallet, const ZTXOSelector& selector, SpendableInputs& spendable, const std::vector& payments, const CChain& chain, - TransactionStrategy strategy, - CAmount fee, - int anchorHeight) const; + const TransactionStrategy& strategy, + std::optional fee, + bool afterNU5) const; /** * Compute the internal and external OVKs to use in transaction construction, given * the spendable inputs. @@ -363,14 +411,16 @@ public: const ZTXOSelector& selector, int32_t minDepth) const; - PrepareTransactionResult PrepareTransaction( + tl::expected + PrepareTransaction( CWallet& wallet, const ZTXOSelector& selector, SpendableInputs& spendable, const std::vector& payments, const CChain& chain, TransactionStrategy strategy, - CAmount fee, + /// A fixed fee is used if provided, otherwise it is calculated based on ZIP 317. + std::optional fee, uint32_t anchorConfirmations) const; }; diff --git a/src/zip317.cpp b/src/zip317.cpp index e0bd1c640..b9b65e7c8 100644 --- a/src/zip317.cpp +++ b/src/zip317.cpp @@ -14,6 +14,13 @@ CAmount CalculateConventionalFee(size_t logicalActionCount) { return MARGINAL_FEE * std::max(GRACE_ACTIONS, logicalActionCount); } +template +static size_t GetTxIOFieldSize(const std::vector& txIOs) { + auto size = GetSerializeSize(txIOs, SER_NETWORK, PROTOCOL_VERSION); + auto countSize = GetSizeOfCompactSize(txIOs.size()); + return size - countSize; +} + size_t CalculateLogicalActionCount( const std::vector& vin, const std::vector& vout, @@ -21,8 +28,8 @@ size_t CalculateLogicalActionCount( unsigned int saplingSpendCount, unsigned int saplingOutputCount, unsigned int orchardActionCount) { - const size_t tx_in_total_size = GetSerializeSize(vin, SER_NETWORK, PROTOCOL_VERSION); - const size_t tx_out_total_size = GetSerializeSize(vout, SER_NETWORK, PROTOCOL_VERSION); + const size_t tx_in_total_size = GetTxIOFieldSize(vin); + const size_t tx_out_total_size = GetTxIOFieldSize(vout); return std::max(ceil_div(tx_in_total_size, P2PKH_STANDARD_INPUT_SIZE), ceil_div(tx_out_total_size, P2PKH_STANDARD_OUTPUT_SIZE)) + diff --git a/src/zip317.h b/src/zip317.h index 342b35e99..10657368c 100644 --- a/src/zip317.h +++ b/src/zip317.h @@ -23,6 +23,9 @@ static const int64_t WEIGHT_RATIO_SCALE = INT64_C(10000000000000000); static const int64_t WEIGHT_RATIO_CAP = 4; static const size_t BLOCK_UNPAID_ACTION_LIMIT = 50; +/// This is the lowest the conventional fee can be in ZIP 317. +static const CAmount MINIMUM_FEE = MARGINAL_FEE * GRACE_ACTIONS; + /// Return the conventional fee for the given `logicalActionCount` calculated according to /// . CAmount CalculateConventionalFee(size_t logicalActionCount);