Warp->Axum + OpenAPI documentation#2119
Conversation
OCaml Reference Validation ResultsRepository: https://github.com/MinaProtocol/mina.git Click to see full validation output |
✓ Code Reference Verification PassedAll code references in the documentation have been verified successfully! Total references checked: 1 The documentation is in sync with the codebase on the |
8778c1e to
f5603f4
Compare
f5603f4 to
b011ef3
Compare
ecdd2b1 to
c478564
Compare
dannywillems
left a comment
There was a problem hiding this comment.
LGTM.
Also, the page RPC API can now be removed.
| uses: ./.github/actions/setup-rust | ||
| with: | ||
| components: rustfmt | ||
| # This should be in line with the verison in: |
There was a problem hiding this comment.
Nit: I don't want to block at all, but it was nice to have this to guide updates, in case we have to use a specific nightly version again. Fine to remove it as AI code assistants are good for this now. However, we might want to stay consistent in the codebase and remove other places where we have the list.
| // NB: this is disgusting, the real approach is to do | ||
| // #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema), schema(as = String))] | ||
| // on RequestId<T>, with #[schema(ignore)] on the PhantomData | ||
| // However, ignore doesn't ignore: https://github.com/juhaku/utoipa/issues/1499 |
There was a problem hiding this comment.
Comment: I'm a bit worried about the maintenance state of utoipa, as it seems there has been a patch (juhaku/utoipa#1500) but no review.
There was a problem hiding this comment.
Have you tried #[schema(ignore = true)]? Both are supposed to work, but maybe there's just a bug in one?
| @@ -0,0 +1,83 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Is this actually used?
There was a problem hiding this comment.
I left it in to show comparison methodology. It's also generally handy in case we ever want to compare responses from sources that we're trying to match. It's designed to be agent-friendly (hence all the work around keeping the amount printed small)
|
|
||
| When running a node locally, the same documentation is available at: | ||
|
|
||
| - `http://localhost:3000/api-docs/swagger-ui` |
| @echo "Rust API documentation integrated into website/static/api-docs/" | ||
|
|
||
| .PHONY: docs-openapi-gen | ||
| docs-openapi-gen: ## Generate OpenAPI spec and integrate into website |
a7120b6 to
b375353
Compare
richardpringle
left a comment
There was a problem hiding this comment.
Haha the AI slop comment is the only necessary change request. Everything else is just a comment
| // NB: this is disgusting, the real approach is to do | ||
| // #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema), schema(as = String))] | ||
| // on RequestId<T>, with #[schema(ignore)] on the PhantomData | ||
| // However, ignore doesn't ignore: https://github.com/juhaku/utoipa/issues/1499 |
There was a problem hiding this comment.
Have you tried #[schema(ignore = true)]? Both are supposed to work, but maybe there's just a bug in one?
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "openapi")] |
There was a problem hiding this comment.
Comment for the future, don't even need to change this in a follow up, but I find that multiple feature flags in the same file are a bit of a code smell. It's better to put everything in the same module (whether in a new file or in the same file) and add the attribute to the module definition. Can always do use super::* inside that module, then in the parent, do something like use my_module::*;.
For this case it would especially well given you don't need to export anything.
| fn schemas( | ||
| schemas: &mut Vec<( | ||
| String, | ||
| utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>, | ||
| )>, | ||
| ) { | ||
| schemas.extend([]); | ||
| } |
There was a problem hiding this comment.
AI slop? This is a no op that already has a no-op default implementation.
| fn schemas( | |
| schemas: &mut Vec<( | |
| String, | |
| utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>, | |
| )>, | |
| ) { | |
| schemas.extend([]); | |
| } |
There was a problem hiding this comment.
Reminder @iostat, this is the only thing that I think needs to be changed. Everything else is just commentary
There was a problem hiding this comment.
Worse -- hand-slop from when I was tweaking the AI slop to see if I can get utoipa to stop inlining certain types!
| // LedgerHashTransition (aka SnarkJobId) and LedgerHashTransitionPasses serialize to strings | ||
| // in human-readable format. Manual impls to avoid cascading ToSchema requirement on LedgerHash. | ||
| #[cfg(feature = "openapi")] | ||
| const _: () = { |
There was a problem hiding this comment.
I understand that this works, but it's limiting vs using a module. If you ever wanted to export a new type or anything from the stuff behind the feature flag, the change would be smaller if this was a module.
| const _: () = { | |
| mod openapi { |
crates/p2p-messages/src/v2/manual.rs
Outdated
| ($name:ident, $ty:ty, $version_byte:ident) => { | ||
| pub type $name = Base58CheckOfBytes<$ty, { $crate::b58version::$version_byte }>; | ||
| #[cfg(feature = "openapi")] | ||
| const _: () = { |
There was a problem hiding this comment.
this is one place where I actually think the const block does make sense!
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "openapi")] |
There was a problem hiding this comment.
Why the inconsistency with const block vs multiple feature flags?
| let res = client | ||
| .post(url) | ||
| .body(serde_json::to_string(&offer)?) | ||
| .json(&offer) // Sets Content-Type: application/json automatically |
| COPY poseidon ./poseidon | ||
| COPY tools ./tools | ||
| COPY vendor ./vendor | ||
| COPY xtask ./xtask |
| eprintln!(); | ||
| eprintln!("Commands:"); | ||
| #[cfg(feature = "openapi-gen")] | ||
| eprintln!(" openapi [output] Generate OpenAPI spec"); |
b375353 to
226e3ea
Compare
| }; | ||
| } | ||
|
|
||
| base58check_openapi_impl!(MinaBaseSignedCommandMemoStableV1); |
- Update workspace axum dependency from 0.7 to 0.8
- Update path syntax in testing tools: /:param -> /{param} (axum 0.8 requirement)
- Fix serde::__private::fmt usage in p2p-messages (broke with serde 1.0.228
pulled in by axum 0.8 transitive deps)
- Create http_server/ module structure with AppState, AppError, CORS layer - Implement status routes (build_env, status, healthz, readyz, make_heartbeat) matching warp behavior - Implement WebRTC signaling routes (GET/POST) matching warp behavior - Add stub routes for state, scan_state, snark_pool, snarker, transaction, discovery endpoints - Rename http_server.rs to http_server_warp.rs for coexistence
- Add --http-port-axum CLI flag (default 3001) to run axum server alongside warp - Add http_server_axum_init to service builder spawning axum on separate thread - Add http_server_axum method to NodeBuilder - Both servers share the same RpcSender for identical responses
- Implements all previously stubbed endpoints and add rpc_request!/jsonify_rpc! macros to reduce boilerplate. - All routes tested against warp implementation for parity. Known differences: - axum always sends CORS vary header - WebRTC POST returns 422 vs 400 for malformed JSON (framework default) - Some error responses use bare JSON strings for warp compat (marked TODO)
- Upgrade juniper 0.16 -> 0.17, add juniper_axum 0.3 - Remove warp and juniper_warp dependencies - Delete http_server_warp.rs - Add GraphQL routes to axum (/graphql, /graphiql) - Remove /playground endpoint (redundant with GraphiQL 2.0) - Remove --http-port-axum flag (axum is now the only server)
- Add utoipa, utoipa-axum, utoipa-scalar, utoipa-swagger-ui deps - Add openapi feature to mina-node for ToSchema derives - Add swagger-ui, scalar, graphiql features to mina-node-native - Convert all route modules to use OpenApiRouter with #[utoipa::path] - Serve Swagger UI at /api-docs/swagger-ui - Serve Scalar at /api-docs/scalar - Serve OpenAPI JSON at /api-docs/openapi.json - GraphQL routes kept separate (not documented in OpenAPI)
JS/CSS loaded from CDN, negligible binary size. Feature: stoplight-elements.
- Propagate openapi feature through crates (redux, ledger, core, p2p, account) - Add ToSchema to status route response types - Manual ToSchema impl for RequestId<T> (workaround for utoipa#1499)
- Add ToSchema to ActionStatsResponse, ActionStatsForBlock, ActionStatsForRange, ActionStatsForRanges - Add ToSchema to SyncStatsSnapshot, SyncKind, SyncLedgers, SyncLedger, SyncSnarkedLedger, SyncStagedLedger, SyncBlock, SyncBlockStatus, LedgerResyncKind, LedgerResyncEvent - Add ToSchema to RpcBlockProducerStats, VrfEvaluatorStats - Add TODO(openapi) comments for Base58CheckOfBinProt fields needing macro-generated ToSchema
Add ToSchema derives and manual impls for types used by snark pool and snarker HTTP routes: - Add openapi feature to mina-p2p-messages crate - Propagate openapi feature through mina-core and mina-node - Add manual ToSchema impls for Base58CheckOfBinProt types: CurrencyFeeStableV1, StateHash, StateBodyHash, LedgerHash, NonZeroCurvePoint (all serialize to strings) - Add manual ToSchema impls for SnarkJobId (LedgerHashTransition) and LedgerHashTransitionPasses - Add ToSchema derives to SnarkJobCommitment, SnarkWorkSpecError, ExternalSnarkWorkerError, ExternalSnarkWorkerWorkError, JobSummary, JobCommitment - Simplify RequestId PartialSchema impl to use String schema
Add OpenAPI schema implementations for BigInt, List<T>, Number<T>, and PaddedSeq types used throughout p2p-messages. - BigInt: hex string with 0x prefix pattern, description corrected to "Unsigned 256-bit integer" (not field element) - List<T>: array schema delegating to inner type - Number<T>: string-encoded numeric values - PaddedSeq<T, N>: array schema with minItems/maxItems from const N
Update base58check_of_binprot! and base58check_of_bytes! macros to automatically generate ToSchema implementations. All base58-encoded types now have string schemas. Also adds manual ToSchema for: - MinaBaseSignedCommandMemoStableV1 - NonZeroCurvePoint (not macro-generated) - TransactionHash
Add ToSchema derives for generated types needed by HTTP API responses: - Currency types: CurrencyFeeStableV1, CurrencyAmountStableV1 - Number types: UnsignedExtendedUInt32/64 variants - Slot types: MinaNumbersGlobalSlotSinceGenesisMStableV1 - Account types: MinaBaseAccountIdDigestStableV1 - Transaction status: MinaBaseTransactionStatusFailureStableV2
Add ToSchema to RPC types used in HTTP API responses: - RpcScanStateSummary and related types - RpcSnarkPoolJobSummary, RpcSnarkPoolJobSnarkWork - JobCommitment, SnarkJobCommitment - Timestamp, PeerId wrappers Also adds ToSchema to ledger scan_state types for OpenAPI. Removes redundant schema(value_type = String) annotations from: - StateHash fields in rpc/mod.rs (macro now generates ToSchema) - BlockHash fields in stats_sync.rs, stats_actions.rs, stats_block_producer.rs - AccountPublicKey (derives ToSchema directly)
Replace schema(as = String) derive with proper manual ToSchema impl:
- Base58check encoding pattern: ^[1-9A-HJ-NP-Za-km-z]{49}$
- Constrained to exactly 49 characters (minLength/maxLength)
- Description: "Base58check-encoded peer ID (32 bytes)"
This prevents "String" from leaking into OpenAPI models.
Add body= parameters to utoipa path annotations for proper OpenAPI response documentation: - stats.rs: ActionStatsResponse, SyncStatsSnapshot, BlockProducerStats - state.rs: RpcPeersGetResponse, RpcMessageProgressResponse - status.rs: RpcNodeStatus, RpcHeartbeatGetResponse - scan_state.rs: RpcScanStateSummary with BlockIdentifier param type Uses inline() for type alias responses to prevent "Vec" and "Option" from leaking into OpenAPI models: - RpcPeersGetResponse = Vec<RpcPeerInfo> -> inline() - RpcActionStatsGetResponse = Option<...> -> inline() - RpcSyncStatsGetResponse = Option<Vec<...>> -> inline() - RpcBlockProducerStatsGetResponse = Option<...> -> inline() - RpcHeartbeatGetResponse = Option<...> -> inline() Also reorganizes shared types into routes/mod.rs and improves JsonErrorResponse schema.
Expose a function to extract the OpenAPI spec without starting a server. This enables static generation via xtask for documentation builds.
Add cargo xtask infrastructure for build/CI tooling. The openapi-gen feature enables the `openapi` command which generates the HTTP API spec as JSON. Usage: cargo x-openapi-gen openapi [output-path]
Add three API documentation UI pages that reference static openapi.json: - swagger-ui.html (classic interactive explorer) - stoplight.html (modern API reference) - scalar.html (developer-friendly reference) All UIs load from CDN. Links to be added in Docusaurus docs.
Add docs-openapi-gen target that generates openapi.json directly into website/static/openapi/ via cargo x-openapi-gen. Skips generation if file already exists (for CI artifact pre-placement). Updated docs-build and docs-serve to depend on docs-openapi-gen.
Add section to RPC API docs pointing to the three OpenAPI viewers (Swagger UI, Stoplight Elements, Scalar) both for the static site and for local node instances.
Replace manual dtolnay/rust-toolchain setup with the composite setup-rust action which includes cargo caching. Use nightly toolchain for rustfmt compatibility with build.rs generated code formatting.
Use reqwest's .json() method instead of .body() with manual serialization. This automatically sets Content-Type: application/json, which axum's Json extractor requires (unlike warp which was lenient). Also removes the now-unused Serialize error variant from RTCSignalingError since reqwest handles serialization internally.
Axum's Json extractor requires Content-Type: application/json, returning 415 if missing. Old clients may not send this header. AssumeJson: - Validates Content-Type if present (415 if wrong type) - Assumes JSON if Content-Type is missing - Uses axum's Json parsing (422 on malformed JSON)
226e3ea to
5c9b049
Compare
Description
Migrate the HTTP RPC server from warp to axum and add comprehensive OpenAPI documentation with interactive API explorers.
HTTP Server Migration (warp → axum)
OpenAPI Documentation
/api-docs/swagger-ui- Swagger UI/api-docs/scalar- Scalar/api-docs/stoplight- Stoplight Elements/api-docs/openapi.jsonToSchemaderives across RPC response types in:mina-node(feature-gated behindopenapi)mina-p2p-messages(for base58check types via macro)mina-p2p(manual impl for PeerId)Static OpenAPI Generation for Documentation Website
cargo xtaskcrate withopenapi-genfeature for CI toolingcargo x-openapi-gen [output]generates OpenAPI spec without running a nodewebsite/static/openapi/make docs-openapi-genBreaking Changes
Error response format standardization
Most error responses now use
{"error": "message"}instead of the varied formats in warp:"error message"(bare JSON string){"error": "error message"}{"VariantName": "details"}(enum){"error": "details"}error message(plaintext){"error": "error message"}Affected endpoints:
/stats/actions,/scan-state/summary*,/snark-pool/*,/snarker/workers,/snarker/config,/transaction-pool,/accounts,/best-chain-user-commands,/state(filter errors)Preserved for compatibility:
/snarker/job/commit: Invalid input still returns bare"invalid_input"stringP2pConnectionResponseenum directlyChannel dropped handling
Many endpoints that previously returned
nullin the response body on channel drop now return a proper error:Affected endpoints:
/status,/make_heartbeat,/state/peers,/state/message-progress,/stats/*Health/readiness error format
/healthzand/readyzerror responses changed from plaintext to JSON:Status code changes
/mina/webrtc/signal(POST): Malformed JSON returns 422 instead of 400 (axum vs warp framework defaults; 422 "Unprocessable Entity" is arguably more correct for valid JSON with wrong schema)Removed Endpoints
/playground- GraphQL Playground was deprecated and merged into GraphiQL 2.0. Use/graphiqlinstead.Dependency Changes
Added:
utoipa5.xutoipa-axum0.2utoipa-swagger-ui9.x (workspace, optional)utoipa-scalar0.3 (workspace, optional)juniper_axum0.3Upgraded:
axum0.7 → 0.8juniper0.16 → 0.17Removed:
warp0.3juniper_warp0.8Related Issue(s)
Closes #2077
Closes #1745
Follow-up issues to open
api-endpoints.mdxinto the OpenAPI schemaopenapi.jsonand enforce synchronyToSchemaderived on them. There's a MASSIVE dependency graph, and really the codegen should be updated to properly derive them. This precludes some of the endpoints from being properly documented. That same codegen should be brought into the repo, and possibly run as part of the build -- right now it exists in mina-gossip-p2p, hasn't been touched in 4 years, and apparently there's been manual touches up to the generated types... obviously not sustainable!Type of Change
Testing
The migration was developed with side-by-side comparison testing:
--http-port-axumflag ran both servers simultaneouslyscripts/compare-endpoints.shverified identical responsesChangelog Entry
Changelog Summary:
/api-docs/*