From ff29c621034158fbdcabf89408410734b8f90f76 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:29:31 +0000 Subject: [PATCH 1/4] test: run CoinJoin RPC tests using blank wallet --- test/functional/rpc_coinjoin.py | 42 ++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 76d2e73794..6bd2068dd9 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -19,43 +19,51 @@ class CoinJoinTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() - def run_test(self): - self.test_coinjoin_start_stop() - self.test_coinjoin_setamount() - self.test_coinjoin_setrounds() + def setup_nodes(self): + self.add_nodes(self.num_nodes) + self.start_nodes() - def test_coinjoin_start_stop(self): + def run_test(self): + node = self.nodes[0] + + node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) + w1 = node.get_wallet_rpc('w1') + self.test_coinjoin_start_stop(w1) + self.test_coinjoin_setamount(w1) + self.test_coinjoin_setrounds(w1) + + def test_coinjoin_start_stop(self, node): # Start Mixing - self.nodes[0].coinjoin("start") + node.coinjoin('start') # Get CoinJoin info - cj_info = self.nodes[0].getcoinjoininfo() + cj_info = node.getcoinjoininfo() # Ensure that it started properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], True) # Stop mixing - self.nodes[0].coinjoin("stop") + node.coinjoin('stop') # Get CoinJoin info - cj_info = self.nodes[0].getcoinjoininfo() + cj_info = node.getcoinjoininfo() # Ensure that it stopped properly assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], False) - def test_coinjoin_setamount(self): + def test_coinjoin_setamount(self, node): # Try normal values - self.nodes[0].setcoinjoinamount(50) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinamount(50) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_amount'], 50) # Try large values - self.nodes[0].setcoinjoinamount(1200000) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinamount(1200000) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_amount'], 1200000) - def test_coinjoin_setrounds(self): + def test_coinjoin_setrounds(self, node): # Try normal values - self.nodes[0].setcoinjoinrounds(5) - cj_info = self.nodes[0].getcoinjoininfo() + node.setcoinjoinrounds(5) + cj_info = node.getcoinjoininfo() assert_equal(cj_info['max_rounds'], 5) From c6dd3dd567a8f9e0d2c9780f40c1f496df013b6f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 07:38:30 +0000 Subject: [PATCH 2/4] test: rename test functions to reflect RPC used, simplify them --- test/functional/rpc_coinjoin.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 6bd2068dd9..935d2817f6 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -29,8 +29,8 @@ class CoinJoinTest(BitcoinTestFramework): node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) w1 = node.get_wallet_rpc('w1') self.test_coinjoin_start_stop(w1) - self.test_coinjoin_setamount(w1) - self.test_coinjoin_setrounds(w1) + self.test_setcoinjoinamount(w1) + self.test_setcoinjoinrounds(w1) def test_coinjoin_start_stop(self, node): # Start Mixing @@ -49,22 +49,16 @@ class CoinJoinTest(BitcoinTestFramework): assert_equal(cj_info['enabled'], True) assert_equal(cj_info['running'], False) - def test_coinjoin_setamount(self, node): - # Try normal values - node.setcoinjoinamount(50) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_amount'], 50) + def test_setcoinjoinamount(self, node): + # Test normal and large values + for value in [50, 1200000]: + node.setcoinjoinamount(value) + assert_equal(node.getcoinjoininfo()['max_amount'], value) - # Try large values - node.setcoinjoinamount(1200000) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_amount'], 1200000) - - def test_coinjoin_setrounds(self, node): - # Try normal values + def test_setcoinjoinrounds(self, node): + # Test normal values node.setcoinjoinrounds(5) - cj_info = node.getcoinjoininfo() - assert_equal(cj_info['max_rounds'], 5) + assert_equal(node.getcoinjoininfo()['max_rounds'], 5) if __name__ == '__main__': From a1b256b06f2bc30c6a3686f11a1849e0dbdb92be Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:33:22 +0000 Subject: [PATCH 3/4] test: extend CoinJoin RPC tests to include more cases, add logging --- test/functional/rpc_coinjoin.py | 54 ++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 935d2817f6..8ca9c865fa 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -4,13 +4,21 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.messages import ( + COIN, + MAX_MONEY, +) +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) -''' -rpc_coinjoin.py - -Tests CoinJoin basic RPC -''' +# See coinjoin/options.h +COINJOIN_ROUNDS_DEFAULT = 4 +COINJOIN_ROUNDS_MAX = 16 +COINJOIN_ROUNDS_MIN = 2 +COINJOIN_TARGET_MAX = int(MAX_MONEY / COIN) +COINJOIN_TARGET_MIN = 2 class CoinJoinTest(BitcoinTestFramework): def set_test_params(self): @@ -33,33 +41,45 @@ class CoinJoinTest(BitcoinTestFramework): self.test_setcoinjoinrounds(w1) def test_coinjoin_start_stop(self, node): - # Start Mixing + self.log.info('"coinjoin" subcommands should update mixing status') + # Start mix session and ensure it's reported node.coinjoin('start') - # Get CoinJoin info cj_info = node.getcoinjoininfo() - # Ensure that it started properly 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 node.coinjoin('stop') - # Get CoinJoin info cj_info = node.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') + + # Reset mix session + assert_equal(node.coinjoin('reset'), "Mixing was reset") def test_setcoinjoinamount(self, node): + self.log.info('"setcoinjoinamount" should update mixing target') # Test normal and large values - for value in [50, 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_setcoinjoinrounds(self, node): - # Test normal values - node.setcoinjoinrounds(5) - assert_equal(node.getcoinjoininfo()['max_rounds'], 5) - + self.log.info('"setcoinjoinrounds" should update mixing rounds') + # Test acceptable values + for value in [COINJOIN_ROUNDS_MIN, COINJOIN_ROUNDS_DEFAULT, COINJOIN_ROUNDS_MAX]: + 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) if __name__ == '__main__': CoinJoinTest().main() From 16c2e13fb48c3e7cdab945310280766e12e2f260 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:31:46 +0000 Subject: [PATCH 4/4] test: add functional tests for `coinjoinsalt` RPC --- test/functional/rpc_coinjoin.py | 73 +++++++++++++++++++++++++++++++++ test/functional/test_runner.py | 2 - 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py index 8ca9c865fa..70f9955377 100755 --- a/test/functional/rpc_coinjoin.py +++ b/test/functional/rpc_coinjoin.py @@ -3,13 +3,17 @@ # 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.messages import ( COIN, MAX_MONEY, + uint256_to_string, ) from test_framework.util import ( assert_equal, + assert_is_hex_string, assert_raises_rpc_error, ) @@ -36,9 +40,22 @@ class CoinJoinTest(BitcoinTestFramework): node.createwallet(wallet_name='w1', blank=True, disable_private_keys=False) w1 = node.get_wallet_rpc('w1') + 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') @@ -81,5 +98,61 @@ class CoinJoinTest(BitcoinTestFramework): 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() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2b311fca34..e1c2b7680a 100755 --- a/test/functional/test_runner.py +++ b/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