Closes #2639. Adds optional limit parameter with a default value of 50.

The new parameter is to satisfy the principle of least astonishment
by providing a sensible default for the maximum number of transparent
inputs to shield.  If users do not configure -mempooltxinputlimit
it is possible for them to create transactions with hundreds of
inputs which suffer from mining delay, due to the current state of
the network where some miners have configured -mempooltxinputlimit
as a way to deal with the problem of quadratic hashing.
This commit is contained in:
Simon 2017-11-01 10:40:24 -07:00
parent c2d3bafeaa
commit c5dabd2b66
5 changed files with 65 additions and 10 deletions

View File

@ -72,7 +72,7 @@ Command | Parameters | Description
--- | --- | ---
z_listreceivedbyaddress<br> | zaddr [minconf=1] | Return a list of amounts received by a zaddr belonging to the nodes wallet.<br><br>Optionally set the minimum number of confirmations which a received amount must have in order to be included in the result. Use 0 to count unconfirmed transactions.<br><br>Output:<br>[{<br>“txid”: “4a0f…”,<br>“amount”: 0.54,<br>“memo”:”F0FF…”,}, {...}, {...}<br>]
z_sendmany<br> | fromaddress amounts [minconf=1] [fee=0.0001] | _This is an Asynchronous RPC call_<br><br>Send funds from an address to multiple outputs. The address can be either a taddr or a zaddr.<br><br>Amounts is a list containing key/value pairs corresponding to the addresses and amount to pay. Each output address can be in taddr or zaddr format.<br><br>When sending to a zaddr, you also have the option of attaching a memo in hexadecimal format.<br><br>**NOTE:**When sending coinbase funds to a zaddr, the node's wallet does not allow any change. Put another way, spending a partial amount of a coinbase utxo is not allowed. This is not a consensus rule but a local wallet rule due to the current implementation of z_sendmany. In future, this rule may be removed.<br><br>Example of Outputs parameter:<br>[{“address”:”t123…”, “amount”:0.005},<br>,{“address”:”z010…”,”amount”:0.03, “memo”:”f508af…”}]<br><br>Optionally set the minimum number of confirmations which a private or transparent transaction must have in order to be used as an input. When sending from a zaddr, minconf must be greater than zero.<br><br>Optionally set a transaction fee, which by default is 0.0001 ZEC.<br><br>Any transparent change will be sent to a new transparent address. Any private change will be sent back to the zaddr being used as the source of funds.<br><br>Returns an operationid. You use the operationid value with z_getoperationstatus and z_getoperationresult to obtain the result of sending funds, which if successful, will be a txid.
z_shieldcoinbase<br> | fromaddress toaddress [fee=0.0001] | _This is an Asynchronous RPC call_<br><br>Shield transparent coinbase funds by sending to a shielded z address. Utxos selected for shielding will be locked. If there is an error, they are unlocked. The RPC call `listlockunspent` can be used to return a list of locked utxos. The number of coinbase utxos selected for shielding is limited by both the -mempooltxinputlimit=xxx option and a consensus rule defining a maximum transaction size of 100000 bytes. <br><br>The from address is a taddr or "*" for all taddrs belonging to the wallet. The to address is a zaddr. The default fee is 0.0001.<br><br>Returns an object containing an operationid which can be used with z_getoperationstatus and z_getoperationresult, along with key-value pairs regarding how many utxos are being shielded in this trasaction and what remains to be shielded.
z_shieldcoinbase<br> | fromaddress toaddress [fee=0.0001] [limit=50] | _This is an Asynchronous RPC call_<br><br>Shield transparent coinbase funds by sending to a shielded z address. Utxos selected for shielding will be locked. If there is an error, they are unlocked. The RPC call `listlockunspent` can be used to return a list of locked utxos.<br><br>The number of coinbase utxos selected for shielding can be set with the limit parameter, which has a default value of 50. If the parameter is set to 0, the number of utxos selected is limited by the `-mempooltxinputlimit` option. Any limit is constrained by a consensus rule defining a maximum transaction size of 100000 bytes. <br><br>The from address is a taddr or "*" for all taddrs belonging to the wallet. The to address is a zaddr. The default fee is 0.0001.<br><br>Returns an object containing an operationid which can be used with z_getoperationstatus and z_getoperationresult, along with key-value pairs regarding how many utxos are being shielded in this trasaction and what remains to be shielded.
### Operations

