Private headers should not be needed to build CCF apps

[eddyashton]

Longer term follow-up to #697. We currently implement much of the project in headers, and so our public headers include private headers which must be installed to build CCF apps. We should break these dependency chains to make a hard distinction between publicly supported APIs and implementation details. This will likely involve moving more implementations to .cpp files, and separating the project into individually built libraries.

[achamayou]

~/CCF/include/ccf$ find . -type f | xargs grep "#include \"" | grep -v 'ccf/' | cut -d ' ' -f 2 | sort -u

./tx_id.h:#include "ds/hash.h"
./tx_id.h:#include "ds/json.h"
./claims_digest.h:#include "crypto/hash_provider.h"
./indexing/strategy.h:#include "kv/store.h"
./indexing/strategies/visit_each_entry_in_map.h:#include "indexing/indexing_types.h"
./indexing/strategies/seqnos_by_key_bucketed.h:#include "ds/lru.h"
./indexing/lfs_types.h:#include "ds/ring_buffer_types.h"
./base_endpoint_registry.h:#include "enclave/node_context.h"
./endpoint_registry.h:#include "ds/ccf_deprecated.h"
./endpoint_registry.h:#include "ds/json_schema.h"
./endpoint_registry.h:#include "ds/openapi.h"
./endpoint_registry.h:#include "http/http_consts.h"
./endpoint_registry.h:#include "node/endpoint_metrics.h"
./endpoint_registry.h:#include "node/rpc/serialization.h"
./endpoint_context.h:#include "enclave/rpc_context.h"
./endpoint_context.h:#include "http/authentication/authentication_types.h"
./endpoint.h:#include "ds/json.h"
./endpoint.h:#include "ds/openapi.h"
./endpoint.h:#include "kv/serialise_entry_blit.h"
./endpoint.h:#include "node/service_map.h"
./http_query.h:#include "ds/nonstd.h"
./tx.h:#include "crypto/hash.h"
./tx.h:#include "ds/ccf_assert.h"
./tx.h:#include "kv/kv_types.h"
./tx.h:#include "kv/untyped_map.h"
./historical_queries_interface.h:#include "consensus/ledger_enclave_types.h"
./historical_queries_interface.h:#include "ds/contiguous_set.h"
./historical_queries_interface.h:#include "kv/store.h"
./historical_queries_interface.h:#include "node/history.h"
./historical_queries_interface.h:#include "node/tx_receipt.h"
./receipt.h:#include "crypto/hash.h"
./receipt.h:#include "ds/json.h"
./entity_id.h:#include "ds/json.h"
./entity_id.h:#include "ds/serialized.h"
./app_interface.h:#include "js_plugin.h"
./user_frontend.h:#include "enclave/node_context.h"
./user_frontend.h:#include "node/network_tables.h"
./user_frontend.h:#include "node/rpc/frontend.h"
./json_handler.h:#include "enclave/rpc_context.h"
./json_handler.h:#include "http/http_accept.h"
./json_handler.h:#include "http/http_consts.h"
./json_handler.h:#include "node/rpc/error.h"
./json_handler.h:#include "node/rpc/rpc_exception.h"
./json_handler.h:#include "node/rpc/serdes.h"
./common_auth_policies.h:#include "http/authentication/cert_auth.h"
./common_auth_policies.h:#include "http/authentication/jwt_auth.h"
./common_auth_policies.h:#include "http/authentication/sig_auth.h"

[eddyashton]

Listing some thoughts on the refactoring needed for remaining components, so I don't lose track of them:

• tx.h - Need to expose an abstraction of many of the KV types (Map, Tx, Handle etc), so they can be used by apps without relying on the implementation details. The key here is probably to make the untyped implementation non-templated, so they can be easily forward-declared, and then make all of the type machinery above this public. In Separate public KV headers from implementation details #3578
• contiguous_set.h - We want to expose the API of this container so it can be passed around between indexing and historical queries, and maybe expose some direct access for apps. But ideally we do this explicitly for SeqNoContainer, and move the fact that it's implemented by a custom container to private implementation files.
• node_interface.h - We currently expose this to apps, even though it mostly contains methods explicitly for the node or gov frontends that we don't want apps to call. We should split this up into smaller interfaces, and pass each only where it is needed. This will probably mean a breaking change removing functionality from node_context.h, but this functionality should be unused. Relatedly I'm tempted to rewrite AbstractNodeContext as a more generic interface container/service discovery point, which would make it clearer that we can (and should!) shard the systems we register with it into their minimal atoms. In Remove public get_node_state() API #3552
• indexing/strategies - I think these can now just be moved to private implementation, unless I've missed something. Included in Public header cleanup, part 2 #3543.
• frontend.h - This is a requirement of the get_rpc_handler() entry point, but provides no value to users. Should be replaced by an alternative entry point that just returns an EndpointRegistry. In Change C++ application entry point to remove dependency on frontends #3562

[eddyashton]

Our sample apps also directly includes some private headers, rather than going via public includes:

CCF/samples/apps$ find . -type f | xargs grep -e "#include \"" | grep -v "ccf" | sort
./logging/logging.cpp:#include "apps/utils/metrics_tracker.h"
./logging/logging.cpp:#include "crypto/verifier.h"
./logging/logging.cpp:#include "logging_schema.h"
./logging/logging.cpp:#include "node/tx_receipt.h"
./nobuiltins/nobuiltins.cpp:#include "node/rpc/call_types.h"
./nobuiltins/nobuiltins.cpp:#include "node/rpc/frontend.h"
./nobuiltins/nobuiltins.cpp:#include "node/rpc/serialization.h"
./nobuiltins/nobuiltins.cpp:#include "service/network_tables.h"

I think these are all simple to fix, though the nobuiltins fix may involve making a bunch of the C++ types backing our builtin HTTP endpoints public? Hopefully we can break that just by creating new types for this app's endpoints.

[eddyashton]

We've finished the major work here, and the only major type that we still need to expose is kv::Store (required to use the historical query API, currently visible to apps via chains like node/tx_receipt.h -> node/history.h -> kv/store.h).

Beyond that, the remaining cleanup tasks are:

• metrics_tracker.h - this can be removed? Done in Remove metrics tracker #3599
• tx_receipt.h - expose interface of this, and supporting hash types, so that apps can call describe. In Remove dependency on TxReceipt from public API #3610
• network_tables.h - move some service table definitions, and supporting types, to public headers, so they can be safely consumed by C++ apps.
• rpc/[call_types/serialization].h - currently used by nobuiltins, but probably shouldn't be. This app should produce its own types for RPC responses. (Separately, combine these files, and move all their type definitions where they're actually needed!)
• kv/store.h - currently seen via tx_receipt.h, and needed for historical queries.