From 28e670d1d6f002cda9c19e13e78e109a1233528b Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 11 May 2022 12:54:23 +0200 Subject: [PATCH 1/8] ci: increase task timeout --- .cirrus.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cirrus.yml b/.cirrus.yml index 5aea224a..78fc9e30 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -2,6 +2,7 @@ task: name: 'Functional tests' container: image: rust:latest + timeout_in: 90m # https://cirrus-ci.org/faq/#instance-timed-out env: EXECUTOR_WORKERS: 1 From 4a1480dbe134fab70b5d620d100d58c010138cf2 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 12 May 2022 11:15:12 +0200 Subject: [PATCH 2/8] ci: don't run in VERBOSE, we don't use it --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 78fc9e30..01facdcf 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -6,7 +6,7 @@ task: env: EXECUTOR_WORKERS: 1 - VERBOSE: 1 + VERBOSE: 0 LOG_LEVEL: debug TIMEOUT: 300 BITCOIND_VERSION: 22.0 From dbcbcff9de10fd424f6feb3e0bcf359383183c8d Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 13 May 2022 10:55:31 +0200 Subject: [PATCH 3/8] ci: increase the number of workers --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 01facdcf..66f9ae49 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -5,7 +5,7 @@ task: timeout_in: 90m # https://cirrus-ci.org/faq/#instance-timed-out env: - EXECUTOR_WORKERS: 1 + EXECUTOR_WORKERS: 3 VERBOSE: 0 LOG_LEVEL: debug TIMEOUT: 300 From 87e9b257ecbcffd9ec1c8cbfdeae6fbd8c7fcb7b Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 10 May 2022 21:21:38 +0200 Subject: [PATCH 4/8] qa: stop daemons in parrallel in revault_network fixture --- tests/test_framework/revault_network.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_framework/revault_network.py b/tests/test_framework/revault_network.py index 8e341348..ed9eeb06 100644 --- a/tests/test_framework/revault_network.py +++ b/tests/test_framework/revault_network.py @@ -799,7 +799,8 @@ def start_wallets(self): j.result(TIMEOUT) def cleanup(self): - for n in self.daemons: - n.cleanup() + jobs = [self.executor.submit(w.cleanup) for w in self.daemons] + for j in jobs: + j.result(TIMEOUT) if self.bitcoind_proxy is not None: self.bitcoind_proxy.stop() From 744bf6d3d38043b2b141247735c981b36b37974a Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 10 May 2022 17:27:46 +0200 Subject: [PATCH 5/8] qa: implement an in-RAM simple coordinator Using a single Postgre backend and a regular coordinator per instance worked for a time, but it's getting to its limits. The functional tests, when ran in parrallel, put an unreasonable load on the Postgre backend. This leads to spurious failures and timeouts that are hard to deal with in the `coordinatord` or `revaultd` binaries themselves without optimizing for functional tests.. Which isn't a realistic load. I tried to implement a cache to get_sigs requests using a proxy between the revaultds and the Coordinator in a single instance. Although it (partially) solved the timeouts in the RPC commands that hit the coordinator, it slowed tests down by a lot. Instead, here we implement a fully in-RAM coordinator for the lowest possible response time. After all, we don't need to persist the data for the functional tests and we can afford a few more MB of RAM usage. Of course, we keep the possibility of running using the real coordinator, but we would probably not do so in the CI that is already too slow. 3 tests would be skipped, though. But not the most important ones, and we don't only rely on CI anyways. As a by-product, we can now run the functional tests without having to start a Postgre backend beforehand. I think it can help new contributors a lot. --- tests/fixtures.py | 6 - tests/requirements.txt | 1 + tests/test_chain.py | 8 - tests/test_framework/bitcoind.py | 2 +- tests/test_framework/coordinatord.py | 205 +++++++++++++++++++++++- tests/test_framework/revault_network.py | 53 +++--- tests/test_framework/revaultd.py | 3 +- tests/test_framework/utils.py | 2 +- tests/test_misc.py | 11 -- tests/test_rpc.py | 17 -- tests/test_spend.py | 8 - tests/test_watchtowers.py | 4 - 12 files changed, 243 insertions(+), 77 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 851faf9b..ba7568c6 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -224,12 +224,6 @@ def revaultd_manager(bitcoind, directory): @pytest.fixture def revault_network(directory, bitcoind, executor): - if not POSTGRES_IS_SETUP: - raise ValueError( - "Please set the POSTGRES_USER, POSTGRES_PASS and " - "POSTGRES_HOST environment variables." - ) - factory = RevaultNetwork( directory, bitcoind, executor, POSTGRES_USER, POSTGRES_PASS, POSTGRES_HOST ) diff --git a/tests/requirements.txt b/tests/requirements.txt index 6230e565..cb70946e 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -6,6 +6,7 @@ bip32~=3.0 bip380==0.0.3 psycopg2-binary==2.9 pynacl==1.4 +noiseprotocol==0.3.1 # For the bitcoind proxy Flask==2.0.3 diff --git a/tests/test_chain.py b/tests/test_chain.py index 0d2128a2..32909b55 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -5,12 +5,10 @@ """ import logging -import pytest from fixtures import * from test_framework import serializations from test_framework.utils import ( - POSTGRES_IS_SETUP, wait_for, ) @@ -248,7 +246,6 @@ def reorg_deposit(revault_network, bitcoind, deposit, stop_wallets, target_statu # TODO: try with tx malleation -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_reorged_deposit_status_1(revault_network, bitcoind): # NOTE: bitcoind would discard updating the mempool if the reorg is >10 blocks long. revault_network.deploy(4, 2, csv=12, with_watchtowers=False) @@ -287,7 +284,6 @@ def test_reorged_deposit_status_1(revault_network, bitcoind): # TODO: same with 'emergency' -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_reorged_deposit_status_2(revault_network, bitcoind): # NOTE: bitcoind would discard updating the mempool if the reorg is >10 blocks long. revault_network.deploy(4, 2, csv=3, with_watchtowers=False) @@ -319,7 +315,6 @@ def test_reorged_deposit_status_2(revault_network, bitcoind): # TODO: same with 'unvault_emergency' -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_reorged_unvault(revault_network, bitcoind): """Test various scenarii with reorgs around the Unvault transaction of a vault.""" CSV = 12 @@ -513,7 +508,6 @@ def test_reorged_unvault(revault_network, bitcoind): assert vault[field] is None, field -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_reorged_spend(revault_network, bitcoind): CSV = 12 revault_network.deploy(4, 2, csv=CSV, with_watchtowers=False) @@ -561,7 +555,6 @@ def test_reorged_spend(revault_network, bitcoind): assert vault[field] is None, field -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_reorged_cancel(revault_network, bitcoind): revault_network.deploy(4, 2, csv=12, with_watchtowers=False) stks = revault_network.stks() @@ -655,7 +648,6 @@ def test_reorged_cancel(revault_network, bitcoind): assert vault[field] is None, field -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_retrieve_vault_status(revault_network, bitcoind): """Test we keep track of coins that moved without us actively noticing it.""" CSV = 3 diff --git a/tests/test_framework/bitcoind.py b/tests/test_framework/bitcoind.py index d8c6956b..4ac608e3 100644 --- a/tests/test_framework/bitcoind.py +++ b/tests/test_framework/bitcoind.py @@ -4,7 +4,7 @@ import os import threading -from cheroot.wsgi import PathInfoDispatcher, Server +from cheroot.wsgi import Server from decimal import Decimal from ephemeral_port_reserve import reserve from flask import Flask, request, Response diff --git a/tests/test_framework/coordinatord.py b/tests/test_framework/coordinatord.py index a11290c0..b541dab7 100644 --- a/tests/test_framework/coordinatord.py +++ b/tests/test_framework/coordinatord.py @@ -1,7 +1,21 @@ +import cryptography +import json +import logging import os import psycopg2 +import select +import socket +import threading -from test_framework.utils import TailableProc, VERBOSE, LOG_LEVEL, COORDINATORD_PATH +from nacl.public import PrivateKey as Curve25519Private +from noise.connection import NoiseConnection, Keypair +from test_framework.utils import ( + TailableProc, + VERBOSE, + LOG_LEVEL, + COORDINATORD_PATH, + TIMEOUT, +) class Coordinatord(TailableProc): @@ -98,3 +112,192 @@ def cleanup(self): except Exception: self.proc.kill() self.postgres_exec(f"DROP DATABASE {self.db_name}") + + +HANDSHAKE_MSG = b"practical_revault_0" + + +class DummyCoordinator: + """A simple in-RAM synchronization server.""" + + def __init__( + self, + port, + coordinator_privkey, + client_pubkeys, + ): + self.port = port + self.coordinator_privkey = coordinator_privkey + self.coordinator_pubkey = bytes( + Curve25519Private(coordinator_privkey).public_key + ) + self.client_pubkeys = client_pubkeys + + # Spin up the coordinator proxy + self.s = socket.socket() + self.s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self.s.bind(("localhost", self.port)) + self.s.listen(1_000) + # Use a pipe to communicate to threads to stop + self.r_close_chann, self.w_close_chann = os.pipe() + + # A mapping from txid to pubkey to signature + self.sigs = {} + # A mapping from deposit_outpoint to base64 PSBT + self.spend_txs = {} + + def __del__(self): + self.cleanup() + + def start(self): + self.server_thread = threading.Thread(target=self.run) + self.server_thread.start() + + def cleanup(self): + # Write to the pipe to notify the thread it needs to stop + os.write(self.w_close_chann, b".") + self.server_thread.join() + + def run(self): + """Accept new connections until we are told to stop.""" + while True: + r_fds, _, _ = select.select([self.r_close_chann, self.s.fileno()], [], []) + + # First check if we've been told to stop, then spawn a new thread per connection + if self.r_close_chann in r_fds: + self.s.close() + break + if self.s.fileno() in r_fds: + t = threading.Thread(target=self.connection_handle, daemon=True) + t.start() + + def connection_handle(self): + """Read and treat requests from this client. Blocking.""" + client_fd, _ = self.s.accept() + client_fd.settimeout(TIMEOUT) + client_noise = self.server_noise_conn(client_fd) + + while True: + # Manually do the select to check if we've been told to stop + r_fds, _, _ = select.select([self.r_close_chann, client_fd], [], []) + if self.r_close_chann in r_fds: + client_fd.close() + break + elif client_fd not in r_fds: + client_fd.close() + break + req = self.read_msg(client_fd, client_noise) + if req is None: + client_fd.close() + break + request = json.loads(req) + method, params = request["method"], request["params"] + + if method == "sig": + # FIXME: the field name is a misimplementation! + # TODO: mutex + if params["id"] not in self.sigs: + self.sigs[params["id"]] = {} + self.sigs[params["id"]][params["pubkey"]] = params["signature"] + # TODO: remove this useless response from the protocol + resp = {"result": {"ack": True}, "id": request["id"]} + self.send_msg(client_fd, client_noise, json.dumps(resp)) + + elif method == "get_sigs": + # FIXME: the field name is a misimplementation of the protocol! + txid = params["id"] + sigs = self.sigs.get(txid, {}) + resp = {"result": {"signatures": sigs}, "id": request["id"]} + self.send_msg(client_fd, client_noise, json.dumps(resp)) + + elif method == "set_spend_tx": + for outpoint in params["deposit_outpoints"]: + self.spend_txs[outpoint] = params["transaction"] + # TODO: remove this useless response from the protocol + resp = {"result": {"ack": True}, "id": request["id"]} + self.send_msg(client_fd, client_noise, json.dumps(resp)) + + elif method == "get_spend_tx": + spend_tx = self.spend_txs.get(params["deposit_outpoint"]) + resp = {"result": {"transaction": spend_tx}, "id": request["id"]} + self.send_msg(client_fd, client_noise, json.dumps(resp)) + + else: + assert False, "Invalid request '{}'".format(method) + + def server_noise_conn(self, fd): + """Do practical-revault's Noise handshake with a given client connection.""" + # Read the first message of the handshake only once + data = self.read_data(fd, 32 + len(HANDSHAKE_MSG) + 16) + + # We brute force all pubkeys. FIXME! + for pubkey in self.client_pubkeys: + # Set the local and remote static keys + conn = NoiseConnection.from_name(b"Noise_KK_25519_ChaChaPoly_SHA256") + conn.set_as_responder() + conn.set_keypair_from_private_bytes( + Keypair.STATIC, self.coordinator_privkey + ) + conn.set_keypair_from_public_bytes(Keypair.REMOTE_STATIC, pubkey) + + # Now, get the first message of the handshake + conn.start_handshake() + try: + plaintext = conn.read_message(data) + except cryptography.exceptions.InvalidTag: + continue + else: + assert plaintext[: len(HANDSHAKE_MSG)] == HANDSHAKE_MSG + + # If it didn't fail it was the right key! Finalize the handshake. + resp = conn.write_message() + fd.sendall(resp) + assert conn.handshake_finished + + return conn + + raise Exception( + f"Unknown client key. Keys: {','.join(k.hex() for k in self.client_pubkeys)}" + ) + + def read_msg(self, fd, noise_conn): + """read a noise-encrypted message from this stream. + + Returns None if the socket closed. + """ + # Read first the length prefix + cypher_header = self.read_data(fd, 2 + 16) + if cypher_header == b"": + return None + msg_header = noise_conn.decrypt(cypher_header) + msg_len = int.from_bytes(msg_header, "big") + + # And then the message + cypher_msg = self.read_data(fd, msg_len) + assert len(cypher_msg) == msg_len + msg = noise_conn.decrypt(cypher_msg).decode("utf-8") + return msg + + def send_msg(self, fd, noise_conn, msg): + """Write a noise-encrypted message from this stream.""" + assert isinstance(msg, str) + + # Compute the message header + msg_raw = msg.encode("utf-8") + length_prefix = (len(msg_raw) + 16).to_bytes(2, "big") + encrypted_header = noise_conn.encrypt(length_prefix) + encrypted_body = noise_conn.encrypt(msg_raw) + + # Then send both the header and the message concatenated + fd.sendall(encrypted_header + encrypted_body) + + def read_data(self, fd, max_len): + """Read data from the given fd until there is nothing to read.""" + data = b"" + while True: + d = fd.recv(max_len) + if len(d) == max_len: + return d + if d == b"": + return data + data += d diff --git a/tests/test_framework/revault_network.py b/tests/test_framework/revault_network.py index ed9eeb06..6b83faef 100644 --- a/tests/test_framework/revault_network.py +++ b/tests/test_framework/revault_network.py @@ -7,7 +7,7 @@ from nacl.public import PrivateKey as Curve25519Private from test_framework import serializations from test_framework.bitcoind import BitcoindRpcProxy -from test_framework.coordinatord import Coordinatord +from test_framework.coordinatord import Coordinatord, DummyCoordinator from test_framework.cosignerd import Cosignerd from test_framework.miradord import Miradord from test_framework.revaultd import ManagerRevaultd, StakeholderRevaultd, StkManRevaultd @@ -18,6 +18,7 @@ wait_for, TIMEOUT, WT_PLUGINS_DIR, + POSTGRES_IS_SETUP, ) @@ -201,24 +202,38 @@ def deploy( f"{stkonly_wt_noisepubs + stkman_wt_noisepubs}\n" ) - # Spin up the "Sync Server" - coord_datadir = os.path.join(self.root_dir, "coordinatord") - os.makedirs(coord_datadir, exist_ok=True) - coordinatord = Coordinatord( - coord_datadir, - coordinator_noisepriv, - man_noisepubs + stkman_noisepubs, - stkonly_noisepubs + stkman_noisepubs, - stkonly_wt_noisepubs + stkman_wt_noisepubs, - self.coordinator_port, - bitcoind_rpcport, - bitcoind_cookie, - self.postgres_user, - self.postgres_pass, - self.postgres_host, - ) - coordinatord.start() - self.daemons.append(coordinatord) + # Use coordinatord if they pointed us to a Postgre backend, otherwise use the dummy + # coordinator. + if POSTGRES_IS_SETUP: + coord_datadir = os.path.join(self.root_dir, "coordinatord") + os.makedirs(coord_datadir, exist_ok=True) + coordinatord = Coordinatord( + coord_datadir, + coordinator_noisepriv, + man_noisepubs + stkman_noisepubs, + stkonly_noisepubs + stkman_noisepubs, + stkonly_wt_noisepubs + stkman_wt_noisepubs, + self.coordinator_port, + bitcoind_rpcport, + bitcoind_cookie, + self.postgres_user, + self.postgres_pass, + self.postgres_host, + ) + coordinatord.start() + self.daemons.append(coordinatord) + else: + coordinator = DummyCoordinator( + self.coordinator_port, + coordinator_noisepriv, + man_noisepubs + + stkonly_noisepubs + + stkman_noisepubs + + stkonly_wt_noisepubs + + stkman_wt_noisepubs, + ) + coordinator.start() + self.daemons.append(coordinator) cosigners_info = [] for (i, noisepub) in enumerate(stkonly_cosig_noisepubs): diff --git a/tests/test_framework/revaultd.py b/tests/test_framework/revaultd.py index 27fe7d9d..39d74890 100644 --- a/tests/test_framework/revaultd.py +++ b/tests/test_framework/revaultd.py @@ -159,7 +159,8 @@ def stop(self, timeout=10): def cleanup(self): try: self.stop() - except Exception: + except Exception as e: + logging.error(f"{self.prefix}: Error when cleaning up: '{e}'") self.proc.kill() diff --git a/tests/test_framework/utils.py b/tests/test_framework/utils.py index fc79c9ee..fdc6d147 100644 --- a/tests/test_framework/utils.py +++ b/tests/test_framework/utils.py @@ -28,7 +28,7 @@ POSTGRES_USER = os.getenv("POSTGRES_USER", "") POSTGRES_PASS = os.getenv("POSTGRES_PASS", "") POSTGRES_HOST = os.getenv("POSTGRES_HOST", "localhost") -POSTGRES_IS_SETUP = POSTGRES_USER and POSTGRES_PASS and POSTGRES_HOST +POSTGRES_IS_SETUP = POSTGRES_USER != "" and POSTGRES_PASS != "" VERBOSE = os.getenv("VERBOSE", "0") == "1" LOG_LEVEL = os.getenv("LOG_LEVEL", "debug") assert LOG_LEVEL in ["trace", "debug", "info", "warn", "error"] diff --git a/tests/test_misc.py b/tests/test_misc.py index 9a6a2375..66b288a2 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1,4 +1,3 @@ -import logging import pytest import os @@ -37,7 +36,6 @@ def test_largewallets(revaultd_stakeholder, bitcoind): revaultd_stakeholder.rpc.listpresignedtransactions() -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_huge_deposit(revault_network, bitcoind): revault_network.deploy(2, 1) stk = revault_network.stk(0) @@ -49,7 +47,6 @@ def test_huge_deposit(revault_network, bitcoind): assert stk.rpc.listvaults([], [deposit])["vaults"][0]["amount"] == amount * COIN -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_revocation_sig_sharing(revault_network): revault_network.deploy(4, 2, n_stkmanagers=1) stks = revault_network.stks() @@ -101,7 +98,6 @@ def test_revocation_sig_sharing(revault_network): wait_for(lambda: len(stk.rpc.listvaults(["secured"], [deposit])["vaults"]) > 0) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_raw_broadcast_cancel(revault_network, bitcoind): """ Test broadcasting a dozen of pair of Unvault and Cancel for vaults with @@ -140,7 +136,6 @@ def test_raw_broadcast_cancel(revault_network, bitcoind): wait_for(lambda: len(w.rpc.listvaults(["canceled"], [deposit])) == 1) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_sigfetcher(revault_network, bitcoind, executor): rn = revault_network rn.deploy(7, 3, n_stkmanagers=2) @@ -248,7 +243,6 @@ def test_sigfetcher_coordinator_dead(revault_network, bitcoind): stk.wait_for_secured_vaults([deposit]) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_sigfetcher_secured_vaults(revault_network, bitcoind): """ Test that unvault sigs are retrieved and stored for secured vault even if the @@ -287,7 +281,6 @@ def test_sigfetcher_secured_vaults(revault_network, bitcoind): ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_stkman_only(revault_network, bitcoind): """Test a setup with only stakehodlers-managers""" rn = revault_network @@ -309,7 +302,6 @@ def test_stkman_only(revault_network, bitcoind): rn.cancel_vault(vaults[0]) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_no_cosig_server(revault_network): """Test a setup with no cosig""" rn = revault_network @@ -337,7 +329,6 @@ def get_unvault_txids(wallet, vaults): return unvault_txids -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_cpfp_transaction(revault_network, bitcoind): CSV = 12 revault_network.deploy( @@ -427,7 +418,6 @@ def test_cpfp_transaction(revault_network, bitcoind): man.wait_for_logs(["Checking if transactions need CPFP...", "Nothing to CPFP"]) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_batched_cpfp_transaction(revault_network, bitcoind): rn = revault_network CSV = 6 @@ -538,7 +528,6 @@ def test_batched_cpfp_transaction(revault_network, bitcoind): wait_for(lambda: len(man.rpc.listvaults(["spent"])["vaults"]) == len(vaults)) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_rbfed_cpfp_transaction(revault_network, bitcoind): """We don't have explicit RBF logic, but since we signal for it we should be able to replace a previous CPFP with a higher-fee, higher-feerate one. diff --git a/tests/test_rpc.py b/tests/test_rpc.py index df9eae33..a72e346a 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -121,7 +121,6 @@ def test_listvaults(revaultd_manager, bitcoind): assert len(vault_list) == 0 -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_getdepositaddress(revault_network, bitcoind): rn = revault_network rn.deploy(4, 2) @@ -147,7 +146,6 @@ def test_getdepositaddress(revault_network, bitcoind): assert addr2 == w.rpc.call("getdepositaddress")["address"] -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_getrevocationtxs(revault_network, bitcoind): rn = revault_network rn.deploy(4, 2) @@ -182,7 +180,6 @@ def test_getrevocationtxs(revault_network, bitcoind): assert txs == n.rpc.getrevocationtxs(deposit) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_getunvaulttx(revault_network): revault_network.deploy(3, 1) mans = revault_network.mans() @@ -206,7 +203,6 @@ def test_getunvaulttx(revault_network): assert tx["unvault_tx"] == stk.rpc.getunvaulttx(outpoint)["unvault_tx"] -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_listpresignedtransactions(revault_network): revault_network.deploy(2, 1) vaultA = revault_network.fund(0.2222221) @@ -246,7 +242,6 @@ def test_listpresignedtransactions(revault_network): assert res["unvault_emergency"] == stk_res["unvault_emergency"] -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_listspendtxs(revault_network, bitcoind): rn = revault_network rn.deploy(n_stakeholders=2, n_managers=2, n_stkmanagers=0, csv=5) @@ -357,7 +352,6 @@ def test_listspendtxs(revault_network, bitcoind): assert deprecated_txs[1]["status"] == "deprecated" -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_listspendtxs_check_indexes(revault_network, bitcoind): # Spending with the change output rn = revault_network @@ -373,7 +367,6 @@ def test_listspendtxs_check_indexes(revault_network, bitcoind): assert spend_txs[0]["cpfp_index"] is not None -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_listonchaintransactions(revault_network): """Just a small sanity check of the API""" rn = revault_network @@ -475,7 +468,6 @@ def psbt_add_invalid_sig(psbt_str): return psbt.serialize() -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_revocationtxs(revault_network): """Sanity checks for the revocationtxs command""" revault_network.deploy(6, 2) @@ -595,7 +587,6 @@ def test_revocationtxs(revault_network): assert len(stks[0].rpc.listvaults(["securing"], [deposit])["vaults"]) == 1 -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_unvaulttx(revault_network): """Sanity checks for the unvaulttx command""" revault_network.deploy(3, 1) @@ -701,7 +692,6 @@ def sign_revocation_txs(stks, deposit): ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_emergency(revault_network, bitcoind): """This tests the 'emergency' RPC command""" rn = revault_network @@ -837,7 +827,6 @@ def test_emergency(revault_network, bitcoind): ) + len(unvaulted_vaults) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_getspendtx(revault_network, bitcoind): revault_network.deploy(2, 1) man = revault_network.man(0) @@ -960,7 +949,6 @@ def test_getspendtx(revault_network, bitcoind): man.rpc.getspendtx(deposits, destinations, feerate) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_revault_command(revault_network, bitcoind, executor): """ Here we manually broadcast the unvualt_tx, followed by the cancel_tx @@ -1149,15 +1137,11 @@ def test_getserverstatus(revault_network, bitcoind): assert not watchtower["reachable"] -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_setspendtx_cpfp_not_enabled(revault_network, bitcoind): CSV = 12 revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV, with_cpfp=False) - man = revault_network.mans()[1] - stks = revault_network.stks() amount = 0.24 vault = revault_network.fund(amount) - deposit = f"{vault['txid']}:{vault['vout']}" revault_network.secure_vault(vault) revault_network.activate_vault(vault) @@ -1168,7 +1152,6 @@ def test_setspendtx_cpfp_not_enabled(revault_network, bitcoind): revault_network.broadcast_unvaults_anyhow([vault], priority=True) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_gethistory(revault_network, bitcoind, executor): """ Retrieve the event History and check the presence of some triggered events. diff --git a/tests/test_spend.py b/tests/test_spend.py index 0c8a04f5..0e6ef57b 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -11,13 +11,11 @@ from test_framework import serializations from test_framework.utils import ( COIN, - POSTGRES_IS_SETUP, RpcError, wait_for, ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_spendtx_management(revault_network, bitcoind): CSV = 12 revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV) @@ -228,7 +226,6 @@ def test_spendtx_management(revault_network, bitcoind): assert rogue_spend_tx["status"] == "deprecated" -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_spends_concurrent(revault_network, bitcoind): """ Here we test the creation and succesful broadcast of both Spend transaction @@ -323,7 +320,6 @@ def test_spends_concurrent(revault_network, bitcoind): ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_spends_conflicting(revault_network, bitcoind): """ Here we test two spends which spends 2 vaults each, with one shared and all vaults @@ -420,7 +416,6 @@ def test_spends_conflicting(revault_network, bitcoind): assert vault["moved_at"] is not None -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_spend_threshold(revault_network, bitcoind, executor): CSV = 20 managers_threshold = 2 @@ -526,7 +521,6 @@ def test_spend_threshold(revault_network, bitcoind, executor): assert vault["moved_at"] is not None -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_large_spends(revault_network, bitcoind, executor): CSV = 2016 # 2 weeks :tm: revault_network.deploy(17, 8, csv=CSV) @@ -608,7 +602,6 @@ def test_large_spends(revault_network, bitcoind, executor): # Tests that getspendtx returns an error when trying to build a spend too big # (it wouldn't be possible to announce it to the coordinator when fully signed) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_not_announceable_spend(revault_network, bitcoind, executor): CSV = 4 revault_network.deploy(5, 7, csv=CSV) @@ -685,7 +678,6 @@ def test_not_announceable_spend(revault_network, bitcoind, executor): assert vault["moved_at"] is not None -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_revaulted_spend(revault_network, bitcoind, executor): """ Revault an ongoing Spend transaction carried out by the managers, under misc diff --git a/tests/test_watchtowers.py b/tests/test_watchtowers.py index 422ed462..c1e463fa 100644 --- a/tests/test_watchtowers.py +++ b/tests/test_watchtowers.py @@ -1,16 +1,13 @@ import os -import pytest from fixtures import * from test_framework.utils import ( wait_for, - POSTGRES_IS_SETUP, WT_PLUGINS_DIR, COIN, ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_wt_share_revocation_txs(revault_network, bitcoind): """Sanity check that we share the revocation signatures with the watchtower.""" rn = revault_network @@ -27,7 +24,6 @@ def test_wt_share_revocation_txs(revault_network, bitcoind): ) -@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") def test_wt_policy(directory, revault_network, bitcoind): """Test we "can't" breach the policies defined by the watchtowers""" rn = revault_network From 356389d5e5584f6c9bc304bfc9cf1731e2112407 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 10 May 2022 22:02:54 +0200 Subject: [PATCH 6/8] qa: decrease the coordinator poll interval We can afford shorter intervals with the dummy coordinator. --- tests/test_framework/revaultd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_framework/revaultd.py b/tests/test_framework/revaultd.py index 39d74890..ff4ca06e 100644 --- a/tests/test_framework/revaultd.py +++ b/tests/test_framework/revaultd.py @@ -65,7 +65,7 @@ def __init__( f.write(f'coordinator_host = "127.0.0.1:{coordinator_port}"\n') f.write(f'coordinator_noise_key = "{coordinator_noise_key}"\n') - f.write("coordinator_poll_seconds = 5\n") + f.write("coordinator_poll_seconds = 1\n") f.write("[scripts_config]\n") f.write(f'deposit_descriptor = "{deposit_desc}"\n') From 995e4e5f6bf66c3739861e42001cd37f0e5e129c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 10 May 2022 22:17:39 +0200 Subject: [PATCH 7/8] qa: don't compile nor use the coordinator, run the dummy one --- .cirrus.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 66f9ae49..573a100a 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -48,7 +48,7 @@ task: folder: tests/servers/cosignerd fingerprint_script: git rev-parse HEAD:tests/servers/cosignerd servers_build_script: | - for server in "coordinatord" "miradord" "cosignerd"; do + for server in "miradord" "cosignerd"; do if [ ! -f tests/servers/$server/Cargo.lock ]; then git submodule update --init --recursive --remote ./tests/servers/$server cd ./tests/servers/$server @@ -57,7 +57,7 @@ task: fi done - deps_script: apt update && apt install -y postgresql python3 python3-pip + deps_script: apt update && apt install -y python3 python3-pip pip_cache: folder: ~/.cache/pip @@ -72,11 +72,6 @@ task: tar -xzf $ARCHIVE_NAME export BITCOIND_PATH=$BITCOIND_DIR_NAME/bin/bitcoind - # Setup the postgres instance for the servers - pg_ctlcluster 13 main start - su -c "psql -c \"CREATE ROLE test CREATEDB LOGIN PASSWORD 'test'\"" - postgres - export POSTGRES_USER=test POSTGRES_PASS=test - # Run the functional tests REVAULTD_PATH=$PWD/target/release/revaultd pytest $TEST_GROUP -vvv -n 2 From 2179813a2b347bb332f4655e9a879ed5472a9006 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 10 May 2022 22:42:36 +0200 Subject: [PATCH 8/8] qa: update the README to make Postgre optional, and some fixups --- tests/README.md | 47 ++++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/tests/README.md b/tests/README.md index edfcaeb9..b36f0d47 100644 --- a/tests/README.md +++ b/tests/README.md @@ -39,18 +39,7 @@ variables: `POSTGRES_USER`, `POSTGRES_PASS` and optionally `POSTGRES_HOST` (if n `localhost`). The framework will take care of creating a database for each process launched, and to drop it at teardown time. -#### Without the servers - -You can write and run tests that don't need the servers. By default, if no `POSTGRES_*` -environment variables are passed, the framework will skip the tests that depends on -servers: - -``` -# Adapt `-n`, `-v`, `timeout` and other environment variables to your needs -pytest tests/ -vvv -n4 --ignore tests/servers/ -``` - -#### With the servers +#### Compiling the servers For spinning up the servers, the framework will need access to the binaries. Therefore you must `init` the submodules (if you did not `clone` with `--recursive`) and compile the @@ -59,21 +48,41 @@ servers code: # From the root of the repository git submodule update --init --recursive cd tests/servers -cd coordinatord && cargo build && cd .. cd cosignerd && cargo build && cd .. cd miradord && cargo build && cd .. ``` -When you need a new version of the servers, you can update the submodules: +When you need a new version of the servers, you can update the submodules. You can use a +script from `contrib/` that updates the servers and recompiles them: ``` # From the root of the repository -cd tests/servers -git submodule update --remote --recursive +./contrib/recompile_tests_servers.sh +``` + +#### Using the real Coordinator + +By default, the functional tests will use a dummy in-RAM coordinator (see +[`test_framework/coordinatord.py`](test_framework/coordinatord.py). + +The tests can be ran using `coordinatord`, and some require its use. + +In order to use it you'll first need to compile it: +``` +# From the root of the repository +git submodule update --init --recursive +cd tests/servers/coordinatord && cargo build +cd ../../../ ``` -To run the server-requiring tests, pass the postgres credentials to the framework: +And then you'll need to set a Postgre backend up. The easiest way to do so is by using Docker: ``` -POSTGRES_USER="test" POSTGRES_PASS="test" TEST_DEBUG=1 pytest -vvv -n8 --timeout=1800 +docker run --rm -d -p 5432:5432 --name postgres-coordinatord -e POSTGRES_PASSWORD=revaultd_tests -e POSTGRES_USER=revaultd_tests -e POSTGRES_DB=coordinator_db postgres:alpine +``` + +To run the tests with, pass the postgres credentials to the framework: +``` +# From the root of the repository +POSTGRES_USER=revaultd_tests POSTGRES_PASS=revaultd_tests pytest -vvv --ignore tests/servers -n 10 ``` @@ -89,7 +98,7 @@ You can override the config at runtime with the `--log-cli-level` option: POSTGRES_USER=test POSTGRES_PASS=test pytest -vvv --log-cli-level=DEBUG -k test_getrevocationtxs ``` -Note that we log each daemon log, and we start them with `log_level = "trace"`. +Note that we log each daemon log if `VERBOSE=1`, and we start them with `log_level = "debug"`. #### Profiling