View File

@ -114,6 +114,20 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
errorString = e.error['message']
assert_equal("Insufficient coinbase funds" in errorString, True)
# Shielding will fail because limit parameter must be at least 0
try:
self.nodes[0].z_shieldcoinbase("*", myzaddr, Decimal('0.001'), -1)
except JSONRPCException,e:
errorString = e.error['message']
assert_equal("Limit on maximum number of utxos cannot be negative" in errorString, True)
# Shielding will fail because limit parameter is absurdly large
try:
self.nodes[0].z_shieldcoinbase("*", myzaddr, Decimal('0.001'), 99999999999999)
except JSONRPCException,e:
errorString = e.error['message']
assert_equal("JSON integer out of range" in errorString, True)
# Shield coinbase utxos from node 0 of value 40, standard fee of 0.00010000
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr)
self.wait_and_assert_operationid_status(0, result['opid'])
@ -151,14 +165,15 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
# Shielding the 800 utxos will occur over two transactions, since max tx size is 100,000 bytes.
# We don't verify shieldingValue as utxos are not selected in any specific order, so value can change on each test run.
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0)
# We set an unrealistically high limit parameter of 99999, to verify that max tx size will constrain the number of utxos.
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 99999)
assert_equal(result["shieldingUTXOs"], Decimal('662'))
assert_equal(result["remainingUTXOs"], Decimal('138'))
remainingValue = result["remainingValue"]
opid1 = result['opid']
# Verify that utxos are locked (not available for selection) by queuing up another shielding operation
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr)
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 0)
assert_equal(result["shieldingValue"], Decimal(remainingValue))
assert_equal(result["shieldingUTXOs"], Decimal('138'))
assert_equal(result["remainingValue"], Decimal('0'))
@ -176,8 +191,9 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
self.sync_all()
# Verify maximum number of utxos which node 2 can shield is limited by option -mempooltxinputlimit
# This option is used when the limit parameter is set to 0.
mytaddr = self.nodes[2].getnewaddress()
result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, 0)
result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 0)
assert_equal(result["shieldingUTXOs"], Decimal('7'))
assert_equal(result["remainingUTXOs"], Decimal('13'))
self.wait_and_assert_operationid_status(2, result['opid'])
@ -185,5 +201,23 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
self.nodes[1].generate(1)
self.sync_all()
# Verify maximum number of utxos which node 0 can shield is set by default limit parameter of 50
self.nodes[0].generate(200)
self.sync_all()
mytaddr = self.nodes[0].getnewaddress()
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'))
assert_equal(result["shieldingUTXOs"], Decimal('50'))
assert_equal(result["remainingUTXOs"], Decimal('50'))
self.wait_and_assert_operationid_status(0, result['opid'])
# Verify maximum number of utxos which node 0 can shield can be set by the limit parameter
result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 33)
assert_equal(result["shieldingUTXOs"], Decimal('33'))
assert_equal(result["remainingUTXOs"], Decimal('17'))
self.wait_and_assert_operationid_status(0, result['opid'])
sync_blocks(self.nodes) # don't sync on mempool as node 2 will reject thix tx due to its mempooltxinputlimit
self.nodes[1].generate(1)
self.sync_all()
if __name__ == '__main__':
WalletShieldCoinbaseTest().main()

View File

@ -110,6 +110,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "z_sendmany", 2},
{ "z_sendmany", 3},
{ "z_shieldcoinbase", 2},
{ "z_shieldcoinbase", 3},
{ "z_getoperationstatus", 0},
{ "z_getoperationresult", 0},
{ "z_importkey", 2 },

