allow UA as z_shieldcoinbase destination

qa/pull-tester/rpc-tests.py

@@ -46,6 +46,8 @@ BASE_SCRIPTS= [
     'zcjoinsplit.py',
     'mergetoaddress_mixednotes.py',
     'wallet_shieldcoinbase_sapling.py',
+    'wallet_shieldcoinbase_ua_sapling.py',
+    'wallet_shieldcoinbase_ua_nu5.py',
     'turnstile.py',
     'walletbackup.py',
     'zkey_import_export.py',

qa/rpc-tests/wallet_shieldcoinbase.py

@@ -7,21 +7,24 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.authproxy import JSONRPCException
 from test_framework.util import assert_equal, initialize_chain_clean, \
     start_node, connect_nodes_bi, sync_blocks, sync_mempools, \
-    wait_and_assert_operationid_status, get_coinbase_address, DEFAULT_FEE
+    wait_and_assert_operationid_status, get_coinbase_address, DEFAULT_FEE, \
+    NU5_BRANCH_ID, nuparams
 
 from decimal import Decimal
 
 class WalletShieldCoinbaseTest (BitcoinTestFramework):
-    def __init__(self, addr_type):
-        super(WalletShieldCoinbaseTest, self).__init__()
-        self.addr_type = addr_type
-
     def setup_chain(self):
         print("Initializing test directory "+self.options.tmpdir)
         initialize_chain_clean(self.options.tmpdir, 4)
 
     def setup_network(self, split=False):
-        args = ['-regtestprotectcoinbase', '-debug=zrpcunsafe']
+        args = [
+            '-regtestprotectcoinbase',
+            '-debug=zrpcunsafe',
+            '-experimentalfeatures',
+            '-orchardwallet',
+            nuparams(NU5_BRANCH_ID, self.nu5_activation),
+        ]
         self.nodes = []
         self.nodes.append(start_node(0, self.options.tmpdir, args))
         self.nodes.append(start_node(1, self.options.tmpdir, args))
@@ -52,11 +55,13 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
         assert_equal(self.nodes[1].getbalance(), 10)
         assert_equal(self.nodes[2].getbalance(), 30)
 
+        # create one zaddr that is the target of all shielding
+        myzaddr = self.test_init_zaddr(self.nodes[0])
+
         do_not_shield_taddr = get_coinbase_address(self.nodes[0], 1)
 
         # Prepare to send taddr->zaddr
         mytaddr = get_coinbase_address(self.nodes[0], 4)
-        myzaddr = self.nodes[0].z_getnewaddress(self.addr_type)
 
         # Shielding will fail when trying to spend from watch-only address
         self.nodes[2].importaddress(mytaddr)
@@ -111,7 +116,7 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
         # Confirm balances and that do_not_shield_taddr containing funds of 10 was left alone
         assert_equal(self.nodes[0].getbalance(), 10)
         assert_equal(self.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('10.0'))
-        assert_equal(self.nodes[0].z_getbalance(myzaddr), Decimal('40.0') - DEFAULT_FEE)
+        self.test_check_balance_zaddr(self.nodes[0], Decimal('40.0') - DEFAULT_FEE)
         assert_equal(self.nodes[1].getbalance(), 20)
         assert_equal(self.nodes[2].getbalance(), 30)
 
@@ -123,7 +128,7 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
         self.sync_all()
 
         assert_equal(self.nodes[0].getbalance(), 10)
-        assert_equal(self.nodes[0].z_getbalance(myzaddr), Decimal('70.0') - DEFAULT_FEE)
+        self.test_check_balance_zaddr(self.nodes[0], Decimal('70.0') - DEFAULT_FEE)
         assert_equal(self.nodes[1].getbalance(), 30)
         assert_equal(self.nodes[2].getbalance(), 0)
 
@@ -185,3 +190,6 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework):
         sync_mempools(self.nodes[:2])
         self.nodes[1].generate(1)
         self.sync_all()
+
+# Note, no "if __name__ == '__main__" and call the test here; it's called from
+# pool-specific derived classes in wallet_shieldcoinbase_*.py

qa/rpc-tests/wallet_shieldcoinbase_sapling.py

@@ -1,10 +1,22 @@
 #!/usr/bin/env python3
 
 from wallet_shieldcoinbase import WalletShieldCoinbaseTest
+from test_framework.util import assert_equal
 
 class WalletShieldCoinbaseSapling(WalletShieldCoinbaseTest):
     def __init__(self):
-        super(WalletShieldCoinbaseSapling, self).__init__('sapling')
+        super(WalletShieldCoinbaseSapling, self).__init__()
+        self.nu5_activation = 99999
+
+    def test_init_zaddr(self, node):
+        self.addr = node.z_getnewaddress('sapling')
+        return self.addr
+
+    def test_check_balance_zaddr(self, node, expected):
+        balance = node.z_getbalance(self.addr)
+        assert_equal(balance, expected)
+
 
 if __name__ == '__main__':
+    print("Test shielding to a sapling address")
     WalletShieldCoinbaseSapling().main()

qa/rpc-tests/wallet_shieldcoinbase_sprout.py

