From 80e8773538d2ce1ff8d5adafd6c68430132ccef1 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 7 Nov 2025 16:33:08 +0000 Subject: [PATCH 01/12] Consistency TV to LONG_TESTS --- CMakeLists.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d4a0eefdad57..6ad86d86f5e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1073,11 +1073,6 @@ if(BUILD_TESTS) NAME connections PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/connections.py ) - add_e2e_test( - NAME consistency_trace_validation - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/consistency_trace_validation.py - ) - add_e2e_test( NAME fuzz_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/fuzzing.py ) @@ -1225,6 +1220,11 @@ if(BUILD_TESTS) add_e2e_test( NAME e2e_curl PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/e2e_curl.py ) + + add_e2e_test( + NAME consistency_trace_validation + PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/consistency_trace_validation.py + ) endif() endif() From db241e49b25b7bdcaa2d91a55712c5994f327fb9 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 7 Nov 2025 16:39:23 +0000 Subject: [PATCH 02/12] There is only 2024-07-01 --- CMakeLists.txt | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ad86d86f5e5..166b42f07d49 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -329,15 +329,6 @@ configure_file( @ONLY ) -file(READ ${CCF_DIR}/doc/schemas/gov/2023-06-01-preview/gov.json - GOV_API_SCHEMA_2023_06_01_PREVIEW -) -set_property( - DIRECTORY - APPEND - PROPERTY CMAKE_CONFIGURE_DEPENDS - ${CCF_DIR}/doc/schemas/gov/2023-06-01-preview/gov.json -) file(READ ${CCF_DIR}/doc/schemas/gov/2024-07-01/gov.json GOV_API_SCHEMA_2024_07_01 ) @@ -846,9 +837,9 @@ if(BUILD_TESTS) endif() add_e2e_test( - NAME recovery_test_api_1 + NAME recovery_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/recovery.py - ADDITIONAL_ARGS ${ADDITIONAL_RECOVERY_ARGS} --gov-api-version "2024-07-01" + ADDITIONAL_ARGS ${ADDITIONAL_RECOVERY_ARGS} ) add_e2e_test( @@ -1036,9 +1027,7 @@ if(BUILD_TESTS) ) add_e2e_test( - NAME membership_api_1 - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/membership.py - ADDITIONAL_ARGS --gov-api-version "2024-07-01" + NAME membership PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/membership.py ) add_e2e_test( From 1a9b8edb38b8438f87cb5180506ad0ec66cbcdf7 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 10 Nov 2025 14:16:36 +0000 Subject: [PATCH 03/12] Continue to test schema for 2023-06-01-preview for now --- CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 166b42f07d49..206f6286a1a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -329,6 +329,15 @@ configure_file( @ONLY ) +file(READ ${CCF_DIR}/doc/schemas/gov/2023-06-01-preview/gov.json + GOV_API_SCHEMA_2023_06_01_PREVIEW +) +set_property( + DIRECTORY + APPEND + PROPERTY CMAKE_CONFIGURE_DEPENDS + ${CCF_DIR}/doc/schemas/gov/2023-06-01-preview/gov.json +) file(READ ${CCF_DIR}/doc/schemas/gov/2024-07-01/gov.json GOV_API_SCHEMA_2024_07_01 ) From 3469a72d956a847b5285622ccfaacd368bffb8e6 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 12 Nov 2025 12:46:11 +0000 Subject: [PATCH 04/12] Clean-up legacy JWK (#7442) Co-authored-by: Eddy Ashton --- CHANGELOG.md | 2 + include/ccf/service/tables/jwt.h | 15 ----- samples/constitutions/default/actions.js | 25 +++++++ src/endpoints/authentication/jwt_auth.cpp | 56 ---------------- src/node/rpc/jwt_management.h | 48 -------------- src/service/network_tables.h | 15 +---- tests/infra/consortium.py | 9 +++ tests/lts_compatibility.py | 81 +++++++++++++++++++++-- 8 files changed, 114 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 166b2ff14452..7660f6ac1440 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Removed the unused experimental `ccf.host.triggerSubprocess()` JS API - Removed ACME client and support for ACME-endorsed interfaces (#7414). +- Removed fallback JWT authentication (#7442) + - It is recommended to clean up the old tables for services started before 6.x - check out `cleanup_legacy_jwt_records` proposal in the default sample constitution. ### Fixed diff --git a/include/ccf/service/tables/jwt.h b/include/ccf/service/tables/jwt.h index ec192cd25a90..595558622f69 100644 --- a/include/ccf/service/tables/jwt.h +++ b/include/ccf/service/tables/jwt.h @@ -71,21 +71,6 @@ namespace ccf static constexpr auto JWT_PUBLIC_SIGNING_KEYS_METADATA = "public:ccf.gov.jwt.public_signing_keys_metadata_v2"; - - namespace Legacy - { - static constexpr auto JWT_PUBLIC_SIGNING_KEYS = - "public:ccf.gov.jwt.public_signing_key"; - static constexpr auto JWT_PUBLIC_SIGNING_KEY_ISSUER = - "public:ccf.gov.jwt.public_signing_key_issuer"; - static constexpr auto JWT_PUBLIC_SIGNING_KEYS_METADATA = - "public:ccf.gov.jwt.public_signing_keys_metadata"; - - using JwtPublicSigningKeys = - ccf::kv::RawCopySerialisedMap; - using JwtPublicSigningKeyIssuer = - ccf::kv::RawCopySerialisedMap; - } } struct JsonWebKeySet diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index a58a552d2aba..a85be75d196e 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -1577,4 +1577,29 @@ const actions = new Map([ function (args) {}, ), ], + [ + "cleanup_legacy_jwt_records", + new Action( + function (args) { + checkType( + args.ensure_new_records_exist, + "boolean?", + "ensure_new_records_exist", + ); + }, + function (args) { + if ( + args.ensure_new_records_exist && + ccf.kv["public:ccf.gov.jwt.public_signing_keys_metadata_v2"].size === + 0 + ) { + throw new Error("No new JWT public signing keys records found"); + } + + ccf.kv["public:ccf.gov.jwt.public_signing_keys"].clear(); + ccf.kv["public:ccf.gov.jwt.public_signing_keys_metadata"].clear(); + ccf.kv["public:ccf.gov.jwt.public_signing_key_issuer"].clear(); + }, + ), + ], ]); diff --git a/src/endpoints/authentication/jwt_auth.cpp b/src/endpoints/authentication/jwt_auth.cpp index e75774e4ef90..15c96a8026eb 100644 --- a/src/endpoints/authentication/jwt_auth.cpp +++ b/src/endpoints/authentication/jwt_auth.cpp @@ -178,62 +178,6 @@ namespace ccf const auto key_id = token.header_typed.kid; auto token_keys = keys->get(key_id); - // For metadata KID->(cert,issuer,constraint). - // - // Note, that Legacy keys are stored as certs, new approach is raw keys, so - // conversion from cert to raw key is needed. - if (!token_keys) - { - auto* fallback_certs = tx.ro( - ccf::Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS_METADATA); - auto fallback_data = fallback_certs->get(key_id); - if (fallback_data) - { - auto new_keys = std::vector(); - for (const auto& metadata : *fallback_data) - { - auto verifier = ccf::crypto::make_unique_verifier(metadata.cert); - new_keys.push_back(OpenIDJWKMetadata{ - .public_key = verifier->public_key_der(), - .issuer = metadata.issuer, - .constraint = metadata.constraint}); - } - if (!new_keys.empty()) - { - token_keys = new_keys; - } - } - } - - // For metadata as two separate tables, KID->JwtIssuer and KID->Cert. - // - // Note, that Legacy keys are stored as certs, new approach is raw keys, so - // conversion from certs to keys is needed. - if (!token_keys) - { - auto* fallback_keys = tx.ro( - ccf::Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS); - auto* fallback_issuers = tx.ro( - ccf::Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER); - - auto fallback_cert = fallback_keys->get(key_id); - auto fallback_issuer = fallback_issuers->get(key_id); - if (fallback_cert) - { - if (!fallback_issuer) - { - error_reason = fmt::format( - "JWT signing key fallback issuers not found for kid {}", key_id); - return nullptr; - } - auto verifier = ccf::crypto::make_unique_verifier(*fallback_cert); - token_keys = std::vector{OpenIDJWKMetadata{ - .public_key = verifier->public_key_der(), - .issuer = fallback_issuer.value(), - .constraint = std::nullopt}}; - } - } - if (!token_keys || token_keys->empty()) { error_reason = diff --git a/src/node/rpc/jwt_management.h b/src/node/rpc/jwt_management.h index b50b0d031743..143487407a7d 100644 --- a/src/node/rpc/jwt_management.h +++ b/src/node/rpc/jwt_management.h @@ -122,49 +122,6 @@ namespace namespace ccf { - static void legacy_remove_jwt_public_signing_keys( - ccf::kv::Tx& tx, std::string issuer) - { - auto keys = tx.rw( - Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS); - auto key_issuer = tx.rw( - Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER); - - key_issuer->foreach( - [&issuer, &keys, &key_issuer](const auto& k, const auto& v) { - if (v == issuer) - { - keys->remove(k); - key_issuer->remove(k); - } - return true; - }); - - auto metadata = tx.rw( - Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS_METADATA); - metadata->foreach([&issuer, &metadata](const auto& k, const auto& v) { - std::vector updated; - for (const auto& key : v) - { - if (key.issuer != issuer) - { - updated.push_back(key); - } - } - - if (updated.empty()) - { - metadata->remove(k); - } - else if (updated.size() < v.size()) - { - metadata->put(k, updated); - } - - return true; - }); - } - static bool check_issuer_constraint( const std::string& issuer, const std::string& constraint) { @@ -199,11 +156,6 @@ namespace ccf static void remove_jwt_public_signing_keys( ccf::kv::Tx& tx, std::string issuer) { - // Unlike resetting JWT keys for a particular issuer, removing keys can be - // safely done on both table revisions, as soon as the application - // shouldn't use them anyway after being ask about that explicitly. - legacy_remove_jwt_public_signing_keys(tx, issuer); - auto keys = tx.rw( Tables::JWT_PUBLIC_SIGNING_KEYS_METADATA); diff --git a/src/service/network_tables.h b/src/service/network_tables.h index 36ab930fe926..679f6a5bcef7 100644 --- a/src/service/network_tables.h +++ b/src/service/network_tables.h @@ -164,24 +164,11 @@ namespace ccf const JwtIssuers jwt_issuers = {Tables::JWT_ISSUERS}; const JwtPublicSigningKeysMetadata jwt_public_signing_keys_metadata = { Tables::JWT_PUBLIC_SIGNING_KEYS_METADATA}; - const JwtPublicSigningKeysMetadataLegacy - legacy_jwt_public_signing_keys_metadata = { - Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS_METADATA}; - const Tables::Legacy::JwtPublicSigningKeys legacy_jwt_public_signing_keys = - {Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS}; - const Tables::Legacy::JwtPublicSigningKeyIssuer - legacy_jwt_public_signing_key_issuer = { - Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER}; inline auto get_all_jwt_tables() const { return std::make_tuple( - ca_cert_bundles, - jwt_issuers, - jwt_public_signing_keys_metadata, - legacy_jwt_public_signing_keys_metadata, - legacy_jwt_public_signing_keys, - legacy_jwt_public_signing_key_issuer); + ca_cert_bundles, jwt_issuers, jwt_public_signing_keys_metadata); } // diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index d8a3b9dcc205..d6746ea6174b 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -937,6 +937,15 @@ def force_ledger_chunk(self, remote_node): proposal = self.get_any_active_member().propose(remote_node, proposal_body) return self.vote_using_majority(remote_node, proposal, careful_vote) + def cleanup_legacy_jwt_records(self, remote_node, ensure_new_records_exist=False): + # Submit a proposal to remove legacy JWT records + proposal_body, careful_vote = self.make_proposal( + "cleanup_legacy_jwt_records", + ensure_new_records_exist=ensure_new_records_exist, + ) + proposal = self.get_any_active_member().propose(remote_node, proposal_body) + return self.vote_using_majority(remote_node, proposal, careful_vote) + def check_for_service(self, remote_node, status, recovery_count=None): """ Check the certificate associated with current CCF service signing key has been recorded in diff --git a/tests/lts_compatibility.py b/tests/lts_compatibility.py index 922f5a4176fd..c96ac020faea 100644 --- a/tests/lts_compatibility.py +++ b/tests/lts_compatibility.py @@ -12,6 +12,8 @@ import infra.platform_detection import suite.test_requirements as reqs import ccf.ledger +from ccf.tx_id import TxID +import time import os import json import datetime @@ -101,6 +103,7 @@ def test_new_service( library_dir, version, expected_subject_name=None, + test_jwt_cleanup=False, ): if infra.platform_detection.is_snp(): LOG.info( @@ -168,6 +171,67 @@ def test_new_service( # that did not record the now-deprecated "public:first_write_version" table network.txs.verify_range(log_capture=[], from_seqno=1) + if test_jwt_cleanup: + + def get_fresh_public_state(): + with primary.client() as c: + r = c.get("/node/commit") + target_seqno = TxID.from_str(r.body.json()["transaction_id"]).seqno + network.consortium.force_ledger_chunk(primary) + for _ in range(10): + ledger = ccf.ledger.Ledger( + primary.remote.ledger_paths(), committed_only=True + ) + public_state, last_seqno = ledger.get_latest_public_state() + if last_seqno >= target_seqno: + return public_state + + time.sleep(0.1) + else: + assert ( + False + ), f"Failed to up-to-date ledger state, seqno needed: {target_seqno}, last seqno: {last_seqno}" + + def table_has_entries(table_name, public_state): + rows = public_state.get(table_name, None) + return rows is not None and len(rows) > 0 + + legacy_tables = [ + "public:ccf.gov.jwt.public_signing_keys", + "public:ccf.gov.jwt.public_signing_keys_metadata", + "public:ccf.gov.jwt.public_signing_key_issuer", + ] + new_table = "public:ccf.gov.jwt.public_signing_keys_metadata_v2" + + public_state = get_fresh_public_state() + assert all(table_has_entries(table, public_state) for table in legacy_tables) + assert table_has_entries(new_table, public_state) + + network.consortium.cleanup_legacy_jwt_records( + primary, ensure_new_records_exist=True + ) + + public_state = get_fresh_public_state() + assert all( + not table_has_entries(table, public_state) for table in legacy_tables + ) + + # Cannot remove legacy if the current table is not populated but required to be + network.consortium.remove_jwt_issuer(primary, network.jwt_issuer.issuer_url) + public_state = get_fresh_public_state() + assert not table_has_entries(new_table, public_state) + try: + network.consortium.cleanup_legacy_jwt_records( + primary, ensure_new_records_exist=True + ) + except infra.proposal.ProposalNotAccepted: + pass + + # Although can remove it if not ensuring explicitly new records exist + network.consortium.cleanup_legacy_jwt_records( + primary, ensure_new_records_exist=False + ) + # Local build and install bin/ and lib/ directories differ def get_bin_and_lib_dirs_for_install_path(install_path): @@ -443,7 +507,7 @@ def run_code_upgrade_from( to_binary_dir, to_library_dir, to_version, - service_subject_name, + expected_subject_name=service_subject_name, ) network.get_latest_ledger_public_state() @@ -490,7 +554,9 @@ def run_live_compatibility_with_latest( @reqs.description("Run ledger compatibility since first LTS") -def run_ledger_compatibility_since_first(args, local_branch, use_snapshot): +def run_ledger_compatibility_since_first( + args, local_branch, use_snapshot, test_jwt_cleanup +): """ Tests that a service from the very first LTS can be recovered to the next LTS, and so forth, until the version of the local checkout. @@ -654,6 +720,7 @@ def run_ledger_compatibility_since_first(args, local_branch, use_snapshot): binary_dir, library_dir, version, + test_jwt_cleanup=test_jwt_cleanup, ) snapshots_dir = ( @@ -752,13 +819,19 @@ def add(parser): if args.check_ledger_compatibility: compatibility_report["data compatibility"] = {} lts_versions = run_ledger_compatibility_since_first( - args, local_branch, use_snapshot=False + args, + local_branch, + use_snapshot=False, + test_jwt_cleanup=False, ) compatibility_report["data compatibility"].update( {"with previous ledger": lts_versions} ) lts_versions = run_ledger_compatibility_since_first( - args, local_branch, use_snapshot=True + args, + local_branch, + use_snapshot=True, + test_jwt_cleanup=True, ) compatibility_report["data compatibility"].update( {"with previous snapshots": lts_versions} From 1f336785056a5590897c22cf38bb70d3bbeba93d Mon Sep 17 00:00:00 2001 From: cjen1-msft Date: Wed, 12 Nov 2025 14:52:12 +0000 Subject: [PATCH 05/12] Ensure that pre-vote messages are binary compatible with pre-pre-vote messages (#7445) --- .github/workflows/ci-verification.yml | 24 +- .github/workflows/long-verification.yml | 6 +- CHANGELOG.md | 2 +- doc/architecture/raft_tla.rst | 2 +- src/consensus/aft/raft.h | 309 ++++++++++++++++-------- src/consensus/aft/raft_types.h | 100 +++++++- src/consensus/aft/test/driver.h | 58 ++++- tla/consensus/Traceccfraft.tla | 9 +- tla/consensus/ccfraft.tla | 6 + tla/install_deps.py | 29 ++- 10 files changed, 392 insertions(+), 153 deletions(-) diff --git a/.github/workflows/ci-verification.yml b/.github/workflows/ci-verification.yml index 196899a4ad02..c3e513bd9a4f 100644 --- a/.github/workflows/ci-verification.yml +++ b/.github/workflows/ci-verification.yml @@ -41,7 +41,7 @@ jobs: - name: Install TLC dependencies run: | tdnf install -y jre wget - python3 tla/install_deps.py --skip-apt-packages + python3 tla/install_deps.py - run: cd tla && ./tlc.py mc consistency/MCSingleNode.tla - run: cd tla && ./tlc.py mc consistency/MCSingleNodeReads.tla @@ -70,7 +70,7 @@ jobs: - name: Install TLC dependencies run: | sudo apt update - sudo apt install -y default-jre + sudo apt install -y default-jre wget python3 install_deps.py - run: ./tlc_debug.sh --config consistency/MCSingleNodeCommitReachability.cfg mc consistency/MCSingleNodeReads.tla @@ -90,7 +90,7 @@ jobs: - name: Install TLC dependencies run: | sudo apt update - sudo apt install -y default-jre + sudo apt install -y default-jre wget python3 install_deps.py - run: ./tlc.py sim --num 500 --depth 50 consistency/MultiNodeReads.tla @@ -128,7 +128,7 @@ jobs: - name: Install TLC dependencies run: | tdnf install -y jre wget - python3 tla/install_deps.py --skip-apt-packages + python3 tla/install_deps.py - run: cd tla && ./tlc.py mc consensus/MCabs.tla - run: cd tla && ./tlc.py --trace-name 1C2N mc --term-count 2 --request-count 2 --raft-configs 1C2N consensus/MCccfraft.tla @@ -155,7 +155,7 @@ jobs: - name: Install TLC dependencies run: | sudo apt update - sudo apt install -y default-jre + sudo apt install -y default-jre wget python3 install_deps.py - run: ./tlc.py sim consensus/SIMccfraft.tla @@ -193,22 +193,16 @@ jobs: with: fetch-depth: 0 - - name: Install TLC dependencies - run: | - tdnf install -y jre wget - python3 tla/install_deps.py --skip-apt-packages - - name: "Install dependencies" shell: bash run: | set -ex ./scripts/setup-ci.sh - # Parallel - wget https://ftp.gnu.org/gnu/parallel/parallel-latest.tar.bz2 - tar -xjf parallel-latest.tar.bz2 - cd $(ls | grep 'parallel' | grep -v 'tar' | grep -v 'rpm') - ./configure && make && make install + - name: Install TLC dependencies + run: | + tdnf install -y jre wget + python3 tla/install_deps.py --tdnf-extended - name: "Build" run: | diff --git a/.github/workflows/long-verification.yml b/.github/workflows/long-verification.yml index 09bd4126accc..bc8afc2d21bc 100644 --- a/.github/workflows/long-verification.yml +++ b/.github/workflows/long-verification.yml @@ -39,7 +39,7 @@ jobs: - name: Install TLC dependencies run: | tdnf install -y jre wget - python3 tla/install_deps.py --skip-apt-packages + python3 tla/install_deps.py - run: cd tla && ./tlc.py --trace-name 2C2N mc --term-count 2 --request-count 0 --raft-configs 2C2N --disable-check-quorum consensus/MCccfraft.tla @@ -77,7 +77,7 @@ jobs: - name: Install TLC dependencies run: | tdnf install -y jre wget - python3 tla/install_deps.py --skip-apt-packages + python3 tla/install_deps.py - run: cd tla && ./tlc.py --trace-name 3C2N mc --term-count 2 --request-count 0 --raft-configs 3C2N --disable-check-quorum consensus/MCccfraft.tla @@ -102,7 +102,7 @@ jobs: - uses: actions/checkout@v5 - run: | sudo apt update - sudo apt install -y default-jre + sudo apt install -y default-jre wget python3 install_deps.py - run: ./tlc.py sim --max-seconds 3000 --depth 500 consensus/SIMccfraft.tla diff --git a/CHANGELOG.md b/CHANGELOG.md index 7660f6ac1440..b59a2afb0bcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -- Support for PreVote optimisation. Nodes understand and are able to respond to PreVote messages, but will not become pre-vote candidates themselves. (#7419) +- Support for PreVote optimisation. Nodes understand and are able to respond to PreVote messages, but will not become pre-vote candidates themselves. (#7419, #7445) ### Changed diff --git a/doc/architecture/raft_tla.rst b/doc/architecture/raft_tla.rst index e3f5b11ed30a..fdde71565601 100644 --- a/doc/architecture/raft_tla.rst +++ b/doc/architecture/raft_tla.rst @@ -44,7 +44,7 @@ It is possible to produce fresh traces quickly from the driver by running the `` Calling the trace validation on, for example, the ``append`` scenario can then be done with ``./tlc.py --driver-trace ../build/append.ndjson consensus/Traceccfraft.tla``. -Generating a trace of a scenario and validate it in one go can be done with ``./tlc.py --workers 1 tv --scenario ../tests/raft_scenarios/append consensus/Traceccfraft.tla``. +Generating a trace of a scenario and validating it in one go can be done with ``./tlc.py --workers 1 tv --scenario ../tests/raft_scenarios/append consensus/Traceccfraft.tla``. This runs the raft_driver on the scenario, cleans the trace and then validates it against the TLA+ specification. CCF also provides a command line trace visualizer to aid debugging, for example, the ``append`` scenario can be visualized with ``python ../tests/trace_viz.py ../build/append.ndjson``. diff --git a/src/consensus/aft/raft.h b/src/consensus/aft/raft.h index db8bb15fa5ef..8d92f239da6a 100644 --- a/src/consensus/aft/raft.h +++ b/src/consensus/aft/raft.h @@ -772,6 +772,15 @@ namespace aft break; } + case raft_request_pre_vote: + { + RequestPreVote r = + channels->template recv_authenticated( + from, data, size); + recv_request_pre_vote(from, r); + break; + } + case raft_request_vote: { RequestVote r = channels->template recv_authenticated( @@ -780,6 +789,15 @@ namespace aft break; } + case raft_request_pre_vote_response: + { + RequestPreVoteResponse r = + channels->template recv_authenticated( + from, data, size); + recv_request_pre_vote_response(from, r); + break; + } + case raft_request_vote_response: { RequestVoteResponse r = @@ -1040,16 +1058,6 @@ namespace aft const auto prev_term = get_term_internal(prev_idx); const auto term_of_idx = get_term_internal(end_idx); - RAFT_DEBUG_FMT( - "Send append entries from {} to {}: ({}.{}, {}.{}] ({})", - state->node_id, - to, - prev_term, - prev_idx, - term_of_idx, - end_idx, - state->commit_idx); - #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wc99-designator" AppendEntries ae{ @@ -1062,6 +1070,17 @@ namespace aft }; #pragma clang diagnostic pop + RAFT_DEBUG_FMT( + "Send {} from {} to {}: ({}.{}, {}.{}] ({})", + ae.msg, + state->node_id, + to, + prev_term, + prev_idx, + term_of_idx, + end_idx, + state->commit_idx); + auto& node = all_other_nodes.at(to); #ifdef CCF_RAFT_TRACING @@ -1097,12 +1116,14 @@ namespace aft std::unique_lock guard(state->lock); RAFT_DEBUG_FMT( - "Received append entries: {}.{} to {}.{} (from {} in term {})", + "Recv {} to {} from {}: {}.{} to {}.{} in term {}", + r.msg, + state->node_id, + from, r.prev_term, r.prev_idx, r.term_of_idx, r.idx, - from, r.term); #ifdef CCF_RAFT_TRACING @@ -1136,7 +1157,8 @@ namespace aft { // Reply false, since our term is later than the received term. RAFT_INFO_FMT( - "Recv append entries to {} from {} but our term is later ({} > {})", + "Recv {} to {} from {} but our term is later ({} > {})", + r.msg, state->node_id, from, state->current_view, @@ -1157,8 +1179,9 @@ namespace aft if (prev_term == 0) { RAFT_DEBUG_FMT( - "Recv append entries to {} from {} but our log does not yet " + "Recv {} to {} from {} but our log does not yet " "contain index {}", + r.msg, state->node_id, from, r.prev_idx); @@ -1167,8 +1190,9 @@ namespace aft else { RAFT_DEBUG_FMT( - "Recv append entries to {} from {} but our log at {} has the wrong " + "Recv {} to {} from {} but our log at {} has the wrong " "previous term (ours: {}, theirs: {})", + r.msg, state->node_id, from, r.prev_idx, @@ -1194,8 +1218,9 @@ namespace aft if (r.prev_idx < state->commit_idx) { RAFT_DEBUG_FMT( - "Recv append entries to {} from {} but prev_idx ({}) < commit_idx " + "Recv {} to {} from {} but prev_idx ({}) < commit_idx " "({})", + r.msg, state->node_id, from, r.prev_idx, @@ -1209,7 +1234,8 @@ namespace aft else if (r.prev_idx > state->last_idx) { RAFT_FAIL_FMT( - "Recv append entries to {} from {} but prev_idx ({}) > last_idx ({})", + "Recv {} to {} from {} but prev_idx ({}) > last_idx ({})", + r.msg, state->node_id, from, r.prev_idx, @@ -1218,7 +1244,8 @@ namespace aft } RAFT_DEBUG_FMT( - "Recv append entries to {} from {} for index {} and previous index {}", + "Recv {} to {} from {} for index {} and previous index {}", + r.msg, state->node_id, from, r.idx, @@ -1300,7 +1327,8 @@ namespace aft { // This should only fail if there is malformed data. RAFT_FAIL_FMT( - "Recv append entries to {} from {} but the data is malformed: {}", + "Recv {} to {} from {} but the data is malformed: {}", + r.msg, state->node_id, from, e.what()); @@ -1313,8 +1341,9 @@ namespace aft if (ds == nullptr) { RAFT_FAIL_FMT( - "Recv append entries to {} from {} but the entry could not be " + "Recv {} to {} from {} but the entry could not be " "deserialised", + r.msg, state->node_id, from); send_append_entries_response_nack(from); @@ -1513,19 +1542,20 @@ namespace aft aft::Term response_term, aft::Index response_idx) { - RAFT_DEBUG_FMT( - "Send append entries response from {} to {} for index {}: {}", - state->node_id, - to, - response_idx, - (answer == AppendEntriesResponseType::OK ? "ACK" : "NACK")); - AppendEntriesResponse response{ .term = response_term, .last_log_idx = response_idx, .success = answer, }; + RAFT_DEBUG_FMT( + "Send {} from {} to {} for index {}: {}", + response.msg, + state->node_id, + to, + response_idx, + (answer == AppendEntriesResponseType::OK ? "ACK" : "NACK")); + #ifdef CCF_RAFT_TRACING nlohmann::json j = {}; j["function"] = "send_append_entries_response"; @@ -1572,7 +1602,8 @@ namespace aft if (state->leadership_state != ccf::kv::LeadershipState::Leader) { RAFT_INFO_FMT( - "Recv append entries response to {} from {}: no longer leader", + "Recv {} to {} from {}: no longer leader", + r.msg, state->node_id, from); return; @@ -1585,8 +1616,9 @@ namespace aft { // We are behind, update our state. RAFT_DEBUG_FMT( - "Recv append entries response to {} from {}: more recent term ({} " + "Recv {} to {} from {}: more recent term ({} " "> {})", + r.msg, state->node_id, from, r.term, @@ -1604,7 +1636,8 @@ namespace aft if (r.success == AppendEntriesResponseType::OK) { RAFT_DEBUG_FMT( - "Recv append entries response to {} from {}: stale term ({} != {})", + "Recv {} to {} from {}: stale term ({} != {})", + r.msg, state->node_id, from, r.term, @@ -1622,9 +1655,7 @@ namespace aft if (r.success == AppendEntriesResponseType::OK) { RAFT_DEBUG_FMT( - "Recv append entries response to {} from {}: stale idx", - state->node_id, - from); + "Recv {} to {} from {}: stale idx", r.msg, state->node_id, from); return; } } @@ -1634,9 +1665,7 @@ namespace aft { // Failed due to log inconsistency. Reset sent_idx, and try again soon. RAFT_DEBUG_FMT( - "Recv append entries response to {} from {}: failed", - state->node_id, - from); + "Recv {} to {} from {}: failed", r.msg, state->node_id, from); const auto this_match = find_highest_possible_match({r.term, r.last_log_idx}); node->second.sent_idx = std::max( @@ -1652,29 +1681,48 @@ namespace aft } RAFT_DEBUG_FMT( - "Recv append entries response to {} from {} for index {}: success", + "Recv {} to {} from {} for index {}: success", + r.msg, state->node_id, from, r.last_log_idx); update_commit(); } - void send_request_vote(const ccf::NodeId& to, ElectionType election_type) + void send_request_pre_vote(const ccf::NodeId& to) + { + auto last_committable_idx = last_committable_index(); + CCF_ASSERT(last_committable_idx >= state->commit_idx, "lci < ci"); + + RequestPreVote rpv{ + .term = state->current_view, + .last_committable_idx = last_committable_idx, + .term_of_last_committable_idx = + get_term_internal(last_committable_idx)}; + +#ifdef CCF_RAFT_TRACING + nlohmann::json j = {}; + j["function"] = "send_request_vote"; + j["packet"] = rpv; + j["state"] = *state; + COMMITTABLE_INDICES(j["state"], state); + j["to_node_id"] = to; + RAFT_TRACE_JSON_OUT(j); +#endif + + channels->send_authenticated(to, ccf::NodeMsgType::consensus_msg, rpv); + } + + void send_request_vote(const ccf::NodeId& to) { auto last_committable_idx = last_committable_index(); - RAFT_INFO_FMT( - "Send {}request vote from {} to {} at {}", - election_type == ElectionType::PreVote ? "pre-vote " : "", - state->node_id, - to, - last_committable_idx); CCF_ASSERT(last_committable_idx >= state->commit_idx, "lci < ci"); RequestVote rv{ .term = state->current_view, .last_committable_idx = last_committable_idx, - .term_of_last_committable_idx = get_term_internal(last_committable_idx), - .election_type = election_type}; + .term_of_last_committable_idx = + get_term_internal(last_committable_idx)}; #ifdef CCF_RAFT_TRACING nlohmann::json j = {}; @@ -1689,10 +1737,9 @@ namespace aft channels->send_authenticated(to, ccf::NodeMsgType::consensus_msg, rv); } - void recv_request_vote(const ccf::NodeId& from, RequestVote r) + void recv_request_vote_unsafe( + const ccf::NodeId& from, RequestVote r, ElectionType election_type) { - std::lock_guard guard(state->lock); - // Do not check that from is a known node. It is possible to receive // RequestVotes from nodes that this node doesn't yet know, just as it // receives AppendEntries from those nodes. These should be obeyed just @@ -1700,32 +1747,24 @@ namespace aft // produce a primary in the new term, who will then help this node catch // up. -#ifdef CCF_RAFT_TRACING - nlohmann::json j = {}; - j["function"] = "recv_request_vote"; - j["packet"] = r; - j["state"] = *state; - COMMITTABLE_INDICES(j["state"], state); - j["from_node_id"] = from; - RAFT_TRACE_JSON_OUT(j); -#endif - if (state->current_view > r.term) { // Reply false, since our term is later than the received term. RAFT_DEBUG_FMT( - "Recv request vote to {} from {}: our term is later ({} > {})", + "Recv {} to {} from {}: our term is later ({} > {})", + r.msg, state->node_id, from, state->current_view, r.term); - send_request_vote_response(from, false, r.election_type); + send_request_vote_response(from, false, election_type); return; } if (state->current_view < r.term) { RAFT_DEBUG_FMT( - "Recv request vote to {} from {}: their term is later ({} < {})", + "Recv {} to {} from {}: their term is later ({} < {})", + r.msg, state->node_id, from, state->current_view, @@ -1740,12 +1779,12 @@ namespace aft bool grant_vote = true; - if ( - (r.election_type == ElectionType::RegularVote) && leader_id.has_value()) + if ((election_type == ElectionType::RegularVote) && leader_id.has_value()) { // Reply false, since we already know the leader in the current term. RAFT_DEBUG_FMT( - "Recv request vote to {} from {}: leader {} already known in term {}", + "Recv {} to {} from {}: leader {} already known in term {}", + r.msg, state->node_id, from, leader_id.value(), @@ -1755,11 +1794,12 @@ namespace aft auto voted_for_other = (voted_for.has_value()) && (voted_for.value() != from); - if ((r.election_type == ElectionType::RegularVote) && voted_for_other) + if ((election_type == ElectionType::RegularVote) && voted_for_other) { // Reply false, since we already voted for someone else. RAFT_DEBUG_FMT( - "Recv request vote to {} from {}: already voted for {}", + "Recv {} to {} from {}: already voted for {}", + r.msg, state->node_id, from, voted_for.value()); @@ -1779,8 +1819,9 @@ namespace aft if (!log_up_to_date) { RAFT_DEBUG_FMT( - "Request vote to {} from {}: candidate log {}.{} is not up-to-date " + "Recv {} to {} from {}: candidate log {}.{} is not up-to-date " "with ours {}.{}", + r.msg, state->node_id, from, r.term_of_last_committable_idx, @@ -1790,7 +1831,7 @@ namespace aft grant_vote = false; } - if (grant_vote && r.election_type == ElectionType::RegularVote) + if (grant_vote && election_type == ElectionType::RegularVote) { // If we grant our vote to a candidate, then an election is in progress restart_election_timeout(); @@ -1799,9 +1840,9 @@ namespace aft } RAFT_INFO_FMT( - "Request {}vote to {} from {}: {} vote to candidate at {}.{} with " + "Recv {} to {} from {}: {} vote to candidate at {}.{} with " "local state at {}.{}", - r.election_type == ElectionType::PreVote ? "pre-" : "", + r.msg, state->node_id, from, grant_vote ? "granted" : "denied", @@ -1810,30 +1851,89 @@ namespace aft term_of_last_committable_idx, last_committable_idx); - send_request_vote_response(from, grant_vote, r.election_type); + send_request_vote_response(from, grant_vote, election_type); + } + + void recv_request_vote(const ccf::NodeId& from, RequestVote r) + { + std::lock_guard guard(state->lock); + +#ifdef CCF_RAFT_TRACING + nlohmann::json j = {}; + j["function"] = "recv_request_vote"; + j["packet"] = r; + j["state"] = *state; + COMMITTABLE_INDICES(j["state"], state); + j["from_node_id"] = from; + RAFT_TRACE_JSON_OUT(j); +#endif + + recv_request_vote_unsafe(from, r, ElectionType::RegularVote); + } + + void recv_request_pre_vote(const ccf::NodeId& from, RequestPreVote r) + { + std::lock_guard guard(state->lock); + +#ifdef CCF_RAFT_TRACING + nlohmann::json j = {}; + j["function"] = "recv_request_vote"; + j["packet"] = r; + j["state"] = *state; + COMMITTABLE_INDICES(j["state"], state); + j["from_node_id"] = from; + RAFT_TRACE_JSON_OUT(j); +#endif + + // A pre-vote is a speculative request vote, so we translate it back to a + // RequestVote to avoid duplicating the logic. + RequestVote rv{ + .term = r.term, + .last_committable_idx = r.last_committable_idx, + .term_of_last_committable_idx = r.term_of_last_committable_idx, + }; + recv_request_vote_unsafe(from, rv, ElectionType::PreVote); } void send_request_vote_response( const ccf::NodeId& to, bool answer, ElectionType election_type) { - RAFT_INFO_FMT( - "Send request {}vote response from {} to {}: {}", - election_type == ElectionType::PreVote ? "pre-" : "", - state->node_id, - to, - answer); + if (election_type == ElectionType::RegularVote) + { + RequestVoteResponse response{ + .term = state->current_view, .vote_granted = answer}; - RequestVoteResponse response{ - .term = state->current_view, - .vote_granted = answer, - .election_type = election_type}; + RAFT_INFO_FMT( + "Send {} from {} to {}: {}", + response.msg, + state->node_id, + to, + answer); - channels->send_authenticated( - to, ccf::NodeMsgType::consensus_msg, response); + channels->send_authenticated( + to, ccf::NodeMsgType::consensus_msg, response); + } + else + { + RequestPreVoteResponse response{ + .term = state->current_view, .vote_granted = answer}; + + RAFT_INFO_FMT( + "Send {} from {} to {}: {}", + response.msg, + state->node_id, + to, + answer); + + channels->send_authenticated( + to, ccf::NodeMsgType::consensus_msg, response); + } } void recv_request_vote_response( - const ccf::NodeId& from, RequestVoteResponse r) + const ccf::NodeId& from, + RequestVoteResponse r, + ElectionType election_type) { std::lock_guard guard(state->lock); @@ -1852,7 +1952,8 @@ namespace aft state->leadership_state != ccf::kv::LeadershipState::Candidate) { RAFT_INFO_FMT( - "Recv request vote response to {} from: {}: we aren't a candidate", + "Recv {} to {} from: {}: we aren't a candidate", + r.msg, state->node_id, from); return; @@ -1860,11 +1961,12 @@ namespace aft // Stale message if ( - r.election_type == ElectionType::PreVote && + election_type == ElectionType::PreVote && state->leadership_state == ccf::kv::LeadershipState::Candidate) { RAFT_INFO_FMT( - "Recv pre-vote response to {} from {}: no longer in pre-vote", + "Recv {} to {} from {}: no longer in pre-vote", + r.msg, state->node_id, from); return; @@ -1877,12 +1979,13 @@ namespace aft // while still in PreVoteCandidate state something illegal must have // happened. if ( - r.election_type == ElectionType::RegularVote && + election_type == ElectionType::RegularVote && state->leadership_state == ccf::kv::LeadershipState::PreVoteCandidate) { RAFT_FAIL_FMT( - "Recv vote response to {} from {}: We should not yet sent a request " - "vote", + "Recv {} to {} from {}: We should not yet have sent a request " + "vote, as we are still a PreVoteCandidate yet received a response", + r.msg, state->node_id, from); return; @@ -1893,17 +1996,16 @@ namespace aft if (node == all_other_nodes.end()) { RAFT_INFO_FMT( - "Recv request vote response to {} from {}: unknown node", - state->node_id, - from); + "Recv {} to {} from {}: unknown node", r.msg, state->node_id, from); return; } if (state->current_view < r.term) { RAFT_INFO_FMT( - "Recv request vote response to {} from {}: their term is more recent " + "Recv {} to {} from {}: their term is more recent " "({} < {})", + r.msg, state->node_id, from, state->current_view, @@ -1940,6 +2042,19 @@ namespace aft add_vote_for_me(from); } + void recv_request_vote_response( + const ccf::NodeId& from, RequestVoteResponse r) + { + recv_request_vote_response(from, r, ElectionType::RegularVote); + } + + void recv_request_pre_vote_response( + const ccf::NodeId& from, RequestPreVoteResponse r) + { + RequestVoteResponse rvr{.term = r.term, .vote_granted = r.vote_granted}; + recv_request_vote_response(from, rvr, ElectionType::PreVote); + } + void recv_propose_request_vote( const ccf::NodeId& from, ProposeRequestVote r) { @@ -2020,7 +2135,7 @@ namespace aft for (auto const& node_id : other_nodes_in_active_configs()) { // ccfraft!RequestVote - send_request_vote(node_id, ElectionType::PreVote); + send_request_pre_vote(node_id); } } @@ -2067,7 +2182,7 @@ namespace aft for (auto const& node_id : other_nodes_in_active_configs()) { // ccfraft!RequestVote - send_request_vote(node_id, ElectionType::RegularVote); + send_request_vote(node_id); } } diff --git a/src/consensus/aft/raft_types.h b/src/consensus/aft/raft_types.h index 89f022c2c8b7..803d24ba09c8 100644 --- a/src/consensus/aft/raft_types.h +++ b/src/consensus/aft/raft_types.h @@ -102,6 +102,8 @@ namespace aft raft_request_vote, raft_request_vote_response, raft_propose_request_vote, + raft_request_pre_vote, + raft_request_pre_vote_response, }; DECLARE_JSON_ENUM( RaftMsgType, @@ -114,6 +116,9 @@ namespace aft {RaftMsgType::raft_request_vote, "raft_request_vote"}, {RaftMsgType::raft_request_vote_response, "raft_request_vote_response"}, {RaftMsgType::raft_propose_request_vote, "raft_propose_request_vote"}, + {RaftMsgType::raft_request_pre_vote, "raft_request_pre_vote"}, + {RaftMsgType::raft_request_pre_vote_response, + "raft_request_pre_vote_response"}, }); #pragma pack(push, 1) @@ -183,9 +188,6 @@ namespace aft DECLARE_JSON_REQUIRED_FIELDS( AppendEntriesResponse, term, last_log_idx, success); - DECLARE_JSON_TYPE(RaftHeader) - DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg) - enum ElectionType { PreVote = 0, @@ -195,18 +197,31 @@ namespace aft ElectionType, {{ElectionType::PreVote, "PreVote"}, {ElectionType::RegularVote, "RegularVote"}}); + + DECLARE_JSON_TYPE(RaftHeader) + DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg) struct RequestVote : RaftHeader { Term term; Index last_committable_idx; Term term_of_last_committable_idx; - ElectionType election_type = RegularVote; }; - DECLARE_JSON_TYPE_WITH_BASE_AND_OPTIONAL_FIELDS( - RequestVote, RaftHeader); + DECLARE_JSON_TYPE_WITH_BASE(RequestVote, RaftHeader); DECLARE_JSON_REQUIRED_FIELDS( RequestVote, term, last_committable_idx, term_of_last_committable_idx); - DECLARE_JSON_OPTIONAL_FIELDS(RequestVote, election_type) + + DECLARE_JSON_TYPE(RaftHeader); + DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg); + struct RequestPreVote : RaftHeader + { + Term term; + Index last_committable_idx; + Term term_of_last_committable_idx; + }; + DECLARE_JSON_TYPE_WITH_BASE( + RequestPreVote, RaftHeader); + DECLARE_JSON_REQUIRED_FIELDS( + RequestPreVote, term, last_committable_idx, term_of_last_committable_idx); DECLARE_JSON_TYPE(RaftHeader) DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg) @@ -214,12 +229,21 @@ namespace aft { Term term; bool vote_granted; - ElectionType election_type = RegularVote; }; - DECLARE_JSON_TYPE_WITH_BASE_AND_OPTIONAL_FIELDS( + DECLARE_JSON_TYPE_WITH_BASE( RequestVoteResponse, RaftHeader); DECLARE_JSON_REQUIRED_FIELDS(RequestVoteResponse, term, vote_granted); - DECLARE_JSON_OPTIONAL_FIELDS(RequestVoteResponse, election_type) + + DECLARE_JSON_TYPE(RaftHeader) + DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg) + struct RequestPreVoteResponse : RaftHeader + { + Term term; + bool vote_granted; + }; + DECLARE_JSON_TYPE_WITH_BASE( + RequestPreVoteResponse, RaftHeader); + DECLARE_JSON_REQUIRED_FIELDS(RequestPreVoteResponse, term, vote_granted); DECLARE_JSON_TYPE(RaftHeader) DECLARE_JSON_REQUIRED_FIELDS(RaftHeader, msg) @@ -235,3 +259,59 @@ namespace aft #pragma pack(pop) } + +FMT_BEGIN_NAMESPACE +template <> +struct formatter +{ + template + constexpr auto parse(ParseContext& ctx) + { + return ctx.begin(); + } + + template + auto format(const aft::RaftMsgType& msg_type, FormatContext& ctx) const + -> decltype(ctx.out()) + { + switch (msg_type) + { + case (aft::RaftMsgType::raft_append_entries): + { + return fmt::format_to(ctx.out(), "append_entries"); + } + case (aft::RaftMsgType::raft_append_entries_response): + { + return fmt::format_to(ctx.out(), "append_entries_response"); + } + case (aft::RaftMsgType::raft_append_entries_signed_response): + { + return fmt::format_to(ctx.out(), "append_entries_signed_response"); + } + case (aft::RaftMsgType::raft_request_vote): + { + return fmt::format_to(ctx.out(), "request_vote"); + } + case (aft::RaftMsgType::raft_request_vote_response): + { + return fmt::format_to(ctx.out(), "request_vote_response"); + } + case (aft::RaftMsgType::raft_propose_request_vote): + { + return fmt::format_to(ctx.out(), "propose_request_vote"); + } + case (aft::RaftMsgType::raft_request_pre_vote): + { + return fmt::format_to(ctx.out(), "request_pre_vote"); + } + case (aft::RaftMsgType::raft_request_pre_vote_response): + { + return fmt::format_to(ctx.out(), "request_pre_vote_response"); + } + default: + throw std::runtime_error( + fmt::format("Unhandled RaftMsgType: {}", uint64_t(msg_type))); + } + } +}; +FMT_END_NAMESPACE diff --git a/src/consensus/aft/test/driver.h b/src/consensus/aft/test/driver.h index 70f5ff27c0e4..9e1217b0d601 100644 --- a/src/consensus/aft/test/driver.h +++ b/src/consensus/aft/test/driver.h @@ -3,6 +3,7 @@ #pragma once #include "consensus/aft/raft.h" +#include "consensus/aft/raft_types.h" #include "ds/internal_logger.h" #include "logging_stub.h" @@ -390,6 +391,21 @@ class RaftDriver "{}--{}{}: {}", first, (dropped ? "X" : ">>"), second, message); } + void log_msg_details( + ccf::NodeId node_id, + ccf::NodeId tgt_node_id, + aft::RequestPreVote rv, + bool dropped) + { + const auto s = fmt::format( + "{} for term {}, at tx {}.{}", + rv.msg, + rv.term, + rv.term_of_last_committable_idx, + rv.last_committable_idx); + log(node_id, tgt_node_id, s, dropped); + } + void log_msg_details( ccf::NodeId node_id, ccf::NodeId tgt_node_id, @@ -397,13 +413,25 @@ class RaftDriver bool dropped) { const auto s = fmt::format( - "request_vote for term {}, at tx {}.{}", + "{} for term {}, at tx {}.{}", + rv.msg, rv.term, rv.term_of_last_committable_idx, rv.last_committable_idx); log(node_id, tgt_node_id, s, dropped); } + void log_msg_details( + ccf::NodeId node_id, + ccf::NodeId tgt_node_id, + aft::RequestPreVoteResponse rv, + bool dropped) + { + const auto s = fmt::format( + "{} for term {} = {}", rv.msg, rv.term, (rv.vote_granted ? "Y" : "N")); + rlog(node_id, tgt_node_id, s, dropped); + } + void log_msg_details( ccf::NodeId node_id, ccf::NodeId tgt_node_id, @@ -411,9 +439,7 @@ class RaftDriver bool dropped) { const auto s = fmt::format( - "request_vote_response for term {} = {}", - rv.term, - (rv.vote_granted ? "Y" : "N")); + "{} for term {} = {}", rv.msg, rv.term, (rv.vote_granted ? "Y" : "N")); rlog(node_id, tgt_node_id, s, dropped); } @@ -424,7 +450,8 @@ class RaftDriver bool dropped) { const auto s = fmt::format( - "append_entries ({}.{}, {}.{}] (term {}, commit {})", + "{} ({}.{}, {}.{}] (term {}, commit {})", + ae.msg, ae.prev_term, ae.prev_idx, ae.term_of_idx, @@ -455,10 +482,7 @@ class RaftDriver } } const auto s = fmt::format( - "append_entries_response {} for {}.{}", - success, - aer.term, - aer.last_log_idx); + "{} {} for {}.{}", aer.msg, success, aer.term, aer.last_log_idx); rlog(node_id, tgt_node_id, s, dropped); } @@ -468,7 +492,7 @@ class RaftDriver aft::ProposeRequestVote prv, bool dropped) { - const auto s = fmt::format("propose_request_vote for term {}", prv.term); + const auto s = fmt::format("{} for term {}", prv.msg, prv.term); log(node_id, tgt_node_id, s, dropped); } @@ -493,7 +517,21 @@ class RaftDriver log_msg_details(node_id, tgt_node_id, rv, dropped); break; } + case (aft::RaftMsgType::raft_request_pre_vote): + { + auto rpv = *(aft::RequestPreVote*)data; + packet = rpv; + log_msg_details(node_id, tgt_node_id, rpv, dropped); + break; + } case (aft::RaftMsgType::raft_request_vote_response): + { + auto rvr = *(aft::RequestPreVoteResponse*)data; + packet = rvr; + log_msg_details(node_id, tgt_node_id, rvr, dropped); + break; + } + case (aft::RaftMsgType::raft_request_pre_vote_response): { auto rvr = *(aft::RequestVoteResponse*)data; packet = rvr; diff --git a/tla/consensus/Traceccfraft.tla b/tla/consensus/Traceccfraft.tla index 5281b6c324df..a539b7216d98 100644 --- a/tla/consensus/Traceccfraft.tla +++ b/tla/consensus/Traceccfraft.tla @@ -4,7 +4,8 @@ EXTENDS ccfraft, Json, IOUtils, Sequences, MCAliases \* raft_types.h enum RaftMsgType RaftMsgType == "raft_append_entries" :> AppendEntriesRequest @@ "raft_append_entries_response" :> AppendEntriesResponse @@ - "raft_request_vote" :> RequestVoteRequest @@ "raft_request_vote_response" :> RequestVoteResponse @@ + "raft_request_vote" :> RequestVoteRequest @@ "raft_request_pre_vote" :> RequestVoteRequest @@ + "raft_request_vote_response" :> RequestVoteResponse @@ "raft_request_pre_vote_response" :> RequestVoteResponse @@ "raft_propose_request_vote" :> ProposeVoteRequest ToLeadershipState == @@ -56,9 +57,9 @@ IsRequestVoteRequest(msg, dst, src, logline) == /\ IsHeader(msg, dst, src, logline, RequestVoteRequest) /\ msg.lastCommittableIndex = logline.msg.packet.last_committable_idx /\ msg.lastCommittableTerm = logline.msg.packet.term_of_last_committable_idx - /\ IF "election_type" \in DOMAIN logline.msg.packet - THEN msg.isPreVote = (logline.msg.packet.election_type = "PreVote") - ELSE msg.isPreVote = FALSE + /\ IF logline.msg.packet.msg = "raft_request_vote" + THEN msg.isPreVote = FALSE + ELSE msg.isPreVote = TRUE IsRequestVoteResponse(msg, dst, src, logline) == /\ IsHeader(msg, dst, src, logline, RequestVoteResponse) diff --git a/tla/consensus/ccfraft.tla b/tla/consensus/ccfraft.tla index a71cf72e1e56..18ed7b0a58bb 100644 --- a/tla/consensus/ccfraft.tla +++ b/tla/consensus/ccfraft.tla @@ -1255,6 +1255,12 @@ DropIgnoredMessage(i,j,m) == \* the next configurations learns that its retirement has been committed. \/ /\ membershipState[i] = RetiredCommitted /\ m.type /= AppendEntriesRequest + \* raft.h::recv_append_entries + \* We drop append entries which start before the commit index + \* This is safe as sentIndex will still be incremented allowing subsequent AppendEntries + \* to start later and hence it won't livelock + \/ /\ m.type = AppendEntriesRequest + /\ m.prevLogIndex < commitIndex[i] /\ Discard(m) /\ UNCHANGED <> diff --git a/tla/install_deps.py b/tla/install_deps.py index 126bcdd1f188..493420e89636 100644 --- a/tla/install_deps.py +++ b/tla/install_deps.py @@ -71,7 +71,8 @@ def _parse_args() -> argparse.Namespace: ) parser.add_argument( - "--skip-apt-packages", action="store_false", default=True, dest="apt_packages" + "--tdnf-extended", + action="store_true", ) return parser.parse_args() @@ -89,6 +90,21 @@ def install_tlc(): def install_deps(args: argparse.Namespace): + if args.tdnf_extended: + with open("/etc/yum.repos.d/tdnf.repo", "w", encoding="utf-8") as tdnf_repo: + tdnf_repo.write( + """[azurelinux-official-extended] + name=Azure Linux Official Extended $releasever $basearch + baseurl=https://packages.microsoft.com/azurelinux/$releasever/prod/extended/$basearch + gpgkey=file:///etc/pki/rpm-gpg/MICROSOFT-RPM-GPG-KEY + gpgcheck=1 + repo_gpgcheck=1 + enabled=1 + skip_if_unavailable=True + sslverify=1""" + ) + subprocess.check_call(["tdnf", "install", "-y", "parallel"]) + # Setup tools directory tools_dir = os.path.join(TLA_DIR, "tools") @@ -112,17 +128,6 @@ def create_tools_dir(): dest=tools_dir, ) - if args.apt_packages: - subprocess.Popen( - "sudo apt-get install -y --no-install-recommends".split() - + [ - "wget", - "graphviz", - "htop", - "texlive-latex-recommended", - ] - ).wait() - fetch_latest( url="https://nightly.tlapl.us/dist/tla2tools.jar", dest=TLA_DIR, From fb39d8e9168bc4a88bb62d96a7ba3337e2bf919e Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Wed, 12 Nov 2025 15:58:01 +0000 Subject: [PATCH 06/12] Tidier public headers (#7451) --- .clang-tidy | 2 +- include/ccf/base_endpoint_registry.h | 6 +- include/ccf/claims_digest.h | 2 +- .../ccf/cose_signatures_config_interface.h | 6 +- include/ccf/crypto/jwk.h | 2 +- include/ccf/crypto/sha256_hash.h | 12 ++- include/ccf/endpoint.h | 32 +++--- include/ccf/endpoint_registry.h | 23 ++--- .../endpoints/authentication/all_of_auth.h | 15 +-- .../ccf/endpoints/authentication/cert_auth.h | 43 ++++---- .../ccf/endpoints/authentication/cose_auth.h | 59 +++++------ .../ccf/endpoints/authentication/empty_auth.h | 13 +-- include/ccf/endpoints/authentication/js.h | 2 +- .../ccf/endpoints/authentication/jwt_auth.h | 6 +- include/ccf/entity_id.h | 15 ++- include/ccf/historical_queries_adapter.h | 4 +- include/ccf/historical_queries_interface.h | 10 +- include/ccf/http_accept.h | 2 +- include/ccf/http_etag.h | 99 +++++++++---------- include/ccf/http_query.h | 22 ++--- include/ccf/indexing/indexer_interface.h | 2 +- .../strategies/seqnos_by_key_bucketed.h | 2 +- .../strategies/visit_each_entry_in_map.h | 2 +- include/ccf/indexing/strategy.h | 2 +- include/ccf/js/audit_format.h | 2 +- include/ccf/js/core/context.h | 9 +- include/ccf/js/core/runtime.h | 2 +- include/ccf/js/core/wrapped_property_enum.h | 2 +- include/ccf/js/extensions/ccf/historical.h | 2 +- include/ccf/js/extensions/ccf/kv.h | 4 +- include/ccf/js/extensions/console.h | 3 +- include/ccf/js/interpreter_cache_interface.h | 2 +- include/ccf/js/kv_access_permissions.h | 2 +- include/ccf/js/registry.h | 6 +- include/ccf/js/tx_access.h | 2 +- include/ccf/network_identity_interface.h | 2 +- include/ccf/node/cose_signatures_config.h | 4 +- .../ccf/node/node_configuration_interface.h | 2 +- include/ccf/node/quote.h | 2 +- include/ccf/node/startup_config.h | 10 +- include/ccf/node_context.h | 11 ++- include/ccf/node_startup_state.h | 6 +- include/ccf/odata_error.h | 2 +- include/ccf/receipt.h | 37 +++---- include/ccf/rest_verb.h | 4 +- include/ccf/rpc_exception.h | 2 +- include/ccf/tx.h | 27 +++-- include/ccf/tx_id.h | 2 +- include/ccf/tx_status.h | 33 +++---- src/endpoints/endpoint.cpp | 4 +- src/js/extensions/ccf/historical.cpp | 2 +- src/js/interpreter_cache.h | 2 +- src/js/registry.cpp | 2 +- src/node/gov/handlers/proposals.h | 6 +- src/node/historical_queries_adapter.cpp | 4 +- src/node/receipt.cpp | 7 +- src/node/rpc_context_impl.h | 2 + src/node/snapshot_serdes.h | 1 + src/node/snapshotter.h | 1 + src/node/test/receipt.cpp | 7 +- 60 files changed, 315 insertions(+), 286 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index aa8780f0a647..01eb6ffd31b1 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -58,5 +58,5 @@ Checks: > -readability-magic-numbers, WarningsAsErrors: '*' -HeaderFilterRegex: 'include\/ccf\/(ds|threading|service|research|pal)\/.*' +HeaderFilterRegex: 'include\/ccf\/.*' FormatStyle: 'file' \ No newline at end of file diff --git a/include/ccf/base_endpoint_registry.h b/include/ccf/base_endpoint_registry.h index a1999d251167..cf4da9122a9f 100644 --- a/include/ccf/base_endpoint_registry.h +++ b/include/ccf/base_endpoint_registry.h @@ -14,7 +14,7 @@ namespace ccf /** Lists the possible return codes from the versioned APIs in @c * ccf::BaseEndpointRegistry */ - enum class ApiResult + enum class ApiResult : uint8_t { /** Call was successful, results can be used */ OK = 0, @@ -25,7 +25,7 @@ namespace ccf /** One of the arguments passed to the function is invalid. It may be outside the range of known values, or not be in the expected format. */ InvalidArgs, - /** The requsted value was not found. */ + /** The requested value was not found. */ NotFound, /** General error not covered by the cases above. Generally means that an unexpected exception was thrown during execution. */ @@ -66,7 +66,7 @@ namespace ccf /** Lists possible reasons for an ApiResult::InvalidArgs being return in @c * ccf::BaseEndpointRegistry */ - enum class InvalidArgsReason + enum class InvalidArgsReason : uint8_t { NoReason = 0, /** Views start at 1 (one) in CCF */ diff --git a/include/ccf/claims_digest.h b/include/ccf/claims_digest.h index 4baf8ae10d8f..45881ea97436 100644 --- a/include/ccf/claims_digest.h +++ b/include/ccf/claims_digest.h @@ -27,7 +27,7 @@ namespace ccf void set(Digest::Representation&& r) { is_set = true; - digest.set(std::move(r)); + digest.set(r); } [[nodiscard]] bool empty() const diff --git a/include/ccf/cose_signatures_config_interface.h b/include/ccf/cose_signatures_config_interface.h index 32a0df01a6d6..3baebfd20709 100644 --- a/include/ccf/cose_signatures_config_interface.h +++ b/include/ccf/cose_signatures_config_interface.h @@ -15,14 +15,14 @@ namespace ccf::cose class AbstractCOSESignaturesConfig : public ccf::AbstractNodeSubSystem { public: - virtual ~AbstractCOSESignaturesConfig() = default; + ~AbstractCOSESignaturesConfig() override = default; static char const* get_subsystem_name() { return "COSESignaturesConfig"; } - virtual const ccf::COSESignaturesConfig& get_cose_signatures_config() - const = 0; + [[nodiscard]] virtual const ccf::COSESignaturesConfig& + get_cose_signatures_config() const = 0; }; } \ No newline at end of file diff --git a/include/ccf/crypto/jwk.h b/include/ccf/crypto/jwk.h index 5902df5434df..c9a85c4e5871 100644 --- a/include/ccf/crypto/jwk.h +++ b/include/ccf/crypto/jwk.h @@ -48,7 +48,7 @@ namespace ccf::crypto struct JsonWebKeyData { - JsonWebKeyType kty; + JsonWebKeyType kty = JsonWebKeyType::EC; std::optional kid = std::nullopt; std::optional> x5c = std::nullopt; std::optional n = std::nullopt; diff --git a/include/ccf/crypto/sha256_hash.h b/include/ccf/crypto/sha256_hash.h index 192d3c33ed24..13a33128fa2d 100644 --- a/include/ccf/crypto/sha256_hash.h +++ b/include/ccf/crypto/sha256_hash.h @@ -21,9 +21,9 @@ namespace ccf::crypto Sha256Hash() = default; - void set(Representation&& r) + void set(const Representation& r) { - h = std::move(r); + h = r; } Sha256Hash(const uint8_t* data, size_t size); @@ -34,6 +34,14 @@ namespace ccf::crypto const Sha256Hash& first, const Sha256Hash& second, const Sha256Hash& third); + Sha256Hash(const Sha256Hash& hash) = default; + Sha256Hash& operator=(const Sha256Hash& hash) = default; + Sha256Hash(Sha256Hash&& hash) noexcept : h(hash.h) {} + Sha256Hash& operator=(Sha256Hash&& hash) noexcept + { + h = hash.h; + return *this; + } friend std::ostream& operator<<( std::ostream& os, const ccf::crypto::Sha256Hash& h); diff --git a/include/ccf/endpoint.h b/include/ccf/endpoint.h index c8b3025e7e1c..a407c8f47065 100644 --- a/include/ccf/endpoint.h +++ b/include/ccf/endpoint.h @@ -40,7 +40,7 @@ namespace ccf::kv::serialisers { auto str = fmt::format("{} {}", endpoint_key.verb.c_str(), endpoint_key.uri_path); - return SerialisedEntry(str.begin(), str.end()); + return {str.begin(), str.end()}; } static ccf::endpoints::EndpointKey from_serialised( @@ -64,7 +64,7 @@ namespace ccf::endpoints DECLARE_JSON_TYPE(EndpointKey); DECLARE_JSON_REQUIRED_FIELDS(EndpointKey, uri_path, verb); - enum class ForwardingRequired + enum class ForwardingRequired : uint8_t { /** ForwardingRequired::Sometimes is the default value, and should be used * for most read-only operations. If this request is made to a backup node, @@ -90,7 +90,7 @@ namespace ccf::endpoints Never }; - enum class RedirectionStrategy + enum class RedirectionStrategy : uint8_t { /** This operation does not need to be redirected, and can be executed on the receiving node. Most read-only operations can be executed on any @@ -109,14 +109,14 @@ namespace ccf::endpoints ToBackup, }; - enum class Mode + enum class Mode : uint8_t { ReadWrite, ReadOnly, Historical }; - enum QueryParamPresence + enum QueryParamPresence : uint8_t { RequiredParameter, OptionalParameter, @@ -142,10 +142,10 @@ namespace ccf::endpoints struct InterpreterReusePolicy { - enum + enum class Kind : uint8_t { KeyBased - } kind; + } kind = Kind::KeyBased; std::string key; @@ -154,8 +154,11 @@ namespace ccf::endpoints void to_json(nlohmann::json& j, const InterpreterReusePolicy& grp); void from_json(const nlohmann::json& j, InterpreterReusePolicy& grp); - std::string schema_name(const InterpreterReusePolicy*); - void fill_json_schema(nlohmann::json& schema, const InterpreterReusePolicy*); + std::string schema_name( + [[maybe_unused]] const InterpreterReusePolicy* policy); + void fill_json_schema( + nlohmann::json& schema, + [[maybe_unused]] const InterpreterReusePolicy* policy); struct EndpointProperties { @@ -259,8 +262,9 @@ namespace ccf::endpoints struct Installer { virtual void install(Endpoint&) = 0; + virtual ~Installer() = default; }; - Installer* installer; + Installer* installer = nullptr; using SchemaBuilderFn = std::function; @@ -341,6 +345,7 @@ namespace ccf::endpoints * @param status Response status code * @return This Endpoint for further modification */ + // NOLINTNEXTLINE(misc-confusable-identifiers) template Endpoint& set_auto_schema(std::optional status = std::nullopt) { @@ -430,7 +435,7 @@ namespace ccf::endpoints template Endpoint& add_query_parameter( const std::string& param_name, - QueryParamPresence presence = RequiredParameter) + QueryParamPresence presence = QueryParamPresence::RequiredParameter) { schema_builders.push_back( [param_name, @@ -448,7 +453,8 @@ namespace ccf::endpoints auto parameter = nlohmann::json::object(); parameter["name"] = param_name; parameter["in"] = "query"; - parameter["required"] = presence == RequiredParameter; + parameter["required"] = + presence == QueryParamPresence::RequiredParameter; parameter["schema"] = ds::openapi::add_schema_to_components( document, schema_name, query_schema); ds::openapi::add_request_parameter_schema( @@ -488,7 +494,7 @@ struct formatter auto format( const ccf::endpoints::ForwardingRequired& v, FormatContext& ctx) const { - char const* s; + char const* s = nullptr; switch (v) { case ccf::endpoints::ForwardingRequired::Sometimes: diff --git a/include/ccf/endpoint_registry.h b/include/ccf/endpoint_registry.h index 1e5a6578c883..21189cb1bead 100644 --- a/include/ccf/endpoint_registry.h +++ b/include/ccf/endpoint_registry.h @@ -40,12 +40,12 @@ namespace ccf::endpoints struct RequestCompletedEvent { - std::string method = ""; + std::string method; // This contains the path template against which the request matched. For // instance `/user/{user_id}` rather than `/user/Bob`. This should be safe // to log, though doing so still reveals (to anyone with stdout access) // exactly which endpoints were executed and when. - std::string dispatch_path = ""; + std::string dispatch_path; int status = 0; std::chrono::milliseconds exec_time{0}; size_t attempts = 0; @@ -53,7 +53,7 @@ namespace ccf::endpoints struct DispatchFailedEvent { - std::string method = ""; + std::string method; int status = 0; }; @@ -116,7 +116,7 @@ namespace ccf::endpoints class EndpointRegistry : public Endpoint::Installer { public: - enum ReadWrite + enum class ReadWrite : uint8_t { Read, Write @@ -165,11 +165,11 @@ namespace ccf::endpoints ccf::kv::TxHistory* history = nullptr; public: - EndpointRegistry(const std::string& method_prefix_) : - method_prefix(method_prefix_) + EndpointRegistry(std::string method_prefix_) : + method_prefix(std::move(method_prefix_)) {} - virtual ~EndpointRegistry() = default; + ~EndpointRegistry() override = default; /** Create a new endpoint. * @@ -254,12 +254,13 @@ namespace ccf::endpoints * internally, so must be able to populate the document * with the supported endpoints however it defines them. */ - virtual void build_api(nlohmann::json& document, ccf::kv::ReadOnlyTx&); + virtual void build_api( + nlohmann::json& document, [[maybe_unused]] ccf::kv::ReadOnlyTx& tx); virtual void init_handlers(); virtual EndpointDefinitionPtr find_endpoint( - ccf::kv::Tx&, ccf::RpcContext& rpc_ctx); + [[maybe_unused]] ccf::kv::Tx& tx, ccf::RpcContext& rpc_ctx); virtual void execute_endpoint( EndpointDefinitionPtr e, EndpointContext& ctx); @@ -268,7 +269,7 @@ namespace ccf::endpoints EndpointDefinitionPtr e, CommandEndpointContext& ctx, const TxID& tx_id); virtual std::set get_allowed_verbs( - ccf::kv::Tx&, const ccf::RpcContext& rpc_ctx); + [[maybe_unused]] ccf::kv::Tx& tx, const ccf::RpcContext& rpc_ctx); virtual bool request_needs_root(const ccf::RpcContext& rpc_ctx); @@ -276,7 +277,7 @@ namespace ccf::endpoints const std::string& path, const std::vector& matches); - virtual void tick(std::chrono::milliseconds); + virtual void tick([[maybe_unused]] std::chrono::milliseconds duration); void set_consensus(ccf::kv::Consensus* c); diff --git a/include/ccf/endpoints/authentication/all_of_auth.h b/include/ccf/endpoints/authentication/all_of_auth.h index 04476b541682..a8814ad6abd7 100644 --- a/include/ccf/endpoints/authentication/all_of_auth.h +++ b/include/ccf/endpoints/authentication/all_of_auth.h @@ -15,7 +15,7 @@ namespace ccf { std::map> identities; - std::string get_conjoined_name() const; + [[nodiscard]] std::string get_conjoined_name() const; }; class AllOfAuthnPolicy : public AuthnPolicy @@ -37,15 +37,16 @@ namespace ccf const std::vector>& _policies); std::unique_ptr authenticate( - ccf::kv::ReadOnlyTx&, - const std::shared_ptr&, - std::string&) override; + ccf::kv::ReadOnlyTx& tx, + const std::shared_ptr& ctx, + std::string& error_reason) override; void set_unauthenticated_error( - std::shared_ptr, std::string&&) override; + std::shared_ptr ctx, + std::string&& error_reason) override; - std::optional get_openapi_security_schema() - const override; + [[nodiscard]] std::optional + get_openapi_security_schema() const override; std::string get_security_scheme_name() override; }; diff --git a/include/ccf/endpoints/authentication/cert_auth.h b/include/ccf/endpoints/authentication/cert_auth.h index 844716db7f3a..2648362791a7 100644 --- a/include/ccf/endpoints/authentication/cert_auth.h +++ b/include/ccf/endpoints/authentication/cert_auth.h @@ -7,15 +7,12 @@ namespace ccf { - namespace + inline std::optional get_cert_based_security_schema() { - std::optional get_cert_based_security_schema() - { - // There is currently no OpenAPI-compliant way to describe cert-based TLS - // auth, so this policy is not documented. This should change in - // OpenAPI3.1: https://github.com/OAI/OpenAPI-Specification/pull/1764 - return std::nullopt; - } + // There is currently no OpenAPI-compliant way to describe cert-based TLS + // auth, so this policy is not documented. This should change in + // OpenAPI3.1: https://github.com/OAI/OpenAPI-Specification/pull/1764 + return std::nullopt; } struct UserCertAuthnIdentity : public AuthnIdentity @@ -35,20 +32,20 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "user_cert"; UserCertAuthnPolicy(); - virtual ~UserCertAuthnPolicy(); + ~UserCertAuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, const std::shared_ptr& ctx, std::string& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return get_cert_based_security_schema(); } - virtual std::string get_security_scheme_name() override + std::string get_security_scheme_name() override { return SECURITY_SCHEME_NAME; }; @@ -69,20 +66,20 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "member_cert"; MemberCertAuthnPolicy(); - virtual ~MemberCertAuthnPolicy(); + ~MemberCertAuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, const std::shared_ptr& ctx, std::string& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return get_cert_based_security_schema(); } - virtual std::string get_security_scheme_name() override + std::string get_security_scheme_name() override { return SECURITY_SCHEME_NAME; }; @@ -103,13 +100,13 @@ namespace ccf const std::shared_ptr& ctx, std::string& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return get_cert_based_security_schema(); } - virtual std::string get_security_scheme_name() override + std::string get_security_scheme_name() override { return SECURITY_SCHEME_NAME; }; @@ -130,20 +127,20 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "any_cert"; AnyCertAuthnPolicy(); - virtual ~AnyCertAuthnPolicy(); + ~AnyCertAuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, const std::shared_ptr& ctx, std::string& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return get_cert_based_security_schema(); } - virtual std::string get_security_scheme_name() override + std::string get_security_scheme_name() override { return SECURITY_SCHEME_NAME; }; diff --git a/include/ccf/endpoints/authentication/cose_auth.h b/include/ccf/endpoints/authentication/cose_auth.h index 03a59bb0fe6e..546b08b96cab 100644 --- a/include/ccf/endpoints/authentication/cose_auth.h +++ b/include/ccf/endpoints/authentication/cose_auth.h @@ -11,7 +11,7 @@ namespace ccf { struct ProtectedHeader { - int64_t alg; + int64_t alg = 0; std::string kid; }; @@ -19,7 +19,7 @@ namespace ccf { std::optional gov_msg_type; std::optional gov_msg_proposal_id; - uint64_t gov_msg_created_at; + uint64_t gov_msg_created_at = 0; }; struct TimestampedProtectedHeader : ProtectedHeader @@ -70,13 +70,13 @@ namespace ccf const std::span& content_, const std::span& envelope_, const std::span& signature_, - const MemberId& member_id_, - const ccf::crypto::Pem& member_cert_, - const GovernanceProtectedHeader& protected_header_) : + MemberId member_id_, + ccf::crypto::Pem member_cert_, + GovernanceProtectedHeader protected_header_) : COSESign1AuthnIdentity(content_, envelope_, signature_), - member_id(member_id_), - member_cert(member_cert_), - protected_header(protected_header_) + member_id(std::move(member_id_)), + member_cert(std::move(member_cert_)), + protected_header(std::move(protected_header_)) {} }; @@ -95,13 +95,13 @@ namespace ccf const std::span& content_, const std::span& envelope_, const std::span& signature_, - const UserId& user_id_, - const ccf::crypto::Pem& user_cert_, - const TimestampedProtectedHeader& protected_header_) : + UserId user_id_, + ccf::crypto::Pem user_cert_, + TimestampedProtectedHeader protected_header_) : COSESign1AuthnIdentity(content_, envelope_, signature_), - user_id(user_id_), - user_cert(user_cert_), - protected_header(protected_header_) + user_id(std::move(user_id_)), + user_cert(std::move(user_cert_)), + protected_header(std::move(protected_header_)) {} }; @@ -122,7 +122,7 @@ namespace ccf MemberCOSESign1AuthnPolicy( std::optional gov_msg_type_ = std::nullopt); - ~MemberCOSESign1AuthnPolicy(); + ~MemberCOSESign1AuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, @@ -133,8 +133,8 @@ namespace ccf std::shared_ptr ctx, std::string&& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return security_schema; } @@ -190,12 +190,12 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "user_cose_sign1"; UserCOSESign1AuthnPolicy( - const std::string& msg_type_name_ = "ccf.msg.type", - const std::string& msg_created_at_name_ = "ccf.msg.created_at") : - msg_type_name(msg_type_name_), - msg_created_at_name(msg_created_at_name_) + std::string msg_type_name_ = "ccf.msg.type", + std::string msg_created_at_name_ = "ccf.msg.created_at") : + msg_type_name(std::move(msg_type_name_)), + msg_created_at_name(std::move(msg_created_at_name_)) {} - ~UserCOSESign1AuthnPolicy(); + ~UserCOSESign1AuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, @@ -206,8 +206,8 @@ namespace ccf std::shared_ptr ctx, std::string&& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return security_schema; } @@ -232,11 +232,12 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "typed_user_cose_sign1"; TypedUserCOSESign1AuthnPolicy( - const std::string& expected_msg_type_, - const std::string& msg_type_name_ = "ccf.msg.type", - const std::string& msg_created_at_name_ = "ccf.msg.created_at") : - UserCOSESign1AuthnPolicy(msg_type_name_, msg_created_at_name_), - expected_msg_type(expected_msg_type_) + std::string expected_msg_type_, + std::string msg_type_name_ = "ccf.msg.type", + std::string msg_created_at_name_ = "ccf.msg.created_at") : + UserCOSESign1AuthnPolicy( + std::move(msg_type_name_), std::move(msg_created_at_name_)), + expected_msg_type(std::move(expected_msg_type_)) {} std::unique_ptr authenticate( diff --git a/include/ccf/endpoints/authentication/empty_auth.h b/include/ccf/endpoints/authentication/empty_auth.h index f5ec3870e575..2827b5fa426a 100644 --- a/include/ccf/endpoints/authentication/empty_auth.h +++ b/include/ccf/endpoints/authentication/empty_auth.h @@ -17,15 +17,16 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "no_auth"; std::unique_ptr authenticate( - ccf::kv::ReadOnlyTx&, - const std::shared_ptr&, - std::string&) override; + [[maybe_unused]] ccf::kv::ReadOnlyTx& tx, + [[maybe_unused]] const std::shared_ptr& ctx, + [[maybe_unused]] std::string& error_reason) override; void set_unauthenticated_error( - std::shared_ptr, std::string&&) override; + [[maybe_unused]] std::shared_ptr ctx, + [[maybe_unused]] std::string&& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return unauthenticated_schema; } diff --git a/include/ccf/endpoints/authentication/js.h b/include/ccf/endpoints/authentication/js.h index 2a853d4ccbeb..6894b4a54f14 100644 --- a/include/ccf/endpoints/authentication/js.h +++ b/include/ccf/endpoints/authentication/js.h @@ -56,7 +56,7 @@ namespace ccf } template - static inline constexpr char const* get_policy_name_from_ident(const T*) + static constexpr char const* get_policy_name_from_ident(const T* /*unused*/) { if constexpr (std::is_same_v) { diff --git a/include/ccf/endpoints/authentication/jwt_auth.h b/include/ccf/endpoints/authentication/jwt_auth.h index 70d4f3f7c813..2471410cb83f 100644 --- a/include/ccf/endpoints/authentication/jwt_auth.h +++ b/include/ccf/endpoints/authentication/jwt_auth.h @@ -34,7 +34,7 @@ namespace ccf static constexpr auto SECURITY_SCHEME_NAME = "jwt"; JwtAuthnPolicy(); - virtual ~JwtAuthnPolicy(); + ~JwtAuthnPolicy() override; std::unique_ptr authenticate( ccf::kv::ReadOnlyTx& tx, @@ -45,8 +45,8 @@ namespace ccf std::shared_ptr ctx, std::string&& error_reason) override; - std::optional get_openapi_security_schema() - const override + [[nodiscard]] std::optional + get_openapi_security_schema() const override { return security_schema; } diff --git a/include/ccf/entity_id.h b/include/ccf/entity_id.h index 9c12ad3f800e..f6a87ed5282f 100644 --- a/include/ccf/entity_id.h +++ b/include/ccf/entity_id.h @@ -26,20 +26,27 @@ namespace ccf EntityId(const EntityId& id_) = default; EntityId(const Value& id_) : id(id_) {} EntityId(Value&& id_) : id(std::move(id_)) {} + EntityId(EntityId&& id_) noexcept : id(std::move(id_)) {} + EntityId& operator=(EntityId&& other) = default; operator std::string() const { return id; } - void operator=(const EntityId& other) + EntityId& operator=(const EntityId& other) { - id = other.id; + if (this != &other) + { + id = other.id; + } + return *this; } - void operator=(const Value& id_) + EntityId& operator=(const Value& id_) { id = id_; + return *this; } bool operator==(const EntityId& other) const @@ -157,6 +164,7 @@ namespace ccf using NodeId = EntityId; } +// NOLINTBEGIN(cert-dcl58-cpp) namespace std { template @@ -183,6 +191,7 @@ namespace std } }; } +// NOLINTEND(cert-dcl58-cpp) FMT_BEGIN_NAMESPACE template diff --git a/include/ccf/historical_queries_adapter.h b/include/ccf/historical_queries_adapter.h index 01abd9720227..e95252c85bee 100644 --- a/include/ccf/historical_queries_adapter.h +++ b/include/ccf/historical_queries_adapter.h @@ -39,7 +39,7 @@ namespace ccf::historical std::optional txid_from_header( endpoints::CommandEndpointContext& args); - enum class HistoricalQueryErrorCode + enum class HistoricalQueryErrorCode : uint8_t { InternalError, TransactionPending, @@ -63,7 +63,7 @@ namespace ccf::historical std::string reason, endpoints::CommandEndpointContext& args); - enum class HistoricalTxStatus + enum class HistoricalTxStatus : uint8_t { Error, PendingOrUnknown, diff --git a/include/ccf/historical_queries_interface.h b/include/ccf/historical_queries_interface.h index f4bc6ba9f4d4..0867b859c535 100644 --- a/include/ccf/historical_queries_interface.h +++ b/include/ccf/historical_queries_interface.h @@ -23,11 +23,11 @@ namespace ccf::historical ccf::TxID transaction_id; State( - const ccf::kv::ReadOnlyStorePtr& store_, - const TxReceiptImplPtr& receipt_, + ccf::kv::ReadOnlyStorePtr store_, + TxReceiptImplPtr receipt_, const ccf::TxID& transaction_id_) : - store(store_), - receipt(receipt_), + store(std::move(store_)), + receipt(std::move(receipt_)), transaction_id(transaction_id_) {} @@ -66,7 +66,7 @@ namespace ccf::historical class AbstractStateCache : public ccf::AbstractNodeSubSystem { public: - virtual ~AbstractStateCache() = default; + ~AbstractStateCache() override = default; static char const* get_subsystem_name() { diff --git a/include/ccf/http_accept.h b/include/ccf/http_accept.h index 53093bfde33b..8e294c7e885b 100644 --- a/include/ccf/http_accept.h +++ b/include/ccf/http_accept.h @@ -22,7 +22,7 @@ namespace ccf::http return s == "*"; } - bool matches(const std::string& mime) const + [[nodiscard]] bool matches(const std::string& mime) const { const auto [t, st] = ccf::nonstd::split_1(mime, "/"); diff --git a/include/ccf/http_etag.h b/include/ccf/http_etag.h index 8047539abaaa..68f3f0b04dde 100644 --- a/include/ccf/http_etag.h +++ b/include/ccf/http_etag.h @@ -7,70 +7,67 @@ #include #include -namespace ccf +namespace ccf::http { - namespace http + /** Utility class to resolve If-Match and If-None-Match as described + * in https://www.rfc-editor.org/rfc/rfc9110#field.if-match + */ + class Matcher { - /** Utility class to resolve If-Match and If-None-Match as described - * in https://www.rfc-editor.org/rfc/rfc9110#field.if-match + private: + /// If-Match header is present and has the value "*" + bool any_value = false; + /// If-Match header is present and has specific etag values + std::set if_etags; + + public: + /** Construct a Matcher from a match header + * + * Note: Weak tags are not supported. */ - class Matcher + Matcher(const std::string& match_header) { - private: - /// If-Match header is present and has the value "*" - bool any_value = false; - /// If-Match header is present and has specific etag values - std::set if_etags; - - public: - /** Construct a Matcher from a match header - * - * Note: Weak tags are not supported. - */ - Matcher(const std::string& match_header) + if (match_header == "*") { - if (match_header == "*") - { - any_value = true; - return; - } - - std::regex etag_rx("\\\"([0-9a-f]+)\\\",?\\s*"); - auto etags_begin = std::sregex_iterator( - match_header.begin(), match_header.end(), etag_rx); - auto etags_end = std::sregex_iterator(); - ssize_t last_matched = 0; - - for (std::sregex_iterator i = etags_begin; i != etags_end; ++i) - { - if (i->position() != last_matched) - { - throw std::runtime_error("Invalid If-Match header"); - } - std::smatch match = *i; - if_etags.insert(match[1].str()); - last_matched = match.position() + match.length(); - } + any_value = true; + return; + } - ssize_t last_index_in_header = match_header.size(); + std::regex etag_rx(R"(\"([0-9a-f]+)\",?\s*)"); + auto etags_begin = + std::sregex_iterator(match_header.begin(), match_header.end(), etag_rx); + auto etags_end = std::sregex_iterator(); + ssize_t last_matched = 0; - if (last_matched != last_index_in_header || if_etags.empty()) + for (std::sregex_iterator i = etags_begin; i != etags_end; ++i) + { + if (i->position() != last_matched) { throw std::runtime_error("Invalid If-Match header"); } + const std::smatch& match = *i; + if_etags.insert(match[1].str()); + last_matched = match.position() + match.length(); } - /// Check if a given ETag matches the If-Match/If-None-Match header - bool matches(const std::string& etag) const - { - return any_value || if_etags.contains(etag); - } + ssize_t last_index_in_header = match_header.size(); - /// Check if the header will match any ETag (*) - bool is_any() const + if (last_matched != last_index_in_header || if_etags.empty()) { - return any_value; + throw std::runtime_error("Invalid If-Match header"); } - }; - } + } + + /// Check if a given ETag matches the If-Match/If-None-Match header + [[nodiscard]] bool matches(const std::string& etag) const + { + return any_value || if_etags.contains(etag); + } + + /// Check if the header will match any ETag (*) + [[nodiscard]] bool is_any() const + { + return any_value; + } + }; } \ No newline at end of file diff --git a/include/ccf/http_query.h b/include/ccf/http_query.h index 88157af8d530..d52854cfe495 100644 --- a/include/ccf/http_query.h +++ b/include/ccf/http_query.h @@ -63,19 +63,18 @@ namespace ccf::http val = true; return true; } - else if (param_val == "false") + + if (param_val == "false") { val = false; return true; } - else - { - error_reason = fmt::format( - "Unable to parse value '{}' as bool in parameter '{}'", - param_val, - param_key); - return false; - } + + error_reason = fmt::format( + "Unable to parse value '{}' as bool in parameter '{}'", + param_val, + param_key); + return false; } else if constexpr (std::is_integral_v) { @@ -108,9 +107,6 @@ namespace ccf::http { return val; } - else - { - return std::nullopt; - } + return std::nullopt; } } diff --git a/include/ccf/indexing/indexer_interface.h b/include/ccf/indexing/indexer_interface.h index 58cb70f75afa..b92d894c4429 100644 --- a/include/ccf/indexing/indexer_interface.h +++ b/include/ccf/indexing/indexer_interface.h @@ -24,7 +24,7 @@ namespace ccf::indexing std::set strategies; public: - virtual ~IndexingStrategies() = default; + ~IndexingStrategies() override = default; static char const* get_subsystem_name() { diff --git a/include/ccf/indexing/strategies/seqnos_by_key_bucketed.h b/include/ccf/indexing/strategies/seqnos_by_key_bucketed.h index 96ec1d15ff46..7d121d0381cf 100644 --- a/include/ccf/indexing/strategies/seqnos_by_key_bucketed.h +++ b/include/ccf/indexing/strategies/seqnos_by_key_bucketed.h @@ -34,7 +34,7 @@ namespace ccf::indexing::strategies size_t seqnos_per_bucket_ = 1000, size_t max_buckets_ = 10); - size_t max_requestable_range() const; + [[nodiscard]] size_t max_requestable_range() const; }; template diff --git a/include/ccf/indexing/strategies/visit_each_entry_in_map.h b/include/ccf/indexing/strategies/visit_each_entry_in_map.h index fb6b9613e058..d5dbb2eaf671 100644 --- a/include/ccf/indexing/strategies/visit_each_entry_in_map.h +++ b/include/ccf/indexing/strategies/visit_each_entry_in_map.h @@ -31,7 +31,7 @@ namespace ccf::indexing::strategies const std::string& map_name_, const std::string& strategy_prefix = "VisitEachEntryIn"); - virtual ~VisitEachEntryInMap() = default; + ~VisitEachEntryInMap() override = default; void handle_committed_transaction( const ccf::TxID& tx_id, const ccf::kv::ReadOnlyStorePtr& store) override; diff --git a/include/ccf/indexing/strategy.h b/include/ccf/indexing/strategy.h index 3d3773468072..d7e1cdabf5de 100644 --- a/include/ccf/indexing/strategy.h +++ b/include/ccf/indexing/strategy.h @@ -25,7 +25,7 @@ namespace ccf::indexing const std::string name; public: - Strategy(const std::string& name) : name(name) {} + Strategy(std::string name) : name(std::move(name)) {} virtual ~Strategy() = default; [[nodiscard]] std::string get_name() const diff --git a/include/ccf/js/audit_format.h b/include/ccf/js/audit_format.h index 96a36932990f..151a2eddc3b4 100644 --- a/include/ccf/js/audit_format.h +++ b/include/ccf/js/audit_format.h @@ -8,7 +8,7 @@ namespace ccf { - enum class ActionFormat + enum class ActionFormat : uint8_t { COSE = 0, JSON = 1 diff --git a/include/ccf/js/core/context.h b/include/ccf/js/core/context.h index fa5946e0dfff..f1672d2f5c6d 100644 --- a/include/ccf/js/core/context.h +++ b/include/ccf/js/core/context.h @@ -36,8 +36,8 @@ namespace ccf::js::core struct InterruptData { std::chrono::high_resolution_clock::time_point start_time; - std::chrono::milliseconds max_execution_time; - ccf::js::TxAccess access; + std::chrono::milliseconds max_execution_time = {}; + ccf::js::TxAccess access = ccf::js::TxAccess::APP_RO; bool request_timed_out = false; }; @@ -142,7 +142,7 @@ namespace ccf::js::core [[nodiscard]] JSWrappedValue new_string_len( const char* buf, size_t buf_len) const; [[nodiscard]] JSWrappedValue new_string_len( - const std::span buf) const; + std::span buf) const; JSWrappedValue new_type_error(const char* fmt, ...) const; JSWrappedValue new_internal_error(const char* fmt, ...) const; [[nodiscard]] JSWrappedValue new_tag_value(int tag, int32_t val = 0) const; @@ -197,8 +197,7 @@ namespace ccf::js::core { for (auto& extension : extensions) { - if (TExtension* t = dynamic_cast(extension.get()); - t != nullptr) + if (auto* t = dynamic_cast(extension.get()); t != nullptr) { return t; } diff --git a/include/ccf/js/core/runtime.h b/include/ccf/js/core/runtime.h index 6794c5fa4982..8d36446cd25e 100644 --- a/include/ccf/js/core/runtime.h +++ b/include/ccf/js/core/runtime.h @@ -9,7 +9,7 @@ namespace ccf::js::core { - enum class RuntimeLimitsPolicy + enum class RuntimeLimitsPolicy : uint8_t { NONE, NO_LOWER_THAN_DEFAULTS diff --git a/include/ccf/js/core/wrapped_property_enum.h b/include/ccf/js/core/wrapped_property_enum.h index 9b0aa0cce7ca..10068577da8a 100644 --- a/include/ccf/js/core/wrapped_property_enum.h +++ b/include/ccf/js/core/wrapped_property_enum.h @@ -48,7 +48,7 @@ namespace ccf::js::core return prop_enum[i].atom; } - size_t size() const + [[nodiscard]] size_t size() const { return prop_count; } diff --git a/include/ccf/js/extensions/ccf/historical.h b/include/ccf/js/extensions/ccf/historical.h index 772175d3c305..6e581f62da49 100644 --- a/include/ccf/js/extensions/ccf/historical.h +++ b/include/ccf/js/extensions/ccf/historical.h @@ -28,7 +28,7 @@ namespace ccf::js::extensions std::unique_ptr impl; HistoricalExtension(ccf::historical::AbstractStateCache* hs); - ~HistoricalExtension(); + ~HistoricalExtension() override; void install(js::core::Context& ctx) override; diff --git a/include/ccf/js/extensions/ccf/kv.h b/include/ccf/js/extensions/ccf/kv.h index a0094a9b7533..d021c014a018 100644 --- a/include/ccf/js/extensions/ccf/kv.h +++ b/include/ccf/js/extensions/ccf/kv.h @@ -29,9 +29,9 @@ namespace ccf::js::extensions ccf::js::NamespaceRestriction namespace_restriction; KvExtension(ccf::kv::Tx* t, ccf::js::NamespaceRestriction nr = {}); - ~KvExtension(); + ~KvExtension() override; - void install(js::core::Context& ctx); + void install(js::core::Context& ctx) override; void rethrow_trapped_exceptions() const; }; diff --git a/include/ccf/js/extensions/console.h b/include/ccf/js/extensions/console.h index 0de9158e522f..6188701f2513 100644 --- a/include/ccf/js/extensions/console.h +++ b/include/ccf/js/extensions/console.h @@ -27,7 +27,6 @@ namespace ccf::js::extensions void install(js::core::Context& ctx) override; - static void log_info_with_tag( - const ccf::js::TxAccess access, std::string_view s); + static void log_info_with_tag(ccf::js::TxAccess access, std::string_view s); }; } diff --git a/include/ccf/js/interpreter_cache_interface.h b/include/ccf/js/interpreter_cache_interface.h index c0ee446881fd..0c9ff9e02c4b 100644 --- a/include/ccf/js/interpreter_cache_interface.h +++ b/include/ccf/js/interpreter_cache_interface.h @@ -22,7 +22,7 @@ namespace ccf::js class AbstractInterpreterCache : public ccf::AbstractNodeSubSystem { public: - virtual ~AbstractInterpreterCache() = default; + ~AbstractInterpreterCache() override = default; static char const* get_subsystem_name() { diff --git a/include/ccf/js/kv_access_permissions.h b/include/ccf/js/kv_access_permissions.h index 9553813d573b..9ce17b0bc402 100644 --- a/include/ccf/js/kv_access_permissions.h +++ b/include/ccf/js/kv_access_permissions.h @@ -6,7 +6,7 @@ namespace ccf::js { - enum class KVAccessPermissions + enum class KVAccessPermissions : uint8_t { ILLEGAL = 0, READ_ONLY = 1 << 0, diff --git a/include/ccf/js/registry.h b/include/ccf/js/registry.h index 9cdca205dbbd..d94d16d7fb1a 100644 --- a/include/ccf/js/registry.h +++ b/include/ccf/js/registry.h @@ -145,11 +145,11 @@ namespace ccf::js void build_api(nlohmann::json& document, ccf::kv::ReadOnlyTx& tx) override; std::set get_allowed_verbs( - ccf::kv::Tx&, const ccf::RpcContext& rpc_ctx) override; + ccf::kv::Tx& tx, const ccf::RpcContext& rpc_ctx) override; ///@} virtual ccf::js::extensions::Extensions get_extensions( - const ccf::endpoints::EndpointContext& endpoint_ctx) + [[maybe_unused]] const ccf::endpoints::EndpointContext& endpoint_ctx) { return {}; }; @@ -195,7 +195,7 @@ namespace ccf::js ccf::ApiResult check_action_not_replayed_v1( ccf::kv::Tx& tx, uint64_t created_at, - const std::span action, + std::span action, ccf::InvalidArgsReason& reason); }; } diff --git a/include/ccf/js/tx_access.h b/include/ccf/js/tx_access.h index 9433af65a9b9..2614750312bd 100644 --- a/include/ccf/js/tx_access.h +++ b/include/ccf/js/tx_access.h @@ -6,7 +6,7 @@ namespace ccf::js { /// Describes the context in which JS script is currently executing. Used to /// determine which KV tables should be accessible. - enum class TxAccess + enum class TxAccess : uint8_t { /// Application code, during evaluation of an endpoint handler function /// marked as readonly diff --git a/include/ccf/network_identity_interface.h b/include/ccf/network_identity_interface.h index c484de5a5535..510524199392 100644 --- a/include/ccf/network_identity_interface.h +++ b/include/ccf/network_identity_interface.h @@ -15,7 +15,7 @@ namespace ccf class NetworkIdentitySubsystemInterface : public ccf::AbstractNodeSubSystem { public: - virtual ~NetworkIdentitySubsystemInterface() = default; + ~NetworkIdentitySubsystemInterface() override = default; static char const* get_subsystem_name() { diff --git a/include/ccf/node/cose_signatures_config.h b/include/ccf/node/cose_signatures_config.h index 974de3f84445..f4e19300359a 100644 --- a/include/ccf/node/cose_signatures_config.h +++ b/include/ccf/node/cose_signatures_config.h @@ -10,8 +10,8 @@ namespace ccf { struct COSESignaturesConfig { - std::string issuer = ""; - std::string subject = ""; + std::string issuer; + std::string subject; bool operator==(const COSESignaturesConfig& other) const = default; }; diff --git a/include/ccf/node/node_configuration_interface.h b/include/ccf/node/node_configuration_interface.h index bb164014e11a..2e913411e6e1 100644 --- a/include/ccf/node/node_configuration_interface.h +++ b/include/ccf/node/node_configuration_interface.h @@ -22,7 +22,7 @@ namespace ccf class NodeConfigurationInterface : public AbstractNodeSubSystem { public: - virtual ~NodeConfigurationInterface() = default; + ~NodeConfigurationInterface() override = default; static char const* get_subsystem_name() { diff --git a/include/ccf/node/quote.h b/include/ccf/node/quote.h index f5b2d73e55ef..a1e9fa93eef9 100644 --- a/include/ccf/node/quote.h +++ b/include/ccf/node/quote.h @@ -14,7 +14,7 @@ namespace ccf { - enum class QuoteVerificationResult + enum class QuoteVerificationResult : uint8_t { Verified = 0, Failed, diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index bd2684459d85..d02ce53eb4bd 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -28,12 +28,12 @@ namespace ccf ccf::ds::SizeString historical_cache_soft_limit = {"512MB"}; ccf::consensus::Configuration consensus = {}; - ccf::NodeInfoNetwork network = {}; + ccf::NodeInfoNetwork network; struct NodeCertificateInfo { std::string subject_name = "CN=CCF Node"; - std::vector subject_alt_names = {}; + std::vector subject_alt_names; ccf::crypto::CurveID curve_id = ccf::crypto::CurveID::SECP384R1; size_t initial_validity_days = 1; @@ -44,7 +44,7 @@ namespace ccf struct Ledger { std::string directory = "ledger"; - std::vector read_only_directories = {}; + std::vector read_only_directories; ccf::ds::SizeString chunk_size = {"5MB"}; bool operator==(const Ledger&) const = default; @@ -70,7 +70,7 @@ namespace ccf struct Attestation { - ccf::pal::snp::EndorsementsServers snp_endorsements_servers = {}; + ccf::pal::snp::EndorsementsServers snp_endorsements_servers; std::optional snp_security_policy_file = std::nullopt; std::optional snp_uvm_endorsements_file = std::nullopt; std::optional snp_endorsements_file = std::nullopt; @@ -135,7 +135,7 @@ namespace ccf { ccf::NodeInfoNetwork::NetAddress target_rpc_address; ccf::ds::TimeString retry_timeout = {"1000ms"}; - std::vector service_cert = {}; + std::vector service_cert; bool follow_redirect = true; }; Join join = {}; diff --git a/include/ccf/node_context.h b/include/ccf/node_context.h index c073822dd792..9d3d358bdc75 100644 --- a/include/ccf/node_context.h +++ b/include/ccf/node_context.h @@ -34,7 +34,8 @@ namespace ccf } template - std::shared_ptr get_subsystem(const std::string& name) const + [[nodiscard]] std::shared_ptr get_subsystem( + const std::string& name) const { const auto it = subsystems.find(name); if (it != subsystems.end()) @@ -56,7 +57,7 @@ namespace ccf } template - std::shared_ptr get_subsystem() const + [[nodiscard]] std::shared_ptr get_subsystem() const { return get_subsystem(T::get_subsystem_name()); } @@ -71,7 +72,8 @@ namespace ccf return {}; } - ccf::historical::AbstractStateCache& get_historical_state() + [[nodiscard]] ccf::historical::AbstractStateCache& get_historical_state() + const { auto historical_state_cache = get_subsystem(); @@ -83,7 +85,8 @@ namespace ccf return *historical_state_cache; } - ccf::indexing::IndexingStrategies& get_indexing_strategies() + [[nodiscard]] ccf::indexing::IndexingStrategies& get_indexing_strategies() + const { auto indexer = get_subsystem(); if (indexer == nullptr) diff --git a/include/ccf/node_startup_state.h b/include/ccf/node_startup_state.h index 639e09d63d12..9d283d3bce36 100644 --- a/include/ccf/node_startup_state.h +++ b/include/ccf/node_startup_state.h @@ -6,7 +6,7 @@ namespace ccf { - enum class NodeStartupState + enum class NodeStartupState : uint8_t { uninitialized, initialized, @@ -29,6 +29,7 @@ namespace ccf } // Used by fmtlib to render ccf::State +// NOLINTBEGIN(cert-dcl58-cpp) namespace std { inline std::ostream& operator<<(std::ostream& os, ccf::NodeStartupState s) @@ -37,4 +38,5 @@ namespace std to_json(j, s); return os << j.dump(); } -} \ No newline at end of file +} +// NOLINTEND(cert-dcl58-cpp) \ No newline at end of file diff --git a/include/ccf/odata_error.h b/include/ccf/odata_error.h index ea23ce7494a3..94022799223d 100644 --- a/include/ccf/odata_error.h +++ b/include/ccf/odata_error.h @@ -56,7 +56,7 @@ namespace ccf struct ErrorDetails { - http_status status; + http_status status = HTTP_STATUS_BAD_REQUEST; std::string code; std::string msg; }; diff --git a/include/ccf/receipt.h b/include/ccf/receipt.h index b814e2364d77..c835452f0501 100644 --- a/include/ccf/receipt.h +++ b/include/ccf/receipt.h @@ -50,11 +50,12 @@ namespace ccf struct ProofStep { - enum + enum class Direction : uint8_t { Left, Right - } direction; + }; + Direction direction = Direction::Left; ccf::crypto::Sha256Hash hash; @@ -74,7 +75,7 @@ namespace ccf for (const auto& element : proof) { - if (element.direction == ProofStep::Left) + if (element.direction == ProofStep::Direction::Left) { current = ccf::crypto::Sha256Hash(element.hash, current); } @@ -87,21 +88,17 @@ namespace ccf return current; } - ccf::crypto::Sha256Hash get_leaf_digest() + [[nodiscard]] ccf::crypto::Sha256Hash get_leaf_digest() const { ccf::crypto::Sha256Hash ce_dgst(leaf_components.commit_evidence); if (!leaf_components.claims_digest.empty()) { - return ccf::crypto::Sha256Hash( + return { leaf_components.write_set_digest, ce_dgst, - leaf_components.claims_digest.value()); - } - else - { - return ccf::crypto::Sha256Hash( - leaf_components.write_set_digest, ce_dgst); + leaf_components.claims_digest.value()}; } + return {leaf_components.write_set_digest, ce_dgst}; } [[nodiscard]] bool is_signature_transaction() const override @@ -137,6 +134,7 @@ namespace ccf nlohmann::json describe_receipt_v1(const TxReceiptImpl& receipt); ReceiptPtr describe_receipt_v2(const TxReceiptImpl& in); + // NOLINTNEXTLINE(performance-enum-size) enum MerkleProofLabel : int64_t { // Values set in @@ -160,19 +158,24 @@ namespace ccf void to_json(nlohmann::json& j, const ProofReceipt::Components& components); void from_json(const nlohmann::json& j, ProofReceipt::Components& components); - std::string schema_name(const ProofReceipt::Components*); + std::string schema_name( + [[maybe_unused]] const ProofReceipt::Components* components); void fill_json_schema( - nlohmann::json& schema, const ProofReceipt::Components*); + nlohmann::json& schema, + [[maybe_unused]] const ProofReceipt::Components* components); void to_json(nlohmann::json& j, const ProofReceipt::ProofStep& step); void from_json(const nlohmann::json& j, ProofReceipt::ProofStep& step); - std::string schema_name(const ProofReceipt::ProofStep*); - void fill_json_schema(nlohmann::json& schema, const ProofReceipt::ProofStep*); + std::string schema_name([[maybe_unused]] const ProofReceipt::ProofStep* step); + void fill_json_schema( + nlohmann::json& schema, + [[maybe_unused]] const ProofReceipt::ProofStep* step); void to_json(nlohmann::json& j, const ReceiptPtr& receipt); void from_json(const nlohmann::json& j, ReceiptPtr& receipt); - std::string schema_name(const ReceiptPtr*); - void fill_json_schema(nlohmann::json& schema, const ReceiptPtr*); + std::string schema_name([[maybe_unused]] const ReceiptPtr* receipt); + void fill_json_schema( + nlohmann::json& schema, [[maybe_unused]] const ReceiptPtr* receipt); template void add_schema_components( diff --git a/include/ccf/rest_verb.h b/include/ccf/rest_verb.h index 018e8a040b65..2f63de688c6c 100644 --- a/include/ccf/rest_verb.h +++ b/include/ccf/rest_verb.h @@ -51,7 +51,7 @@ namespace ccf RESTVerb(const llhttp_method& hm) : verb(hm) {} RESTVerb(const std::string& s) { - verb = http_method_from_str(s.c_str()); + verb = http_method_from_str(s); } [[nodiscard]] std::optional get_http_method() const @@ -100,7 +100,7 @@ namespace ccf std::string s = j.get(); ccf::nonstd::to_upper(s); - verb = RESTVerb(s.c_str()); + verb = RESTVerb(s); } inline std::string schema_name([[maybe_unused]] const RESTVerb* verb_type) diff --git a/include/ccf/rpc_exception.h b/include/ccf/rpc_exception.h index 0f6fc02ad1b4..e5e49332aa58 100644 --- a/include/ccf/rpc_exception.h +++ b/include/ccf/rpc_exception.h @@ -18,7 +18,7 @@ namespace ccf error{status, code, std::move(msg)} {} - const char* what() const throw() override + [[nodiscard]] const char* what() const noexcept override { return error.msg.c_str(); } diff --git a/include/ccf/tx.h b/include/ccf/tx.h index ee0971a7d334..a2851271e2d9 100644 --- a/include/ccf/tx.h +++ b/include/ccf/tx.h @@ -86,27 +86,26 @@ namespace ccf::kv { auto& [abstract_map, change_set] = it->second; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) auto typed_handle = new THandle(*change_set, map_name); std::unique_ptr abstract_handle(typed_handle); retain_handle(map_name, std::move(abstract_handle)); return typed_handle; } - else - { - auto [abstract_map, change_set] = get_map_and_change_set_by_name( - map_name, track_deletes_on_missing_keys); + auto [abstract_map, change_set] = + get_map_and_change_set_by_name(map_name, track_deletes_on_missing_keys); - if (change_set == nullptr) - { - compacted_version_conflict(map_name); - } - - auto typed_handle = new THandle(*change_set, map_name); - std::unique_ptr abstract_handle(typed_handle); - retain_handle(map_name, std::move(abstract_handle)); - retain_change_set(map_name, std::move(change_set), abstract_map); - return typed_handle; + if (change_set == nullptr) + { + compacted_version_conflict(map_name); } + + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + auto typed_handle = new THandle(*change_set, map_name); + std::unique_ptr abstract_handle(typed_handle); + retain_handle(map_name, std::move(abstract_handle)); + retain_change_set(map_name, std::move(change_set), abstract_map); + return typed_handle; } public: diff --git a/include/ccf/tx_id.h b/include/ccf/tx_id.h index e0fea054509d..1d857883825f 100644 --- a/include/ccf/tx_id.h +++ b/include/ccf/tx_id.h @@ -52,7 +52,7 @@ namespace ccf static std::optional from_str(const std::string_view& sv) { - const auto separator_idx = sv.find("."); + const auto separator_idx = sv.find('.'); if (separator_idx == std::string_view::npos) { return std::nullopt; diff --git a/include/ccf/tx_status.h b/include/ccf/tx_status.h index 64aaff1562d0..6dceea3feac0 100644 --- a/include/ccf/tx_status.h +++ b/include/ccf/tx_status.h @@ -9,7 +9,7 @@ namespace ccf { /** Describes the status of a transaction, as seen by this node. */ - enum class TxStatus + enum class TxStatus : uint8_t { /** This node has not received this transaction, and knows nothing about it */ @@ -92,33 +92,30 @@ namespace ccf { return TxStatus::Committed; } - else - { - return TxStatus::Invalid; - } + return TxStatus::Invalid; } - else if (views_match) + + if (views_match) { // This node knows about the requested tx id, but it is not globally // committed return TxStatus::Pending; } - else if (committed_view > target_view) + + if (committed_view > target_view) { // This node has seen the seqno in a different view, and committed // further, so the requested tx id is impossible return TxStatus::Invalid; } - else - { - // Otherwise, we cannot state anything about this tx id. The most common - // reason is that the local_view is unknown (this transaction has never - // existed, or has not reached this node yet). It is also possible that - // this node believes locally that this tx id is impossible, but does not - // have a global commit to back this up - it will eventually receive - // either a global commit confirming this belief, or an election and - // global commit making this tx id invalid - return TxStatus::Unknown; - } + + // Otherwise, we cannot state anything about this tx id. The most common + // reason is that the local_view is unknown (this transaction has never + // existed, or has not reached this node yet). It is also possible that + // this node believes locally that this tx id is impossible, but does not + // have a global commit to back this up - it will eventually receive + // either a global commit confirming this belief, or an election and + // global commit making this tx id invalid + return TxStatus::Unknown; } } \ No newline at end of file diff --git a/src/endpoints/endpoint.cpp b/src/endpoints/endpoint.cpp index 0b5c3ad0f962..c4ca626790a2 100644 --- a/src/endpoints/endpoint.cpp +++ b/src/endpoints/endpoint.cpp @@ -137,7 +137,7 @@ namespace ccf::endpoints { switch (grp.kind) { - case InterpreterReusePolicy::KeyBased: + case InterpreterReusePolicy::Kind::KeyBased: { j = nlohmann::json::object(); j["key"] = grp.key; @@ -152,7 +152,7 @@ namespace ccf::endpoints const auto key_it = j.find("key"); if (key_it != j.end()) { - grp.kind = InterpreterReusePolicy::KeyBased; + grp.kind = InterpreterReusePolicy::Kind::KeyBased; grp.key = key_it->get(); } } diff --git a/src/js/extensions/ccf/historical.cpp b/src/js/extensions/ccf/historical.cpp index f457a96b71d9..e86e78347cd3 100644 --- a/src/js/extensions/ccf/historical.cpp +++ b/src/js/extensions/ccf/historical.cpp @@ -246,7 +246,7 @@ namespace ccf::js::extensions JS_CHECK_EXC(js_element); auto is_left = - element.direction == ccf::ProofReceipt::ProofStep::Left; + element.direction == ccf::ProofReceipt::ProofStep::Direction::Left; const auto hash_hex = ds::to_hex(element.hash.h); auto js_hash = jsctx.new_string(hash_hex); diff --git a/src/js/interpreter_cache.h b/src/js/interpreter_cache.h index 7230669b35da..f5a8ddcf229a 100644 --- a/src/js/interpreter_cache.h +++ b/src/js/interpreter_cache.h @@ -60,7 +60,7 @@ namespace ccf::js { switch (interpreter_reuse->kind) { - case ccf::endpoints::InterpreterReusePolicy::KeyBased: + case ccf::endpoints::InterpreterReusePolicy::Kind::KeyBased: { auto key = interpreter_reuse->key; if (access == js::TxAccess::APP_RW) diff --git a/src/js/registry.cpp b/src/js/registry.cpp index b0757bd616a9..576306eb3dd2 100644 --- a/src/js/registry.cpp +++ b/src/js/registry.cpp @@ -906,7 +906,7 @@ namespace ccf::js } std::set BaseDynamicJSEndpointRegistry::get_allowed_verbs( - ccf::kv::Tx& tx, const ccf::RpcContext& rpc_ctx) + [[maybe_unused]] ccf::kv::Tx& tx, const ccf::RpcContext& rpc_ctx) { const auto method = rpc_ctx.get_method(); diff --git a/src/node/gov/handlers/proposals.h b/src/node/gov/handlers/proposals.h index 51b9af30f67b..bf96846ebee9 100644 --- a/src/node/gov/handlers/proposals.h +++ b/src/node/gov/handlers/proposals.h @@ -215,7 +215,11 @@ namespace ccf::gov::endpoints v.set( "member_id", js_context.new_string_len(member_id.data(), member_id.size())); - v.set_bool("vote", vote_result); + if (v.set_bool("vote", vote_result) != 1) + { + throw std::runtime_error( + "Failed to set vote property on vote result object"); + } vs.set_at_index(index++, std::move(v)); } diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp index e0c429ad5940..eed55ad4a11b 100644 --- a/src/node/historical_queries_adapter.cpp +++ b/src/node/historical_queries_adapter.cpp @@ -153,8 +153,8 @@ namespace ccf { const auto direction = node.direction == ccf::HistoryTree::Path::Direction::PATH_LEFT ? - ccf::ProofReceipt::ProofStep::Left : - ccf::ProofReceipt::ProofStep::Right; + ccf::ProofReceipt::ProofStep::Direction::Left : + ccf::ProofReceipt::ProofStep::Direction::Right; const auto hash = ccf::crypto::Sha256Hash::from_span( std::span( node.hash.bytes, sizeof(node.hash.bytes))); diff --git a/src/node/receipt.cpp b/src/node/receipt.cpp index 934077bbecf3..24308c10b6b0 100644 --- a/src/node/receipt.cpp +++ b/src/node/receipt.cpp @@ -111,7 +111,8 @@ namespace ccf { j = nlohmann::json::object(); const auto* const key = - step.direction == ProofReceipt::ProofStep::Left ? "left" : "right"; + step.direction == ProofReceipt::ProofStep::Direction::Left ? "left" : + "right"; j[key] = step.hash; } @@ -135,12 +136,12 @@ namespace ccf if (l_it != j.end()) { - step.direction = ProofReceipt::ProofStep::Left; + step.direction = ProofReceipt::ProofStep::Direction::Left; step.hash = l_it.value(); } else { - step.direction = ProofReceipt::ProofStep::Right; + step.direction = ProofReceipt::ProofStep::Direction::Right; step.hash = r_it.value(); } } diff --git a/src/node/rpc_context_impl.h b/src/node/rpc_context_impl.h index 82dbbc965cbb..6d3fe5dd1331 100644 --- a/src/node/rpc_context_impl.h +++ b/src/node/rpc_context_impl.h @@ -49,10 +49,12 @@ namespace ccf } ccf::ClaimsDigest claims = ccf::empty_claims(); + // NOLINTBEGIN(performance-move-const-arg) void set_claims_digest(ccf::ClaimsDigest::Digest&& digest) override { claims.set(std::move(digest)); } + // NOLINTEND(performance-move-const-arg) ccf::PathParams path_params = {}; virtual const ccf::PathParams& get_request_path_params() override diff --git a/src/node/snapshot_serdes.h b/src/node/snapshot_serdes.h index 54dfe02db207..fa9f1c41405b 100644 --- a/src/node/snapshot_serdes.h +++ b/src/node/snapshot_serdes.h @@ -177,6 +177,7 @@ namespace ccf ccf::MerkleTreeHistory history(tree); auto proof = history.get_proof(seqno); ccf::ClaimsDigest cd; + // NOLINTNEXTLINE(performance-move-const-arg) cd.set(std::move(claims_digest)); ccf::TxReceiptImpl tx_receipt( sig, diff --git a/src/node/snapshotter.h b/src/node/snapshotter.h index bddfbfa2189c..5cc25ec4615b 100644 --- a/src/node/snapshotter.h +++ b/src/node/snapshotter.h @@ -144,6 +144,7 @@ namespace ccf evidence->put({snapshot_hash, snapshot_version}); ccf::ClaimsDigest cd; + // NOLINTNEXTLINE(performance-move-const-arg) cd.set(std::move(snapshot_hash)); ccf::crypto::Sha256Hash ws_digest; diff --git a/src/node/test/receipt.cpp b/src/node/test/receipt.cpp index e0f5a2d022fe..28186ca4dcdf 100644 --- a/src/node/test/receipt.cpp +++ b/src/node/test/receipt.cpp @@ -46,14 +46,15 @@ void populate_receipt(std::shared_ptr receipt) const auto num_proof_steps = rand() % 8; for (auto i = 0; i < num_proof_steps; ++i) { - const auto dir = rand() % 2 == 0 ? ccf::ProofReceipt::ProofStep::Left : - ccf::ProofReceipt::ProofStep::Right; + const auto dir = rand() % 2 == 0 ? + ccf::ProofReceipt::ProofStep::Direction::Left : + ccf::ProofReceipt::ProofStep::Direction::Right; const auto digest = rand_digest(); ccf::ProofReceipt::ProofStep step{dir, digest}; receipt->proof.push_back(step); - if (dir == ccf::ProofReceipt::ProofStep::Left) + if (dir == ccf::ProofReceipt::ProofStep::Direction::Left) { current_digest = ccf::crypto::Sha256Hash(digest, current_digest); } From 2c274d7e168e5b49ef743993488312a157939322 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Wed, 12 Nov 2025 17:50:23 +0000 Subject: [PATCH 07/12] Comprehensively cleanse temporary keys used for RSA AES wrap/unwrap (#7456) --- src/crypto/key_wrap.cpp | 92 +++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/src/crypto/key_wrap.cpp b/src/crypto/key_wrap.cpp index 5738c7afbeea..f032334c293b 100644 --- a/src/crypto/key_wrap.cpp +++ b/src/crypto/key_wrap.cpp @@ -93,25 +93,35 @@ namespace ccf::crypto // - Generates temporary random AES key of ulAESKeyBits length. This key is // not accessible to the user - no handle is returned. std::vector taeskey(aes_key_size / CHAR_BIT); - RAND_bytes(taeskey.data(), taeskey.size()); - - // - Wraps the AES key with the wrapping RSA key using CKM_RSA_PKCS_OAEP - // with parameters of OAEPParams. - std::vector w_aeskey = wrapping_key->rsa_oaep_wrap(taeskey, label); - - // - Wraps the target key with the temporary AES key using - // CKM_AES_KEY_WRAP_PAD (RFC5649). - auto aes = std::make_unique(taeskey); - std::vector w_target = aes->ckm_aes_key_wrap_pad(unwrapped); - - // - Zeroizes the temporary AES key. - memset(taeskey.data(), 0, taeskey.size()); - - // - Concatenates two wrapped keys and outputs the concatenated blob. - std::vector r; - r.insert(r.end(), w_aeskey.begin(), w_aeskey.end()); - r.insert(r.end(), w_target.begin(), w_target.end()); - return r; + try + { + RAND_bytes(taeskey.data(), taeskey.size()); + + // - Wraps the AES key with the wrapping RSA key using CKM_RSA_PKCS_OAEP + // with parameters of OAEPParams. + std::vector w_aeskey = + wrapping_key->rsa_oaep_wrap(taeskey, label); + + // - Wraps the target key with the temporary AES key using + // CKM_AES_KEY_WRAP_PAD (RFC5649). + auto aes = std::make_unique(taeskey); + std::vector w_target = aes->ckm_aes_key_wrap_pad(unwrapped); + + // - Zeroizes the temporary AES key. + OPENSSL_cleanse(taeskey.data(), taeskey.size()); + + // - Concatenates two wrapped keys and outputs the concatenated blob. + std::vector r; + r.insert(r.end(), w_aeskey.begin(), w_aeskey.end()); + r.insert(r.end(), w_target.begin(), w_target.end()); + return r; + } + catch (...) + { + // Ensure temporary key is zeroed even on exceptions + OPENSSL_cleanse(taeskey.data(), taeskey.size()); + throw; + } } std::vector ckm_rsa_aes_key_wrap( @@ -149,25 +159,35 @@ namespace ccf::crypto std::vector t_aeskey = wrapping_key->rsa_oaep_unwrap(w_aeskey, label); - if ( - t_aeskey.size() != AES_KEY_SIZE_128_BYTES && - t_aeskey.size() != AES_KEY_SIZE_192_BYTES && - t_aeskey.size() != AES_KEY_SIZE_256_BYTES) + try { - throw std::runtime_error("invalid key size"); + if ( + t_aeskey.size() != AES_KEY_SIZE_128_BYTES && + t_aeskey.size() != AES_KEY_SIZE_192_BYTES && + t_aeskey.size() != AES_KEY_SIZE_256_BYTES) + { + throw std::runtime_error("invalid key size"); + } + + // - Un-wraps the target key from the second part with the temporary AES + // key + // using CKM_AES_KEY_WRAP_PAD (RFC5649). + + auto aes = std::make_unique(t_aeskey); + std::vector target = aes->ckm_aes_key_unwrap_pad(w_target); + + // - Zeroizes the temporary AES key. + OPENSSL_cleanse(t_aeskey.data(), t_aeskey.size()); + + // - Returns the handle to the newly unwrapped target key. + return target; + } + catch (...) + { + // Ensure temporary key is zeroed even on exceptions + OPENSSL_cleanse(t_aeskey.data(), t_aeskey.size()); + throw; } - - // - Un-wraps the target key from the second part with the temporary AES key - // using CKM_AES_KEY_WRAP_PAD (RFC5649). - - auto aes = std::make_unique(t_aeskey); - std::vector target = aes->ckm_aes_key_unwrap_pad(w_target); - - // - Zeroizes the temporary AES key. - memset(t_aeskey.data(), 0, t_aeskey.size()); - - // - Returns the handle to the newly unwrapped target key. - return target; } std::vector ckm_rsa_aes_key_unwrap( From 6ae075d6ace84f23fda47c79c1735b7b50f71d5b Mon Sep 17 00:00:00 2001 From: cjen1-msft Date: Thu, 13 Nov 2025 10:48:12 +0000 Subject: [PATCH 08/12] Wait for sealed secrets (#7450) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Amaury Chamayou --- tests/e2e_operations.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index d6193e8f8d53..a73ec90ad89b 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -1428,6 +1428,29 @@ def run_initial_tcb_version_checks(const_args): assert False, "No TCB_version found in recovery ledger" +def wait_for_sealed_secrets(node, min_seqno=0, timeout=10): + out, _ = node.remote.get_logs() + start = time.time() + while time.time() < start + timeout: + with open(out, "r") as outf: + for line in outf.readlines(): + if "Sealing complete of ledger secret to" in line: + try: + path = line.split()[-1] + filename = os.path.basename(path) + seqno = int(filename.split(".")[0]) + if seqno >= min_seqno: + return + except (IndexError, ValueError): + continue + + time.sleep(0.1) + + raise TimeoutError( + f"Could not find sealed secrets for seqno {min_seqno} after {timeout}s in logs" + ) + + def run_recovery_local_unsealing( const_args, recovery_f=0, rekey=False, recovery_shares_refresh=False ): @@ -1444,10 +1467,19 @@ def run_recovery_local_unsealing( primary, _ = network.find_primary() if rekey: + network.wait_for_node_commit_sync() + with primary.client() as c: + r = c.get("/node/commit").body.json() + min_seqno = TxID.from_str(r["transaction_id"]).seqno network.consortium.trigger_ledger_rekey(primary) + else: + min_seqno = 0 if recovery_shares_refresh: network.consortium.trigger_recovery_shares_refresh(primary) + for node in network.nodes: + wait_for_sealed_secrets(node, min_seqno=min_seqno) + node_secret_map = { node.local_node_id: node.save_sealed_ledger_secret() for node in network.nodes @@ -1503,6 +1535,8 @@ def run_recovery_unsealing_validate_audit(const_args): network.start_and_open(args) network.save_service_identity(args) + for node in network.nodes: + wait_for_sealed_secrets(node) node0_secrets = network.nodes[0].save_sealed_ledger_secret() latest_public_tables, _ = network.get_latest_ledger_public_state() @@ -1586,6 +1620,8 @@ def run_recovery_unsealing_corrupt(const_args, recovery_f=0): network.start_and_open(args) network.save_service_identity(args) + for node in network.nodes: + wait_for_sealed_secrets(node) node_secret_map = { node.local_node_id: node.save_sealed_ledger_secret() From 8762f8639716c0ab3ee290d6ed28eb7b36379465 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 13 Nov 2025 14:00:31 +0000 Subject: [PATCH 09/12] Remove unused variables, fields, and captures (#7455) --- cmake/preproject.cmake | 2 +- include/ccf/js/core/wrapped_value.h | 10 +- samples/apps/basic/basic.cpp | 2 +- samples/apps/logging/logging.cpp | 78 +++++++--------- .../apps/programmability/programmability.cpp | 31 ++++--- src/crypto/csr.h | 1 - src/endpoints/authentication/cose_auth.cpp | 1 - src/js/checks.h | 9 ++ src/js/core/context.cpp | 3 +- src/js/extensions/ccf/consensus.cpp | 14 +-- src/js/extensions/ccf/converters.cpp | 30 +++--- src/js/extensions/ccf/crypto.cpp | 92 ++++++++++--------- src/js/extensions/ccf/gov.cpp | 7 +- src/js/extensions/ccf/gov_effects.cpp | 13 +-- src/js/extensions/ccf/historical.cpp | 12 +-- src/js/extensions/ccf/kv.cpp | 2 +- src/js/extensions/ccf/network.cpp | 15 +-- src/js/extensions/ccf/node.cpp | 23 ++--- src/js/extensions/ccf/request.cpp | 84 +++++++++-------- src/js/extensions/ccf/rpc.cpp | 11 ++- src/js/extensions/console.cpp | 15 ++- src/js/extensions/math/random.cpp | 4 +- src/js/extensions/snp_attestation.cpp | 8 +- src/js/registry.cpp | 7 +- src/kv/serialised_entry_format.h | 3 +- src/kv/store.h | 3 - src/node/gov/handlers/proposals.h | 13 +-- src/node/node_state.h | 2 - src/node/quote.cpp | 1 - src/node/rpc/jwt_management.h | 3 +- src/node/rpc/node_frontend.h | 13 +-- src/quic/quic_session.h | 2 - src/tasks/ordered_tasks.cpp | 2 +- 33 files changed, 267 insertions(+), 249 deletions(-) diff --git a/cmake/preproject.cmake b/cmake/preproject.cmake index cc75d0a3d07b..9bc9f478d95d 100644 --- a/cmake/preproject.cmake +++ b/cmake/preproject.cmake @@ -68,8 +68,8 @@ function(add_warning_checks name) -Werror -Wundef -Wpedantic - -Wno-unused -Wno-unused-parameter + -Wno-unused-function -Wshadow ) endfunction() diff --git a/include/ccf/js/core/wrapped_value.h b/include/ccf/js/core/wrapped_value.h index f36bec7a0076..7b4f832d620d 100644 --- a/include/ccf/js/core/wrapped_value.h +++ b/include/ccf/js/core/wrapped_value.h @@ -29,9 +29,10 @@ namespace ccf::js::core JSWrappedValue operator[](uint32_t i) const; - int set(const char* prop, JSWrappedValue&& value) const; + [[nodiscard]] int set(const char* prop, JSWrappedValue&& value) const; - int set_getter(const char* prop, JSWrappedValue&& getter) const; + [[nodiscard]] int set_getter( + const char* prop, JSWrappedValue&& getter) const; [[nodiscard]] int set( const std::string& prop, JSWrappedValue&& value) const; @@ -46,7 +47,8 @@ namespace ccf::js::core [[nodiscard]] int set_bool(const std::string& prop, bool b) const; - int set_at_index(uint32_t index, JSWrappedValue&& value) const; + [[nodiscard]] int set_at_index( + uint32_t index, JSWrappedValue&& value) const; [[nodiscard]] bool is_exception() const; @@ -60,6 +62,6 @@ namespace ccf::js::core [[nodiscard]] bool is_undefined() const; - JSValue take(); + [[nodiscard]] JSValue take(); }; } diff --git a/samples/apps/basic/basic.cpp b/samples/apps/basic/basic.cpp index 556f48e33d4b..8c0adeb8602b 100644 --- a/samples/apps/basic/basic.cpp +++ b/samples/apps/basic/basic.cpp @@ -95,7 +95,7 @@ namespace basicapp .set_forwarding_required(ccf::endpoints::ForwardingRequired::Never) .install(); - auto post = [this](ccf::endpoints::EndpointContext& ctx) { + auto post = [](ccf::endpoints::EndpointContext& ctx) { const nlohmann::json body = nlohmann::json::parse(ctx.rpc_ctx->get_request_body()); diff --git a/samples/apps/logging/logging.cpp b/samples/apps/logging/logging.cpp index a099fe00c14f..36f2d3631684 100644 --- a/samples/apps/logging/logging.cpp +++ b/samples/apps/logging/logging.cpp @@ -500,7 +500,7 @@ namespace loggingapp ccf::user_cose_sign1_auth_policy}; // SNIPPET_START: record - auto record = [this](auto& ctx, nlohmann::json&& params) { + auto record = [](auto& ctx, nlohmann::json&& params) { // SNIPPET_START: macro_validation_record const auto in = params.get(); // SNIPPET_END: macro_validation_record @@ -546,7 +546,7 @@ namespace loggingapp ctx.rpc_ctx->set_response_body(nlohmann::json(*out).dump()); }; - auto record_v2 = [this](auto& ctx, nlohmann::json&& params) { + auto record_v2 = [](auto& ctx, nlohmann::json&& params) { const auto in = params.get(); if (in.msg.empty()) @@ -591,7 +591,7 @@ namespace loggingapp .install(); // SNIPPET_START: get - auto get = [this](auto& ctx, nlohmann::json&&) { + auto get = [](auto& ctx, nlohmann::json&&) { // Parse id from query const auto parsed_query = ccf::http::parse_query(ctx.rpc_ctx->get_request_query()); @@ -758,7 +758,7 @@ namespace loggingapp .add_query_parameter("id") .install(); - auto remove = [this](auto& ctx, nlohmann::json&&) { + auto remove = [](auto& ctx, nlohmann::json&&) { // Parse id from query const auto parsed_query = ccf::http::parse_query(ctx.rpc_ctx->get_request_query()); @@ -786,7 +786,7 @@ namespace loggingapp .add_query_parameter("id") .install(); - auto clear = [this](auto& ctx, nlohmann::json&&) { + auto clear = [](auto& ctx, nlohmann::json&&) { auto records_handle = ctx.tx.template rw(private_records(ctx)); records_handle->clear(); @@ -800,7 +800,7 @@ namespace loggingapp .set_auto_schema() .install(); - auto count = [this](auto& ctx, nlohmann::json&&) { + auto count = [](auto& ctx, nlohmann::json&&) { auto records_handle = ctx.tx.template ro(private_records(ctx)); return ccf::make_success(records_handle->size()); @@ -811,7 +811,7 @@ namespace loggingapp .install(); // SNIPPET_START: record_public - auto record_public = [this](auto& ctx, nlohmann::json&& params) { + auto record_public = [](auto& ctx, nlohmann::json&& params) { const auto in = params.get(); if (in.msg.empty()) @@ -905,7 +905,7 @@ namespace loggingapp .install(); // SNIPPET_START: get_public - auto get_public = [this](auto& ctx, nlohmann::json&&) { + auto get_public = [](auto& ctx, nlohmann::json&&) { // Parse id from query const auto parsed_query = ccf::http::parse_query(ctx.rpc_ctx->get_request_query()); @@ -998,7 +998,7 @@ namespace loggingapp .add_query_parameter("id") .install(); - auto remove_public = [this](auto& ctx, nlohmann::json&&) { + auto remove_public = [](auto& ctx, nlohmann::json&&) { // Parse id from query const auto parsed_query = ccf::http::parse_query(ctx.rpc_ctx->get_request_query()); @@ -1075,7 +1075,7 @@ namespace loggingapp .add_query_parameter("id") .install(); - auto clear_public = [this](auto& ctx, nlohmann::json&&) { + auto clear_public = [](auto& ctx, nlohmann::json&&) { auto public_records_handle = ctx.tx.template rw(public_records(ctx)); public_records_handle->clear(); @@ -1089,7 +1089,7 @@ namespace loggingapp .set_auto_schema() .install(); - auto count_public = [this](auto& ctx, nlohmann::json&&) { + auto count_public = [](auto& ctx, nlohmann::json&&) { auto public_records_handle = ctx.tx.template ro(public_records(ctx)); return ccf::make_success(public_records_handle->size()); @@ -1103,7 +1103,7 @@ namespace loggingapp .install(); // SNIPPET_START: log_record_prefix_cert - auto log_record_prefix_cert = [this](auto& ctx) { + auto log_record_prefix_cert = [](auto& ctx) { const auto& caller_ident = ctx.template get_caller(); @@ -1137,7 +1137,7 @@ namespace loggingapp .install(); // SNIPPET_END: log_record_prefix_cert - auto log_record_anonymous = [this](auto& ctx, nlohmann::json&& params) { + auto log_record_anonymous = [](auto& ctx, nlohmann::json&& params) { const auto in = params.get(); if (in.msg.empty()) { @@ -1221,7 +1221,7 @@ namespace loggingapp // SNIPPET_END: custom_auth_endpoint // SNIPPET_START: log_record_text - auto log_record_text = [this](auto& ctx) { + auto log_record_text = [](auto& ctx) { const auto* const expected = ccf::http::headervalues::contenttype::TEXT; const auto actual = ctx.rpc_ctx->get_request_header(ccf::http::headers::CONTENT_TYPE) @@ -1264,7 +1264,7 @@ namespace loggingapp // SNIPPET_END: log_record_text // SNIPPET_START: get_historical - auto get_historical = [this]( + auto get_historical = []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { // Parse id from query @@ -1410,7 +1410,7 @@ namespace loggingapp // SNIPPET_START: get_historical_with_receipt auto get_historical_with_receipt = - [this]( + []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { // Parse id from query @@ -1459,7 +1459,7 @@ namespace loggingapp // SNIPPET_END: get_historical_with_receipt auto get_historical_with_receipt_and_claims = - [this]( + []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { // Parse id from query @@ -1863,17 +1863,6 @@ namespace loggingapp // NB: Currently ignoring pagination, as this endpoint is temporary - // Use hash of request as RequestHandle. WARNING: This means identical - // requests from different users will collide, and overwrite each - // other's progress! - auto make_handle = [](size_t begin, size_t end, size_t id) { - size_t raw[] = {begin, end, id}; - auto size = sizeof(raw); - std::vector v(size); - memcpy(v.data(), reinterpret_cast(raw), size); - return std::hash()(v); - }; - ccf::historical::RequestHandle handle = 0; { std::hash h; @@ -1997,7 +1986,7 @@ namespace loggingapp .set_auto_schema() .install(); - auto get_request_query = [this](auto& ctx) { + auto get_request_query = [](auto& ctx) { ctx.rpc_ctx->set_response_status(HTTP_STATUS_OK); std::vector rq( ctx.rpc_ctx->get_request_query().begin(), @@ -2013,19 +2002,18 @@ namespace loggingapp .set_auto_schema() .install(); - auto post_cose_signed_content = - [this](ccf::endpoints::EndpointContext& ctx) { - const auto& caller_identity = - ctx.template get_caller(); + auto post_cose_signed_content = [](ccf::endpoints::EndpointContext& ctx) { + const auto& caller_identity = + ctx.template get_caller(); - ctx.rpc_ctx->set_response_header( - ccf::http::headers::CONTENT_TYPE, - ccf::http::headervalues::contenttype::TEXT); - std::vector response_body( - caller_identity.content.begin(), caller_identity.content.end()); - ctx.rpc_ctx->set_response_body(response_body); - ctx.rpc_ctx->set_response_status(HTTP_STATUS_OK); - }; + ctx.rpc_ctx->set_response_header( + ccf::http::headers::CONTENT_TYPE, + ccf::http::headervalues::contenttype::TEXT); + std::vector response_body( + caller_identity.content.begin(), caller_identity.content.end()); + ctx.rpc_ctx->set_response_body(response_body); + ctx.rpc_ctx->set_response_status(HTTP_STATUS_OK); + }; make_endpoint( "/log/cose_signed_content", @@ -2036,7 +2024,7 @@ namespace loggingapp .install(); auto get_cbor_merkle_proof = - [this]( + []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { auto historical_tx = historical_state->store->create_read_only_tx(); @@ -2069,7 +2057,7 @@ namespace loggingapp .install(); auto get_cose_endorsements = - [this]( + []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { auto historical_tx = historical_state->store->create_read_only_tx(); @@ -2104,7 +2092,7 @@ namespace loggingapp .set_forwarding_required(ccf::endpoints::ForwardingRequired::Never) .install(); - auto get_cose_signature = [this]( + auto get_cose_signature = []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { auto historical_tx = historical_state->store->create_read_only_tx(); @@ -2135,7 +2123,7 @@ namespace loggingapp .set_forwarding_required(ccf::endpoints::ForwardingRequired::Never) .install(); - auto get_cose_receipt = [this]( + auto get_cose_receipt = []( ccf::endpoints::ReadOnlyEndpointContext& ctx, ccf::historical::StatePtr historical_state) { auto historical_tx = historical_state->store->create_read_only_tx(); diff --git a/samples/apps/programmability/programmability.cpp b/samples/apps/programmability/programmability.cpp index 4218eb63a194..4d337b82fe2c 100644 --- a/samples/apps/programmability/programmability.cpp +++ b/samples/apps/programmability/programmability.cpp @@ -165,19 +165,28 @@ namespace programmabilityapp // Insert a constant string into the JS environment, accessible at // my_object.my_constant - my_global_object.set("my_constant", ctx.new_string("Hello world")); + if (my_global_object.set("my_constant", ctx.new_string("Hello world")) != 1) + { + throw std::runtime_error( + "Unable to set JS field my_constant while constructing MyExtension"); + } // Insert a function into the JS environment, called at my_object.has_role - my_global_object.set( - // Name of field on object - "hasRole", - ctx.new_c_function( - // C/C++ function implementing this JS function - js_has_role_permitting_action, - // Repeated name of function, used in callstacks + if ( + my_global_object.set( + // Name of field on object "hasRole", - // Number of arguments to this function - 2)); + ctx.new_c_function( + // C/C++ function implementing this JS function + js_has_role_permitting_action, + // Repeated name of function, used in callstacks + "hasRole", + // Number of arguments to this function + 2)) != 1) + { + throw std::runtime_error( + "Unable to set JS field hasRole while constructing MyExtension"); + } } // This sample shows the features of DynamicJSEndpointRegistry. This sample @@ -339,7 +348,7 @@ namespace programmabilityapp .set_forwarding_required(ccf::endpoints::ForwardingRequired::Never) .install(); - auto post = [this](ccf::endpoints::EndpointContext& ctx) { + auto post = [](ccf::endpoints::EndpointContext& ctx) { const nlohmann::json body = nlohmann::json::parse(ctx.rpc_ctx->get_request_body()); diff --git a/src/crypto/csr.h b/src/crypto/csr.h index 73cc2789f533..46f9bf94bf66 100644 --- a/src/crypto/csr.h +++ b/src/crypto/csr.h @@ -15,7 +15,6 @@ namespace ccf::crypto */ Pem public_key_pem_from_csr(const Pem& signing_request) { - X509* icrt = NULL; OpenSSL::Unique_BIO mem(signing_request); OpenSSL::Unique_X509_REQ csr(mem); OpenSSL::Unique_BIO buf; diff --git a/src/endpoints/authentication/cose_auth.cpp b/src/endpoints/authentication/cose_auth.cpp index b65473180b94..88671ebc9113 100644 --- a/src/endpoints/authentication/cose_auth.cpp +++ b/src/endpoints/authentication/cose_auth.cpp @@ -225,7 +225,6 @@ namespace ccf header_items[MSG_TYPE].uLabelType = QCBOR_TYPE_TEXT_STRING; header_items[MSG_TYPE].uDataType = QCBOR_TYPE_TEXT_STRING; - const auto* gov_msg_proposal_created_at = HEADER_PARAM_MSG_CREATED_AT; header_items[MSG_CREATED_AT].label.string = UsefulBuf_FromSZ(created_at_name.c_str()); header_items[MSG_CREATED_AT].uLabelType = QCBOR_TYPE_TEXT_STRING; diff --git a/src/js/checks.h b/src/js/checks.h index a5850f6e5e9b..d22c0e7beb40 100644 --- a/src/js/checks.h +++ b/src/js/checks.h @@ -19,3 +19,12 @@ return ccf::js::core::constants::Exception; \ } \ } while (0) + +#define JS_CHECK_OR_THROW(val) \ + do \ + { \ + if (val != 1) \ + { \ + throw std::runtime_error("Unable to populate JS object"); \ + } \ + } while (0) diff --git a/src/js/core/context.cpp b/src/js/core/context.cpp index 492a0bfe5d72..3df33c30d934 100644 --- a/src/js/core/context.cpp +++ b/src/js/core/context.cpp @@ -9,6 +9,7 @@ #include "ccf/js/extensions/console.h" #include "ccf/js/tx_access.h" #include "ds/internal_logger.h" +#include "js/checks.h" #include "js/global_class_ids.h" #include @@ -228,7 +229,7 @@ namespace ccf::js::core if (val.is_undefined()) { val = default_value; - g.set(s, std::move(default_value)); + JS_CHECK_OR_THROW(g.set(s, std::move(default_value))); } return val; diff --git a/src/js/extensions/ccf/consensus.cpp b/src/js/extensions/ccf/consensus.cpp index 5f094278987e..ce808229dc84 100644 --- a/src/js/extensions/ccf/consensus.cpp +++ b/src/js/extensions/ccf/consensus.cpp @@ -176,20 +176,20 @@ namespace ccf::js::extensions { auto consensus = ctx.new_obj(); - consensus.set( + JS_CHECK_OR_THROW(consensus.set( "getLastCommittedTxId", ctx.new_c_function( - js_consensus_get_last_committed_txid, "getLastCommittedTxId", 0)); - consensus.set( + js_consensus_get_last_committed_txid, "getLastCommittedTxId", 0))); + JS_CHECK_OR_THROW(consensus.set( "getStatusForTxId", ctx.new_c_function( - js_consensus_get_status_for_txid, "getStatusForTxId", 2)); - consensus.set( + js_consensus_get_status_for_txid, "getStatusForTxId", 2))); + JS_CHECK_OR_THROW(consensus.set( "getViewForSeqno", ctx.new_c_function( - js_consensus_get_view_for_seqno, "getViewForSeqno", 1)); + js_consensus_get_view_for_seqno, "getViewForSeqno", 1))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("consensus", std::move(consensus)); + JS_CHECK_OR_THROW(ccf.set("consensus", std::move(consensus))); } } diff --git a/src/js/extensions/ccf/converters.cpp b/src/js/extensions/ccf/converters.cpp index e600df4445d8..8d5d1b7f122b 100644 --- a/src/js/extensions/ccf/converters.cpp +++ b/src/js/extensions/ccf/converters.cpp @@ -264,29 +264,33 @@ namespace ccf::js::extensions { auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("strToBuf", ctx.new_c_function(js_str_to_buf, "strToBuf", 1)); - ccf.set("bufToStr", ctx.new_c_function(js_buf_to_str, "bufToStr", 1)); + JS_CHECK_OR_THROW( + ccf.set("strToBuf", ctx.new_c_function(js_str_to_buf, "strToBuf", 1))); + JS_CHECK_OR_THROW( + ccf.set("bufToStr", ctx.new_c_function(js_buf_to_str, "bufToStr", 1))); - ccf.set( + JS_CHECK_OR_THROW(ccf.set( "jsonCompatibleToBuf", - ctx.new_c_function(js_json_compatible_to_buf, "jsonCompatibleToBuf", 1)); - ccf.set( + ctx.new_c_function(js_json_compatible_to_buf, "jsonCompatibleToBuf", 1))); + JS_CHECK_OR_THROW(ccf.set( "bufToJsonCompatible", - ctx.new_c_function(js_buf_to_json_compatible, "bufToJsonCompatible", 1)); + ctx.new_c_function(js_buf_to_json_compatible, "bufToJsonCompatible", 1))); - ccf.set( + JS_CHECK_OR_THROW(ccf.set( "enableUntrustedDateTime", ctx.new_c_function( - js_enable_untrusted_date_time, "enableUntrustedDateTime", 1)); + js_enable_untrusted_date_time, "enableUntrustedDateTime", 1))); - ccf.set( + JS_CHECK_OR_THROW(ccf.set( "enableMetricsLogging", - ctx.new_c_function(js_enable_metrics_logging, "enableMetricsLogging", 1)); + ctx.new_c_function( + js_enable_metrics_logging, "enableMetricsLogging", 1))); - ccf.set("pemToId", ctx.new_c_function(js_pem_to_id, "pemToId", 1)); + JS_CHECK_OR_THROW( + ccf.set("pemToId", ctx.new_c_function(js_pem_to_id, "pemToId", 1))); - ccf.set( + JS_CHECK_OR_THROW(ccf.set( "tcbHexToPolicy", - ctx.new_c_function(js_tcb_hex_to_policy, "tcbHexToPolicy", 2)); + ctx.new_c_function(js_tcb_hex_to_policy, "tcbHexToPolicy", 2))); } } diff --git a/src/js/extensions/ccf/crypto.cpp b/src/js/extensions/ccf/crypto.cpp index 7515f3ce8d9e..44ccc25efbe6 100644 --- a/src/js/extensions/ccf/crypto.cpp +++ b/src/js/extensions/ccf/crypto.cpp @@ -542,7 +542,7 @@ namespace ccf::js::extensions } catch (const std::exception& ex) { - auto e = JS_ThrowInternalError( + return JS_ThrowInternalError( ctx, "Failed to convert jwk to pem %s", ex.what()); } @@ -1121,93 +1121,97 @@ namespace ccf::js::extensions { auto crypto = ctx.new_obj(); - crypto.set("sign", ctx.new_c_function(js_sign, "sign", 3)); - crypto.set( + JS_CHECK_OR_THROW( + crypto.set("sign", ctx.new_c_function(js_sign, "sign", 3))); + JS_CHECK_OR_THROW(crypto.set( "verifySignature", - ctx.new_c_function(js_verify_signature, "verifySignature", 4)); - crypto.set( + ctx.new_c_function(js_verify_signature, "verifySignature", 4))); + JS_CHECK_OR_THROW(crypto.set( "pubPemToJwk", ctx.new_c_function( - js_pem_to_jwk, "pubPemToJwk", 1)); - crypto.set( + js_pem_to_jwk, "pubPemToJwk", 1))); + JS_CHECK_OR_THROW(crypto.set( "pemToJwk", ctx.new_c_function( - js_pem_to_jwk, "pemToJwk", 1)); - crypto.set( + js_pem_to_jwk, "pemToJwk", 1))); + JS_CHECK_OR_THROW(crypto.set( "pubRsaPemToJwk", ctx.new_c_function( - js_pem_to_jwk, "pubRsaPemToJwk", 1)); - crypto.set( + js_pem_to_jwk, "pubRsaPemToJwk", 1))); + JS_CHECK_OR_THROW(crypto.set( "rsaPemToJwk", ctx.new_c_function( - js_pem_to_jwk, "rsaPemToJwk", 1)); - crypto.set( + js_pem_to_jwk, "rsaPemToJwk", 1))); + JS_CHECK_OR_THROW(crypto.set( "pubEddsaPemToJwk", ctx.new_c_function( js_pem_to_jwk, "pubEddsaPemToJwk", - 1)); - crypto.set( + 1))); + JS_CHECK_OR_THROW(crypto.set( "eddsaPemToJwk", ctx.new_c_function( js_pem_to_jwk, "eddsaPemToJwk", - 1)); - crypto.set( + 1))); + JS_CHECK_OR_THROW(crypto.set( "pubJwkToPem", ctx.new_c_function( - js_jwk_to_pem, "pubJwkToPem", 1)); - crypto.set( + js_jwk_to_pem, "pubJwkToPem", 1))); + JS_CHECK_OR_THROW(crypto.set( "jwkToPem", ctx.new_c_function( - js_jwk_to_pem, "jwkToPem", 1)); - crypto.set( + js_jwk_to_pem, "jwkToPem", 1))); + JS_CHECK_OR_THROW(crypto.set( "pubRsaJwkToPem", ctx.new_c_function( - js_jwk_to_pem, "pubRsaJwkToPem", 1)); - crypto.set( + js_jwk_to_pem, "pubRsaJwkToPem", 1))); + JS_CHECK_OR_THROW(crypto.set( "rsaJwkToPem", ctx.new_c_function( - js_jwk_to_pem, "rsaJwkToPem", 1)); - crypto.set( + js_jwk_to_pem, "rsaJwkToPem", 1))); + JS_CHECK_OR_THROW(crypto.set( "pubEddsaJwkToPem", ctx.new_c_function( js_jwk_to_pem, "pubEddsaJwkToPem", - 1)); - crypto.set( + 1))); + JS_CHECK_OR_THROW(crypto.set( "eddsaJwkToPem", ctx.new_c_function( js_jwk_to_pem, "eddsaJwkToPem", - 1)); - crypto.set( + 1))); + JS_CHECK_OR_THROW(crypto.set( "generateAesKey", - ctx.new_c_function(js_generate_aes_key, "generateAesKey", 1)); - crypto.set( + ctx.new_c_function(js_generate_aes_key, "generateAesKey", 1))); + JS_CHECK_OR_THROW(crypto.set( "generateRsaKeyPair", - ctx.new_c_function(js_generate_rsa_key_pair, "generateRsaKeyPair", 1)); - crypto.set( + ctx.new_c_function(js_generate_rsa_key_pair, "generateRsaKeyPair", 1))); + JS_CHECK_OR_THROW(crypto.set( "generateEcdsaKeyPair", ctx.new_c_function( - js_generate_ecdsa_key_pair, "generateEcdsaKeyPair", 1)); - crypto.set( + js_generate_ecdsa_key_pair, "generateEcdsaKeyPair", 1))); + JS_CHECK_OR_THROW(crypto.set( "generateEddsaKeyPair", ctx.new_c_function( - js_generate_eddsa_key_pair, "generateEddsaKeyPair", 1)); - crypto.set("wrapKey", ctx.new_c_function(js_wrap_key, "wrapKey", 3)); - crypto.set("unwrapKey", ctx.new_c_function(js_unwrap_key, "unwrapKey", 3)); - crypto.set("digest", ctx.new_c_function(js_digest, "digest", 2)); - crypto.set( + js_generate_eddsa_key_pair, "generateEddsaKeyPair", 1))); + JS_CHECK_OR_THROW( + crypto.set("wrapKey", ctx.new_c_function(js_wrap_key, "wrapKey", 3))); + JS_CHECK_OR_THROW(crypto.set( + "unwrapKey", ctx.new_c_function(js_unwrap_key, "unwrapKey", 3))); + JS_CHECK_OR_THROW( + crypto.set("digest", ctx.new_c_function(js_digest, "digest", 2))); + JS_CHECK_OR_THROW(crypto.set( "isValidX509CertBundle", ctx.new_c_function( - js_is_valid_x509_cert_bundle, "isValidX509CertBundle", 1)); - crypto.set( + js_is_valid_x509_cert_bundle, "isValidX509CertBundle", 1))); + JS_CHECK_OR_THROW(crypto.set( "isValidX509CertChain", ctx.new_c_function( - js_is_valid_x509_cert_chain, "isValidX509CertChain", 2)); + js_is_valid_x509_cert_chain, "isValidX509CertChain", 2))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("crypto", std::move(crypto)); + JS_CHECK_OR_THROW(ccf.set("crypto", std::move(crypto))); } } \ No newline at end of file diff --git a/src/js/extensions/ccf/gov.cpp b/src/js/extensions/ccf/gov.cpp index c920fadfe69c..1bf0fef509ab 100644 --- a/src/js/extensions/ccf/gov.cpp +++ b/src/js/extensions/ccf/gov.cpp @@ -4,6 +4,7 @@ #include "ccf/js/extensions/ccf/gov.h" #include "ccf/js/core/context.h" +#include "js/checks.h" #include #include @@ -135,11 +136,11 @@ namespace ccf::js::extensions { auto gov = ctx.new_obj(); - gov.set( + JS_CHECK_OR_THROW(gov.set( "validateConstitution", - ctx.new_c_function(js_validate_constitution, "validateConstitution", 1)); + ctx.new_c_function(js_validate_constitution, "validateConstitution", 1))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("gov", std::move(gov)); + JS_CHECK_OR_THROW(ccf.set("gov", std::move(gov))); } } diff --git a/src/js/extensions/ccf/gov_effects.cpp b/src/js/extensions/ccf/gov_effects.cpp index 457304ad15d6..59ce1015d9eb 100644 --- a/src/js/extensions/ccf/gov_effects.cpp +++ b/src/js/extensions/ccf/gov_effects.cpp @@ -8,6 +8,7 @@ #include "ccf/js/core/context.h" #include "ccf/version.h" +#include "js/checks.h" #include "js/modules/kv_module_loader.h" #include "node/rpc/jwt_management.h" @@ -229,19 +230,19 @@ namespace ccf::js::extensions { auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set( + JS_CHECK_OR_THROW(ccf.set( "refreshAppBytecodeCache", ctx.new_c_function( - js_refresh_app_bytecode_cache, "refreshAppBytecodeCache", 0)); - ccf.set( + js_refresh_app_bytecode_cache, "refreshAppBytecodeCache", 0))); + JS_CHECK_OR_THROW(ccf.set( "setJwtPublicSigningKeys", ctx.new_c_function( - js_gov_set_jwt_public_signing_keys, "setJwtPublicSigningKeys", 3)); - ccf.set( + js_gov_set_jwt_public_signing_keys, "setJwtPublicSigningKeys", 3))); + JS_CHECK_OR_THROW(ccf.set( "removeJwtPublicSigningKeys", ctx.new_c_function( js_gov_remove_jwt_public_signing_keys, "removeJwtPublicSigningKeys", - 1)); + 1))); } } diff --git a/src/js/extensions/ccf/historical.cpp b/src/js/extensions/ccf/historical.cpp index e86e78347cd3..221bbb4c123e 100644 --- a/src/js/extensions/ccf/historical.cpp +++ b/src/js/extensions/ccf/historical.cpp @@ -85,8 +85,6 @@ namespace ccf::js::extensions ctx, "Invalid handle or seqno or expiry: cannot be negative"); } - ccf::View view = 0; - ccf::SeqNo seqno = 0; std::vector states; try { @@ -383,16 +381,16 @@ namespace ccf::js::extensions { auto historical = ctx.new_obj(); - historical.set( + JS_CHECK_OR_THROW(historical.set( "getStateRange", - ctx.new_c_function(js_historical_get_state_range, "getStateRange", 4)); - historical.set( + ctx.new_c_function(js_historical_get_state_range, "getStateRange", 4))); + JS_CHECK_OR_THROW(historical.set( "dropCachedStates", ctx.new_c_function( - js_historical_drop_cached_states, "dropCachedStates", 1)); + js_historical_drop_cached_states, "dropCachedStates", 1))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("historical", std::move(historical)); + JS_CHECK_OR_THROW(ccf.set("historical", std::move(historical))); } js::core::JSWrappedValue HistoricalExtension::create_historical_state_object( diff --git a/src/js/extensions/ccf/kv.cpp b/src/js/extensions/ccf/kv.cpp index 75878b2699ff..dc10689f87d5 100644 --- a/src/js/extensions/ccf/kv.cpp +++ b/src/js/extensions/ccf/kv.cpp @@ -160,7 +160,7 @@ namespace ccf::js::extensions auto kv = ctx.new_obj_class(kv_class_id); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("kv", std::move(kv)); + JS_CHECK_OR_THROW(ccf.set("kv", std::move(kv))); } void KvExtension::rethrow_trapped_exceptions() const diff --git a/src/js/extensions/ccf/network.cpp b/src/js/extensions/ccf/network.cpp index 75809f1c7936..cdfdd744a834 100644 --- a/src/js/extensions/ccf/network.cpp +++ b/src/js/extensions/ccf/network.cpp @@ -4,6 +4,7 @@ #include "js/extensions/ccf/network.h" #include "ccf/js/core/context.h" +#include "js/checks.h" #include "node/network_state.h" #include @@ -182,24 +183,24 @@ namespace ccf::js::extensions { auto network = ctx.new_obj(); - network.set( + JS_CHECK_OR_THROW(network.set( "getLatestLedgerSecretSeqno", ctx.new_c_function( js_network_latest_ledger_secret_seqno, "getLatestLedgerSecretSeqno", - 0)); - network.set( + 0))); + JS_CHECK_OR_THROW(network.set( "generateEndorsedCertificate", ctx.new_c_function( js_network_generate_endorsed_certificate, "generateEndorsedCertificate", - 0)); - network.set( + 0))); + JS_CHECK_OR_THROW(network.set( "generateNetworkCertificate", ctx.new_c_function( - js_network_generate_certificate, "generateNetworkCertificate", 0)); + js_network_generate_certificate, "generateNetworkCertificate", 0))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("network", std::move(network)); + JS_CHECK_OR_THROW(ccf.set("network", std::move(network))); } } diff --git a/src/js/extensions/ccf/node.cpp b/src/js/extensions/ccf/node.cpp index eaf4b1396809..b5005873576f 100644 --- a/src/js/extensions/ccf/node.cpp +++ b/src/js/extensions/ccf/node.cpp @@ -4,6 +4,7 @@ #include "js/extensions/ccf/node.h" #include "ccf/js/core/context.h" +#include "js/checks.h" #include "node/rpc/gov_logging.h" #include @@ -283,28 +284,28 @@ namespace ccf::js::extensions { auto node = ctx.new_obj(); - node.set( + JS_CHECK_OR_THROW(node.set( "triggerLedgerRekey", ctx.new_c_function( - js_node_trigger_ledger_rekey, "triggerLedgerRekey", 0)); - node.set( + js_node_trigger_ledger_rekey, "triggerLedgerRekey", 0))); + JS_CHECK_OR_THROW(node.set( "transitionServiceToOpen", ctx.new_c_function( - js_node_transition_service_to_open, "transitionServiceToOpen", 2)); - node.set( + js_node_transition_service_to_open, "transitionServiceToOpen", 2))); + JS_CHECK_OR_THROW(node.set( "triggerRecoverySharesRefresh", ctx.new_c_function( js_node_trigger_recovery_shares_refresh, "triggerRecoverySharesRefresh", - 0)); - node.set( + 0))); + JS_CHECK_OR_THROW(node.set( "triggerLedgerChunk", - ctx.new_c_function(js_trigger_ledger_chunk, "triggerLedgerChunk", 0)); - node.set( + ctx.new_c_function(js_trigger_ledger_chunk, "triggerLedgerChunk", 0))); + JS_CHECK_OR_THROW(node.set( "triggerSnapshot", - ctx.new_c_function(js_trigger_snapshot, "triggerSnapshot", 0)); + ctx.new_c_function(js_trigger_snapshot, "triggerSnapshot", 0))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("node", std::move(node)); + JS_CHECK_OR_THROW(ccf.set("node", std::move(node))); } } \ No newline at end of file diff --git a/src/js/extensions/ccf/request.cpp b/src/js/extensions/ccf/request.cpp index df45590089fd..656de48a9392 100644 --- a/src/js/extensions/ccf/request.cpp +++ b/src/js/extensions/ccf/request.cpp @@ -11,6 +11,7 @@ #include "ccf/endpoints/authentication/js.h" #include "ccf/endpoints/authentication/jwt_auth.h" #include "ccf/js/core/context.h" +#include "js/checks.h" #include @@ -132,17 +133,19 @@ namespace ccf::js::extensions const auto* jwt_ident = dynamic_cast(ident.get())) { - caller.set( - "policy", ctx.new_string(ccf::get_policy_name_from_ident(jwt_ident))); + JS_CHECK_OR_THROW(caller.set( + "policy", + ctx.new_string(ccf::get_policy_name_from_ident(jwt_ident)))); auto jwt = ctx.new_obj(); - jwt.set( + JS_CHECK_OR_THROW(jwt.set( "keyIssuer", ctx.new_string_len( - jwt_ident->key_issuer.data(), jwt_ident->key_issuer.size())); - jwt.set("header", ctx.parse_json(jwt_ident->header)); - jwt.set("payload", ctx.parse_json(jwt_ident->payload)); - caller.set("jwt", std::move(jwt)); + jwt_ident->key_issuer.data(), jwt_ident->key_issuer.size()))); + JS_CHECK_OR_THROW(jwt.set("header", ctx.parse_json(jwt_ident->header))); + JS_CHECK_OR_THROW( + jwt.set("payload", ctx.parse_json(jwt_ident->payload))); + JS_CHECK_OR_THROW(caller.set("jwt", std::move(jwt))); return caller; } @@ -150,9 +153,9 @@ namespace ccf::js::extensions const auto* empty_ident = dynamic_cast(ident.get())) { - caller.set( + JS_CHECK_OR_THROW(caller.set( "policy", - ctx.new_string(ccf::get_policy_name_from_ident(empty_ident))); + ctx.new_string(ccf::get_policy_name_from_ident(empty_ident)))); return caller; } if ( @@ -163,12 +166,12 @@ namespace ccf::js::extensions uint32_t i = 0; for (const auto& [name, sub_ident] : all_of_ident->identities) { - policy.set_at_index(i++, ctx.new_string(name)); - caller.set( + JS_CHECK_OR_THROW(policy.set_at_index(i++, ctx.new_string(name))); + JS_CHECK_OR_THROW(caller.set( name, - create_caller_ident_obj(ctx, endpoint_ctx, sub_ident, registry)); + create_caller_ident_obj(ctx, endpoint_ctx, sub_ident, registry))); } - caller.set("policy", std::move(policy)); + JS_CHECK_OR_THROW(caller.set("policy", std::move(policy))); return caller; } @@ -182,9 +185,9 @@ namespace ccf::js::extensions { const auto* policy_name = ccf::get_policy_name_from_ident(any_cert_ident); - caller.set("policy", ctx.new_string(policy_name)); + JS_CHECK_OR_THROW(caller.set("policy", ctx.new_string(policy_name))); auto pem_cert = ccf::crypto::cert_der_to_pem(any_cert_ident->cert); - caller.set("cert", ctx.new_string(pem_cert.str())); + JS_CHECK_OR_THROW(caller.set("cert", ctx.new_string(pem_cert.str()))); return caller; } @@ -217,11 +220,11 @@ namespace ccf::js::extensions is_member = false; auto cose = ctx.new_obj(); - cose.set( + JS_CHECK_OR_THROW(cose.set( "content", ctx.new_array_buffer_copy( - user_cose_ident->content.data(), user_cose_ident->content.size())); - caller.set("cose", std::move(cose)); + user_cose_ident->content.data(), user_cose_ident->content.size()))); + JS_CHECK_OR_THROW(caller.set("cose", std::move(cose))); } if (policy_name == nullptr) @@ -264,10 +267,10 @@ namespace ccf::js::extensions fmt::format("Failed to get certificate for caller {}", id)); } - caller.set("policy", ctx.new_string(policy_name)); - caller.set("id", ctx.new_string(id)); - caller.set("data", ctx.parse_json(data)); - caller.set("cert", ctx.new_string(cert.str())); + JS_CHECK_OR_THROW(caller.set("policy", ctx.new_string(policy_name))); + JS_CHECK_OR_THROW(caller.set("id", ctx.new_string(id))); + JS_CHECK_OR_THROW(caller.set("data", ctx.parse_json(data))); + JS_CHECK_OR_THROW(caller.set("cert", ctx.new_string(cert.str()))); return caller; } @@ -299,36 +302,36 @@ namespace ccf::js::extensions auto headers = ctx.new_obj(); for (const auto& [header_name, header_value] : r_headers) { - headers.set(header_name, ctx.new_string(header_value)); + JS_CHECK_OR_THROW(headers.set(header_name, ctx.new_string(header_value))); } - request.set("headers", std::move(headers)); + JS_CHECK_OR_THROW(request.set("headers", std::move(headers))); const auto& request_query = endpoint_ctx.rpc_ctx->get_request_query(); auto query_str = ctx.new_string(request_query); - request.set("query", std::move(query_str)); + JS_CHECK_OR_THROW(request.set("query", std::move(query_str))); const auto& request_path = endpoint_ctx.rpc_ctx->get_request_path(); auto path_str = ctx.new_string(request_path); - request.set("path", std::move(path_str)); + JS_CHECK_OR_THROW(request.set("path", std::move(path_str))); const auto& request_method = endpoint_ctx.rpc_ctx->get_request_verb(); auto method_str = ctx.new_string(request_method.c_str()); - request.set("method", std::move(method_str)); + JS_CHECK_OR_THROW(request.set("method", std::move(method_str))); const auto host_it = r_headers.find(http::headers::HOST); if (host_it != r_headers.end()) { const auto& request_hostname = host_it->second; auto hostname_str = ctx.new_string(request_hostname); - request.set("hostname", std::move(hostname_str)); + JS_CHECK_OR_THROW(request.set("hostname", std::move(hostname_str))); } else { - request.set_null("hostname"); + JS_CHECK_OR_THROW(request.set_null("hostname")); } auto route_str = ctx.new_string(full_request_path); - request.set("route", std::move(route_str)); + JS_CHECK_OR_THROW(request.set("route", std::move(route_str))); auto request_url = request_path; if (!request_query.empty()) @@ -336,25 +339,28 @@ namespace ccf::js::extensions request_url = fmt::format("{}?{}", request_url, request_query); } auto url_str = ctx.new_string(request_url); - request.set("url", std::move(url_str)); + JS_CHECK_OR_THROW(request.set("url", std::move(url_str))); auto params = ctx.new_obj(); for (const auto& [param_name, param_value] : endpoint_ctx.rpc_ctx->get_request_path_params()) { - params.set(param_name, ctx.new_string(param_value)); + JS_CHECK_OR_THROW(params.set(param_name, ctx.new_string(param_value))); } - request.set("params", std::move(params)); + JS_CHECK_OR_THROW(request.set("params", std::move(params))); auto body = ctx.new_obj(); - body.set("text", ctx.new_c_function(js_body_text, "text", 0)); - body.set("json", ctx.new_c_function(js_body_json, "json", 0)); - body.set( + JS_CHECK_OR_THROW( + body.set("text", ctx.new_c_function(js_body_text, "text", 0))); + JS_CHECK_OR_THROW( + body.set("json", ctx.new_c_function(js_body_json, "json", 0))); + JS_CHECK_OR_THROW(body.set( "arrayBuffer", - ctx.new_c_function(js_body_array_buffer, "arrayBuffer", 0)); - request.set("body", std::move(body)); + ctx.new_c_function(js_body_array_buffer, "arrayBuffer", 0))); + JS_CHECK_OR_THROW(request.set("body", std::move(body))); - request.set("caller", create_caller_obj(ctx, endpoint_ctx, registry)); + JS_CHECK_OR_THROW( + request.set("caller", create_caller_obj(ctx, endpoint_ctx, registry))); return request; } diff --git a/src/js/extensions/ccf/rpc.cpp b/src/js/extensions/ccf/rpc.cpp index 500c839997dc..e1bca281b9ee 100644 --- a/src/js/extensions/ccf/rpc.cpp +++ b/src/js/extensions/ccf/rpc.cpp @@ -5,6 +5,7 @@ #include "ccf/js/core/context.h" #include "ccf/rpc_context.h" +#include "js/checks.h" #include @@ -105,14 +106,14 @@ namespace ccf::js::extensions { auto rpc = ctx.new_obj(); - rpc.set( + JS_CHECK_OR_THROW(rpc.set( "setApplyWrites", - ctx.new_c_function(js_rpc_set_apply_writes, "setApplyWrites", 1)); - rpc.set( + ctx.new_c_function(js_rpc_set_apply_writes, "setApplyWrites", 1))); + JS_CHECK_OR_THROW(rpc.set( "setClaimsDigest", - ctx.new_c_function(js_rpc_set_claims_digest, "setClaimsDigest", 1)); + ctx.new_c_function(js_rpc_set_claims_digest, "setClaimsDigest", 1))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); - ccf.set("rpc", std::move(rpc)); + JS_CHECK_OR_THROW(ccf.set("rpc", std::move(rpc))); } } diff --git a/src/js/extensions/console.cpp b/src/js/extensions/console.cpp index 7108d35153c9..faf00ab08f92 100644 --- a/src/js/extensions/console.cpp +++ b/src/js/extensions/console.cpp @@ -6,6 +6,7 @@ #include "ccf/ds/logger.h" #include "ccf/js/core/context.h" #include "ds/internal_logger.h" +#include "js/checks.h" #include "node/rpc/gov_logging.h" #include @@ -136,10 +137,14 @@ namespace ccf::js::extensions { auto console = jsctx.new_obj(); - console.set("log", jsctx.new_c_function(js_info, "log", 1)); - console.set("info", jsctx.new_c_function(js_info, "info", 1)); - console.set("warn", jsctx.new_c_function(js_fail, "warn", 1)); - console.set("error", jsctx.new_c_function(js_fatal, "error", 1)); + JS_CHECK_OR_THROW( + console.set("log", jsctx.new_c_function(js_info, "log", 1))); + JS_CHECK_OR_THROW( + console.set("info", jsctx.new_c_function(js_info, "info", 1))); + JS_CHECK_OR_THROW( + console.set("warn", jsctx.new_c_function(js_fail, "warn", 1))); + JS_CHECK_OR_THROW( + console.set("error", jsctx.new_c_function(js_fatal, "error", 1))); return console; } @@ -148,7 +153,7 @@ namespace ccf::js::extensions void ConsoleExtension::install(js::core::Context& ctx) { auto global_obj = ctx.get_global_obj(); - global_obj.set("console", create_console_obj(ctx)); + JS_CHECK_OR_THROW(global_obj.set("console", create_console_obj(ctx))); } void ConsoleExtension::log_info_with_tag( diff --git a/src/js/extensions/math/random.cpp b/src/js/extensions/math/random.cpp index 8fc69ba31d84..919a29139f1f 100644 --- a/src/js/extensions/math/random.cpp +++ b/src/js/extensions/math/random.cpp @@ -5,6 +5,7 @@ #include "ccf/crypto/entropy.h" #include "ccf/js/core/context.h" +#include "js/checks.h" #include @@ -54,6 +55,7 @@ namespace ccf::js::extensions { // Overriding built-in Math.random auto math_val = ctx.get_global_property("Math"); - math_val.set("random", ctx.new_c_function(js_random_impl, "random", 0)); + JS_CHECK_OR_THROW( + math_val.set("random", ctx.new_c_function(js_random_impl, "random", 0))); } } diff --git a/src/js/extensions/snp_attestation.cpp b/src/js/extensions/snp_attestation.cpp index f376b28fd2a5..d96cc20c4bd3 100644 --- a/src/js/extensions/snp_attestation.cpp +++ b/src/js/extensions/snp_attestation.cpp @@ -339,11 +339,13 @@ namespace ccf::js::extensions { auto snp_attestation = ctx.new_obj(); - snp_attestation.set( + JS_CHECK_OR_THROW(snp_attestation.set( "verifySnpAttestation", - ctx.new_c_function(js_verify_snp_attestation, "verifySnpAttestation", 4)); + ctx.new_c_function( + js_verify_snp_attestation, "verifySnpAttestation", 4))); auto global_obj = ctx.get_global_obj(); - global_obj.set("snp_attestation", std::move(snp_attestation)); + JS_CHECK_OR_THROW( + global_obj.set("snp_attestation", std::move(snp_attestation))); } } \ No newline at end of file diff --git a/src/js/registry.cpp b/src/js/registry.cpp index 576306eb3dd2..059ee920e41f 100644 --- a/src/js/registry.cpp +++ b/src/js/registry.cpp @@ -13,6 +13,7 @@ #include "ccf/service/tables/modules.h" #include "ccf/version.h" #include "ds/internal_logger.h" +#include "js/checks.h" #include #define FMT_HEADER_ONLY @@ -186,8 +187,6 @@ namespace ccf::js ctx.remove_extension(extension); } - const auto& rt = ctx.runtime(); - if (val.is_exception()) { bool time_out = ctx.interrupt_data.request_timed_out; @@ -455,7 +454,7 @@ namespace ccf::js if (extension != nullptr) { auto val = extension->create_historical_state_object(ctx, state); - ccf.set("historicalState", std::move(val)); + JS_CHECK_OR_THROW(ccf.set("historicalState", std::move(val))); } else { @@ -916,7 +915,7 @@ namespace ccf::js auto* endpoints = tx.template ro(metadata_map); - endpoints->foreach_key([this, &verbs, &method](const auto& key) { + endpoints->foreach_key([&verbs, &method](const auto& key) { const auto opt_spec = ccf::endpoints::PathTemplateSpec::parse(key.uri_path); if (opt_spec.has_value()) diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index 702388861b82..4e8cb0fb4793 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -29,7 +29,8 @@ namespace ccf::kv void set_size(uint64_t size_) { - static constexpr size_t max_entry_size = 1UL << BITS_FOR_SIZE; + [[maybe_unused]] static constexpr size_t max_entry_size = 1UL + << BITS_FOR_SIZE; CCF_ASSERT_FMT( size_ < max_entry_size, "Cannot serialise entry of size {} (max allowed size is {})", diff --git a/src/kv/store.h b/src/kv/store.h index 1b7bc020fe81..6197142cc7e4 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -112,9 +112,6 @@ namespace ccf::kv // Ledger entry header flags uint8_t flags = 0; - size_t size_since_chunk = 0; - size_t chunk_threshold = 0; - bool commit_deserialised( OrderedChanges& changes, Version v, diff --git a/src/node/gov/handlers/proposals.h b/src/node/gov/handlers/proposals.h index bf96846ebee9..ab0e2f922afb 100644 --- a/src/node/gov/handlers/proposals.h +++ b/src/node/gov/handlers/proposals.h @@ -5,6 +5,7 @@ #include "ccf/base_endpoint_registry.h" #include "ccf/js/common_context.h" #include "ccf/js/extensions/ccf/gov_effects.h" +#include "js/checks.h" #include "js/extensions/ccf/network.h" #include "js/extensions/ccf/node.h" #include "node/gov/api_version.h" @@ -212,16 +213,12 @@ namespace ccf::gov::endpoints { auto v = js_context.new_obj(); - v.set( + JS_CHECK_OR_THROW(v.set( "member_id", - js_context.new_string_len(member_id.data(), member_id.size())); - if (v.set_bool("vote", vote_result) != 1) - { - throw std::runtime_error( - "Failed to set vote property on vote result object"); - } + js_context.new_string_len(member_id.data(), member_id.size()))); + JS_CHECK_OR_THROW(v.set_bool("vote", vote_result)); - vs.set_at_index(index++, std::move(v)); + JS_CHECK_OR_THROW(vs.set_at_index(index++, std::move(v))); } argv.push_back(vs); diff --git a/src/node/node_state.h b/src/node/node_state.h index f4b62c3d8f9a..c118dc335b3d 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1189,8 +1189,6 @@ namespace ccf h->set_node_id(self); } - auto service_config = tx.ro(network.config)->get(); - setup_consensus( ServiceStatus::OPENING, ccf::ReconfigurationType::ONE_TRANSACTION, diff --git a/src/node/quote.cpp b/src/node/quote.cpp index 7336a449da24..4f709dbc356e 100644 --- a/src/node/quote.cpp +++ b/src/node/quote.cpp @@ -190,7 +190,6 @@ namespace ccf case QuoteFormat::amd_sev_snp_v1: { - HostData digest{}; HostData::Representation rep{}; pal::PlatformAttestationMeasurement d = {}; pal::PlatformAttestationReportData r = {}; diff --git a/src/node/rpc/jwt_management.h b/src/node/rpc/jwt_management.h index 143487407a7d..b8ec7d01354b 100644 --- a/src/node/rpc/jwt_management.h +++ b/src/node/rpc/jwt_management.h @@ -244,8 +244,7 @@ namespace ccf } std::set existing_kids; - keys->foreach([&existing_kids, &issuer_constraints, &issuer]( - const auto& k, const auto& v) { + keys->foreach([&existing_kids, &issuer](const auto& k, const auto& v) { if (find_if(v.begin(), v.end(), [&](const auto& metadata) { return metadata.issuer == issuer; }) != v.end()) diff --git a/src/node/rpc/node_frontend.h b/src/node/rpc/node_frontend.h index 98df7fbec983..38eb7029db8f 100644 --- a/src/node/rpc/node_frontend.h +++ b/src/node/rpc/node_frontend.h @@ -537,9 +537,6 @@ namespace ccf "No service is available to accept new node."); } - auto config = args.tx.ro(network.config); - auto service_config = config->get(); - if ( active_service->status == ServiceStatus::OPENING || active_service->status == ServiceStatus::RECOVERING) @@ -698,7 +695,7 @@ namespace ccf auto set_retired_committed = [this](auto& ctx, nlohmann::json&&) { auto nodes = ctx.tx.rw(network.nodes); - nodes->foreach([this, &nodes](const auto& node_id, auto node_info) { + nodes->foreach([&nodes](const auto& node_id, auto node_info) { auto gc_node = nodes->get_globally_committed(node_id); if ( gc_node.has_value() && @@ -880,7 +877,7 @@ namespace ccf .install(); auto get_attestations = - [this, get_quotes](auto& args, nlohmann::json&& params) { + [get_quotes](auto& args, nlohmann::json&& params) { const auto res = get_quotes(args, std::move(params)); const auto body = std::get_if(&res); if (body != nullptr) @@ -937,7 +934,7 @@ namespace ccf .set_auto_schema() .install(); - auto service_previous_identity = [this](auto& args, nlohmann::json&&) { + auto service_previous_identity = [](auto& args, nlohmann::json&&) { auto psi_handle = args.tx.template ro( ccf::Tables::PREVIOUS_SERVICE_IDENTITY); const auto psi = psi_handle->get(); @@ -1063,7 +1060,7 @@ namespace ccf auto nodes = args.tx.ro(this->network.nodes); nodes->foreach( - [this, &out, nodes](const NodeId& node_id, const NodeInfo& ni) { + [&out, nodes](const NodeId& node_id, const NodeInfo& ni) { // Only nodes whose retire_committed status is committed can be // safely removed, because any primary elected from here on would // consider them retired, and would consequently not need their @@ -1542,7 +1539,7 @@ namespace ccf .set_auto_schema() .install(); - auto version = [this](auto&, nlohmann::json&&) { + auto version = [](auto&, nlohmann::json&&) { GetVersion::Out result; result.ccf_version = ccf::ccf_version; result.quickjs_version = ccf::quickjs_version; diff --git a/src/quic/quic_session.h b/src/quic/quic_session.h index 9db6fbd6068b..d49c7c65f25a 100644 --- a/src/quic/quic_session.h +++ b/src/quic/quic_session.h @@ -227,7 +227,6 @@ namespace quic if (status != ready) return; - int written = 0; for (auto& write : pending_writes) { LOG_TRACE_FMT("QUIC write_some {} bytes", write.len); @@ -240,7 +239,6 @@ namespace quic stop(error); return; } - written += rc; // Mark for deletion (avoiding invalidating iterator) write.clear = true; diff --git a/src/tasks/ordered_tasks.cpp b/src/tasks/ordered_tasks.cpp index 0d4eb67ab2f7..fd404b10995b 100644 --- a/src/tasks/ordered_tasks.cpp +++ b/src/tasks/ordered_tasks.cpp @@ -48,7 +48,7 @@ namespace ccf::tasks void OrderedTasks::do_task_implementation() { if (pimpl->actions.pop_and_visit( - [this](TaskAction&& action) { action->do_action(); })) + [](TaskAction&& action) { action->do_action(); })) { enqueue_on_board(); } From ace4790c8854214e7d2f4d9640d37fe19092ebf0 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Nov 2025 16:51:14 +0000 Subject: [PATCH 10/12] Consolidate governance tests --- CMakeLists.txt | 4 ---- tests/governance.py | 20 +++++++++++++++++++- tests/infra/runner.py | 2 +- tests/membership.py | 25 ------------------------- 4 files changed, 20 insertions(+), 31 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 206f6286a1a8..279aa64cca1f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1035,10 +1035,6 @@ if(BUILD_TESTS) --http2 ) - add_e2e_test( - NAME membership PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/membership.py - ) - add_e2e_test( NAME partitions PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/partitions_test.py diff --git a/tests/governance.py b/tests/governance.py index 9c72aad9b709..dcbe4ae036e7 100644 --- a/tests/governance.py +++ b/tests/governance.py @@ -25,6 +25,9 @@ from hashlib import md5 import random +import memberclient +import membership + from loguru import logger as LOG @@ -808,4 +811,19 @@ def add(parser): nodes=infra.e2e_args.max_nodes(cr.args, f=0), ) - cr.run(2) + cr.add( + "membership", + membership.run, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.max_nodes(cr.args, f=0), + initial_user_count=0, + ) + + cr.add( + "member_client", + memberclient.run, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.max_nodes(cr.args, f=1), + ) + + cr.run() diff --git a/tests/infra/runner.py b/tests/infra/runner.py index 9ae0759008e0..f3d17adaa221 100644 --- a/tests/infra/runner.py +++ b/tests/infra/runner.py @@ -238,7 +238,7 @@ def run(self, max_concurrent=None): config = { "handlers": [ { - "sink": sys.stderr, + "sink": sys.stdout, "format": "{time:YYYY-MM-DD HH:mm:ss.SSS} | {level: <8} | {{{thread.name}}} {name}:{function}:{line} - {message}", } ] diff --git a/tests/membership.py b/tests/membership.py index 25e85bdc7782..2949b74b5dce 100644 --- a/tests/membership.py +++ b/tests/membership.py @@ -1,11 +1,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the Apache 2.0 License. -import infra.e2e_args import infra.network -import infra.consortium import random -from infra.runner import ConcurrentRunner -import memberclient import infra.proposal import infra.member @@ -637,24 +633,3 @@ def run(args): service_startups(args) recovery_shares_scenario(args) recovery_shares_with_owners_scenario(args) - - -if __name__ == "__main__": - cr = ConcurrentRunner() - - cr.add( - "membership", - run, - package="samples/apps/logging/logging", - nodes=infra.e2e_args.max_nodes(cr.args, f=0), - initial_user_count=0, - ) - - cr.add( - "member_client", - memberclient.run, - package="samples/apps/logging/logging", - nodes=infra.e2e_args.max_nodes(cr.args, f=1), - ) - - cr.run() From ef228a52a64de1558a89de64ae73939725885f13 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Nov 2025 18:11:00 +0000 Subject: [PATCH 11/12] election+rotation+committable->nodes --- CMakeLists.txt | 25 +++++------------- tests/committable.py | 6 ----- tests/{election.py => nodes.py} | 45 +++++++++++++++++++++++++++++++-- tests/rotation.py | 39 ---------------------------- tests/suite/test_suite.py | 12 ++++----- 5 files changed, 56 insertions(+), 71 deletions(-) rename tests/{election.py => nodes.py} (89%) delete mode 100644 tests/rotation.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 279aa64cca1f..a927585f22fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -900,12 +900,6 @@ if(BUILD_TESTS) ${CMAKE_SOURCE_DIR}/samples/templates ) - add_e2e_test( - NAME committable_suffix_test - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/committable.py - ADDITIONAL_ARGS --sig-ms-interval 100 - ) - add_e2e_test( NAME commit_latency PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/commit_latency.py @@ -1127,17 +1121,6 @@ if(BUILD_TESTS) ) endif() - if(LONG_TESTS) - set(ROTATION_TEST_ARGS --rotation-retirements 10) - endif() - - add_e2e_test( - NAME rotation_test - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/rotation.py - LABEL rotation - ADDITIONAL_ARGS ${ROTATION_TEST_ARGS} - ) - set(RECONFIG_TEST_ARGS --ccf-version ${CCF_VERSION}) add_e2e_test( NAME reconfiguration_test @@ -1146,8 +1129,14 @@ if(BUILD_TESTS) ) set_property(TEST reconfiguration_test PROPERTY LABELS reconfiguration) + if(LONG_TESTS) + set(ROTATION_TEST_ARGS --rotation-retirements 10) + endif() + add_e2e_test( - NAME election_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/election.py + NAME nodes_test + PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/nodes.py + ADDITIONAL_ARGS ${ROTATION_TEST_ARGS} ) add_piccolo_test( diff --git a/tests/committable.py b/tests/committable.py index b93bd3c1e32c..8d6415490706 100644 --- a/tests/committable.py +++ b/tests/committable.py @@ -106,9 +106,3 @@ def run(args): # Resume original primary, check that they rejoin correctly, including new transactions primary.resume() network.wait_for_node_commit_sync(timeout=16) - - -if __name__ == "__main__": - args = infra.e2e_args.cli_args() - args.package = "samples/apps/logging/logging" - run(args) diff --git a/tests/election.py b/tests/nodes.py similarity index 89% rename from tests/election.py rename to tests/nodes.py index 19760b47ec33..4eb8bd568d33 100644 --- a/tests/election.py +++ b/tests/nodes.py @@ -16,6 +16,24 @@ from infra.runner import ConcurrentRunner from loguru import logger as LOG +import reconfiguration +import committable + + +def run_rotations(args): + with infra.network.network( + args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb + ) as network: + network.start_and_open(args) + + # Replace primary repeatedly and check the network still operates + LOG.info(f"Retiring primary {args.rotation_retirements} times") + for i in range(args.rotation_retirements): + LOG.warning(f"Retirement {i}") + reconfiguration.test_add_node(network, args) + reconfiguration.test_retire_primary(network, args) + + # This test starts from a given number of nodes (hosts), commits # a transaction, stops the current primary, waits for an election and repeats # this process until no progress can be made (i.e. no primary can be elected @@ -242,10 +260,26 @@ def run(args): if __name__ == "__main__": - cr = ConcurrentRunner() + def add(parser): + parser.add_argument( + "--rotation-retirements", + help="Number of times to retire the primary", + type=int, + default=2, + ) + + cr = ConcurrentRunner(add) args = copy.deepcopy(cr.args) + cr.add( + "rotation", + run_rotations, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.min_nodes(args, f=1), + initial_member_count=1, + ) + cr.add( "cft", run, @@ -254,4 +288,11 @@ def run(args): election_timeout_ms=1000, ) - cr.run(1) + cr.add( + "committable", + committable.run, + package="samples/apps/logging/logging", + sig_ms_interval=100, + ) + + cr.run() diff --git a/tests/rotation.py b/tests/rotation.py deleted file mode 100644 index dda2e72e4f0c..000000000000 --- a/tests/rotation.py +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the Apache 2.0 License. -import infra.e2e_args -import infra.network -import infra.proc -import reconfiguration - -from loguru import logger as LOG - - -def run(args): - with infra.network.network( - args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb - ) as network: - network.start_and_open(args) - - # Replace primary repeatedly and check the network still operates - LOG.info(f"Retiring primary {args.rotation_retirements} times") - for i in range(args.rotation_retirements): - LOG.warning(f"Retirement {i}") - reconfiguration.test_add_node(network, args) - reconfiguration.test_retire_primary(network, args) - - -if __name__ == "__main__": - - def add(parser): - parser.add_argument( - "--rotation-retirements", - help="Number of times to retire the primary", - type=int, - default=2, - ) - - args = infra.e2e_args.cli_args(add=add) - args.package = "samples/apps/logging/logging" - args.nodes = infra.e2e_args.max_nodes(args, f=0) - args.initial_member_count = 1 - run(args) diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index afa154ecd3ba..a2fddd4839eb 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -5,7 +5,7 @@ import memberclient import reconfiguration import recovery -import election +import nodes import code_update import membership import governance_history @@ -61,14 +61,14 @@ reconfiguration.test_retire_primary, e2e_logging.test_rekey, reconfiguration.test_add_node, - election.test_kill_primary, - election.test_commit_view_history, + nodes.test_kill_primary, + nodes.test_commit_view_history, reconfiguration.test_add_node, reconfiguration.test_add_node_from_snapshot, reconfiguration.test_retire_backup, reconfiguration.test_add_node, - election.test_kill_primary, - election.test_commit_view_history, + nodes.test_kill_primary, + nodes.test_commit_view_history, e2e_logging.test_view_history, reconfiguration.test_ledger_invariants, ] @@ -114,7 +114,7 @@ e2e_logging.test_rekey, # election: reconfiguration.test_add_node, - election.test_kill_primary, + nodes.test_kill_primary, # code update: code_update.test_verify_quotes, code_update.test_add_node_with_different_package, From 4ba508dcd87a8b5ec63443f0276d60f953a35b30 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Nov 2025 18:24:27 +0000 Subject: [PATCH 12/12] nodes+reconfiguration->nodes --- CMakeLists.txt | 12 ++---------- tests/nodes.py | 7 +++++++ tests/reconfiguration.py | 13 ------------- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a927585f22fe..34402b2d975d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -874,7 +874,6 @@ if(BUILD_TESTS) --test-duration 300 --test-suite reconfiguration --jinja-templates-path ${CMAKE_SOURCE_DIR}/samples/templates ) - set_property(TEST reconfiguration_test_suite PROPERTY LABELS reconfiguration) if(LONG_TESTS) add_e2e_test( @@ -1121,22 +1120,15 @@ if(BUILD_TESTS) ) endif() - set(RECONFIG_TEST_ARGS --ccf-version ${CCF_VERSION}) - add_e2e_test( - NAME reconfiguration_test - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/reconfiguration.py - ADDITIONAL_ARGS ${RECONFIG_TEST_ARGS} - ) - set_property(TEST reconfiguration_test PROPERTY LABELS reconfiguration) - if(LONG_TESTS) set(ROTATION_TEST_ARGS --rotation-retirements 10) endif() + set(RECONFIG_TEST_ARGS --ccf-version ${CCF_VERSION}) add_e2e_test( NAME nodes_test PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/nodes.py - ADDITIONAL_ARGS ${ROTATION_TEST_ARGS} + ADDITIONAL_ARGS ${RECONFIG_TEST_ARGS} ${ROTATION_TEST_ARGS} ) add_piccolo_test( diff --git a/tests/nodes.py b/tests/nodes.py index 4eb8bd568d33..01544e43ee1d 100644 --- a/tests/nodes.py +++ b/tests/nodes.py @@ -295,4 +295,11 @@ def add(parser): sig_ms_interval=100, ) + cr.add( + "reconfiguration", + reconfiguration.run_all, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.min_nodes(cr.args, f=1), + ) + cr.run() diff --git a/tests/reconfiguration.py b/tests/reconfiguration.py index 58ef3055ac68..9c1cb10ca764 100644 --- a/tests/reconfiguration.py +++ b/tests/reconfiguration.py @@ -20,7 +20,6 @@ from infra.checker import check_can_progress from governance_history import check_signatures from infra.snp import SNP_SUPPORT -from infra.runner import ConcurrentRunner import http import random @@ -916,15 +915,3 @@ def run_join_old_snapshot(const_args): fetch_recent_snapshot=True, timeout=3, ) - - -if __name__ == "__main__": - cr = ConcurrentRunner() - cr.add( - "reconfiguration", - run_all, - package="samples/apps/logging/logging", - nodes=infra.e2e_args.min_nodes(cr.args, f=1), - ) - - cr.run()