View File

@ -1251,7 +1251,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase"), runtime_error);
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase toofewargs"), runtime_error);
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase too many args here"), runtime_error);
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase too many args shown here"), runtime_error);
// bad from address
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase "
@ -1279,6 +1279,13 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
"21000001"
), runtime_error);
// invalid limit, must be at least 0
BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase "
"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ "
"tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB "
"100 -1"
), runtime_error);
// Test constructor of AsyncRPCOperation_sendmany
std::string testnetzaddr = "ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP";
std::string mainnetzaddr = "zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U";

View File

@ -3522,18 +3522,21 @@ When estimating the number of coinbase utxos we can shield in a single transacti
*/
#define CTXIN_SPEND_P2SH_SIZE 400
#define SHIELD_COINBASE_DEFAULT_LIMIT 50
UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
{
if (!EnsureWalletIsAvailable(fHelp))
return NullUniValue;
if (fHelp || params.size() < 2 || params.size() > 3)
if (fHelp || params.size() < 2 || params.size() > 4)
throw runtime_error(
"z_shieldcoinbase \"fromaddress\" \"tozaddress\" ( fee )\n"
"z_shieldcoinbase \"fromaddress\" \"tozaddress\" ( fee ) ( limit )\n"
"\nShield transparent coinbase funds by sending to a shielded zaddr. This is an asynchronous operation and utxos"
"\nselected for shielding will be locked. If there is an error, they are unlocked. The RPC call `listlockunspent`"
"\ncan be used to return a list of locked utxos. The number of coinbase utxos selected for shielding is limited by"
"\nboth the -mempooltxinputlimit=xxx option and a consensus rule defining a maximum transaction size of "
"\ncan be used to return a list of locked utxos. The number of coinbase utxos selected for shielding can be limited"
"\nby the caller. If the limit parameter is set to zero, the -mempooltxinputlimit option will determine the number"
"\nof uxtos. Any limit is constrained by the consensus rule defining a maximum transaction size of "
+ strprintf("%d bytes.", MAX_TX_SIZE)
+ HelpRequiringPassphrase() + "\n"
"\nArguments:\n"
@ -3541,6 +3544,8 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
"2. \"toaddress\" (string, required) The address is a zaddr.\n"
"3. fee (numeric, optional, default="
+ strprintf("%s", FormatMoney(SHIELD_COINBASE_DEFAULT_MINERS_FEE)) + ") The fee amount to attach to this transaction.\n"
"4. limit (numeric, optional, default="
+ strprintf("%d", SHIELD_COINBASE_DEFAULT_LIMIT) + ") Limit on the maximum number of utxos to shield. Set to 0 to use node option -mempooltxinputlimit.\n"
"\nResult:\n"
"{\n"
" \"operationid\": xxx (string) An operationid to pass to z_getoperationstatus to get the result of the operation.\n"
@ -3583,6 +3588,14 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
}
}
int nLimit = SHIELD_COINBASE_DEFAULT_LIMIT;
if (params.size() > 3) {
nLimit = params[3].get_int();
if (nLimit < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Limit on maximum number of utxos cannot be negative");
}
}
// Prepare to get coinbase utxos
std::vector<ShieldCoinbaseUTXO> inputs;
CAmount shieldedValue = 0;
@ -3590,7 +3603,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
size_t estimatedTxSize = 2000; // 1802 joinsplit description + tx overhead + wiggle room
size_t utxoCounter = 0;
bool maxedOutFlag = false;
size_t mempoolLimit = (size_t)GetArg("-mempooltxinputlimit", 0);
size_t mempoolLimit = (nLimit != 0) ? nLimit : (size_t)GetArg("-mempooltxinputlimit", 0);
// Set of addresses to filter utxos by
set<CBitcoinAddress> setAddress = {};