Merge #6492: test: add functional tests for coinjoinsalt RPC

16c2e13fb4 test: add functional tests for `coinjoinsalt` RPC (Kittywhiskers Van Gogh)
a1b256b06f test: extend CoinJoin RPC tests to include more cases, add logging (Kittywhiskers Van Gogh)
c6dd3dd567 test: rename test functions to reflect RPC used, simplify them (Kittywhiskers Van Gogh)
ff29c62103 test: run CoinJoin RPC tests using blank wallet (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Current suite of tests do not check if restoring salt results in restoring CoinJoin balance. This is because functional tests currently do not test CoinJoin mixing (and thus, routines for the same are not currently present).

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 16c2e13fb4
  UdjinM6:
    utACK 16c2e13fb4

Tree-SHA512: 0ce4e67f2caf0619cae42e8158cd39fba24c0a86050e061511ea23c4c0bf34a0eede72917516b6039d7ac15f85730ab36ba9ec1c42d0eb271f6cb4341389bcec

test/functional/rpc_coinjoin.py

@@ -3,14 +3,26 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+import random
+
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.messages import (
+    COIN,
+    MAX_MONEY,
+    uint256_to_string,
+)
+from test_framework.util import (
+    assert_equal,
+    assert_is_hex_string,
+    assert_raises_rpc_error,
+)
 
-'''
+# See coinjoin/options.h
-rpc_coinjoin.py
+COINJOIN_ROUNDS_DEFAULT = 4
-
+COINJOIN_ROUNDS_MAX = 16
-Tests CoinJoin basic RPC
+COINJOIN_ROUNDS_MIN = 2
-'''
+COINJOIN_TARGET_MAX = int(MAX_MONEY / COIN)
+COINJOIN_TARGET_MIN = 2
 
 class CoinJoinTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -19,45 +31,128 @@ class CoinJoinTest(BitcoinTestFramework):
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
-    def run_test(self):
+    def setup_nodes(self):
-        self.test_coinjoin_start_stop()
+        self.add_nodes(self.num_nodes)
-        self.test_coinjoin_setamount()
+        self.start_nodes()
-        self.test_coinjoin_setrounds()
 
-    def test_coinjoin_start_stop(self):
+    def run_test(self):
-        # Start Mixing
+        node = self.nodes[0]
-        self.nodes[0].coinjoin("start")
+
-        # Get CoinJoin info
+        node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False)
-        cj_info = self.nodes[0].getcoinjoininfo()
+        w1 = node.get_wallet_rpc('w1')
-        # Ensure that it started properly
+        self.test_salt_presence(w1)
+        self.test_coinjoin_start_stop(w1)
+        self.test_setcoinjoinamount(w1)
+        self.test_setcoinjoinrounds(w1)
+        self.test_coinjoinsalt(w1)
+        w1.unloadwallet()
+
+        node.createwallet(wallet_name='w2', blank=True, disable_private_keys=True)
+        w2 = node.get_wallet_rpc('w2')
+        self.test_coinjoinsalt_disabled(w2)
+        w2.unloadwallet()
+
+    def test_salt_presence(self, node):
+        self.log.info('Salt should be automatically generated in new wallet')
+        # Will raise exception if no salt generated
+        assert_is_hex_string(node.coinjoinsalt('get'))
+
+    def test_coinjoin_start_stop(self, node):
+        self.log.info('"coinjoin" subcommands should update mixing status')
+        # Start mix session and ensure it's reported
+        node.coinjoin('start')
+        cj_info = node.getcoinjoininfo()
         assert_equal(cj_info['enabled'], True)
         assert_equal(cj_info['running'], True)
+        # Repeated start should yield error
+        assert_raises_rpc_error(-32603, 'Mixing has been started already.', node.coinjoin, 'start')
 
-        # Stop mixing
+        # Stop mix session and ensure it's reported
-        self.nodes[0].coinjoin("stop")
+        node.coinjoin('stop')
-        # Get CoinJoin info
+        cj_info = node.getcoinjoininfo()
-        cj_info = self.nodes[0].getcoinjoininfo()
-        # Ensure that it stopped properly
         assert_equal(cj_info['enabled'], True)
         assert_equal(cj_info['running'], False)
+        # Repeated stop should yield error
+        assert_raises_rpc_error(-32603, 'No mix session to stop', node.coinjoin, 'stop')
 
-    def test_coinjoin_setamount(self):
+        # Reset mix session
-        # Try normal values
+        assert_equal(node.coinjoin('reset'), "Mixing was reset")
-        self.nodes[0].setcoinjoinamount(50)
-        cj_info = self.nodes[0].getcoinjoininfo()
-        assert_equal(cj_info['max_amount'], 50)
 
-        # Try large values
+    def test_setcoinjoinamount(self, node):
-        self.nodes[0].setcoinjoinamount(1200000)
+        self.log.info('"setcoinjoinamount" should update mixing target')
-        cj_info = self.nodes[0].getcoinjoininfo()
+        # Test normal and large values
-        assert_equal(cj_info['max_amount'], 1200000)
+        for value in [COINJOIN_TARGET_MIN, 50, 1200000, COINJOIN_TARGET_MAX]:
+            node.setcoinjoinamount(value)
+            assert_equal(node.getcoinjoininfo()['max_amount'], value)
+        # Test values below minimum and above maximum
+        for value in [COINJOIN_TARGET_MIN - 1, COINJOIN_TARGET_MAX + 1]:
+            assert_raises_rpc_error(-8, "Invalid amount of DASH as mixing goal amount", node.setcoinjoinamount, value)
 