@@ -1,10 +1,22 @@
 #!/usr/bin/env python3
 
 from wallet_shieldcoinbase import WalletShieldCoinbaseTest
+from test_framework.util import assert_equal
 
 class WalletShieldCoinbaseSprout(WalletShieldCoinbaseTest):
     def __init__(self):
-        super(WalletShieldCoinbaseSprout, self).__init__('sprout')
+        super(WalletShieldCoinbaseSprout, self).__init__()
+        self.nu5_activation = 99999
+
+    def test_init_zaddr(self, node):
+        self.addr = node.z_getnewaddress('sprout')
+        return self.addr
+
+    def test_check_balance_zaddr(self, node, expected):
+        balance = node.z_getbalance(self.addr)
+        assert_equal(balance, expected)
+
 
 if __name__ == '__main__':
+    print("Test shielding to a sapling address")
     WalletShieldCoinbaseSprout().main()

qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py

@@ -0,0 +1,32 @@
+#!/usr/bin/env python3
+
+from wallet_shieldcoinbase import WalletShieldCoinbaseTest
+from test_framework.util import assert_equal
+from test_framework.mininode import COIN
+
+class WalletShieldCoinbaseUANU5(WalletShieldCoinbaseTest):
+    def __init__(self):
+        super(WalletShieldCoinbaseUANU5, self).__init__()
+        self.account = None
+        # activate after initial setup, before the first z_shieldcoinbase RPC
+        self.nu5_activation = 109
+
+    def test_init_zaddr(self, node):
+        # this function may be called no more than once
+        assert(self.account is None)
+        self.account = node.z_getnewaccount()['account']
+        self.addr = node.z_getaddressforaccount(self.account)['unifiedaddress']
+        return self.addr
+
+    def test_check_balance_zaddr(self, node, expected):
+        balances = node.z_getbalanceforaccount(self.account)
+        assert('transparent' not in balances['pools'])
+        assert('sprout' not in balances['pools'])
+        # assert('sapling' not in balances['pools'])
+        assert_equal(balances['pools']['sapling']['valueZat'], expected * COIN)
+        # assert_equal(balances['pools']['orchard']['valueZat'], expected * COIN)
+
+
+if __name__ == '__main__':
+    print("Test shielding to a unified address with NU5 activated")
+    WalletShieldCoinbaseUANU5().main()

qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py

@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+
+from wallet_shieldcoinbase import WalletShieldCoinbaseTest
+from test_framework.util import assert_equal
+from test_framework.mininode import COIN
+
+class WalletShieldCoinbaseUASapling(WalletShieldCoinbaseTest):
+    def __init__(self):
+        super(WalletShieldCoinbaseUASapling, self).__init__()
+        self.account = None
+        self.nu5_activation = 99999
+
+    def test_init_zaddr(self, node):
+        # this function may be called no more than once
+        assert(self.account is None)
+        self.account = node.z_getnewaccount()['account']
+        self.addr = node.z_getaddressforaccount(self.account)['unifiedaddress']
+        return self.addr
+
+    def test_check_balance_zaddr(self, node, expected):
+        balances = node.z_getbalanceforaccount(self.account)
+        assert('transparent' not in balances['pools'])
+        assert('sprout' not in balances['pools'])
+        assert_equal(balances['pools']['sapling']['valueZat'], expected * COIN)
+        assert('orchard' not in balances['pools'])
+
+
+if __name__ == '__main__':
+    print("Test shielding to a unified address with sapling activated (but not NU5)")
+    WalletShieldCoinbaseUASapling().main()

src/rpc/client.cpp

@@ -121,6 +121,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "z_getaddressforaccount", 2},
     { "z_getbalance", 1},
     { "z_getbalance", 2},
+    { "z_getbalanceforaccount", 0},
     { "z_gettotalbalance", 0},
     { "z_gettotalbalance", 1},
     { "z_gettotalbalance", 2},

src/wallet/asyncrpcoperation_shieldcoinbase.cpp

@@ -93,8 +93,8 @@ AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase(
         [&](libzcash::SproutPaymentAddress addr) {
             tozaddr_ = addr;
         },
-        [&](libzcash::UnifiedAddress) {
+        [&](libzcash::UnifiedAddress addr) {
-            throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a unified address.");
+            tozaddr_ = addr;
         }
     }, toAddress);
 
@@ -279,7 +279,12 @@ bool ShieldToAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) c
 }
 
 bool ShieldToAddress::operator()(const libzcash::UnifiedAddress &uaddr) const {
-    // TODO
+    // TODO check if an Orchard address is present, send to it if so.
+    const auto receiver{uaddr.GetSaplingReceiver()};
+    if (receiver.has_value()) {
+        return ShieldToAddress(m_op, sendAmount)(receiver.value());
+    }
+    // This UA must contain a transparent address, which can't be the destination of coinbase shielding.
     return false;
 }
 

src/wallet/rpcwallet.cpp

@@ -4743,7 +4743,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
                 }
             },
             [&](const libzcash::UnifiedAddress& ua) {
-                throw JSONRPCError(RPC_VERIFY_REJECTED, "Cannot shield coinbase output to a unified address.");
+                // OK
             }
         }, destaddress.value());
     } else {