Take endpoints out of the source dependency cycle - #8475
Merged
Amaury Chamayou (achamayou) merged 1 commit intoSep 30, 2026
Merged
Amaury Chamayou (achamayou) merged 1 commit into
Amaury Chamayou (achamayou) merged 1 commit into
Conversation
endpoints was in the {endpoints, js, node} cycle only through six
includes of node headers from three files. Move each piece to its owner:
- rpc_context_impl.h moves from node/ to endpoints/.
- build_receipt_for_committed_tx() is now defined in
node/historical_queries_adapter.cpp, next to the other public receipt
functions implemented in node, in the same ccf_endpoints library.
- GetCommit, GetTxStatus and GetAPI move into
common_endpoint_registry.cpp, their only user.
- cose::Signature, is_ecdsa_alg() and is_rsa_alg() move to
crypto/cose.h, next to the alg constants.
Add an endpoints dependency policy. The remaining cycle is {js, node}.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Amaury Chamayou (achamayou)
September 30, 2026 14:16
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The relocations preserve definitions and behavior, and the new dependency policy passes validation.
Review effort: Balanced
Findings: None
What changed in this PR
Moves endpoint-owned code out of node, removing endpoints from the source dependency cycle without changing runtime behavior.
Changes:
- Relocates endpoint context, receipt, RPC schema, and COSE helpers.
- Adds and validates an explicit dependency policy for
endpoints.
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
| File | Description |
|---|---|
src/node/rpc/serialization.h |
Removes relocated endpoint JSON declarations. |
src/node/rpc/http_rpc_context.h |
Includes the relocated RPC context. |
src/node/rpc/call_types.h |
Removes endpoint-only call types. |
src/node/historical_queries_adapter.cpp |
Hosts committed-transaction receipt construction. |
src/node/cose_common.h |
Removes relocated COSE helpers. |
src/js/registry.cpp |
Uses the endpoint-owned RPC context. |
src/endpoints/rpc_context_impl.h |
Defines the relocated RPC context implementation. |
src/endpoints/endpoint_registry.cpp |
Removes node-owned receipt dependencies. |
src/endpoints/common_endpoint_registry.cpp |
Localizes endpoint call types and serialization. |
src/endpoints/authentication/cose_auth.cpp |
Uses crypto-owned COSE helpers. |
src/crypto/cose.h |
Defines shared COSE signature and algorithm helpers. |
scripts/source-dependencies.json |
Adds the validated endpoints dependency policy. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Max (maxtropets)
approved these changes
Sep 30, 2026
Amaury Chamayou (achamayou)
deleted the
achamayou-issue-3517-clarify-dependencies-between-framework-s-cd5d31
branch
September 30, 2026 14:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Partially addresses #3517. This implements the
endpoints -> nodestep from the latest analysis, which #8472 made the next cut.Since #8472, the source dependency cycle has been
{endpoints, js, node}.endpointsis in it only because three of its files include sixnodeheaders. Each of those is either a type thatendpointseffectively owns, or node code that happens to be implemented insrc/endpoints/. This PR moves them, soendpointsleaves the cycle and gets a dependency policy.endpoints,js,nodejs,nodeThe diagram shows every direct dependency among the three components that formed the cycle, each labelled with the number of
#includedirectives behind it after this PR. The dashed red edge is the one this PR removes (6 before).graph LR subgraph core["remaining cycle"] js -- 8 --> node node -- 14 --> js end js -- 8 --> endpoints node -- 5 --> endpoints endpoints -.->|6, removed| node linkStyle 4 stroke:#cf222e,stroke-width:3pxjs -> nodeis now the last edge to cut before the source graph has no cycles.Implementation summary
Code moves only, one per removed include:
rpc_context_impl.hmoves fromnode/toendpoints/. It only depends on public headers, and both endpoint registries (endpoint_registry.cppandjs/registry.cpp) cast toRpcContextImplto fill in path parameters.node/rpc/http_rpc_context.hnow includes it fromendpoints, which is the intended direction. This also removes onejs -> nodeinclude.build_receipt_for_committed_tx()moves tonode/historical_queries_adapter.cpp. It is declared in the publicccf/endpoint_registry.h, but it is node code: it looks up theSignatureCacheInterfacesubsystem and rebuilds aMerkleTreeHistoryto extract a proof. It now sits next todescribe_receipt_v1()and the other public receipt functions implemented innode. Both files are compiled intoccf_endpoints, so linking does not change. This removes thetx_receipt_impl.handsignature_cache_interface.hincludes, without the new subsystem or Merkle tree relocation that the analysis expected.endpoint_registry.cppalso dropsccf/node_context.h, which it no longer uses.GetCommit,GetTxStatusandGetAPImove intocommon_endpoint_registry.cpp, their only user, with their JSON declarations. Their names, and so the generated OpenAPI schemas, do not change. This removes thenode/rpc/call_types.handnode/rpc/serialization.hincludes.cose::Signature,is_ecdsa_alg()andis_rsa_alg()move fromnode/cose_common.htocrypto/cose.h, next to thealgconstants they test.cose_auth.cppused nothing else fromcose_common.h.cose_common.halready includescrypto/cose.h, so its other users are unaffected.source-dependencies.jsongains anendpointspolicy (ccf-api,crypto,ds,http,kv,service), which is exactly the measured set.Following includes transitively,
src/endpoints/now reaches 125 files outside itself instead of 198, and none innode(previously 16).Locally, a clang 21 Debug build of all targets produces no warnings, and all unit tests,
schema_test,e2e_logging(including/log/blocking/private/receipt, which callsbuild_receipt_for_committed_tx()) andgovernance_testpass.The only open PR that touches the same files is draft #8368, which replaces
fmt::formatin them, including in the moved body ofbuild_receipt_for_committed_tx(). Rebasing it is mechanical.Safety and compatibility
Production behaviour does not change. Every function and type keeps its body, and only the file that defines it changes:
/commit,/txand/apiresponses, and the COSE governance signature algorithm checks run the same code as before.build_receipt_for_committed_tx()keeps its public declaration and stays in theccf_endpointslibrary, so applications compile and link as before.Nothing under
include/changes. There are no wire, ledger or KV format changes, and no effect on mixed-version operation or recovery. Nothing is user-facing, so there is noCHANGELOG.mdentry.Fixes: #3517