-    def test_coinjoin_setrounds(self):
+    def test_setcoinjoinrounds(self, node):
-        # Try normal values
+        self.log.info('"setcoinjoinrounds" should update mixing rounds')
-        self.nodes[0].setcoinjoinrounds(5)
+        # Test acceptable values
-        cj_info = self.nodes[0].getcoinjoininfo()
+        for value in [COINJOIN_ROUNDS_MIN, COINJOIN_ROUNDS_DEFAULT, COINJOIN_ROUNDS_MAX]:
-        assert_equal(cj_info['max_rounds'], 5)
+            node.setcoinjoinrounds(value)
+            assert_equal(node.getcoinjoininfo()['max_rounds'], value)
+        # Test values below minimum and above maximum
+        for value in [COINJOIN_ROUNDS_MIN - 1, COINJOIN_ROUNDS_MAX + 1]:
+            assert_raises_rpc_error(-8, "Invalid number of rounds", node.setcoinjoinrounds, value)
 
+    def test_coinjoinsalt(self, node):
+        self.log.info('"coinjoinsalt generate" should fail if salt already present')
+        assert_raises_rpc_error(-32600, 'Wallet "w1" already has set CoinJoin salt!', node.coinjoinsalt, 'generate')
+
+        self.log.info('"coinjoinsalt" subcommands should succeed if no balance and not mixing')
+        # 'coinjoinsalt generate' should return a new salt if overwrite enabled
+        s1 = node.coinjoinsalt('get')
+        assert_equal(node.coinjoinsalt('generate', True), True)
+        s2 = node.coinjoinsalt('get')
+        assert s1 != s2
+
+        # 'coinjoinsalt get' should fetch newly generated value (i.e. new salt should persist)
+        node.unloadwallet('w1')
+        node.loadwallet('w1')
+        node = self.nodes[0].get_wallet_rpc('w1')
+        assert_equal(s2, node.coinjoinsalt('get'))
+
+        # 'coinjoinsalt set' should work with random hashes
+        s1 = uint256_to_string(random.getrandbits(256))
+        node.coinjoinsalt('set', s1)
+        assert_equal(s1, node.coinjoinsalt('get'))
+        assert s1 != s2
+
+        # 'coinjoinsalt set' shouldn't work with nonsense values
+        s2 = format(0, '064x')
+        assert_raises_rpc_error(-8, "Invalid CoinJoin salt value", node.coinjoinsalt, 'set', s2, True)
+        s2 = s2[0:63] + 'h'
+        assert_raises_rpc_error(-8, "salt must be hexadecimal string (not '%s')" % s2, node.coinjoinsalt, 'set', s2, True)
+
+        self.log.info('"coinjoinsalt generate" and "coinjoinsalt set" should fail if mixing')
+        # Start mix session
+        node.coinjoin('start')
+        assert_equal(node.getcoinjoininfo()['running'], True)
+
+        # 'coinjoinsalt generate' and 'coinjoinsalt set' should fail when mixing
+        assert_raises_rpc_error(-4, 'Wallet "w1" is currently mixing, cannot change salt!', node.coinjoinsalt, 'generate', True)
+        assert_raises_rpc_error(-4, 'Wallet "w1" is currently mixing, cannot change salt!', node.coinjoinsalt, 'set', s1, True)
+
+        # 'coinjoinsalt get' should still work
+        assert_equal(node.coinjoinsalt('get'), s1)
+
+        # Stop mix session
+        node.coinjoin('stop')
+        assert_equal(node.getcoinjoininfo()['running'], False)
+
+        # 'coinjoinsalt generate' and 'coinjoinsalt set' should start working again
+        assert_equal(node.coinjoinsalt('generate', True), True)
+        assert_equal(node.coinjoinsalt('set', s1, True), True)
+
+    def test_coinjoinsalt_disabled(self, node):
+        self.log.info('"coinjoinsalt" subcommands should fail if private keys disabled')
+        for subcommand in ['generate', 'get']:
+            assert_raises_rpc_error(-32600, 'Wallet "w2" has private keys disabled, cannot perform CoinJoin!', node.coinjoinsalt, subcommand)
+        s1 = uint256_to_string(random.getrandbits(256))
+        assert_raises_rpc_error(-32600, 'Wallet "w2" has private keys disabled, cannot perform CoinJoin!', node.coinjoinsalt, 'set', s1)
 
 if __name__ == '__main__':
     CoinJoinTest().main()

test/functional/test_runner.py

@@ -892,8 +892,6 @@ class RPCCoverage():
         # Consider RPC generate covered, because it is overloaded in
         # test_framework/test_node.py and not seen by the coverage check.
         covered_cmds = set({'generate'})
-        # TODO: implement functional tests for coinjoinsalt
-        covered_cmds.add('coinjoinsalt')
         # TODO: implement functional tests for voteraw
         covered_cmds.add('voteraw')
         # TODO: implement functional tests for getmerkleblocks