Repository: zcash/zcash (language: rust)

Issue:
Use JSON null instead of -1 as the sentinel for a ZIP 317 conventional fee
This is less error-prone, since a null can't be accidentally treated as a number or have arithmetic done on it.

Since the code using the -1 sentinel was ~~only recently merged~~ in #6524, we should do this before it appears in the 5.5.0 release, to avoid needing to accept -1 as well for compatibility.


Discussion / hints:

Fix patch:
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index d75ce25ca3f..825bf2b97e7 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -4808,7 +4808,7 @@ 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=-1) The fee amount in " + CURRENCY_UNIT + " to attach to this transaction. The default behavior\n"
+            "4. 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"
             "5. privacyPolicy         (string, optional, default=\"LegacyCompat\") Policy for what information leakage is acceptable.\n"
             "                         One of the following strings:\n"
@@ -4835,6 +4835,8 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
             "\nExamples:\n"
             + HelpExampleCli("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" '[{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\", \"amount\": 5.0}]'")
             + HelpExampleCli("z_sendmany", "\"ANY_TADDR\" '[{\"address\": \"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", \"amount\": 2.0}]'")
+            + HelpExampleCli("z_sendmany", "\"ANY_TADDR\" '[{\"address\": \"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", \"amount\": 2.0}]' 1 null 'AllowFullyTransparent'")
+            + HelpExampleCli("z_sendmany", "\"ANY_TADDR\" '[{\"address\": \"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", \"amount\": 2.0}]' 1 5000")
             + HelpExampleRpc("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", [{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\", \"amount\": 5.0}]")
         );
 
@@ -5012,7 +5014,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     int nMinDepth = parseMinconf(DEFAULT_NOTE_CONFIRMATIONS, params, 2, std::nullopt);
 
     std::optional<CAmount> nFee;
-    if (params.size() > 3 && params[3].get_real() != -1.0) {
+    if (params.size() > 3 && !params[3].isNull()) {
         nFee = AmountFromValue( params[3] );
     }
 


Test patch:
diff --git a/qa/rpc-tests/test_framework/zip317.py b/qa/rpc-tests/test_framework/zip317.py
index dbe8f7e514d..1f68ab283d7 100644
--- a/qa/rpc-tests/test_framework/zip317.py
+++ b/qa/rpc-tests/test_framework/zip317.py
@@ -26,7 +26,7 @@
 
 # The zcashd RPC sentinel value to indicate the conventional_fee when a positional argument is
 # required.
-ZIP_317_FEE = -1
+ZIP_317_FEE = None
 
 def conventional_fee_zats(logical_actions):
     return MARGINAL_FEE * max(GRACE_ACTIONS, logical_actions)
diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py
index f3b9b15bee6..ab9f1fadba0 100755
--- a/qa/rpc-tests/wallet_shieldingcoinbase.py
+++ b/qa/rpc-tests/wallet_shieldingcoinbase.py
@@ -309,8 +309,7 @@ def run_test (self):
 
         # Send will fail because fee is negative
         try:
-            # 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)
+            self.nodes[0].z_sendmany(myzaddr, recipients, 1, -1)
         except JSONRPCException as e:
             errorString = e.error['message']
         assert_equal("Amount out of range" in errorString, True)