From b99d99f875aa8d91a2aba4fa333ffb9342488c83 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 06:59:06 +0000 Subject: [PATCH 01/15] Initial plan From f34ee1f367f49842d2c1e15875e5d80a31e500a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 07:30:17 +0000 Subject: [PATCH 02/15] Apply remaining changes Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- CHANGELOG.md | 1 + doc/host_config_schema/host_config.json | 5 ++ include/ccf/node/startup_config.h | 1 + src/common/configuration.h | 6 ++- src/consensus/ledger_enclave.h | 26 +++++++++- src/enclave/enclave.h | 2 + src/enclave/main.cpp | 1 + src/kv/committable_tx.h | 15 +++++- src/kv/generic_serialise_wrapper.h | 22 +++++++-- src/kv/kv_types.h | 12 ++++- src/kv/serialised_entry_format.h | 27 +++++++++-- src/kv/snapshot.h | 6 ++- src/kv/store.h | 41 ++++++++++++++-- src/kv/test/kv_serialisation.cpp | 64 +++++++++++++++++++++++++ src/node/node_state.h | 8 +++- src/node/rpc/frontend.h | 10 ++++ tests/config.jinja | 3 +- tests/infra/e2e_args.py | 6 +++ tests/infra/network.py | 1 + tests/limits.py | 36 ++++++++++++++ 20 files changed, 272 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e2ef85f32c8..5821c85c808d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1278,6 +1278,7 @@ For more information, see [our documentation](https://microsoft.github.io/CCF/ma #### Configuration - The `cchost` configuration file now includes an `idle_connection_timeout` option. This controls how long the node will keep idle connections (for user TLS sessions) before automatically closing them. This may be set to `null` to restore the previous behaviour, where idle connections are never closed. By default connections will be closed after 60s of idle time. +- The `ledger.max_transaction_size` configuration option now limits the serialized transaction body size stored in each ledger entry. The limit excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain. The default value is `100MB`. (#7488) - A soft size limit can now be set for the historical store cache in the node configuration: [`historical_cache_soft_limit`](https://microsoft.github.io/CCF/main/operations/generated_config.html#historical-cache-soft-limit). The default value is `512Mb`. - Path to the enclave file should now be passed as `--enclave-file` CLI argument to `cchost`, rather than `enclave.file` entry within configuration file. - SNP collateral must now be provided through the `snp_security_policy_file`, `snp_uvm_endorsements_file` and `snp_endorsements_servers` configuration values. See [documentation](https://microsoft.github.io/CCF/main/operations/platforms/snp.html) for details and platform-specific configuration samples. diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 30a48cdeaf69..a70247a41e57 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -476,6 +476,11 @@ "type": "string", "default": "5MB", "description": "Minimum size (size string) of the current ledger file after which a new ledger file (chunk) is created" + }, + "max_transaction_size": { + "type": "string", + "default": "100MB", + "description": "Maximum serialised transaction body size (size string). This is compared with the size stored in the ledger entry header, so it excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain." } }, "description": "This section includes configuration for the ledger directories and files", diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index 457b418947eb..84c4382dc331 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -48,6 +48,7 @@ namespace ccf std::string directory = "ledger"; std::vector read_only_directories; ccf::ds::SizeString chunk_size = {"5MB"}; + ccf::ds::SizeString max_transaction_size = {"100MB"}; bool operator==(const Ledger&) const = default; }; diff --git a/src/common/configuration.h b/src/common/configuration.h index 29dcc2e08aca..f09e5eaf079d 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -65,7 +65,11 @@ namespace ccf DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::Ledger); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::Ledger); DECLARE_JSON_OPTIONAL_FIELDS( - CCFConfig::Ledger, directory, read_only_directories, chunk_size); + CCFConfig::Ledger, + directory, + read_only_directories, + chunk_size, + max_transaction_size); DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::LedgerSignatures); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::LedgerSignatures); diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 15a1a6df6e0c..b7119739d273 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -8,6 +8,8 @@ #include "kv/kv_types.h" #include "kv/serialised_entry_format.h" +#include + namespace consensus { class LedgerEnclave @@ -21,11 +23,31 @@ namespace consensus * * @return Raw entry as a vector */ - static std::vector get_entry(const uint8_t*& data, size_t& size) + static std::vector get_entry( + const uint8_t*& data, + size_t& size, + size_t max_transaction_size = + ccf::kv::SerialisedEntryHeader::max_serialised_entry_body_size) { auto header = serialized::peek(data, size); - size_t entry_size = ccf::kv::serialised_entry_header_size + header.size; + const size_t body_size = header.size; + if (body_size > max_transaction_size) + { + throw std::logic_error(ccf::kv::describe_serialized_entry_size_error( + body_size, max_transaction_size, "extract from ledger")); + } + if (body_size + ccf::kv::serialised_entry_header_size > size) + { + throw std::logic_error(fmt::format( + "Cannot read transaction with serialised body size {} bytes from " + "buffer containing {} bytes after the fixed {}-byte ledger entry " + "header", + body_size, + size - ccf::kv::serialised_entry_header_size, + ccf::kv::serialised_entry_header_size)); + } + size_t entry_size = ccf::kv::serialised_entry_header_size + body_size; std::vector entry(data, data + entry_size); serialized::skip(data, size, entry_size); return entry; diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 074aa1eab0a3..c83a94df0d73 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -85,6 +85,7 @@ namespace ccf size_t sig_tx_interval, size_t sig_ms_interval, size_t chunk_threshold, + size_t max_transaction_size, const ccf::consensus::Configuration& consensus_config, const ccf::crypto::CurveID& curve_id, ccf::ds::WorkBeaconPtr work_beacon_, @@ -103,6 +104,7 @@ namespace ccf network.tables->set_chunker( std::make_shared(chunk_threshold)); + network.tables->set_max_transaction_size(max_transaction_size); LOG_TRACE_FMT("Creating node"); node = std::make_unique( diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp index fe1b11530989..b829d915e005 100644 --- a/src/enclave/main.cpp +++ b/src/enclave/main.cpp @@ -106,6 +106,7 @@ namespace ccf ccf_config.ledger_signatures.tx_count, ccf_config.ledger_signatures.delay.count_ms(), ccf_config.ledger.chunk_size, + ccf_config.ledger.max_transaction_size, ccf_config.consensus, ccf_config.node_certificate.curve_id, work_beacon, diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index 32ecb0d7893e..760383c98366 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -91,7 +91,9 @@ namespace ccf::kv entry_type, entry_flags, tx_commit_evidence_digest, - claims_digest_); + claims_digest_, + false /* historical_hint */, + pimpl->store->get_max_transaction_size()); // Process in security domain order for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) @@ -161,6 +163,11 @@ namespace ccf::kv ccf::kv::ConsensusHookPtrs hooks; std::optional new_maps_conflict_version = std::nullopt; + // If serialisation later rejects this transaction because it exceeds the + // configured size limit, roll the store back to the state from before + // apply_changes mutates maps and advances the version. + const auto [rollback_txid, rollback_term] = + pimpl->store->current_txid_and_commit_term(); bool track_deletes_on_missing_keys = false; auto c = apply_changes( @@ -246,6 +253,12 @@ namespace ccf::kv std::move(hooks)), false); } + catch (const MaxTransactionSizeExceeded&) + { + pimpl->store->rollback(rollback_txid, rollback_term); + committed = false; + throw; + } catch (const std::exception& e) { committed = false; diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 160daa1c8dd8..9d028818fcbf 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -26,6 +26,7 @@ namespace ccf::kv TxID tx_id; EntryType entry_type; SerialisedEntryFlags header_flags; + size_t max_transaction_size; std::shared_ptr crypto_util; @@ -70,10 +71,13 @@ namespace ccf::kv // in regular transactions, but absent in snapshots. const ccf::crypto::Sha256Hash& commit_evidence_digest_ = {}, const ccf::ClaimsDigest& claims_digest_ = ccf::no_claims(), - bool historical_hint_ = false) : + bool historical_hint_ = false, + size_t max_transaction_size_ = + SerialisedEntryHeader::max_serialised_entry_body_size) : tx_id(tx_id_), entry_type(entry_type_), header_flags(header_flags_), + max_transaction_size(max_transaction_size_), crypto_util(std::move(e)), historical_hint(historical_hint_) { @@ -175,6 +179,11 @@ namespace ccf::kv size_ += crypto_util->get_header_length() + sizeof(size_t) + serialised_private_domain.size(); } + if (size_ > max_transaction_size) + { + throw MaxTransactionSizeExceeded(describe_serialized_entry_size_error( + size_, max_transaction_size, "serialise")); + } entry_header.set_size(size_); size_ += sizeof(SerialisedEntryHeader); @@ -304,7 +313,9 @@ namespace ccf::kv size_t size, ccf::kv::Term& term, EntryFlags& flags, - bool historical_hint = false) + bool historical_hint = false, + size_t max_transaction_size = + SerialisedEntryHeader::max_serialised_entry_body_size) { current_reader = &public_reader; const auto* data_ = data; @@ -313,6 +324,12 @@ namespace ccf::kv const auto tx_header = serialized::read(data_, size_); + if (tx_header.size > max_transaction_size) + { + throw MaxTransactionSizeExceeded(describe_serialized_entry_size_error( + tx_header.size, max_transaction_size, "deserialise")); + } + flags = static_cast(tx_header.flags); if (tx_header.size != size_) @@ -322,7 +339,6 @@ namespace ccf::kv tx_header.size, size_)); } - const auto* gcm_hdr_data = data_; switch (tx_header.version) diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 18ed95c29afe..c0c5d3d1bcb3 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -328,6 +329,13 @@ namespace ccf::kv } }; + class MaxTransactionSizeExceeded : public std::logic_error + { + public: + MaxTransactionSizeExceeded(const std::string& msg) : std::logic_error(msg) + {} + }; + class TxHistory { public: @@ -637,7 +645,8 @@ namespace ccf::kv virtual ~AbstractSnapshot() = default; [[nodiscard]] virtual Version get_version() const = 0; virtual std::vector serialise( - const std::shared_ptr& encryptor) = 0; + const std::shared_ptr& encryptor, + size_t max_transaction_size) = 0; }; virtual ~AbstractStore() = default; @@ -667,6 +676,7 @@ namespace ccf::kv virtual std::shared_ptr get_history() = 0; virtual std::shared_ptr get_chunker() = 0; virtual EncryptorPtr get_encryptor() = 0; + virtual size_t get_max_transaction_size() const = 0; virtual std::unique_ptr deserialize( const std::vector& data, bool public_only = false, diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index 46e0650fd9cf..a31b7a23bbd2 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -4,7 +4,10 @@ #include "ds/ccf_assert.h" +#include +#include #include +#include namespace ccf::kv { @@ -25,17 +28,17 @@ namespace ccf::kv static constexpr auto BITS_FOR_SIZE = (sizeof(uint64_t) - sizeof(uint8_t) - sizeof(SerialisedEntryFlags)) * CHAR_BIT; + static constexpr uint64_t max_serialised_entry_body_size = + (uint64_t{1} << BITS_FOR_SIZE) - 1; uint64_t size : BITS_FOR_SIZE = 0; void set_size(uint64_t size_) { - [[maybe_unused]] static constexpr size_t max_entry_size = 1UL - << BITS_FOR_SIZE; CCF_ASSERT_FMT( - size_ < max_entry_size, + size_ <= max_serialised_entry_body_size, "Cannot serialise entry of size {} (max allowed size is {})", size_, - max_entry_size); + max_serialised_entry_body_size); size = size_; } }; @@ -43,4 +46,20 @@ namespace ccf::kv static constexpr size_t serialised_entry_header_size = sizeof(SerialisedEntryHeader); + + static inline std::string describe_serialized_entry_size_error( + size_t body_size, size_t max_body_size, const char* operation) + { + return fmt::format( + "Cannot {} transaction with serialised body size {} bytes. The " + "configured maximum is {} bytes. The transaction size compared to this " + "limit is the size stored in the ledger entry header: the serialised " + "transaction body after the fixed {}-byte ledger entry header, " + "including any ledger encryption header, public domain size field, " + "public domain and encrypted private domain.", + operation, + body_size, + max_body_size, + serialised_entry_header_size); + } } \ No newline at end of file diff --git a/src/kv/snapshot.h b/src/kv/snapshot.h index ebe7f759de26..eb228983e5ba 100644 --- a/src/kv/snapshot.h +++ b/src/kv/snapshot.h @@ -40,7 +40,8 @@ namespace ccf::kv } std::vector serialise( - const std::shared_ptr& encryptor) override + const std::shared_ptr& encryptor, + size_t max_transaction_size) override { // Set the execution dependency for the snapshot to be the version // previous to said snapshot to ensure that the correct snapshot is @@ -58,7 +59,8 @@ namespace ccf::kv 0, {}, ccf::no_claims(), - true /* historical_hint */); + true /* historical_hint */, + max_transaction_size); if (hash_at_snapshot.has_value()) { diff --git a/src/kv/store.h b/src/kv/store.h index 3bfd840829de..eeeb2ba4f3a2 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -15,6 +15,7 @@ #include "kv_types.h" #define FMT_HEADER_ONLY +#include #include #include #include @@ -97,6 +98,8 @@ namespace ccf::kv std::shared_ptr chunker = nullptr; EncryptorPtr encryptor = nullptr; SnapshotterPtr snapshotter = nullptr; + size_t max_transaction_size = + SerialisedEntryHeader::max_serialised_entry_body_size; // Generally we will only accept deserialised views if they are contiguous - // at Version N we reject everything but N+1. The exception is when a Store @@ -223,6 +226,30 @@ namespace ccf::kv return encryptor; } + void set_max_transaction_size(size_t max_transaction_size_) + { + static const auto max_allocatable_body = + std::vector().max_size() - serialised_entry_header_size; + const auto effective_max = std::min( + static_cast( + SerialisedEntryHeader::max_serialised_entry_body_size), + max_allocatable_body); + if (max_transaction_size_ > effective_max) + { + throw std::logic_error(fmt::format( + "Configured maximum transaction size {} exceeds the largest " + "supported serialised transaction body size {}", + max_transaction_size_, + effective_max)); + } + max_transaction_size = max_transaction_size_; + } + + size_t get_max_transaction_size() const override + { + return max_transaction_size; + } + void set_snapshotter(const SnapshotterPtr& snapshotter_) { snapshotter = snapshotter_; @@ -387,7 +414,7 @@ namespace ccf::kv std::unique_ptr snapshot) override { auto e = get_encryptor(); - return snapshot->serialise(e); + return snapshot->serialise(e, max_transaction_size); } ApplyResult deserialise_snapshot( @@ -405,7 +432,8 @@ namespace ccf::kv ccf::kv::Term term = 0; ccf::kv::EntryFlags entry_flags = {}; - auto v_ = d.init(data, size, term, entry_flags, is_historical); + auto v_ = d.init( + data, size, term, entry_flags, is_historical, max_transaction_size); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); @@ -732,8 +760,13 @@ namespace ccf::kv public_only ? ccf::kv::SecurityDomain::PUBLIC : std::optional()); - auto v_ = - d.init(data.data(), data.size(), view, entry_flags, is_historical); + auto v_ = d.init( + data.data(), + data.size(), + view, + entry_flags, + is_historical, + max_transaction_size); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 92078bc24784..e5358e59ede9 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -174,6 +174,70 @@ TEST_CASE( } } +TEST_CASE( + "Reject transactions exceeding configured serialised size" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(1024); + + MapTypes::StringString map("public:pub_map"); + + { + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("oversized", std::string(2048, 'A')); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + REQUIRE(!consensus->get_latest_data().has_value()); + } + + { + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("small", "ok"); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + REQUIRE(kv_store.current_version() == 1); + } +} + +TEST_CASE( + "Reject deserialised transactions exceeding configured serialised size" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + + MapTypes::StringString map("public:pub_map"); + + { + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("small", "ok"); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + + ccf::kv::Store kv_store_target; + kv_store_target.set_encryptor(encryptor); + kv_store_target.set_max_transaction_size(1); + + REQUIRE_THROWS_AS( + kv_store_target.deserialize(latest_data.value())->apply(), + ccf::kv::MaxTransactionSizeExceeded); +} + TEST_CASE( "Serialise/deserialise private map and public maps" * doctest::test_suite("serialisation")) diff --git a/src/node/node_state.h b/src/node/node_state.h index 8e66c00b159b..aae66f6c623f 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1474,7 +1474,8 @@ namespace ccf while (size > 0) { - auto entry = ::consensus::LedgerEnclave::get_entry(data, size); + auto entry = ::consensus::LedgerEnclave::get_entry( + data, size, network.tables->get_max_transaction_size()); LOG_INFO_FMT( "Deserialising public ledger entry #{} [{} bytes]", @@ -1763,7 +1764,8 @@ namespace ccf while (size > 0) { - auto entry = ::consensus::LedgerEnclave::get_entry(data, size); + auto entry = ::consensus::LedgerEnclave::get_entry( + data, size, network.tables->get_max_transaction_size()); LOG_INFO_FMT( "Deserialising private ledger entry {} [{}]", @@ -1978,6 +1980,8 @@ namespace ccf recovery_store = std::make_shared( true /* Check transactions in order */, true /* Make use of historical secrets */); + recovery_store->set_max_transaction_size( + network.tables->get_max_transaction_size()); auto recovery_history = std::make_shared( *recovery_store, self, diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 96891f09e0af..975b93780277 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -949,6 +949,16 @@ namespace ccf return; } + catch (const ccf::kv::MaxTransactionSizeExceeded& e) + { + ctx->clear_response_headers(); + ctx->set_error( + HTTP_STATUS_PAYLOAD_TOO_LARGE, + ccf::errors::RequestBodyTooLarge, + e.what()); + + return; + } catch (const ccf::kv::KvSerialiserException& e) { // If serialising the committed transaction fails, there is no way diff --git a/tests/config.jinja b/tests/config.jinja index d4a232fe0cec..bfef039cfe27 100644 --- a/tests/config.jinja +++ b/tests/config.jinja @@ -57,7 +57,8 @@ { "directory": "{{ ledger_dir }}", "read_only_directories": {{ read_only_ledger_dirs|tojson }}, - "chunk_size": "{{ ledger_chunk_bytes }}" + "chunk_size": "{{ ledger_chunk_bytes }}", + "max_transaction_size": "{{ ledger_max_transaction_bytes }}" }, "snapshots": { diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 9a135f836fe2..3f180ad78b14 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -252,6 +252,12 @@ def cli_args( type=str, default=ledger_chunk_bytes_override or "20KB", ) + parser.add_argument( + "--ledger-max-transaction-bytes", + help="Maximum serialised transaction body size (bytes)", + type=str, + default="100MB", + ) parser.add_argument( "--snapshot-tx-interval", help="Number of transactions between two snapshots", diff --git a/tests/infra/network.py b/tests/infra/network.py index 52839fc9c4f6..26fe91488322 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -192,6 +192,7 @@ class Network: "join_timer_s", "worker_threads", "ledger_chunk_bytes", + "ledger_max_transaction_bytes", "subject_alt_names", "snapshot_tx_interval", "snapshot_min_tx_interval", diff --git a/tests/limits.py b/tests/limits.py index 2c4202b9d0fa..45dfaa63d821 100644 --- a/tests/limits.py +++ b/tests/limits.py @@ -49,6 +49,21 @@ def test_forward_larger_than_default_requests(network, args): assert r.status_code == http.HTTPStatus.OK.value, r +def test_transaction_size_limit(network, args): + primary, _ = network.find_primary() + + with primary.client("user0") as c: + r = c.post("/app/log/private", {"id": 1, "msg": "small"}) + assert r.status_code == http.HTTPStatus.OK.value, r + + msg = "A" * 64 * 1024 + r = c.post("/app/log/private", {"id": 2, "msg": msg}) + assert r.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value, r + + r = c.post("/app/log/private", {"id": 3, "msg": "still processing"}) + assert r.status_code == http.HTTPStatus.OK.value, r + + def run_parser_limits_checks(args): new_args = copy.copy(args) # Deliberately large because some builds take @@ -66,6 +81,20 @@ def run_parser_limits_checks(args): test_forward_larger_than_default_requests(network, new_args) +def run_transaction_size_limit_checks(args): + new_args = copy.copy(args) + new_args.ledger_max_transaction_bytes = "20KB" + with infra.network.network( + new_args.nodes, + new_args.binary_dir, + new_args.debug_nodes, + pdb=args.pdb, + ) as network: + network.start_and_open(new_args) + + test_transaction_size_limit(network, new_args) + + if __name__ == "__main__": cr = ConcurrentRunner() @@ -78,4 +107,11 @@ def run_parser_limits_checks(args): nodes=infra.e2e_args.max_nodes(cr.args, f=0), ) + cr.add( + "transaction_size_limit", + run_transaction_size_limit_checks, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.max_nodes(cr.args, f=0), + ) + cr.run() From d86be953dbbaad6d53813615d97d6edba67146b0 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Fri, 3 Jul 2026 15:40:35 +0000 Subject: [PATCH 03/15] Fix CI failures: mark get_max_transaction_size [[nodiscard]], fix transaction_size_limit e2e test - clang-tidy (modernize-use-nodiscard) required get_max_transaction_size() to be marked [[nodiscard]], matching the convention already used for other const getters in AbstractStore. - The transaction_size_limit e2e test configured ledger.max_transaction_size to 20KB before starting the network, which is smaller than the constitution scripts written to the KV store during service creation, causing genesis to fail. Raise the configured limit to 512KB (comfortably above the genesis transaction size) and the oversized test payload to 1MB so the 413 path is still exercised. --- src/kv/kv_types.h | 2 +- src/kv/store.h | 2 +- tests/limits.py | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index c0c5d3d1bcb3..41aa32dd9782 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -676,7 +676,7 @@ namespace ccf::kv virtual std::shared_ptr get_history() = 0; virtual std::shared_ptr get_chunker() = 0; virtual EncryptorPtr get_encryptor() = 0; - virtual size_t get_max_transaction_size() const = 0; + [[nodiscard]] virtual size_t get_max_transaction_size() const = 0; virtual std::unique_ptr deserialize( const std::vector& data, bool public_only = false, diff --git a/src/kv/store.h b/src/kv/store.h index eeeb2ba4f3a2..34d9a92a7f15 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -245,7 +245,7 @@ namespace ccf::kv max_transaction_size = max_transaction_size_; } - size_t get_max_transaction_size() const override + [[nodiscard]] size_t get_max_transaction_size() const override { return max_transaction_size; } diff --git a/tests/limits.py b/tests/limits.py index 45dfaa63d821..131005eb5f26 100644 --- a/tests/limits.py +++ b/tests/limits.py @@ -56,7 +56,7 @@ def test_transaction_size_limit(network, args): r = c.post("/app/log/private", {"id": 1, "msg": "small"}) assert r.status_code == http.HTTPStatus.OK.value, r - msg = "A" * 64 * 1024 + msg = "A" * 1024 * 1024 r = c.post("/app/log/private", {"id": 2, "msg": msg}) assert r.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value, r @@ -83,7 +83,10 @@ def run_parser_limits_checks(args): def run_transaction_size_limit_checks(args): new_args = copy.copy(args) - new_args.ledger_max_transaction_bytes = "20KB" + # Deliberately larger than the constitution scripts written to the KV + # store as part of service creation, but well under the oversized + # request used below, so only the latter is rejected. + new_args.ledger_max_transaction_bytes = "512KB" with infra.network.network( new_args.nodes, new_args.binary_dir, From 80bad361f5089d03e9c07104b0c79c4b6e76e9bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 13:44:33 +0000 Subject: [PATCH 04/15] Document ledger entry size parameter Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/consensus/ledger_enclave.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index b7119739d273..c1136c5c4b79 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -20,6 +20,7 @@ namespace consensus * * @param data Serialised entries * @param size Size of overall serialised entries + * @param max_transaction_size Maximum allowed serialised entry body size * * @return Raw entry as a vector */ From 82b243c4ba117ab45162c21eddc20d6218c57c56 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Thu, 9 Jul 2026 15:38:33 +0000 Subject: [PATCH 05/15] Address review feedback on max transaction size enforcement - Exempt snapshots from the per-transaction limit (serialise and deserialise) and add a regression test; snapshots capture whole-store state and may legitimately exceed a single-transaction cap. - Move the CHANGELOG entry from the released 5.0.0 section to 7.0.7 (Changed), fix 'serialized' -> 'serialised', and reference PR #7992 instead of tracking issue #7488. - Rename describe_serialised_entry_size_error to British spelling to match the surrounding code. - Document that historical-query stores deliberately do not apply the limit (read-only reconstruction). - Add a Store::set_max_transaction_size out-of-range validation test. - Add missing trailing newline to serialised_entry_format.h. --- CHANGELOG.md | 2 +- src/consensus/ledger_enclave.h | 2 +- src/kv/generic_serialise_wrapper.h | 4 +-- src/kv/kv_types.h | 3 +- src/kv/serialised_entry_format.h | 4 +-- src/kv/snapshot.h | 6 ++-- src/kv/store.h | 12 +++++-- src/kv/test/kv_serialisation.cpp | 19 +++++++++++ src/kv/test/kv_snapshot.cpp | 54 ++++++++++++++++++++++++++++++ src/node/historical_queries.h | 7 ++++ 10 files changed, 98 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acc63e450124..7c94c1c60d28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed +- The `ledger.max_transaction_size` configuration option now limits the serialised transaction body size stored in each ledger entry. The limit excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain. The default value is `100MB`. (#7992) - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). @@ -1294,7 +1295,6 @@ For more information, see [our documentation](https://microsoft.github.io/CCF/ma #### Configuration - The `cchost` configuration file now includes an `idle_connection_timeout` option. This controls how long the node will keep idle connections (for user TLS sessions) before automatically closing them. This may be set to `null` to restore the previous behaviour, where idle connections are never closed. By default connections will be closed after 60s of idle time. -- The `ledger.max_transaction_size` configuration option now limits the serialized transaction body size stored in each ledger entry. The limit excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain. The default value is `100MB`. (#7488) - A soft size limit can now be set for the historical store cache in the node configuration: [`historical_cache_soft_limit`](https://microsoft.github.io/CCF/main/operations/generated_config.html#historical-cache-soft-limit). The default value is `512Mb`. - Path to the enclave file should now be passed as `--enclave-file` CLI argument to `cchost`, rather than `enclave.file` entry within configuration file. - SNP collateral must now be provided through the `snp_security_policy_file`, `snp_uvm_endorsements_file` and `snp_endorsements_servers` configuration values. See [documentation](https://microsoft.github.io/CCF/main/operations/platforms/snp.html) for details and platform-specific configuration samples. diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index c1136c5c4b79..975c1871a10b 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -35,7 +35,7 @@ namespace consensus const size_t body_size = header.size; if (body_size > max_transaction_size) { - throw std::logic_error(ccf::kv::describe_serialized_entry_size_error( + throw std::logic_error(ccf::kv::describe_serialised_entry_size_error( body_size, max_transaction_size, "extract from ledger")); } if (body_size + ccf::kv::serialised_entry_header_size > size) diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index e315b5efbf98..006a2cb8fa61 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -178,7 +178,7 @@ namespace ccf::kv } if (size_ > max_transaction_size) { - throw MaxTransactionSizeExceeded(describe_serialized_entry_size_error( + throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( size_, max_transaction_size, "serialise")); } entry_header.set_size(size_); @@ -324,7 +324,7 @@ namespace ccf::kv if (tx_header.size > max_transaction_size) { - throw MaxTransactionSizeExceeded(describe_serialized_entry_size_error( + throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( tx_header.size, max_transaction_size, "deserialise")); } diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index dfc1a7d84987..ce53eb841c7b 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -702,8 +702,7 @@ namespace ccf::kv virtual ~AbstractSnapshot() = default; [[nodiscard]] virtual Version get_version() const = 0; virtual std::vector serialise( - const std::shared_ptr& encryptor, - size_t max_transaction_size) = 0; + const std::shared_ptr& encryptor) = 0; }; virtual ~AbstractStore() = default; diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index a31b7a23bbd2..2efb7a4a5f49 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -47,7 +47,7 @@ namespace ccf::kv static constexpr size_t serialised_entry_header_size = sizeof(SerialisedEntryHeader); - static inline std::string describe_serialized_entry_size_error( + static inline std::string describe_serialised_entry_size_error( size_t body_size, size_t max_body_size, const char* operation) { return fmt::format( @@ -62,4 +62,4 @@ namespace ccf::kv max_body_size, serialised_entry_header_size); } -} \ No newline at end of file +} diff --git a/src/kv/snapshot.h b/src/kv/snapshot.h index 79bfe82350a5..7176c1220572 100644 --- a/src/kv/snapshot.h +++ b/src/kv/snapshot.h @@ -41,8 +41,7 @@ namespace ccf::kv } std::vector serialise( - const std::shared_ptr& encryptor, - size_t max_transaction_size) override + const std::shared_ptr& encryptor) override { // Set the execution dependency for the snapshot to be the version // previous to said snapshot to ensure that the correct snapshot is @@ -60,8 +59,7 @@ namespace ccf::kv 0, {}, ccf::no_claims(), - true /* historical_hint */, - max_transaction_size); + true /* historical_hint */); if (hash_at_snapshot.has_value()) { diff --git a/src/kv/store.h b/src/kv/store.h index 5ee88884a5f6..0654ee02161d 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -414,7 +414,11 @@ namespace ccf::kv std::unique_ptr snapshot) override { auto e = get_encryptor(); - return snapshot->serialise(e, max_transaction_size); + // Snapshots capture the entire committed state and are legitimately much + // larger than any single transaction, so the configured + // max_transaction_size (a per-transaction write limit) is deliberately + // not applied here. + return snapshot->serialise(e); } ApplyResult deserialise_snapshot( @@ -432,8 +436,10 @@ namespace ccf::kv ccf::kv::Term term = 0; ccf::kv::EntryFlags entry_flags = {}; - auto v_ = d.init( - data, size, term, entry_flags, is_historical, max_transaction_size); + // Snapshots are not subject to the per-transaction max_transaction_size + // limit (see serialise_snapshot), so it is not applied when reading one + // back either. + auto v_ = d.init(data, size, term, entry_flags, is_historical); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index e5358e59ede9..635a5a3b765a 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -8,6 +8,7 @@ #include #undef FAIL +#include #include #include @@ -238,6 +239,24 @@ TEST_CASE( ccf::kv::MaxTransactionSizeExceeded); } +TEST_CASE( + "Reject configuring a maximum transaction size beyond the serialisable " + "limit" * + doctest::test_suite("serialisation")) +{ + ccf::kv::Store kv_store; + + // The largest size the ledger entry header can represent is accepted. + REQUIRE_NOTHROW(kv_store.set_max_transaction_size( + ccf::kv::SerialisedEntryHeader::max_serialised_entry_body_size)); + + // A value larger than can ever be serialised is rejected at configuration + // time, rather than being deferred to a later serialisation failure. + REQUIRE_THROWS_AS( + kv_store.set_max_transaction_size(std::numeric_limits::max()), + std::logic_error); +} + TEST_CASE( "Serialise/deserialise private map and public maps" * doctest::test_suite("serialisation")) diff --git a/src/kv/test/kv_snapshot.cpp b/src/kv/test/kv_snapshot.cpp index 08ed6f959e98..7ba6dd7caae1 100644 --- a/src/kv/test/kv_snapshot.cpp +++ b/src/kv/test/kv_snapshot.cpp @@ -7,6 +7,7 @@ #include #undef FAIL +#include struct MapTypes { @@ -19,6 +20,59 @@ struct MapTypes MapTypes::StringString string_map("public:string_map"); MapTypes::NumNum num_map("public:num_map"); +TEST_CASE( + "Snapshots are not subject to the transaction size limit" * + doctest::test_suite("snapshot")) +{ + auto encryptor = std::make_shared(); + + ccf::kv::Store store; + store.set_encryptor(encryptor); + + // A deliberately small per-transaction limit. + constexpr size_t small_limit = 512; + store.set_max_transaction_size(small_limit); + + // Accumulate committed state larger than the limit across several + // transactions, each of which individually stays under the limit. + constexpr size_t num_entries = 32; + constexpr size_t value_size = 64; + ccf::kv::Version snapshot_version = ccf::kv::NoVersion; + for (size_t i = 0; i < num_entries; ++i) + { + auto tx = store.create_tx(); + auto handle = tx.rw(string_map); + handle->put("key_" + std::to_string(i), std::string(value_size, 'x')); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + snapshot_version = tx.commit_version(); + } + REQUIRE(num_entries * value_size > small_limit); + + std::unique_ptr snapshot = nullptr; + { + ccf::kv::ScopedStoreMapsLock maps_lock(&store); + snapshot = store.snapshot_unsafe_maps(snapshot_version); + } + + // Serialising a snapshot larger than the per-transaction limit must not be + // rejected by that limit. + std::vector serialised_snapshot; + REQUIRE_NOTHROW( + serialised_snapshot = store.serialise_snapshot(std::move(snapshot))); + + // And it can be read back into a store configured with the same small limit. + ccf::kv::Store new_store; + new_store.set_encryptor(encryptor); + new_store.set_max_transaction_size(small_limit); + + ccf::kv::ConsensusHookPtrs hooks; + REQUIRE_EQ( + new_store.deserialise_snapshot( + serialised_snapshot.data(), serialised_snapshot.size(), hooks), + ccf::kv::ApplyResult::PASS); + REQUIRE_EQ(new_store.current_version(), snapshot_version); +} + TEST_CASE("Simple snapshot" * doctest::test_suite("snapshot")) { ccf::kv::Store store; diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 55fd7b7c81f0..d12e4041f22c 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -1485,6 +1485,13 @@ namespace ccf::historical false /* Do not start from very first seqno */, true /* Make use of historical secrets */); + // The configured max_transaction_size limit is deliberately not set on + // this store. That limit is a write-time guard preventing oversized + // entries from being serialised into the ledger; historical queries only + // reconstruct already-committed state to serve reads, and must remain + // able to deserialise any entry that was validly written, including one + // written under a previously larger or unset limit. + // If this is older than the node's currently known ledger secrets, use // the historical encryptor (which should have older secrets) if (seqno < source_ledger_secrets->get_first().first) From 6e8606b39f50722bd49777c4e5882f9729b05d74 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Fri, 10 Jul 2026 08:49:22 +0000 Subject: [PATCH 06/15] Refine max transaction size: cap serialisation only, on whole-entry size - Exempt all deserialisation from the size cap; only serialisation of new transactions is capped. Remove the limit parameter and check from the deserialiser init, the abstract deserialiser interface, deserialise_views, get_entry, and both recovery loops; drop recovery_store's limit. Buffer- bounds safety checks (header size vs buffer, get_entry truncation) remain. - Apply the cap to the whole serialised ledger entry (fixed 8-byte header plus body) instead of just the body. - Update the error message, CHANGELOG, and host config schema description; remove a stale get_entry doc comment. - Replace the deserialise-rejection unit test with a deserialise-exemption test. --- CHANGELOG.md | 2 +- doc/host_config_schema/host_config.json | 217 +++++++++++++++++++----- src/consensus/ledger_enclave.h | 16 +- src/kv/generic_serialise_wrapper.h | 23 +-- src/kv/kv_types.h | 4 +- src/kv/serialised_entry_format.h | 18 +- src/kv/store.h | 9 +- src/kv/test/kv_serialisation.cpp | 13 +- src/node/node_state.h | 8 +- 9 files changed, 207 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c94c1c60d28..625c0a247cf9 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. ### Changed -- The `ledger.max_transaction_size` configuration option now limits the serialised transaction body size stored in each ledger entry. The limit excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain. The default value is `100MB`. (#7992) +- The `ledger.max_transaction_size` configuration option now limits the total serialised size of a transaction when writing it to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is enforced only when serialising new transactions; deserialising existing entries (including during recovery) and snapshots are not affected. The default value is `100MB`. (#7992) - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 3e3a4c4e47e4..bf6c609ae996 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -21,7 +21,9 @@ } }, "description": "Addresses (host:port) to listen on for incoming node-to-node connections (e.g. internal consensus messages). IPv6 literals must be bracketed, e.g. ``[::1]:8081``", - "required": ["bind_address"], + "required": [ + "bind_address" + ], "additionalProperties": false }, "rpc_interfaces": { @@ -100,12 +102,19 @@ "properties": { "authority": { "type": "string", - "enum": ["Node", "Service", "ACME", "Unsecured"], + "enum": [ + "Node", + "Service", + "ACME", + "Unsecured" + ], "default": "Service", "description": "The type of endorsement for the TLS certificate used in client sessions. If the endorsement is not available, client sessions will be terminated, before the TLS handshake is complete. 'Node' means self-signed, 'Service' means service-endorsed, 'ACME' is deprecated, 'Unsecured' means unencrypted traffic and no endorsement authority" } }, - "required": ["authority"], + "required": [ + "authority" + ], "additionalProperties": false }, "accepted_endpoints": { @@ -138,19 +147,28 @@ "enabled_operator_features": { "type": "array", "items": { - "enum": ["SnapshotRead", "LedgerChunkRead", "SnapshotCreate"], + "enum": [ + "SnapshotRead", + "LedgerChunkRead", + "SnapshotCreate" + ], "type": "string" }, "description": "An array of features which should be enabled on this interface, providing access to endpoints with specific security or performance constraints." } }, - "required": ["bind_address"] + "required": [ + "bind_address" + ] }, "description": "Interfaces to listen on for incoming client TLS connections, as a dictionary from unique interface name to RPC interface information" } }, "description": "This section includes configuration for the interfaces a node listens on (for both client and node-to-node communications)", - "required": ["node_to_node_interface", "rpc_interfaces"], + "required": [ + "node_to_node_interface", + "rpc_interfaces" + ], "additionalProperties": false }, "command": { @@ -158,7 +176,11 @@ "properties": { "type": { "type": "string", - "enum": ["Start", "Join", "Recover"], + "enum": [ + "Start", + "Join", + "Recover" + ], "description": "Type of CCF node" }, "service_certificate_file": { @@ -222,20 +244,32 @@ "description": "Path to member x509 identity certificate (PEM)" }, "encryption_public_key_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to member encryption public key (PEM)" }, "data_json_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to member data file (JSON)" }, "recovery_role": { "type": "string", - "enum": ["NonParticipant", "Participant", "Owner"], + "enum": [ + "NonParticipant", + "Participant", + "Owner" + ], "description": "Whether the member acts as a recovery participant and gets assigned a share that can contribute towards a recovery threshold or as an owner and gets assigned a full recovery key" } }, - "required": ["certificate_file"], + "required": [ + "certificate_file" + ], "additionalProperties": false }, "description": "List of initial consortium members files, including identity certificates, public encryption keys and member data files" @@ -267,15 +301,22 @@ "minimum": 1 } }, - "required": ["recovery_threshold"], + "required": [ + "recovery_threshold" + ], "additionalProperties": false } }, - "required": ["constitution_files", "members"], + "required": [ + "constitution_files", + "members" + ], "additionalProperties": false } }, - "required": ["start"] + "required": [ + "start" + ] } }, { @@ -327,16 +368,23 @@ "description": "Maximum size of snapshot this node is willing to fetch" }, "host_data_transparent_statement_path": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "default": null, "description": "Path to a SCITT Transparent Statement over the attested host_data of the node" } }, - "required": ["target_rpc_address"], + "required": [ + "target_rpc_address" + ], "additionalProperties": false } }, - "required": ["join"] + "required": [ + "join" + ] } }, { @@ -363,7 +411,9 @@ "description": "Path to the previous service certificate (PEM) file" } }, - "required": ["previous_service_identity_file"], + "required": [ + "previous_service_identity_file" + ], "additionalProperties": false } } @@ -371,7 +421,9 @@ } ], "description": "This section includes configuration of how the node should start (either start, join or recover) and associated information", - "required": ["type"] + "required": [ + "type" + ] }, "node_certificate": { "type": "object", @@ -390,7 +442,10 @@ }, "curve_id": { "type": "string", - "enum": ["Secp384R1", "Secp256R1"], + "enum": [ + "Secp384R1", + "Secp256R1" + ], "default": "Secp384R1", "description": "Elliptic curve to use for node identity key" }, @@ -405,22 +460,34 @@ "additionalProperties": false }, "node_data_json_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to file (JSON) containing initial node data. It is intended to store correlation IDs describing the node's deployment, such as a VM name or Pod identifier" }, "attestation": { "type": "object", "properties": { "snp_security_policy_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to file containing the security policy (SEV-SNP only), can contain environment variables, such as $UVM_SECURITY_CONTEXT_DIR" }, "snp_uvm_endorsements_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to file containing UVM endorsements as a base64-encoded COSE Sign1 (SEV-SNP only). Can contain environment variables, such as $UVM_SECURITY_CONTEXT_DIR" }, "snp_endorsements_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to file containing AMD VCEK hardware endorsements (a PEM certificate chain), base-64 encoded. Can contain environment variables, such as $UVM_SECURITY_CONTEXT_DIR. Will be used in preference to snp_endorsements_servers if the tcbm in this file matches that of the attestation" }, "snp_endorsements_servers": { @@ -430,7 +497,11 @@ "properties": { "type": { "type": "string", - "enum": ["Azure", "AMD", "THIM"], + "enum": [ + "Azure", + "AMD", + "THIM" + ], "default": "Azure", "description": "Type of server used to retrieve attestation report endorsement certificates (SEV-SNP only)" }, @@ -444,7 +515,9 @@ "description": "Maximum number of retries to fetch endorsements from the server" } }, - "required": ["url"], + "required": [ + "url" + ], "additionalProperties": false }, "description": "List of servers used to retrieve attestation report endorsement certificates (SEV-SNP only). The first server in the list is always used and other servers are only specified as fallback. If set, attestation endorsements from ``--snp-security-context-dir-var`` are ignored, but uvm endorsements from that directory are still used." @@ -454,7 +527,10 @@ "additionalProperties": false }, "service_data_json_file": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to file (JSON) containing initial service data. It is used when the node starts in 'Start' or 'Recover' mode and is intended to store arbitrary information about the service" }, "ledger": { @@ -480,7 +556,7 @@ "max_transaction_size": { "type": "string", "default": "100MB", - "description": "Maximum serialised transaction body size (size string). This is compared with the size stored in the ledger entry header, so it excludes the fixed 8-byte ledger entry header and includes the ledger encryption header, public domain size field, public domain and encrypted private domain." + "description": "Maximum total serialised size of a transaction (size string). This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain." } }, "description": "This section includes configuration for the ledger directories and files", @@ -511,7 +587,10 @@ "description": "Time interval after which a snapshot should be triggered, provided more than min_tx_count transactions have elapsed since the last snapshot. Set this to 0s to disable time-based snapshotting." }, "read_only_directory": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Path to read-only snapshots directory. Deprecated: this option is deprecated and will be removed in a future release. Use join.fetch_recent_snapshot and snapshots.backup_fetch to have joining and/or backup nodes automatically fetch snapshots from the primary node instead." }, "backup_fetch": { @@ -555,13 +634,19 @@ "type": "object", "properties": { "max_snapshots": { - "type": ["integer", "null"], + "type": [ + "integer", + "null" + ], "default": null, "description": "Maximum number of committed snapshot files to retain. When the number of committed snapshots exceeds this value, the oldest snapshots are deleted. Must be at least 1 if set. If null or unset, no automated snapshot garbage collection is performed.", "minimum": 1 }, "max_committed_ledger_chunks": { - "type": ["integer", "null"], + "type": [ + "integer", + "null" + ], "default": null, "description": "Maximum number of committed ledger chunk files to retain in the main ledger directory. When the number of committed chunks exceeds this value, the oldest chunks are deleted, but only after verifying that an identical copy (by SHA-256 digest) exists in at least one read-only ledger directory. Chunks whose entries extend to or beyond the sequence number of the newest committed snapshot are never deleted, ensuring a complete ledger history from that snapshot for disaster recovery. Requires at least one ledger.read_only_directories entry; the node will refuse to start otherwise. If null or unset, no automated ledger chunk garbage collection is performed." }, @@ -597,13 +682,22 @@ "properties": { "host_level": { "type": "string", - "enum": ["Trace", "Debug", "Info", "Fail", "Fatal"], + "enum": [ + "Trace", + "Debug", + "Info", + "Fail", + "Fatal" + ], "default": "Info", "description": "Logging level for the untrusted host. DEPRECATED, use the --log-level CLI switch instead." }, "format": { "type": "string", - "enum": ["Text", "Json"], + "enum": [ + "Text", + "Json" + ], "default": "Text", "description": "If 'json', node logs will be formatted as JSON" } @@ -704,7 +798,10 @@ "description": "Maximum duration of I/O operations (ledger and snapshots) after which slow operations will be logged to node log" }, "node_client_interface": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Address to bind to for node-to-node client connections. If unspecified, this is automatically assigned by the OS. This option is particularly useful for testing purposes (e.g. establishing network partitions between nodes)" }, "client_connection_timeout": { @@ -713,7 +810,10 @@ "description": "Maximum duration after which unestablished client connections will be marked as timed out and either re-established or discarded" }, "idle_connection_timeout": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "default": "60s", "description": "Timeout for idle connections. Null is a valid option, and means idle connections are retained indefinitely" }, @@ -774,7 +874,10 @@ "type": "string" } }, - "required": ["name", "address"], + "required": [ + "name", + "address" + ], "additionalProperties": false }, "recovery_decision_protocol": { @@ -793,7 +896,10 @@ "type": "string" } }, - "required": ["name", "address"], + "required": [ + "name", + "address" + ], "additionalProperties": false } }, @@ -807,26 +913,38 @@ "description": "Timeout duration before failover forcibly advances the recovery_decision_protocol, allowing recovery to proceed even in the presence of unresponsive nodes. Set to 0 to disable failover." } }, - "required": ["expected_locations"], + "required": [ + "expected_locations" + ], "additionalProperties": false } }, - "required": ["location"], + "required": [ + "location" + ], "additionalProperties": false } }, - "required": ["network", "command"], + "required": [ + "network", + "command" + ], "additionalProperties": false, "$defs": { "RedirectionResolver": { "type": "object", "properties": { "kind": { - "enum": ["NodeByRole", "StaticAddress"] + "enum": [ + "NodeByRole", + "StaticAddress" + ] }, "target": {} }, - "required": ["kind"], + "required": [ + "kind" + ], "allOf": [ { "if": { @@ -842,7 +960,10 @@ "type": "object", "properties": { "role": { - "enum": ["primary", "backup"], + "enum": [ + "primary", + "backup" + ], "default": "primary" } }, @@ -868,15 +989,19 @@ "type": "string" } }, - "required": ["address"], + "required": [ + "address" + ], "additionalProperties": false } }, - "required": ["target"] + "required": [ + "target" + ] } } ], "additionalProperties": false } } -} +} \ No newline at end of file diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 975c1871a10b..52dae4f5d10d 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -20,24 +20,18 @@ namespace consensus * * @param data Serialised entries * @param size Size of overall serialised entries - * @param max_transaction_size Maximum allowed serialised entry body size * * @return Raw entry as a vector */ - static std::vector get_entry( - const uint8_t*& data, - size_t& size, - size_t max_transaction_size = - ccf::kv::SerialisedEntryHeader::max_serialised_entry_body_size) + static std::vector get_entry(const uint8_t*& data, size_t& size) { auto header = serialized::peek(data, size); const size_t body_size = header.size; - if (body_size > max_transaction_size) - { - throw std::logic_error(ccf::kv::describe_serialised_entry_size_error( - body_size, max_transaction_size, "extract from ledger")); - } + // This is a buffer-bounds safety check, not the configurable transaction + // size limit: that limit applies only when serialising new transactions. + // Deserialisation (including recovery replay) is exempt, so entries + // written under a larger or unset limit can always be read back. if (body_size + ccf::kv::serialised_entry_header_size > size) { throw std::logic_error(fmt::format( diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 006a2cb8fa61..abab15950a3d 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -176,15 +176,18 @@ namespace ccf::kv size_ += crypto_util->get_header_length() + sizeof(size_t) + serialised_private_domain.size(); } - if (size_ > max_transaction_size) - { - throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( - size_, max_transaction_size, "serialise")); - } entry_header.set_size(size_); + // The configured limit applies to the whole serialised ledger entry, + // including the fixed-size entry header. size_ += sizeof(SerialisedEntryHeader); + if (size_ > max_transaction_size) + { + throw MaxTransactionSizeExceeded( + describe_serialised_entry_size_error(size_, max_transaction_size)); + } + std::vector entry(size_); auto* data_ = entry.data(); @@ -311,9 +314,7 @@ namespace ccf::kv size_t size, ccf::kv::Term& term, EntryFlags& flags, - bool historical_hint = false, - size_t max_transaction_size = - SerialisedEntryHeader::max_serialised_entry_body_size) override + bool historical_hint = false) override { current_reader = &public_reader; const auto* data_ = data; @@ -322,12 +323,6 @@ namespace ccf::kv const auto tx_header = serialized::read(data_, size_); - if (tx_header.size > max_transaction_size) - { - throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( - tx_header.size, max_transaction_size, "deserialise")); - } - flags = static_cast(tx_header.flags); if (tx_header.size != size_) diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index ce53eb841c7b..96de7bc58572 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -343,9 +343,7 @@ namespace ccf::kv size_t size, ccf::kv::Term& term, EntryFlags& flags, - bool historical_hint = false, - size_t max_transaction_size = - SerialisedEntryHeader::max_serialised_entry_body_size) = 0; + bool historical_hint = false) = 0; virtual std::optional start_map() = 0; virtual Version deserialise_entry_version() = 0; virtual uint64_t deserialise_read_header() = 0; diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index 2efb7a4a5f49..e0aef2745608 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -48,18 +48,16 @@ namespace ccf::kv sizeof(SerialisedEntryHeader); static inline std::string describe_serialised_entry_size_error( - size_t body_size, size_t max_body_size, const char* operation) + size_t entry_size, size_t max_entry_size) { return fmt::format( - "Cannot {} transaction with serialised body size {} bytes. The " - "configured maximum is {} bytes. The transaction size compared to this " - "limit is the size stored in the ledger entry header: the serialised " - "transaction body after the fixed {}-byte ledger entry header, " - "including any ledger encryption header, public domain size field, " - "public domain and encrypted private domain.", - operation, - body_size, - max_body_size, + "Cannot serialise transaction with total serialised size {} bytes. The " + "configured maximum is {} bytes. The size compared to this limit is the " + "whole ledger entry: the fixed {}-byte ledger entry header plus the " + "serialised transaction body (any ledger encryption header, public " + "domain size field, public domain and encrypted private domain).", + entry_size, + max_entry_size, serialised_entry_header_size); } } diff --git a/src/kv/store.h b/src/kv/store.h index 0654ee02161d..1f461385d3ee 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -766,13 +766,8 @@ namespace ccf::kv public_only ? ccf::kv::SecurityDomain::PUBLIC : std::optional()); - auto v_ = d.init( - data.data(), - data.size(), - view, - entry_flags, - is_historical, - max_transaction_size); + auto v_ = + d.init(data.data(), data.size(), view, entry_flags, is_historical); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 635a5a3b765a..857c16f9df11 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -208,12 +208,13 @@ TEST_CASE( } TEST_CASE( - "Reject deserialised transactions exceeding configured serialised size" * + "Deserialisation is not subject to the transaction size limit" * doctest::test_suite("serialisation")) { auto consensus = std::make_shared(); auto encryptor = std::make_shared(); + // Serialise a transaction under a permissive (default) limit. ccf::kv::Store kv_store; kv_store.set_consensus(consensus); kv_store.set_encryptor(encryptor); @@ -223,20 +224,22 @@ TEST_CASE( { auto tx = kv_store.create_tx(); auto handle = tx.rw(map); - handle->put("small", "ok"); + handle->put("large", std::string(2048, 'A')); REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); } const auto latest_data = consensus->get_latest_data(); REQUIRE(latest_data.has_value()); + // The cap applies only to serialisation. A tiny limit on the target store + // must not prevent it from deserialising an already-serialised transaction. ccf::kv::Store kv_store_target; kv_store_target.set_encryptor(encryptor); kv_store_target.set_max_transaction_size(1); - REQUIRE_THROWS_AS( - kv_store_target.deserialize(latest_data.value())->apply(), - ccf::kv::MaxTransactionSizeExceeded); + REQUIRE( + kv_store_target.deserialize(latest_data.value())->apply() == + ccf::kv::ApplyResult::PASS); } TEST_CASE( diff --git a/src/node/node_state.h b/src/node/node_state.h index eba3ce0d2684..a6587897aa32 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1477,8 +1477,7 @@ namespace ccf while (size > 0) { - auto entry = ::consensus::LedgerEnclave::get_entry( - data, size, network.tables->get_max_transaction_size()); + auto entry = ::consensus::LedgerEnclave::get_entry(data, size); LOG_INFO_FMT( "Deserialising public ledger entry #{} [{} bytes]", @@ -1767,8 +1766,7 @@ namespace ccf while (size > 0) { - auto entry = ::consensus::LedgerEnclave::get_entry( - data, size, network.tables->get_max_transaction_size()); + auto entry = ::consensus::LedgerEnclave::get_entry(data, size); LOG_INFO_FMT( "Deserialising private ledger entry {} [{}]", @@ -1983,8 +1981,6 @@ namespace ccf recovery_store = std::make_shared( true /* Check transactions in order */, true /* Make use of historical secrets */); - recovery_store->set_max_transaction_size( - network.tables->get_max_transaction_size()); auto recovery_history = std::make_shared( *recovery_store, self, From 35d80e6b5f7668363f01581f6c0e9bc1db0153c1 Mon Sep 17 00:00:00 2001 From: achamayou Date: Wed, 12 Aug 2026 23:08:11 +0100 Subject: [PATCH 07/15] Check the transaction size limit before applying the transaction Enforcing the limit during serialisation meant the transaction had already taken a version and mutated the maps, so it could only be undone by calling Store::rollback. That is unsafe: apply_changes notes that other non-conflicting transactions may commit at later versions concurrently, so rolling back to the pre-apply TxID can discard their changes, and Store::rollback also clears pending_txs and bumps rollback_count. Project the exact size of the ledger entry before apply_changes instead, by serialising the change sets (which apply_changes does not modify) through a serialiser that only measures them. An oversized transaction is then rejected without the store having been touched, so it neither writes a value nor stops later transactions. The limit is still checked when serialising, but as a fatal KvSerialiserException, matching how other post-apply serialisation failures are treated. Use a distinct TransactionTooLarge error code, so the rejection cannot be confused with the HTTP parser's RequestBodyTooLarge, which is also a 413 and closes the session. Move the CHANGELOG entry to the current version, under Added. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- include/ccf/odata_error.h | 1 + src/consensus/ledger_enclave.h | 9 +-- src/kv/committable_tx.h | 104 ++++++++++++++++++++--------- src/kv/generic_serialise_wrapper.h | 44 +++++++++--- src/kv/kv_serialiser.h | 1 + src/kv/raw_serialise.h | 78 ++++++++++++++++++++++ src/kv/serialised_entry_format.h | 6 ++ src/kv/store.h | 27 +++----- src/kv/test/kv_serialisation.cpp | 89 +++++++++++++++++++++++- src/node/historical_queries.h | 10 ++- src/node/rpc/frontend.h | 4 +- tests/limits.py | 25 +++++-- 13 files changed, 325 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c17f7745dc9f..2744319493ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). +- New `ledger.max_transaction_size` node configuration option (default `100MB`), which caps the total serialised size of a transaction written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. It applies only to newly serialised transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable (#7992). ### Fixed @@ -87,7 +88,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed -- The `ledger.max_transaction_size` configuration option now limits the total serialised size of a transaction when writing it to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is enforced only when serialising new transactions; deserialising existing entries (including during recovery) and snapshots are not affected. The default value is `100MB`. (#7992) - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). diff --git a/include/ccf/odata_error.h b/include/ccf/odata_error.h index b0bfa65a3675..d3221d05993b 100644 --- a/include/ccf/odata_error.h +++ b/include/ccf/odata_error.h @@ -100,6 +100,7 @@ namespace ccf ERROR(TransactionNotFound) ERROR(TransactionCommitAttemptsExceedLimit) ERROR(TransactionReplicationFailed) + ERROR(TransactionTooLarge) ERROR(UnknownCertificate) ERROR(VoteNotFound) ERROR(VoteAlreadyExists) diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 52dae4f5d10d..458b9c1a2f69 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -28,10 +28,11 @@ namespace consensus auto header = serialized::peek(data, size); const size_t body_size = header.size; - // This is a buffer-bounds safety check, not the configurable transaction - // size limit: that limit applies only when serialising new transactions. - // Deserialisation (including recovery replay) is exempt, so entries - // written under a larger or unset limit can always be read back. + // The size in the entry header is not trusted: check it against the + // buffer we were given before allocating. This is distinct from the + // configured max_transaction_size, which applies only when serialising + // new transactions, so that entries written under a larger or unset + // limit can always be read back. if (body_size + ccf::kv::serialised_entry_header_size > size) { throw std::logic_error(fmt::format( diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index 4b128a465dda..9d62a8c27348 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -36,6 +36,61 @@ namespace ccf::kv TxFlags flags = 0; SerialisedEntryFlags entry_flags = 0; + // Serialises the same changes as serialise(), into a serialiser which only + // measures their size, to determine how large this transaction's ledger + // entry will be. Change sets are not modified by apply_changes(), so this + // can be called before the transaction is applied to the store, and the + // result is exactly the size serialise() will produce. + size_t projected_serialised_size( + const ccf::ClaimsDigest& claims_digest_, bool include_reads = false) + { + auto e = pimpl->store->get_encryptor(); + if (e == nullptr) + { + throw KvSerialiserException("No encryptor set"); + } + + SizeKvStoreSerialiser size_serialiser( + e, + {pimpl->commit_view, NoVersion}, + EntryType::WriteSetWithCommitEvidenceAndClaims, + entry_flags, + // Both digests are fixed-size, so their values do not affect the + // size of the resulting entry + ccf::crypto::Sha256Hash{}, + claims_digest_); + + serialise_all_changes(size_serialiser, include_reads); + + return size_serialiser.get_serialised_size(); + } + + void serialise_all_changes( + KvStoreSerialiser& serialiser, bool include_reads) + { + // Process in security domain order + for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) + { + for (const auto& it : all_changes) + { + const auto& map = it.second.map; + const auto& changeset = it.second.changeset; + if (map->get_security_domain() == domain && changeset->has_writes()) + { + map->serialise_changes(changeset.get(), serialiser, include_reads); + } + } + } + } + + [[nodiscard]] bool has_writes() const + { + return std::any_of( + all_changes.begin(), all_changes.end(), [](const auto& it) { + return it.second.changeset->has_writes(); + }); + } + std::vector serialise( ccf::crypto::Sha256Hash& commit_evidence_digest, std::string& commit_evidence, @@ -58,12 +113,7 @@ namespace ccf::kv } // If no transactions made changes, return a zero length vector. - const bool any_changes = - std::any_of(all_changes.begin(), all_changes.end(), [](const auto& it) { - return it.second.changeset->has_writes(); - }); - - if (!any_changes) + if (!has_writes()) { return {}; } @@ -95,20 +145,7 @@ namespace ccf::kv false /* historical_hint */, pimpl->store->get_max_transaction_size()); - // Process in security domain order - for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) - { - for (const auto& it : all_changes) - { - const auto& map = it.second.map; - const auto& changeset = it.second.changeset; - if (map->get_security_domain() == domain && changeset->has_writes()) - { - map->serialise_changes( - changeset.get(), replicated_serialiser, include_reads); - } - } - } + serialise_all_changes(replicated_serialiser, include_reads); // Return serialised Tx. return replicated_serialiser.get_raw_data(); @@ -152,6 +189,22 @@ namespace ccf::kv return CommitResult::SUCCESS; } + // Reject transactions whose ledger entry would exceed the configured + // limit before any change is applied. Once apply_changes() below has + // taken a version and mutated the maps, the transaction can no longer be + // abandoned without losing the entry at that version. + if (has_writes()) + { + const auto max_transaction_size = + pimpl->store->get_max_transaction_size(); + const auto entry_size = projected_serialised_size(claims); + if (entry_size > max_transaction_size) + { + throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( + entry_size, max_transaction_size)); + } + } + // If this transaction creates any maps, ensure that commit gets a // consistent snapshot of the existing map set const bool maps_created = !pimpl->created_maps.empty(); @@ -163,11 +216,6 @@ namespace ccf::kv ccf::kv::ConsensusHookPtrs hooks; std::optional new_maps_conflict_version = std::nullopt; - // If serialisation later rejects this transaction because it exceeds the - // configured size limit, roll the store back to the state from before - // apply_changes mutates maps and advances the version. - const auto [rollback_txid, rollback_term] = - pimpl->store->current_txid_and_commit_term(); bool track_deletes_on_missing_keys = false; auto c = apply_changes( @@ -253,12 +301,6 @@ namespace ccf::kv std::move(hooks)), false); } - catch (const MaxTransactionSizeExceeded&) - { - pimpl->store->rollback(rollback_txid, rollback_term); - committed = false; - throw; - } catch (const std::exception& e) { committed = false; diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 162b58b0c0e1..fb11f9e6ad32 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -70,8 +70,7 @@ namespace ccf::kv const ccf::crypto::Sha256Hash& commit_evidence_digest_ = {}, const ccf::ClaimsDigest& claims_digest_ = ccf::no_claims(), bool historical_hint_ = false, - size_t max_transaction_size_ = - SerialisedEntryHeader::max_serialised_entry_body_size) : + size_t max_transaction_size_ = max_serialised_entry_size) : tx_id(tx_id_), entry_type(entry_type_), header_flags(header_flags_), @@ -160,6 +159,27 @@ namespace ccf::kv public_writer.get_raw_data(), private_writer.get_raw_data()); } + /** Size of the ledger entry which get_raw_data() would produce for + * everything serialised so far. + * + * This covers the whole entry: the fixed-size ledger entry header, the + * ledger encryption header, the public domain size field, and both + * domains. Encryption does not change the size of the private domain, so + * this is exact. + */ + [[nodiscard]] size_t get_serialised_size() const + { + size_t size_ = public_writer.size(); + + if (crypto_util) + { + size_ += crypto_util->get_header_length() + sizeof(size_t) + + private_writer.size(); + } + + return size_ + sizeof(SerialisedEntryHeader); + } + std::vector serialise_domains( const std::vector& serialised_public_domain, const std::vector& serialised_private_domain) override @@ -177,18 +197,23 @@ namespace ccf::kv size_ += crypto_util->get_header_length() + sizeof(size_t) + serialised_private_domain.size(); } - entry_header.set_size(size_); // The configured limit applies to the whole serialised ledger entry, // including the fixed-size entry header. - size_ += sizeof(SerialisedEntryHeader); - - if (size_ > max_transaction_size) + const size_t entry_size = size_ + sizeof(SerialisedEntryHeader); + if (entry_size > max_transaction_size) { - throw MaxTransactionSizeExceeded( - describe_serialised_entry_size_error(size_, max_transaction_size)); + // CommittableTx checks this limit before it applies its changes, so + // reaching this point means the entry cannot be written and the + // transaction cannot be undone. Throw the fatal serialisation error + // rather than one callers may try to recover from. + throw KvSerialiserException(describe_serialised_entry_size_error( + entry_size, max_transaction_size)); } + entry_header.set_size(size_); + size_ = entry_size; + std::vector entry(size_); auto* data_ = entry.data(); @@ -315,7 +340,7 @@ namespace ccf::kv size_t size, ccf::kv::Term& term, EntryFlags& flags, - bool historical_hint = false) override + bool historical_hint) override { current_reader = &public_reader; const auto* data_ = data; @@ -333,6 +358,7 @@ namespace ccf::kv tx_header.size, size_)); } + const auto* gcm_hdr_data = data_; switch (tx_header.version) diff --git a/src/kv/kv_serialiser.h b/src/kv/kv_serialiser.h index 8772969ffb08..57db53070599 100644 --- a/src/kv/kv_serialiser.h +++ b/src/kv/kv_serialiser.h @@ -8,5 +8,6 @@ namespace ccf::kv { using RawKvStoreSerialiser = GenericSerialiseWrapper; + using SizeKvStoreSerialiser = GenericSerialiseWrapper; using RawKvStoreDeserialiser = GenericDeserialiseWrapper; } diff --git a/src/kv/raw_serialise.h b/src/kv/raw_serialise.h index dd7d91ade216..1e8103eb8ce3 100644 --- a/src/kv/raw_serialise.h +++ b/src/kv/raw_serialise.h @@ -15,6 +15,43 @@ namespace ccf::kv { + /** Number of bytes RawWriter::append() will emit for this entry. + * + * Must be kept in sync with RawWriter::append(), which is checked by + * "RawWriter and SizeWriter agree" in kv_serialisation.cpp. + */ + template + size_t serialised_size(const T& entry) + { + if constexpr ( + ccf::nonstd::is_std_vector::value || + std::is_same_v) + { + return sizeof(size_t) + (entry.size() * sizeof(typename T::value_type)); + } + else if constexpr (std::is_same_v) + { + return sizeof(entry.h); + } + else if constexpr (std::is_same_v) + { + return sizeof(uint8_t); + } + else if constexpr (std::is_same_v) + { + return sizeof(size_t) + entry.size(); + } + else if constexpr (std::is_integral_v) + { + return sizeof(T); + } + else + { + static_assert( + ccf::nonstd::dependent_false::value, "Can't serialise this type"); + } + } + class RawWriter { private: @@ -122,12 +159,53 @@ namespace ccf::kv buf.clear(); } + [[nodiscard]] size_t size() const + { + return buf.size(); + } + std::vector get_raw_data() { return {buf.data(), buf.data() + buf.size()}; } }; + /** Writer which only accounts for the size of what it is given, without + * serialising it. + * + * Used to determine how large a transaction's ledger entry will be, before + * the transaction is applied to the store. + */ + class SizeWriter + { + private: + size_t total_size = 0; + + public: + SizeWriter() = default; + + template + void append(const T& entry) + { + total_size += serialised_size(entry); + } + + void clear() + { + total_size = 0; + } + + [[nodiscard]] size_t size() const + { + return total_size; + } + + static std::vector get_raw_data() + { + throw std::logic_error("SizeWriter does not retain serialised data"); + } + }; + class RawReader { private: diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index e0aef2745608..53979ea4ab97 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -47,6 +47,12 @@ namespace ccf::kv static constexpr size_t serialised_entry_header_size = sizeof(SerialisedEntryHeader); + // Largest ledger entry which can be described by a SerialisedEntryHeader, + // including that header + static constexpr size_t max_serialised_entry_size = + SerialisedEntryHeader::max_serialised_entry_body_size + + serialised_entry_header_size; + static inline std::string describe_serialised_entry_size_error( size_t entry_size, size_t max_entry_size) { diff --git a/src/kv/store.h b/src/kv/store.h index abd73c4b75ad..d8659a6127dd 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -106,8 +106,7 @@ namespace ccf::kv std::shared_ptr chunker = nullptr; EncryptorPtr encryptor = nullptr; SnapshotterPtr snapshotter = nullptr; - size_t max_transaction_size = - SerialisedEntryHeader::max_serialised_entry_body_size; + size_t max_transaction_size = max_serialised_entry_size; // Generally we will only accept deserialised views if they are contiguous - // at Version N we reject everything but N+1. The exception is when a Store @@ -259,17 +258,17 @@ namespace ccf::kv void set_max_transaction_size(size_t max_transaction_size_) { - static const auto max_allocatable_body = - std::vector().max_size() - serialised_entry_header_size; - const auto effective_max = std::min( - static_cast( - SerialisedEntryHeader::max_serialised_entry_body_size), - max_allocatable_body); + // The limit covers the whole ledger entry, so it can never usefully + // exceed the largest entry the header can describe, nor the largest + // buffer the allocator can produce + static const size_t max_allocatable = std::vector().max_size(); + const auto effective_max = + std::min(max_serialised_entry_size, max_allocatable); if (max_transaction_size_ > effective_max) { throw std::logic_error(fmt::format( "Configured maximum transaction size {} exceeds the largest " - "supported serialised transaction body size {}", + "serialisable ledger entry size {}", max_transaction_size_, effective_max)); } @@ -445,10 +444,9 @@ namespace ccf::kv std::unique_ptr snapshot) override { auto e = get_encryptor(); - // Snapshots capture the entire committed state and are legitimately much - // larger than any single transaction, so the configured - // max_transaction_size (a per-transaction write limit) is deliberately - // not applied here. + // Snapshots capture the entire committed state and are legitimately + // larger than any single transaction, so max_transaction_size does not + // apply to them. return snapshot->serialise(e); } @@ -467,9 +465,6 @@ namespace ccf::kv ccf::kv::Term term = 0; ccf::kv::EntryFlags entry_flags = {}; - // Snapshots are not subject to the per-transaction max_transaction_size - // limit (see serialise_snapshot), so it is not applied when reading one - // back either. auto v_ = d.init(data, size, term, entry_flags, is_historical); if (!v_.has_value()) { diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 007d647f367d..3fd4f9822d2e 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -327,6 +327,7 @@ TEST_CASE( MapTypes::StringString map("public:pub_map"); { + INFO("An oversized transaction is rejected before it is applied"); auto tx = kv_store.create_tx(); auto handle = tx.rw(map); handle->put("oversized", std::string(2048, 'A')); @@ -336,6 +337,7 @@ TEST_CASE( } { + INFO("Later transactions are unaffected"); auto tx = kv_store.create_tx(); auto handle = tx.rw(map); handle->put("small", "ok"); @@ -344,6 +346,61 @@ TEST_CASE( } } +TEST_CASE( + "The transaction size limit is compared against the exact entry size" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + MapTypes::StringString map("public:pub_map"); + const auto value = std::string(512, 'A'); + + // Serialise the transaction under a permissive limit to find the exact size + // of the ledger entry it produces. + size_t entry_size = 0; + { + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + entry_size = latest_data->size(); + } + + // The size projected before the transaction is applied must match the size + // of the entry which is eventually written, exactly. A limit of precisely + // that size is accepted, and one byte less is not. + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(entry_size); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(entry_size - 1); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + } +} + TEST_CASE( "Deserialisation is not subject to the transaction size limit" * doctest::test_suite("serialisation")) @@ -379,6 +436,32 @@ TEST_CASE( ccf::kv::ApplyResult::PASS); } +TEST_CASE( + "RawWriter and SizeWriter agree" * doctest::test_suite("serialisation")) +{ + const auto check = [](const auto& entry) { + ccf::kv::RawWriter raw_writer; + ccf::kv::SizeWriter size_writer; + raw_writer.append(entry); + size_writer.append(entry); + REQUIRE(raw_writer.size() == size_writer.size()); + REQUIRE(size_writer.size() == ccf::kv::serialised_size(entry)); + }; + + check(ccf::kv::EntryType::WriteSetWithCommitEvidenceAndClaims); + check(uint8_t(42)); + check(uint64_t(42)); + check(ccf::kv::Version(42)); + check(ccf::crypto::Sha256Hash(std::string("some content"))); + check(std::string()); + check(std::string("a string of some length")); + check(std::vector()); + check(std::vector(37, 'x')); + check(std::vector{1, 2, 3}); + check(ccf::kv::serialisers::SerialisedEntry()); + check(ccf::kv::serialisers::SerialisedEntry(91, 'y')); +} + TEST_CASE( "Reject configuring a maximum transaction size beyond the serialisable " "limit" * @@ -386,9 +469,9 @@ TEST_CASE( { ccf::kv::Store kv_store; - // The largest size the ledger entry header can represent is accepted. - REQUIRE_NOTHROW(kv_store.set_max_transaction_size( - ccf::kv::SerialisedEntryHeader::max_serialised_entry_body_size)); + // The largest entry the ledger entry header can describe is accepted. + REQUIRE_NOTHROW( + kv_store.set_max_transaction_size(ccf::kv::max_serialised_entry_size)); // A value larger than can ever be serialised is rejected at configuration // time, rather than being deferred to a later serialisation failure. diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index d12e4041f22c..0c98e69c89fa 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -1485,12 +1485,10 @@ namespace ccf::historical false /* Do not start from very first seqno */, true /* Make use of historical secrets */); - // The configured max_transaction_size limit is deliberately not set on - // this store. That limit is a write-time guard preventing oversized - // entries from being serialised into the ledger; historical queries only - // reconstruct already-committed state to serve reads, and must remain - // able to deserialise any entry that was validly written, including one - // written under a previously larger or unset limit. + // max_transaction_size is deliberately not set on this store. It is a + // write-time limit, and historical queries must remain able to + // reconstruct any entry which was validly written, including under a + // previously larger or unset limit. // If this is older than the node's currently known ledger secrets, use // the historical encryptor (which should have older secrets) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index bd7f57735260..d1e86618a8cc 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -1019,10 +1019,12 @@ namespace ccf } catch (const ccf::kv::MaxTransactionSizeExceeded& e) { + // Thrown before the transaction is applied, so the store is + // unchanged and later transactions are unaffected ctx->clear_response_headers(); ctx->set_error( HTTP_STATUS_PAYLOAD_TOO_LARGE, - ccf::errors::RequestBodyTooLarge, + ccf::errors::TransactionTooLarge, e.what()); return; diff --git a/tests/limits.py b/tests/limits.py index f001a37b6026..536f9046ceab 100644 --- a/tests/limits.py +++ b/tests/limits.py @@ -10,6 +10,7 @@ import infra.jwt_issuer import infra.network import infra.proc +import suite.test_requirements as reqs from infra.runner import ConcurrentRunner @@ -63,6 +64,8 @@ def get_request_payload_too_large_errors(): assert r.status_code == http.HTTPStatus.OK.value, r +@reqs.description("Transactions larger than ledger.max_transaction_size are rejected") +@reqs.supports_methods("/app/log/private") def test_transaction_size_limit(network, args): primary, _ = network.find_primary() @@ -70,13 +73,27 @@ def test_transaction_size_limit(network, args): r = c.post("/app/log/private", {"id": 1, "msg": "small"}) assert r.status_code == http.HTTPStatus.OK.value, r - msg = "A" * 1024 * 1024 + # Comfortably under the interface's max_http_body_size, so this is + # rejected by the ledger transaction size limit rather than by the HTTP + # request parser, which reports RequestBodyTooLarge and closes the + # session. + msg = "A" * 600 * 1024 r = c.post("/app/log/private", {"id": 2, "msg": msg}) assert r.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value, r + assert r.body.json()["error"]["code"] == "TransactionTooLarge", r + + # The rejected transaction was never applied, so it neither wrote a + # value nor prevented later transactions from being processed + r = c.get("/app/log/private?id=2") + assert r.status_code == http.HTTPStatus.NOT_FOUND.value, r r = c.post("/app/log/private", {"id": 3, "msg": "still processing"}) assert r.status_code == http.HTTPStatus.OK.value, r + r = c.get("/app/log/private?id=3") + assert r.status_code == http.HTTPStatus.OK.value, r + assert r.body.json()["msg"] == "still processing", r + def run_parser_limits_checks(args): new_args = copy.copy(args) @@ -97,9 +114,9 @@ def run_parser_limits_checks(args): def run_transaction_size_limit_checks(args): new_args = copy.copy(args) - # Deliberately larger than the constitution scripts written to the KV - # store as part of service creation, but well under the oversized - # request used below, so only the latter is rejected. + # Larger than the constitution scripts written to the KV store as part of + # service creation, and than any governance transaction, but smaller than + # the oversized request used by the test new_args.ledger_max_transaction_bytes = "512KB" with infra.network.network( new_args.nodes, From e1dc2e3e7af8d6240b00e49a99057f32e80f308c Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Aug 2026 12:46:11 +0100 Subject: [PATCH 08/15] Avoid duplicate transaction serialisation Serialise transaction domains once before applying changes, reuse the retained bytes after assigning the transaction version, and exempt reserved signature transactions from the configurable cap. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- doc/host_config_schema/host_config.json | 2 +- src/kv/committable_tx.h | 153 ++++++++++++------------ src/kv/generic_serialise_wrapper.h | 58 ++++++++- src/kv/kv_serialiser.h | 1 - src/kv/raw_serialise.h | 134 ++++++++++----------- src/kv/test/kv_serialisation.cpp | 49 ++++++-- tests/infra/e2e_args.py | 5 +- 8 files changed, 240 insertions(+), 164 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2744319493ec..d33a4595fc88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). -- New `ledger.max_transaction_size` node configuration option (default `100MB`), which caps the total serialised size of a transaction written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. It applies only to newly serialised transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable (#7992). +- New `ledger.max_transaction_size` node configuration option (default `100MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable (#7992). ### Fixed diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 80ba2893afc2..f2e1ca2220a4 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -481,7 +481,7 @@ "max_transaction_size": { "type": "string", "default": "100MB", - "description": "Maximum total serialised size (size string) of a transaction written to the ledger. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain" + "description": "Maximum total serialised size (size string) of a transaction written to the ledger. Reserved internal signature transactions are exempt. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain" } }, "description": "This section includes configuration for the ledger directories and files", diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index 9d62a8c27348..d4545bec5c8a 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -36,35 +36,6 @@ namespace ccf::kv TxFlags flags = 0; SerialisedEntryFlags entry_flags = 0; - // Serialises the same changes as serialise(), into a serialiser which only - // measures their size, to determine how large this transaction's ledger - // entry will be. Change sets are not modified by apply_changes(), so this - // can be called before the transaction is applied to the store, and the - // result is exactly the size serialise() will produce. - size_t projected_serialised_size( - const ccf::ClaimsDigest& claims_digest_, bool include_reads = false) - { - auto e = pimpl->store->get_encryptor(); - if (e == nullptr) - { - throw KvSerialiserException("No encryptor set"); - } - - SizeKvStoreSerialiser size_serialiser( - e, - {pimpl->commit_view, NoVersion}, - EntryType::WriteSetWithCommitEvidenceAndClaims, - entry_flags, - // Both digests are fixed-size, so their values do not affect the - // size of the resulting entry - ccf::crypto::Sha256Hash{}, - claims_digest_); - - serialise_all_changes(size_serialiser, include_reads); - - return size_serialiser.get_serialised_size(); - } - void serialise_all_changes( KvStoreSerialiser& serialiser, bool include_reads) { @@ -91,31 +62,56 @@ namespace ccf::kv }); } - std::vector serialise( - ccf::crypto::Sha256Hash& commit_evidence_digest, - std::string& commit_evidence, + std::unique_ptr prepare_serialisation( const ccf::ClaimsDigest& claims_digest_, + size_t max_transaction_size, bool include_reads = false) { - if (!committed) + if (claims_digest_.empty()) { - throw std::logic_error("Transaction not yet committed"); + throw std::logic_error("Missing claims"); } - if (!success) + auto e = pimpl->store->get_encryptor(); + if (e == nullptr) { - throw std::logic_error("Transaction aborted"); + throw KvSerialiserException("No encryptor set"); } - if (claims_digest_.empty()) + if (tx_flag_enabled(TxFlag::LEDGER_CHUNK_BEFORE_THIS_TX)) { - throw std::logic_error("Missing claims"); + entry_flags |= EntryFlags::FORCE_LEDGER_CHUNK_BEFORE; } - // If no transactions made changes, return a zero length vector. - if (!has_writes()) + auto serialiser = std::make_unique( + e, + TxID{pimpl->commit_view, NoVersion}, + EntryType::WriteSetWithCommitEvidenceAndClaims, + entry_flags, + ccf::crypto::Sha256Hash{}, + claims_digest_, + false /* historical_hint */, + max_transaction_size, + true /* enforce_max_transaction_size */); + + serialise_all_changes(*serialiser, include_reads); + + return serialiser; + } + + std::vector finalise_serialisation( + RawKvStoreSerialiser& serialiser, + ccf::crypto::Sha256Hash& commit_evidence_digest, + std::string& commit_evidence) + { + if (!committed) { - return {}; + throw std::logic_error("Transaction not yet committed"); + } + + if (!success) + { + throw std::logic_error("Transaction aborted"); } auto e = pimpl->store->get_encryptor(); @@ -128,27 +124,29 @@ namespace ccf::kv LOG_TRACE_FMT("Commit evidence: {}", commit_evidence); ccf::crypto::Sha256Hash tx_commit_evidence_digest(commit_evidence); commit_evidence_digest = tx_commit_evidence_digest; - auto entry_type = EntryType::WriteSetWithCommitEvidenceAndClaims; + serialiser.set_tx_id({pimpl->commit_view, version}); + serialiser.set_commit_evidence_digest(tx_commit_evidence_digest); - if (tx_flag_enabled(TxFlag::LEDGER_CHUNK_BEFORE_THIS_TX)) + // Return serialised Tx. + return serialiser.get_raw_data(); + } + + std::vector serialise( + ccf::crypto::Sha256Hash& commit_evidence_digest, + std::string& commit_evidence, + const ccf::ClaimsDigest& claims_digest_, + size_t max_transaction_size, + bool include_reads = false) + { + if (!has_writes()) { - entry_flags |= EntryFlags::FORCE_LEDGER_CHUNK_BEFORE; + return {}; } - RawKvStoreSerialiser replicated_serialiser( - e, - {pimpl->commit_view, version}, - entry_type, - entry_flags, - tx_commit_evidence_digest, - claims_digest_, - false /* historical_hint */, - pimpl->store->get_max_transaction_size()); - - serialise_all_changes(replicated_serialiser, include_reads); - - // Return serialised Tx. - return replicated_serialiser.get_raw_data(); + auto serialiser = prepare_serialisation( + claims_digest_, max_transaction_size, include_reads); + return finalise_serialisation( + *serialiser, commit_evidence_digest, commit_evidence); } public: @@ -189,20 +187,16 @@ namespace ccf::kv return CommitResult::SUCCESS; } - // Reject transactions whose ledger entry would exceed the configured - // limit before any change is applied. Once apply_changes() below has - // taken a version and mutated the maps, the transaction can no longer be - // abandoned without losing the entry at that version. + std::unique_ptr replicated_serialiser; + + // Serialise the write set and reject oversized entries before any change + // is applied. The retained domain buffers are patched with the assigned + // version and commit evidence after apply_changes(), then encrypted and + // packaged without walking the write set again. if (has_writes()) { - const auto max_transaction_size = - pimpl->store->get_max_transaction_size(); - const auto entry_size = projected_serialised_size(claims); - if (entry_size > max_transaction_size) - { - throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( - entry_size, max_transaction_size)); - } + replicated_serialiser = prepare_serialisation( + claims, pimpl->store->get_max_transaction_size()); } // If this transaction creates any maps, ensure that commit gets a @@ -277,12 +271,14 @@ namespace ccf::kv { ccf::crypto::Sha256Hash commit_evidence_digest; std::string commit_evidence; - auto data = serialise(commit_evidence_digest, commit_evidence, claims); - - if (data.empty()) + if (replicated_serialiser == nullptr) { - return CommitResult::SUCCESS; + throw std::logic_error( + "Missing serialised write set for committed transaction"); } + auto data = finalise_serialisation( + *replicated_serialiser, commit_evidence_digest, commit_evidence); + replicated_serialiser.reset(); if (write_set_observer != nullptr) { @@ -500,7 +496,14 @@ namespace ccf::kv committed = true; auto claims = ccf::empty_claims(); - auto data = serialise(commit_evidence_digest, commit_evidence, claims); + // Reserved transactions are used solely for signatures. They must always + // fill their reserved version, so the operator-configured transaction + // size limit does not apply to them. + auto data = serialise( + commit_evidence_digest, + commit_evidence, + claims, + max_serialised_entry_size); return { CommitResult::SUCCESS, diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index fb11f9e6ad32..b124c54d7969 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -25,6 +25,10 @@ namespace ccf::kv EntryType entry_type; SerialisedEntryFlags header_flags; size_t max_transaction_size; + size_t version_offset = 0; + std::optional commit_evidence_digest_offset = std::nullopt; + bool enforce_max_transaction_size; + bool finalised = false; std::shared_ptr crypto_util; @@ -37,6 +41,21 @@ namespace ccf::kv template void serialise_internal(const T& t) { + if (enforce_max_transaction_size) + { + const auto current_size = get_serialised_size(); + const auto additional_size = W::serialised_size(t); + if ( + current_size > max_transaction_size || + additional_size > max_transaction_size - current_size) + { + throw MaxTransactionSizeExceeded(fmt::format( + "Cannot serialise transaction because its serialised size exceeds " + "the configured maximum of {} bytes", + max_transaction_size)); + } + } + current_writer->append(t); } @@ -70,16 +89,19 @@ namespace ccf::kv const ccf::crypto::Sha256Hash& commit_evidence_digest_ = {}, const ccf::ClaimsDigest& claims_digest_ = ccf::no_claims(), bool historical_hint_ = false, - size_t max_transaction_size_ = max_serialised_entry_size) : + size_t max_transaction_size_ = max_serialised_entry_size, + bool enforce_max_transaction_size_ = false) : tx_id(tx_id_), entry_type(entry_type_), header_flags(header_flags_), max_transaction_size(max_transaction_size_), + enforce_max_transaction_size(enforce_max_transaction_size_), crypto_util(std::move(e)), historical_hint(historical_hint_) { set_current_domain(SecurityDomain::PUBLIC); serialise_internal(entry_type); + version_offset = public_writer.size(); serialise_internal(tx_id.seqno); if (has_claims(entry_type)) { @@ -87,6 +109,7 @@ namespace ccf::kv } if (has_commit_evidence(entry_type)) { + commit_evidence_digest_offset = public_writer.size(); serialise_internal(commit_evidence_digest_); } // Write a placeholder max_conflict_version for compatibility @@ -148,8 +171,33 @@ namespace ccf::kv serialise_internal(k); } + void set_tx_id(const TxID& tx_id_) + { + tx_id = tx_id_; + public_writer.overwrite(version_offset, tx_id.seqno); + } + + void set_commit_evidence_digest( + const ccf::crypto::Sha256Hash& commit_evidence_digest) + { + if (!commit_evidence_digest_offset.has_value()) + { + throw std::logic_error( + "Cannot set commit evidence digest on entry without commit evidence"); + } + + public_writer.overwrite( + commit_evidence_digest_offset.value(), commit_evidence_digest); + } + std::vector get_raw_data() override { + if (finalised) + { + throw std::logic_error("Serialiser has already been finalised"); + } + finalised = true; + // make sure the private buffer is empty when we return auto writer_guard_func = [](W* writer) { writer->clear(); }; std::unique_ptr @@ -203,10 +251,10 @@ namespace ccf::kv const size_t entry_size = size_ + sizeof(SerialisedEntryHeader); if (entry_size > max_transaction_size) { - // CommittableTx checks this limit before it applies its changes, so - // reaching this point means the entry cannot be written and the - // transaction cannot be undone. Throw the fatal serialisation error - // rather than one callers may try to recover from. + // Non-reserved transactions check this exact size before applying + // their changes. Reserved signature transactions are exempt from the + // configured limit and use the largest representable entry size here. + // Reaching this point is therefore always a fatal serialisation error. throw KvSerialiserException(describe_serialised_entry_size_error( entry_size, max_transaction_size)); } diff --git a/src/kv/kv_serialiser.h b/src/kv/kv_serialiser.h index 57db53070599..8772969ffb08 100644 --- a/src/kv/kv_serialiser.h +++ b/src/kv/kv_serialiser.h @@ -8,6 +8,5 @@ namespace ccf::kv { using RawKvStoreSerialiser = GenericSerialiseWrapper; - using SizeKvStoreSerialiser = GenericSerialiseWrapper; using RawKvStoreDeserialiser = GenericDeserialiseWrapper; } diff --git a/src/kv/raw_serialise.h b/src/kv/raw_serialise.h index 1e8103eb8ce3..0e3012b81634 100644 --- a/src/kv/raw_serialise.h +++ b/src/kv/raw_serialise.h @@ -15,43 +15,6 @@ namespace ccf::kv { - /** Number of bytes RawWriter::append() will emit for this entry. - * - * Must be kept in sync with RawWriter::append(), which is checked by - * "RawWriter and SizeWriter agree" in kv_serialisation.cpp. - */ - template - size_t serialised_size(const T& entry) - { - if constexpr ( - ccf::nonstd::is_std_vector::value || - std::is_same_v) - { - return sizeof(size_t) + (entry.size() * sizeof(typename T::value_type)); - } - else if constexpr (std::is_same_v) - { - return sizeof(entry.h); - } - else if constexpr (std::is_same_v) - { - return sizeof(uint8_t); - } - else if constexpr (std::is_same_v) - { - return sizeof(size_t) + entry.size(); - } - else if constexpr (std::is_integral_v) - { - return sizeof(T); - } - else - { - static_assert( - ccf::nonstd::dependent_false::value, "Can't serialise this type"); - } - } - class RawWriter { private: @@ -118,6 +81,38 @@ namespace ccf::kv public: RawWriter() = default; + template + static size_t serialised_size(const T& entry) + { + if constexpr ( + ccf::nonstd::is_std_vector::value || + std::is_same_v) + { + return sizeof(size_t) + (entry.size() * sizeof(typename T::value_type)); + } + else if constexpr (std::is_same_v) + { + return sizeof(entry.h); + } + else if constexpr (std::is_same_v) + { + return sizeof(uint8_t); + } + else if constexpr (std::is_same_v) + { + return sizeof(size_t) + entry.size(); + } + else if constexpr (std::is_integral_v) + { + return sizeof(T); + } + else + { + static_assert( + ccf::nonstd::dependent_false::value, "Can't serialise this type"); + } + } + template void append(const T& entry) { @@ -164,45 +159,46 @@ namespace ccf::kv return buf.size(); } - std::vector get_raw_data() - { - return {buf.data(), buf.data() + buf.size()}; - } - }; - - /** Writer which only accounts for the size of what it is given, without - * serialising it. - * - * Used to determine how large a transaction's ledger entry will be, before - * the transaction is applied to the store. - */ - class SizeWriter - { - private: - size_t total_size = 0; - - public: - SizeWriter() = default; - template - void append(const T& entry) + void overwrite(size_t offset, const T& entry) { - total_size += serialised_size(entry); - } + if constexpr (std::is_same_v) + { + if (offset > buf.size() || sizeof(entry.h) > buf.size() - offset) + { + throw std::logic_error("Cannot overwrite outside serialised data"); + } - void clear() - { - total_size = 0; - } + auto* data_ = buf.data() + offset; + auto size_ = buf.size() - offset; + serialized::write( + data_, + size_, + reinterpret_cast(entry.h.data()), + sizeof(entry.h)); + } + else if constexpr (std::is_integral_v) + { + if (offset > buf.size() || sizeof(T) > buf.size() - offset) + { + throw std::logic_error("Cannot overwrite outside serialised data"); + } - [[nodiscard]] size_t size() const - { - return total_size; + auto* data_ = buf.data() + offset; + auto size_ = buf.size() - offset; + serialized::write(data_, size_, entry); + } + else + { + static_assert( + ccf::nonstd::dependent_false::value, + "Can't overwrite this serialised type"); + } } - static std::vector get_raw_data() + std::vector get_raw_data() { - throw std::logic_error("SizeWriter does not retain serialised data"); + return {buf.data(), buf.data() + buf.size()}; } }; diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 3fd4f9822d2e..aed79784a351 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -374,9 +374,9 @@ TEST_CASE( entry_size = latest_data->size(); } - // The size projected before the transaction is applied must match the size - // of the entry which is eventually written, exactly. A limit of precisely - // that size is accepted, and one byte less is not. + // The retained serialised domains must report the exact size of the entry + // which is eventually written. A limit of precisely that size is accepted, + // and one byte less is not. { ccf::kv::Store kv_store; kv_store.set_encryptor(encryptor); @@ -437,15 +437,14 @@ TEST_CASE( } TEST_CASE( - "RawWriter and SizeWriter agree" * doctest::test_suite("serialisation")) + "RawWriter append size estimates are exact" * + doctest::test_suite("serialisation")) { const auto check = [](const auto& entry) { - ccf::kv::RawWriter raw_writer; - ccf::kv::SizeWriter size_writer; - raw_writer.append(entry); - size_writer.append(entry); - REQUIRE(raw_writer.size() == size_writer.size()); - REQUIRE(size_writer.size() == ccf::kv::serialised_size(entry)); + ccf::kv::RawWriter writer; + const auto expected_size = ccf::kv::RawWriter::serialised_size(entry); + writer.append(entry); + REQUIRE(writer.size() == expected_size); }; check(ccf::kv::EntryType::WriteSetWithCommitEvidenceAndClaims); @@ -462,6 +461,26 @@ TEST_CASE( check(ccf::kv::serialisers::SerialisedEntry(91, 'y')); } +TEST_CASE( + "Reserved signature transactions ignore the configured transaction size " + "limit" * + doctest::test_suite("serialisation")) +{ + ccf::kv::Store kv_store; + auto encryptor = std::make_shared(); + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(1); + + MapTypes::StringString map("public:signature"); + auto tx = kv_store.create_reserved_tx(kv_store.next_txid()); + tx.rw(map)->put("signature", std::string(512, 'A')); + + const auto [result, data, claims, commit_evidence, hooks] = + tx.commit_reserved(); + REQUIRE(result == ccf::kv::CommitResult::SUCCESS); + REQUIRE(data.size() > kv_store.get_max_transaction_size()); +} + TEST_CASE( "Reject configuring a maximum transaction size beyond the serialisable " "limit" * @@ -990,6 +1009,7 @@ TEST_CASE( ccf::ClaimsDigest claims_digest; claims_digest.set(ccf::crypto::Sha256Hash("claim text")); + ccf::crypto::Sha256Hash expected_commit_evidence_digest; INFO("Commit to source store, including claims"); { @@ -1001,9 +1021,11 @@ TEST_CASE( handle_pub->put("pubk1", "pubv1"); REQUIRE(tx.commit(claims_digest) == ccf::kv::CommitResult::SUCCESS); + expected_commit_evidence_digest = ccf::crypto::Sha256Hash( + encryptor->get_commit_evidence({tx.commit_term(), tx.commit_version()})); } - INFO("Deserialise transaction in target store and extract claims"); + INFO("Deserialise transaction in target store and extract digests"); { const auto latest_data = consensus->get_latest_data(); REQUIRE(latest_data.has_value()); @@ -1011,6 +1033,11 @@ TEST_CASE( REQUIRE(wrapper->apply() != ccf::kv::ApplyResult::FAIL); auto deserialised_claims = wrapper->consume_claims_digest(); REQUIRE(claims_digest == deserialised_claims); + auto deserialised_commit_evidence = + wrapper->consume_commit_evidence_digest(); + REQUIRE(deserialised_commit_evidence.has_value()); + REQUIRE( + deserialised_commit_evidence.value() == expected_commit_evidence_digest); auto tx_target = kv_store_target.create_tx(); auto handle_priv = tx_target.rw(priv_map); diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 711aa88e9879..98b06d1035b1 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -288,7 +288,10 @@ def cli_args( ) parser.add_argument( "--ledger-max-transaction-bytes", - help="Maximum serialised transaction body size (bytes)", + help=( + "Maximum total serialised ledger entry size, including its header " + "(size string)" + ), type=str, default="100MB", ) From 99351f5f8ebd462b51c40f39bf951aac80804700 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Aug 2026 15:20:16 +0100 Subject: [PATCH 09/15] Test encrypted transaction size boundary Cover exact size enforcement with a real encryption header and mixed public/private writes, and satisfy clang-tidy by initializing the serialised version offset in the constructor initializer list. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/kv/generic_serialise_wrapper.h | 2 +- src/kv/test/kv_serialisation.cpp | 58 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index b124c54d7969..7e990b77c7da 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -95,13 +95,13 @@ namespace ccf::kv entry_type(entry_type_), header_flags(header_flags_), max_transaction_size(max_transaction_size_), + version_offset(W::serialised_size(entry_type_)), enforce_max_transaction_size(enforce_max_transaction_size_), crypto_util(std::move(e)), historical_hint(historical_hint_) { set_current_domain(SecurityDomain::PUBLIC); serialise_internal(entry_type); - version_offset = public_writer.size(); serialise_internal(tx_id.seqno); if (has_claims(entry_type)) { diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index aed79784a351..615f9142306a 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -6,6 +6,7 @@ #include "kv/store.h" #include "kv/test/null_encryptor.h" #include "kv/test/stub_consensus.h" +#include "node/encryptor.h" #include #undef FAIL @@ -401,6 +402,63 @@ TEST_CASE( } } +TEST_CASE( + "The transaction size limit includes encrypted private data" * + doctest::test_suite("serialisation")) +{ + const auto create_encryptor = []() { + auto secrets = std::make_shared(); + secrets->init(); + return std::make_shared(secrets); + }; + + MapTypes::StringString public_map("public:pub_map"); + MapTypes::StringString private_map("priv_map"); + const auto value = std::string(512, 'A'); + + const auto populate = [&](ccf::kv::CommittableTx& tx) { + tx.rw(public_map)->put("public_key", value); + tx.rw(private_map)->put("private_key", value); + }; + + size_t entry_size = 0; + { + auto consensus = std::make_shared(); + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(create_encryptor()); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + entry_size = latest_data->size(); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(create_encryptor()); + kv_store.set_max_transaction_size(entry_size); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(create_encryptor()); + kv_store.set_max_transaction_size(entry_size - 1); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + } +} + TEST_CASE( "Deserialisation is not subject to the transaction size limit" * doctest::test_suite("serialisation")) From d9856e9d7ed433bef74b02e6c3a6dd5ebce26a05 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Aug 2026 16:26:51 +0100 Subject: [PATCH 10/15] Validate ledger entry and size string bounds Reject truncated ledger headers and bodies consistently on read and skip paths, NACK malformed duplicate entries, and use checked integer scaling for size strings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- include/ccf/ds/unit_strings.h | 23 +++++++++++++- src/consensus/aft/raft.h | 17 +++++++++- src/consensus/aft/test/enclave.cpp | 37 ++++++++++++++++++++++ src/consensus/ledger_enclave.h | 51 ++++++++++++++++++++---------- src/ds/test/unit_strings.cpp | 14 ++++++++ 5 files changed, 123 insertions(+), 19 deletions(-) diff --git a/include/ccf/ds/unit_strings.h b/include/ccf/ds/unit_strings.h index db0b7786813d..ee0919a0b7cb 100644 --- a/include/ccf/ds/unit_strings.h +++ b/include/ccf/ds/unit_strings.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -81,7 +82,27 @@ namespace ccf::ds return UnitStringConverter::convert( input, size_suffix_to_power, [](size_t value, size_t power) { - return value * std::pow(1024, power); + constexpr size_t base = 1024; + size_t factor = 1; + for (size_t i = 0; i < power; ++i) + { + if (factor > std::numeric_limits::max() / base) + { + throw std::logic_error("Size string unit multiplier is too large"); + } + factor *= base; + } + + if (value > std::numeric_limits::max() / factor) + { + throw std::logic_error(fmt::format( + "Size string value {} with multiplier {} exceeds the largest " + "representable size", + value, + factor)); + } + + return value * factor; }); } diff --git a/src/consensus/aft/raft.h b/src/consensus/aft/raft.h index 286a3e0ebb8c..3e82569961fc 100644 --- a/src/consensus/aft/raft.h +++ b/src/consensus/aft/raft.h @@ -1307,7 +1307,22 @@ namespace aft { // If the current entry has already been deserialised, skip the // payload for that entry - ledger->skip_entry(data, size); + try + { + ledger->skip_entry(data, size); + } + catch (const std::logic_error& e) + { + // This should only fail if there is malformed data. + RAFT_FAIL_FMT( + "Recv {} to {} from {} but the data is malformed: {}", + r.msg, + state->node_id, + from, + e.what()); + send_append_entries_response_nack(from); + return; + } continue; } } diff --git a/src/consensus/aft/test/enclave.cpp b/src/consensus/aft/test/enclave.cpp index dee7e1f3c672..590ded913bca 100644 --- a/src/consensus/aft/test/enclave.cpp +++ b/src/consensus/aft/test/enclave.cpp @@ -13,6 +13,43 @@ using namespace consensus; using WFactory = ringbuffer::WriterFactory; +TEST_CASE("Enclave rejects malformed entries") +{ + const auto check_rejected = [](const std::vector& entry) { + { + const auto* data = entry.data(); + auto size = entry.size(); + REQUIRE_THROWS_AS(LedgerEnclave::get_entry(data, size), std::logic_error); + } + + { + const auto* data = entry.data(); + auto size = entry.size(); + REQUIRE_THROWS_AS( + LedgerEnclave::skip_entry(data, size), std::logic_error); + } + }; + + SUBCASE("Truncated header") + { + check_rejected( + std::vector(ccf::kv::serialised_entry_header_size - 1)); + } + + SUBCASE("Claimed body exceeds buffer") + { + ccf::kv::SerialisedEntryHeader header; + header.set_size(2); + + std::vector entry(ccf::kv::serialised_entry_header_size + 1); + auto* data = entry.data(); + auto size = entry.size(); + serialized::write(data, size, header); + + check_rejected(entry); + } +} + TEST_CASE("Enclave put") { constexpr auto buffer_size = 1024; diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 458b9c1a2f69..c8e908196807 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -14,36 +14,55 @@ namespace consensus { class LedgerEnclave { - public: - /** - * Retrieve a single entry, advancing offset to the next entry. - * - * @param data Serialised entries - * @param size Size of overall serialised entries - * - * @return Raw entry as a vector - */ - static std::vector get_entry(const uint8_t*& data, size_t& size) + private: + static size_t get_entry_size(const uint8_t* data, size_t size) { - auto header = + if (size < ccf::kv::serialised_entry_header_size) + { + throw std::logic_error(fmt::format( + "Cannot read transaction header: buffer contains {} bytes, but the " + "fixed ledger entry header requires {} bytes", + size, + ccf::kv::serialised_entry_header_size)); + } + + const auto header = serialized::peek(data, size); const size_t body_size = header.size; + const auto available_body_size = + size - ccf::kv::serialised_entry_header_size; + // The size in the entry header is not trusted: check it against the // buffer we were given before allocating. This is distinct from the // configured max_transaction_size, which applies only when serialising // new transactions, so that entries written under a larger or unset // limit can always be read back. - if (body_size + ccf::kv::serialised_entry_header_size > size) + if (body_size > available_body_size) { throw std::logic_error(fmt::format( "Cannot read transaction with serialised body size {} bytes from " "buffer containing {} bytes after the fixed {}-byte ledger entry " "header", body_size, - size - ccf::kv::serialised_entry_header_size, + available_body_size, ccf::kv::serialised_entry_header_size)); } - size_t entry_size = ccf::kv::serialised_entry_header_size + body_size; + + return ccf::kv::serialised_entry_header_size + body_size; + } + + public: + /** + * Retrieve a single entry, advancing offset to the next entry. + * + * @param data Serialised entries + * @param size Size of overall serialised entries + * + * @return Raw entry as a vector + */ + static std::vector get_entry(const uint8_t*& data, size_t& size) + { + const auto entry_size = get_entry_size(data, size); std::vector entry(data, data + entry_size); serialized::skip(data, size, entry_size); return entry; @@ -107,9 +126,7 @@ namespace consensus */ static void skip_entry(const uint8_t*& data, size_t& size) { - auto header = - serialized::read(data, size); - serialized::skip(data, size, header.size); + serialized::skip(data, size, get_entry_size(data, size)); } /** diff --git a/src/ds/test/unit_strings.cpp b/src/ds/test/unit_strings.cpp index 1488fe3e0afb..a73d041e6b8c 100644 --- a/src/ds/test/unit_strings.cpp +++ b/src/ds/test/unit_strings.cpp @@ -5,6 +5,7 @@ #include #include +#include using namespace ccf::ds; @@ -29,6 +30,19 @@ TEST_CASE("Size strings" * doctest::test_suite("unit strings")) REQUIRE(convert_size_string("3GB") == 3 * std::pow(1024, 3)); REQUIRE(convert_size_string("3TB") == 3 * std::pow(1024, 4)); REQUIRE(convert_size_string("3PB") == 3 * std::pow(1024, 5)); + + const auto max_size = std::numeric_limits::max(); + REQUIRE(convert_size_string(std::to_string(max_size) + "B") == max_size); + + const auto max_kb_value = max_size / 1024; + REQUIRE( + convert_size_string(std::to_string(max_kb_value) + "KB") == + max_kb_value * 1024); + REQUIRE_THROWS_AS( + convert_size_string(std::to_string(max_kb_value + 1) + "KB"), + std::logic_error); + REQUIRE_THROWS_AS( + convert_size_string(std::to_string(max_size) + "PB"), std::logic_error); } TEST_CASE("Time strings" * doctest::test_suite("unit strings")) From c41d26d7e1692bd254f398abecc6a0a417563f61 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Aug 2026 17:46:50 +0100 Subject: [PATCH 11/15] Use allocation-free transaction sizing Measure transaction entries without materialising bytes before conflict detection, serialise only successful writes, align the transaction default to 64MB, and preserve ring-buffer response headroom with a 65MB message default and startup validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- doc/host_config_schema/host_config.json | 2 +- include/ccf/node/startup_config.h | 2 +- samples/config/join_config.json | 2 +- samples/config/recover_config.json | 2 +- samples/config/start_config.json | 2 +- src/consensus/ledger_enclave_types.h | 2 + src/host/configuration.h | 2 +- src/host/ledger.h | 3 +- src/host/run.cpp | 24 +++++ src/kv/committable_tx.h | 113 +++++++++++++----------- src/kv/generic_serialise_wrapper.h | 57 +----------- src/kv/kv_serialiser.h | 1 + src/kv/raw_serialise.h | 57 ++++++------ src/kv/test/kv_serialisation.cpp | 14 +-- tests/e2e_batched.py | 1 + tests/e2e_operations.py | 1 + tests/infra/e2e_args.py | 4 +- 18 files changed, 136 insertions(+), 155 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d33a4595fc88..0ddc259b8cef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). -- New `ledger.max_transaction_size` node configuration option (default `100MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable (#7992). +- New `ledger.max_transaction_size` node configuration option (default `64MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. The default `memory.max_msg_size` is now `65MB`, leaving sufficient ring-buffer response headroom for a maximum-sized transaction (#7992). ### Fixed diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index f2e1ca2220a4..6f74a6f4b338 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -480,7 +480,7 @@ }, "max_transaction_size": { "type": "string", - "default": "100MB", + "default": "65MB", "description": "Maximum total serialised size (size string) of a transaction written to the ledger. Reserved internal signature transactions are exempt. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain" } }, diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index a0a56c376414..bbbed1c45a09 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -48,7 +48,7 @@ namespace ccf std::string directory = "ledger"; std::vector read_only_directories; ccf::ds::SizeString chunk_size = {"5MB"}; - ccf::ds::SizeString max_transaction_size = {"100MB"}; + ccf::ds::SizeString max_transaction_size = {"64MB"}; bool operator==(const Ledger&) const = default; }; diff --git a/samples/config/join_config.json b/samples/config/join_config.json index c7fc6266d06f..96dcd8cfe552 100644 --- a/samples/config/join_config.json +++ b/samples/config/join_config.json @@ -60,7 +60,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "64MB", + "max_msg_size": "65MB", "max_fragment_size": "256KB" } } diff --git a/samples/config/recover_config.json b/samples/config/recover_config.json index cf757419e88c..81fa04c95135 100644 --- a/samples/config/recover_config.json +++ b/samples/config/recover_config.json @@ -60,7 +60,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "64MB", + "max_msg_size": "65MB", "max_fragment_size": "256KB" } } diff --git a/samples/config/start_config.json b/samples/config/start_config.json index 30145a278241..9af915c2f59f 100644 --- a/samples/config/start_config.json +++ b/samples/config/start_config.json @@ -89,7 +89,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "64MB", + "max_msg_size": "65MB", "max_fragment_size": "256KB" } } diff --git a/src/consensus/ledger_enclave_types.h b/src/consensus/ledger_enclave_types.h index 6464092de516..73426d12bf6d 100644 --- a/src/consensus/ledger_enclave_types.h +++ b/src/consensus/ledger_enclave_types.h @@ -8,6 +8,8 @@ namespace consensus { + static constexpr size_t ledger_range_response_metadata_size = 2048; + using Index = uint64_t; enum LedgerRequestPurpose : uint8_t diff --git a/src/host/configuration.h b/src/host/configuration.h index e19e5bcdfad8..a1c73701b63d 100644 --- a/src/host/configuration.h +++ b/src/host/configuration.h @@ -75,7 +75,7 @@ namespace host struct Memory { ccf::ds::SizeString circuit_size = {"16MB"}; - ccf::ds::SizeString max_msg_size = {"64MB"}; + ccf::ds::SizeString max_msg_size = {"65MB"}; ccf::ds::SizeString max_fragment_size = {"256KB"}; bool operator==(const Memory&) const = default; diff --git a/src/host/ledger.h b/src/host/ledger.h index dde825ab6cc5..6cddfbca63fd 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -1885,9 +1885,8 @@ namespace asynchost // Ledger entries response has metadata so cap total entries size // accordingly - constexpr size_t write_ledger_range_response_metadata_size = 2048; auto max_entries_size = to_enclave->get_max_message_size() - - write_ledger_range_response_metadata_size; + ::consensus::ledger_range_response_metadata_size; if (is_in_committed_file(to_idx)) { diff --git a/src/host/run.cpp b/src/host/run.cpp index 68eb671002a8..0a06448cfcd7 100644 --- a/src/host/run.cpp +++ b/src/host/run.cpp @@ -21,6 +21,7 @@ #include "common/enclave_interface_types.h" #include "config_schema.h" #include "configuration.h" +#include "consensus/ledger_enclave_types.h" #include "crypto/openssl/hash.h" #include "ds/files.h" #include "ds/internal_logger.h" @@ -96,6 +97,28 @@ static constexpr size_t retry_interval_ms = 100; namespace ccf { + void validate_ledger_transaction_size(const host::HostConfig& config) + { + const auto max_message_size = config.memory.max_msg_size.count_bytes(); + const auto max_transaction_size = + config.ledger.max_transaction_size.count_bytes(); + const auto response_overhead = + ::consensus::ledger_range_response_metadata_size; + + if ( + max_message_size <= response_overhead || + max_transaction_size > max_message_size - response_overhead) + { + throw std::logic_error(fmt::format( + "ledger.max_transaction_size ({}) must be at least {} bytes smaller " + "than memory.max_msg_size ({}) so a single ledger entry fits in a " + "ring-buffer range response", + max_transaction_size, + response_overhead, + max_message_size)); + } + } + void validate_and_adjust_recovery_threshold(host::HostConfig& config) { if (config.command.type != StartType::Start) @@ -1019,6 +1042,7 @@ namespace ccf try { + validate_ledger_transaction_size(config); validate_and_adjust_recovery_threshold(config); } catch (const std::logic_error& e) diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index d4545bec5c8a..e47877e983bd 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -62,10 +62,8 @@ namespace ccf::kv }); } - std::unique_ptr prepare_serialisation( - const ccf::ClaimsDigest& claims_digest_, - size_t max_transaction_size, - bool include_reads = false) + size_t projected_serialised_size( + const ccf::ClaimsDigest& claims_digest_, bool include_reads = false) { if (claims_digest_.empty()) { @@ -78,31 +76,27 @@ namespace ccf::kv throw KvSerialiserException("No encryptor set"); } - if (tx_flag_enabled(TxFlag::LEDGER_CHUNK_BEFORE_THIS_TX)) - { - entry_flags |= EntryFlags::FORCE_LEDGER_CHUNK_BEFORE; - } - - auto serialiser = std::make_unique( + SizeKvStoreSerialiser size_serialiser( e, TxID{pimpl->commit_view, NoVersion}, EntryType::WriteSetWithCommitEvidenceAndClaims, entry_flags, + // Both digests are fixed-size, so their values do not affect the + // projected size. ccf::crypto::Sha256Hash{}, - claims_digest_, - false /* historical_hint */, - max_transaction_size, - true /* enforce_max_transaction_size */); + claims_digest_); - serialise_all_changes(*serialiser, include_reads); + serialise_all_changes(size_serialiser, include_reads); - return serialiser; + return size_serialiser.get_serialised_size(); } - std::vector finalise_serialisation( - RawKvStoreSerialiser& serialiser, + std::vector serialise( ccf::crypto::Sha256Hash& commit_evidence_digest, - std::string& commit_evidence) + std::string& commit_evidence, + const ccf::ClaimsDigest& claims_digest_, + size_t max_transaction_size, + bool include_reads = false) { if (!committed) { @@ -114,6 +108,16 @@ namespace ccf::kv throw std::logic_error("Transaction aborted"); } + if (claims_digest_.empty()) + { + throw std::logic_error("Missing claims"); + } + + if (!has_writes()) + { + return {}; + } + auto e = pimpl->store->get_encryptor(); if (e == nullptr) { @@ -124,29 +128,24 @@ namespace ccf::kv LOG_TRACE_FMT("Commit evidence: {}", commit_evidence); ccf::crypto::Sha256Hash tx_commit_evidence_digest(commit_evidence); commit_evidence_digest = tx_commit_evidence_digest; - serialiser.set_tx_id({pimpl->commit_view, version}); - serialiser.set_commit_evidence_digest(tx_commit_evidence_digest); - // Return serialised Tx. - return serialiser.get_raw_data(); - } - - std::vector serialise( - ccf::crypto::Sha256Hash& commit_evidence_digest, - std::string& commit_evidence, - const ccf::ClaimsDigest& claims_digest_, - size_t max_transaction_size, - bool include_reads = false) - { - if (!has_writes()) + if (tx_flag_enabled(TxFlag::LEDGER_CHUNK_BEFORE_THIS_TX)) { - return {}; + entry_flags |= EntryFlags::FORCE_LEDGER_CHUNK_BEFORE; } - auto serialiser = prepare_serialisation( - claims_digest_, max_transaction_size, include_reads); - return finalise_serialisation( - *serialiser, commit_evidence_digest, commit_evidence); + RawKvStoreSerialiser serialiser( + e, + {pimpl->commit_view, version}, + EntryType::WriteSetWithCommitEvidenceAndClaims, + entry_flags, + tx_commit_evidence_digest, + claims_digest_, + false /* historical_hint */, + max_transaction_size); + + serialise_all_changes(serialiser, include_reads); + return serialiser.get_raw_data(); } public: @@ -187,16 +186,21 @@ namespace ccf::kv return CommitResult::SUCCESS; } - std::unique_ptr replicated_serialiser; + std::optional projected_entry_size = std::nullopt; - // Serialise the write set and reject oversized entries before any change - // is applied. The retained domain buffers are patched with the assigned - // version and commit evidence after apply_changes(), then encrypted and - // packaged without walking the write set again. + // Measure the write set and reject oversized entries before any change is + // applied. This pass performs no allocations or copies. Actual + // serialisation is deferred until after conflict detection. if (has_writes()) { - replicated_serialiser = prepare_serialisation( - claims, pimpl->store->get_max_transaction_size()); + const auto max_transaction_size = + pimpl->store->get_max_transaction_size(); + projected_entry_size = projected_serialised_size(claims); + if (projected_entry_size.value() > max_transaction_size) + { + throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( + projected_entry_size.value(), max_transaction_size)); + } } // If this transaction creates any maps, ensure that commit gets a @@ -271,14 +275,17 @@ namespace ccf::kv { ccf::crypto::Sha256Hash commit_evidence_digest; std::string commit_evidence; - if (replicated_serialiser == nullptr) - { - throw std::logic_error( - "Missing serialised write set for committed transaction"); - } - auto data = finalise_serialisation( - *replicated_serialiser, commit_evidence_digest, commit_evidence); - replicated_serialiser.reset(); + auto data = serialise( + commit_evidence_digest, + commit_evidence, + claims, + pimpl->store->get_max_transaction_size()); + CCF_ASSERT_FMT( + projected_entry_size.has_value() && + data.size() == projected_entry_size.value(), + "Projected ledger entry size {} does not match serialised size {}", + projected_entry_size.value_or(0), + data.size()); if (write_set_observer != nullptr) { diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 7e990b77c7da..d0e282185615 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -25,10 +25,6 @@ namespace ccf::kv EntryType entry_type; SerialisedEntryFlags header_flags; size_t max_transaction_size; - size_t version_offset = 0; - std::optional commit_evidence_digest_offset = std::nullopt; - bool enforce_max_transaction_size; - bool finalised = false; std::shared_ptr crypto_util; @@ -41,21 +37,6 @@ namespace ccf::kv template void serialise_internal(const T& t) { - if (enforce_max_transaction_size) - { - const auto current_size = get_serialised_size(); - const auto additional_size = W::serialised_size(t); - if ( - current_size > max_transaction_size || - additional_size > max_transaction_size - current_size) - { - throw MaxTransactionSizeExceeded(fmt::format( - "Cannot serialise transaction because its serialised size exceeds " - "the configured maximum of {} bytes", - max_transaction_size)); - } - } - current_writer->append(t); } @@ -89,14 +70,11 @@ namespace ccf::kv const ccf::crypto::Sha256Hash& commit_evidence_digest_ = {}, const ccf::ClaimsDigest& claims_digest_ = ccf::no_claims(), bool historical_hint_ = false, - size_t max_transaction_size_ = max_serialised_entry_size, - bool enforce_max_transaction_size_ = false) : + size_t max_transaction_size_ = max_serialised_entry_size) : tx_id(tx_id_), entry_type(entry_type_), header_flags(header_flags_), max_transaction_size(max_transaction_size_), - version_offset(W::serialised_size(entry_type_)), - enforce_max_transaction_size(enforce_max_transaction_size_), crypto_util(std::move(e)), historical_hint(historical_hint_) { @@ -109,7 +87,6 @@ namespace ccf::kv } if (has_commit_evidence(entry_type)) { - commit_evidence_digest_offset = public_writer.size(); serialise_internal(commit_evidence_digest_); } // Write a placeholder max_conflict_version for compatibility @@ -171,33 +148,8 @@ namespace ccf::kv serialise_internal(k); } - void set_tx_id(const TxID& tx_id_) - { - tx_id = tx_id_; - public_writer.overwrite(version_offset, tx_id.seqno); - } - - void set_commit_evidence_digest( - const ccf::crypto::Sha256Hash& commit_evidence_digest) - { - if (!commit_evidence_digest_offset.has_value()) - { - throw std::logic_error( - "Cannot set commit evidence digest on entry without commit evidence"); - } - - public_writer.overwrite( - commit_evidence_digest_offset.value(), commit_evidence_digest); - } - std::vector get_raw_data() override { - if (finalised) - { - throw std::logic_error("Serialiser has already been finalised"); - } - finalised = true; - // make sure the private buffer is empty when we return auto writer_guard_func = [](W* writer) { writer->clear(); }; std::unique_ptr @@ -251,10 +203,9 @@ namespace ccf::kv const size_t entry_size = size_ + sizeof(SerialisedEntryHeader); if (entry_size > max_transaction_size) { - // Non-reserved transactions check this exact size before applying - // their changes. Reserved signature transactions are exempt from the - // configured limit and use the largest representable entry size here. - // Reaching this point is therefore always a fatal serialisation error. + // Non-reserved transactions measure this exact size before applying + // their changes. Reserved signature transactions use the largest + // representable entry size here. Reaching this point is always fatal. throw KvSerialiserException(describe_serialised_entry_size_error( entry_size, max_transaction_size)); } diff --git a/src/kv/kv_serialiser.h b/src/kv/kv_serialiser.h index 8772969ffb08..57db53070599 100644 --- a/src/kv/kv_serialiser.h +++ b/src/kv/kv_serialiser.h @@ -8,5 +8,6 @@ namespace ccf::kv { using RawKvStoreSerialiser = GenericSerialiseWrapper; + using SizeKvStoreSerialiser = GenericSerialiseWrapper; using RawKvStoreDeserialiser = GenericDeserialiseWrapper; } diff --git a/src/kv/raw_serialise.h b/src/kv/raw_serialise.h index 0e3012b81634..8b1f061750ae 100644 --- a/src/kv/raw_serialise.h +++ b/src/kv/raw_serialise.h @@ -159,46 +159,39 @@ namespace ccf::kv return buf.size(); } + std::vector get_raw_data() + { + return {buf.data(), buf.data() + buf.size()}; + } + }; + + class SizeWriter + { + private: + size_t total_size = 0; + + public: + SizeWriter() = default; + template - void overwrite(size_t offset, const T& entry) + void append(const T& entry) { - if constexpr (std::is_same_v) - { - if (offset > buf.size() || sizeof(entry.h) > buf.size() - offset) - { - throw std::logic_error("Cannot overwrite outside serialised data"); - } + total_size += RawWriter::serialised_size(entry); + } - auto* data_ = buf.data() + offset; - auto size_ = buf.size() - offset; - serialized::write( - data_, - size_, - reinterpret_cast(entry.h.data()), - sizeof(entry.h)); - } - else if constexpr (std::is_integral_v) - { - if (offset > buf.size() || sizeof(T) > buf.size() - offset) - { - throw std::logic_error("Cannot overwrite outside serialised data"); - } + void clear() + { + total_size = 0; + } - auto* data_ = buf.data() + offset; - auto size_ = buf.size() - offset; - serialized::write(data_, size_, entry); - } - else - { - static_assert( - ccf::nonstd::dependent_false::value, - "Can't overwrite this serialised type"); - } + [[nodiscard]] size_t size() const + { + return total_size; } std::vector get_raw_data() { - return {buf.data(), buf.data() + buf.size()}; + throw std::logic_error("SizeWriter does not retain serialised data"); } }; diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 615f9142306a..553d82b3258b 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -375,7 +375,7 @@ TEST_CASE( entry_size = latest_data->size(); } - // The retained serialised domains must report the exact size of the entry + // The allocation-free sizing pass must report the exact size of the entry // which is eventually written. A limit of precisely that size is accepted, // and one byte less is not. { @@ -495,14 +495,16 @@ TEST_CASE( } TEST_CASE( - "RawWriter append size estimates are exact" * - doctest::test_suite("serialisation")) + "RawWriter and SizeWriter agree" * doctest::test_suite("serialisation")) { const auto check = [](const auto& entry) { - ccf::kv::RawWriter writer; + ccf::kv::RawWriter raw_writer; + ccf::kv::SizeWriter size_writer; const auto expected_size = ccf::kv::RawWriter::serialised_size(entry); - writer.append(entry); - REQUIRE(writer.size() == expected_size); + raw_writer.append(entry); + size_writer.append(entry); + REQUIRE(raw_writer.size() == expected_size); + REQUIRE(size_writer.size() == expected_size); }; check(ccf::kv::EntryType::WriteSetWithCommitEvidenceAndClaims); diff --git a/tests/e2e_batched.py b/tests/e2e_batched.py index 7acdc05d0882..0e2eaffaae90 100644 --- a/tests/e2e_batched.py +++ b/tests/e2e_batched.py @@ -139,6 +139,7 @@ def run_to_destruction(args): # Helps ensure expected destruction workflow. See #6373 for details. args.max_msg_size_bytes = f"{1024 * 1024 * 16}" # 16MB + args.ledger_max_transaction_bytes = f"{1024 * 1024 * 15}" # 15MB run(args) run_to_destruction(args) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 3084b1808f9d..e068e05616a0 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -1628,6 +1628,7 @@ def run_file_operations(args): ntf.flush() args.max_msg_size_bytes = f"{1024 ** 2}" + args.ledger_max_transaction_bytes = f"{1024 ** 2 - 2048}" with tempfile.TemporaryDirectory() as tmp_dir: txs = app.LoggingTxs("user0") diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 98b06d1035b1..85a5321c7e76 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -293,7 +293,7 @@ def cli_args( "(size string)" ), type=str, - default="100MB", + default="64MB", ) parser.add_argument( "--snapshot-tx-interval", @@ -433,7 +433,7 @@ def cli_args( "--max-msg-size-bytes", help="Maximum message size (bytes) allowed on the ring buffer", type=str, - default="64MB", + default="65MB", ) parser.add_argument( "--gov-api-version", From 51b2a261c138bf378a77d84244e54f6214bdb2c5 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Thu, 13 Aug 2026 19:17:42 +0100 Subject: [PATCH 12/15] Update batched stress limit behavior Replace the obsolete expectation that an oversized transaction terminates a node with assertions for TransactionTooLarge, continued node health, and a successful subsequent commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/e2e_batched.py | 97 +++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/tests/e2e_batched.py b/tests/e2e_batched.py index 0e2eaffaae90..cd196465b5a8 100644 --- a/tests/e2e_batched.py +++ b/tests/e2e_batched.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the Apache 2.0 License. +import http import itertools import time from hashlib import sha256 @@ -16,7 +17,14 @@ @reqs.description("Running batch submission of new entries") -def test(network, args, batch_size=100, write_key_divisor=1, write_size_multiplier=1): +def test( + network, + args, + batch_size=100, + write_key_divisor=1, + write_size_multiplier=1, + expect_transaction_too_large=False, +): LOG.info(f"Number of batched entries: {batch_size}") primary, _ = network.find_primary() @@ -31,18 +39,24 @@ def test(network, args, batch_size=100, write_key_divisor=1, write_size_multipli ] pre_submit = time.time() - check( - c.post( - "/app/batch/submit", - { - "entries": messages, - "write_key_divisor": write_key_divisor, - "write_size_multiplier": write_size_multiplier, - }, - timeout=30, - ), - result=len(messages), + response = c.post( + "/app/batch/submit", + { + "entries": messages, + "write_key_divisor": write_key_divisor, + "write_size_multiplier": write_size_multiplier, + }, + timeout=30, ) + + if expect_transaction_too_large: + assert ( + response.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value + ) + assert response.body.json()["error"]["code"] == "TransactionTooLarge" + return network + + check(response, result=len(messages)) post_submit = time.time() LOG.warning( f"Submitting {batch_size} new keys took {post_submit - pre_submit}s" @@ -88,48 +102,27 @@ def run(args): # bs += step_size -def run_to_destruction(args): +def run_to_transaction_limit(args): with infra.network.network( args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb ) as network: network.start_and_open(args) - LOG.warning("About to issue transactions until destruction") - try: - wsm = 5000 - while True: - LOG.info(f"Trying with writes scaled by {wsm}") - network = test(network, args, batch_size=10, write_size_multiplier=wsm) - if wsm > 1000000: - LOG.error( - f"Run to destruction still hasn't caused exception with write sizes multiplied by {wsm}. Infinite loop, or not actually submitting?" - ) - raise ValueError(wsm) - else: - wsm += 100000 # Grow very quickly, expect to fail on the second iteration - except Exception as e: - timeout = 120 - - LOG.info("Large write set caused an exception, as expected") - LOG.info(f"Exception was: {e}") - LOG.info(f"Polling for {timeout}s for node to terminate") - - end_time = time.time() + timeout - while time.time() < end_time: - time.sleep(0.1) - exit_codes = [node.remote.remote.proc.poll() for node in network.nodes] - if any(exit_codes): - LOG.info( - f"One or more nodes terminated with exit codes {exit_codes}" - ) - break - - if time.time() > end_time: - raise TimeoutError( - f"Node took longer than {timeout}s to terminate" - ) from e - - network.ignore_errors_on_shutdown() + LOG.warning("About to issue a transaction above the configured limit") + network = test(network, args, batch_size=10, write_size_multiplier=5000) + network = test( + network, + args, + batch_size=10, + write_size_multiplier=105000, + expect_transaction_too_large=True, + ) + + exit_codes = [node.remote.remote.proc.poll() for node in network.nodes] + assert all(exit_code is None for exit_code in exit_codes), exit_codes + + # The rejected transaction must not prevent subsequent commits. + network = test(network, args, batch_size=10) if __name__ == "__main__": @@ -137,9 +130,11 @@ def run_to_destruction(args): args.package = "js_generic" args.nodes = infra.e2e_args.min_nodes(args, f=1) - # Helps ensure expected destruction workflow. See #6373 for details. + # Keep this stress test's successful write below the configured transaction + # limit while allowing the next, much larger write to exercise clean + # rejection. args.max_msg_size_bytes = f"{1024 * 1024 * 16}" # 16MB args.ledger_max_transaction_bytes = f"{1024 * 1024 * 15}" # 15MB run(args) - run_to_destruction(args) + run_to_transaction_limit(args) From 11daadb82bdec18ba53e8106cb3a1d644413c421 Mon Sep 17 00:00:00 2001 From: achamayou Date: Tue, 18 Aug 2026 21:32:12 +0100 Subject: [PATCH 13/15] Use 32MB transaction limit and restore the 64MB max_msg_size default The 64MB transaction limit required raising memory.max_msg_size to 65MB, so that a maximum-sized entry still fit in a ring-buffer range response alongside its metadata. Halving the transaction limit to 32MB leaves ample headroom, so max_msg_size returns to its previous 64MB default. Also corrects the config schema, which documented max_transaction_size as 65MB while the code default was 64MB. That documented value would itself have been rejected by the new startup validation, and it now documents the constraint against memory.max_msg_size. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- doc/host_config_schema/host_config.json | 4 ++-- include/ccf/node/startup_config.h | 2 +- samples/config/join_config.json | 2 +- samples/config/recover_config.json | 2 +- samples/config/start_config.json | 2 +- src/host/configuration.h | 2 +- tests/infra/e2e_args.py | 4 ++-- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ddc259b8cef..bcf5b7d341ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). -- New `ledger.max_transaction_size` node configuration option (default `64MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. The default `memory.max_msg_size` is now `65MB`, leaving sufficient ring-buffer response headroom for a maximum-sized transaction (#7992). +- New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` (unchanged, default `64MB`) by at least the ring-buffer range response overhead, and this is validated at startup (#7992). ### Fixed diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 6f74a6f4b338..781cad582693 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -480,8 +480,8 @@ }, "max_transaction_size": { "type": "string", - "default": "65MB", - "description": "Maximum total serialised size (size string) of a transaction written to the ledger. Reserved internal signature transactions are exempt. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain" + "default": "32MB", + "description": "Maximum total serialised size (size string) of a transaction written to the ledger. Reserved internal signature transactions are exempt. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. Must be smaller than 'memory.max_msg_size', by at least the ring-buffer range response overhead, so that a single ledger entry can be read back from the host" } }, "description": "This section includes configuration for the ledger directories and files", diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index bbbed1c45a09..190fa236c289 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -48,7 +48,7 @@ namespace ccf std::string directory = "ledger"; std::vector read_only_directories; ccf::ds::SizeString chunk_size = {"5MB"}; - ccf::ds::SizeString max_transaction_size = {"64MB"}; + ccf::ds::SizeString max_transaction_size = {"32MB"}; bool operator==(const Ledger&) const = default; }; diff --git a/samples/config/join_config.json b/samples/config/join_config.json index 96dcd8cfe552..c7fc6266d06f 100644 --- a/samples/config/join_config.json +++ b/samples/config/join_config.json @@ -60,7 +60,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "65MB", + "max_msg_size": "64MB", "max_fragment_size": "256KB" } } diff --git a/samples/config/recover_config.json b/samples/config/recover_config.json index 81fa04c95135..cf757419e88c 100644 --- a/samples/config/recover_config.json +++ b/samples/config/recover_config.json @@ -60,7 +60,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "65MB", + "max_msg_size": "64MB", "max_fragment_size": "256KB" } } diff --git a/samples/config/start_config.json b/samples/config/start_config.json index 9af915c2f59f..30145a278241 100644 --- a/samples/config/start_config.json +++ b/samples/config/start_config.json @@ -89,7 +89,7 @@ "worker_threads": 0, "memory": { "circuit_size": "16MB", - "max_msg_size": "65MB", + "max_msg_size": "64MB", "max_fragment_size": "256KB" } } diff --git a/src/host/configuration.h b/src/host/configuration.h index a1c73701b63d..e19e5bcdfad8 100644 --- a/src/host/configuration.h +++ b/src/host/configuration.h @@ -75,7 +75,7 @@ namespace host struct Memory { ccf::ds::SizeString circuit_size = {"16MB"}; - ccf::ds::SizeString max_msg_size = {"65MB"}; + ccf::ds::SizeString max_msg_size = {"64MB"}; ccf::ds::SizeString max_fragment_size = {"256KB"}; bool operator==(const Memory&) const = default; diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 85a5321c7e76..021c3a3e3607 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -293,7 +293,7 @@ def cli_args( "(size string)" ), type=str, - default="64MB", + default="32MB", ) parser.add_argument( "--snapshot-tx-interval", @@ -433,7 +433,7 @@ def cli_args( "--max-msg-size-bytes", help="Maximum message size (bytes) allowed on the ring buffer", type=str, - default="65MB", + default="64MB", ) parser.add_argument( "--gov-api-version", From 76147489f9e5dc3c73946718b0c78f555f0b5113 Mon Sep 17 00:00:00 2001 From: achamayou Date: Tue, 18 Aug 2026 22:04:14 +0100 Subject: [PATCH 14/15] Validate ledger transaction size under --check validate_ledger_transaction_size() ran after the --check early return, so verifying a configuration file reported success even when max_transaction_size left insufficient room in max_msg_size, and the mismatch only surfaced when the node was started for real. Run it before the early return. validate_and_adjust_recovery_threshold() deliberately stays where it is, since it mutates the configuration and is only meaningful for an actual start. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/host/run.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/host/run.cpp b/src/host/run.cpp index 0a06448cfcd7..24a2ec7ccabb 100644 --- a/src/host/run.cpp +++ b/src/host/run.cpp @@ -1017,6 +1017,19 @@ namespace ccf argv + argc, // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) "\" \"")); + // Validated before the --check early return, so that operators verifying a + // configuration file are told about a ledger/ring-buffer size mismatch + // rather than discovering it when the node starts for real + try + { + validate_ledger_transaction_size(config); + } + catch (const std::logic_error& e) + { + LOG_FATAL_FMT("{}. Exiting.", e.what()); + return static_cast(CLI::ExitCodes::ValidationError); + } + if (check_config_only) { LOG_INFO_FMT("Configuration file successfully verified"); @@ -1042,7 +1055,6 @@ namespace ccf try { - validate_ledger_transaction_size(config); validate_and_adjust_recovery_threshold(config); } catch (const std::logic_error& e) From 3a9e18ef2716bdaca3edfa728cd63f305a766edc Mon Sep 17 00:00:00 2001 From: achamayou Date: Tue, 18 Aug 2026 22:18:17 +0100 Subject: [PATCH 15/15] Describe the transaction size limit against the released behaviour The entry described memory.max_msg_size as unchanged, which only made sense relative to an intermediate state of this branch where it had been raised. State the constraint instead, name the user-visible error code, and note that oversized transactions previously terminated the node. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcf5b7d341ae..dcad6c475ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). -- New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is rejected with `413 Payload Too Large` without affecting subsequent transactions. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` (unchanged, default `64MB`) by at least the ring-buffer range response overhead, and this is validated at startup (#7992). +- New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is now rejected with `413 Payload Too Large` and error code `TransactionTooLarge`, and subsequent transactions are unaffected, where previously an excessively large transaction could terminate the node. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` by at least the ring-buffer range response overhead, which is validated at node startup and by `--check` (#7992). ### Fixed