feat(sso): add typed handler and wire macros by pgherveou · Pull Request #651 · paritytech/host-rust-core

pgherveou · 2026-09-08T20:57:38Z

Generate SSO request/response pairing and dispatch from typed handler methods, with compile-time checks that keep handlers and wire messages aligned. Handlers support shared payload types without duplicate request structs.

Stack 1/2; server integration in #628.

Validation: macro/compiler tests and workspace Clippy passed.

socket-security · 2026-09-08T20:58:04Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	cargo/trybuild@1.0.121

View full report

TarikGul · 2026-09-09T01:06:11Z

@lore-bot-app review

lore-bot-app · 2026-09-09T01:09:59Z

Reading the diff and checking what the record says. Back in a few minutes.

lore-bot-app

Gathering is done. Writing the review now.

TL;DR
Splits truapi-macros into one module per macro and adds SsoWire, SsoResponse, and #[sso_service] for the inter-host SSO protocol, checked only by trybuild against a mock of truapi-server. 1 blocking, 3 minor.

Summary
This is PR #651. The existing service, wire, and versioned_type! implementations move unchanged into their own files, with lib.rs reduced to entry points. Three new macros generate the SSO plumbing: SsoWire classifies a hand-written v1::RemoteMessage enum into requests, responses, and disconnect, and adds name(), responding_to(), and with_responding_to(). SsoResponse implements a server trait for structs shaped as responding_to: String plus one Result field, enforcing field order for SCALE. #[sso_service] turns an inherent impl into typed handlers, derives request-to-response pairing from signatures, and emits an exhaustive dispatch. All three hardcode crate::host_logic::sso::{wire, messages::v1} and crate::runtime::{sso_service, authority} paths inside truapi-server. Tests are trybuild pass/fail cases against a mock of those modules, plus unit tests on error strings. Cargo.lock adds trybuild and its two transitive dev-only crates. README, CLAUDE.md, and a new crate README are updated. I could not run cargo in this environment, so this is a static read.

What the record says

The consumer is companion PR #628, "refactor(sso): serve inter-host requests through typed handlers", which introduces the shared typed SSO contract and notes that Rust API renames require consumer updates. Lore lists #651 as its code ref. #628
Lore's entry for this PR says validation was macro/compiler tests plus a workspace clippy pass, and that socket-security flagged the new dependency. #651
The existing wire enum names messages through derive_more::Display with hand-chosen strings, and the CLI terminal UI consumes those strings as SsoEvent.request (its test expects get_account_alias to render as "Get account alias"). https://github.com/paritytech/truapi/blob/4de332bd43ba10a5bf2e4f8ccef12f11befd6919/rust/crates/truapi-host-cli/src/terminal_ui.rs
The record has no prior discussion of proc-macro design, trybuild, or hardcoded crate:: paths in this repo. versioned_type! already hardcodes crate::versioned, so the pattern has precedent in code but no recorded decision.

Concerns

Blocking: the macro contract is only verified against a mock. rust/crates/truapi-macros/src/sso_common.rs:7-14 and rust/crates/truapi-macros/src/sso_service.rs:36 hardcode paths that do not exist in this branch (runtime/sso_service.rs and host_logic/sso/wire.rs are absent, and truapi-server does not depend on truapi-macros). The only definition of SsoResponse, SsoReply, Dispatch, and SsoRequestContext the macros are compiled against is tests/ui/sso/support/{wire,runtime}.rs. If #628's trait signatures differ from the mock in any way (argument order of new, the ResponsePayload alias, finish taking &str), the derives break in the server and nothing in this PR catches it. Merge order matters: either this lands first and #628 is the proof, or the mock drifts silently.
Minor: name() mixes two naming schemes on one enum. rust/crates/truapi-macros/src/sso_wire.rs:85 emits "Disconnected" and line 101 emits the PascalCase variant name for responses, while requests get snake_case stems. Today's Display at rust/crates/truapi-server/src/host_logic/sso/messages/v1.rs:23-106 is uniformly snake_case (disconnected, sign_response). If #628 routes transcript events through name(), response and disconnect strings flip case for hosts that read them. The doc comment says this is intentional, but the enum will then carry two naming functions that disagree.
Minor: handler names are pinned to variant stems, not to today's wire names. rust/crates/truapi-macros/src/sso_service.rs:79-80 requires the handler for RingVrfAliasRequest to be ring_vrf_alias and sets NAME to the same string. The current Display for that variant is get_account_alias, and RingVrfProofRequest is create_account_proof. Either #628 renames the variants (the README example uses GetAccountAliasRequest, which suggests it does) or the action names change. Not visible from this diff.
Minor: compile-fail snapshots against a floating toolchain. rust/crates/truapi-macros/tests/sso.rs:7 runs compile_fail cases whose .stderr files contain rustc help: suggestion blocks (missing_handler.stderr, request_without_variant.stderr, wrong_reply.stderr). CI installs unpinned stable (.github/workflows/ci.yml:35-37) and runs cargo test --workspace (line 68). This is the first trybuild use in the workspace, so each stable release that rewords a suggestion will fail CI for everyone.

Also: CONTRIBUTING.md:59 still describes truapi-macros as the #[wire(id = N)] proc-macro only. README and CLAUDE.md were updated, this one was not.

Questions for the author

Which lands first, this or #628? If #628 already compiles against these macros, say so in the description. If not, can the mock in tests/ui/sso/support/ be derived from or asserted against the real modules?
Does #628 rename the v1::RemoteMessage request variants to match the snake_case stems, and does it replace the Display strings with name() for SsoEvent.request?
Should the crate pin a toolchain for the trybuild job, or should the compile_fail cases be limited to the macro's own syn::Error messages, which are stable across rustc versions?

🤖 Reviewed by Lore (Parity knowledge base) · 30 agent turns · 1470.2s · knowledge as of 2026-09-08

lore-bot-app · 2026-09-09T05:23:25Z

+use quote::quote;
+
+/// Request contract implemented by the service macro.
+pub(super) fn wire_path() -> TokenStream {


Blocking: the macro contract is only verified against a mock. rust/crates/truapi-macros/src/sso_common.rs:7-14 and rust/crates/truapi-macros/src/sso_service.rs:36 hardcode paths that do not exist in this branch (runtime/sso_service.rs and host_logic/sso/wire.rs are absent, and truapi-server does not depend on truapi-macros). The only definition of SsoResponse, SsoReply, Dispatch, and SsoRequestContext the macros are compiled against is tests/ui/sso/support/{wire,runtime}.rs. If #628's trait signatures differ from the mock in any way (argument order of new, the ResponsePayload alias, finish taking &str), the derives break in the server and nothing in this PR catches it. Merge order matters: either this lands first and #628 is the proof, or the mock drifts silently.

lore-bot-app · 2026-09-09T05:23:25Z

+        });
+    }
+    let mut retarget_arms = Vec::new();
+    let mut name_arms = vec![quote! { #enum_ident::#disconnect => #DISCONNECT_VARIANT }];


Minor: name() mixes two naming schemes on one enum. rust/crates/truapi-macros/src/sso_wire.rs:85 emits "Disconnected" and line 101 emits the PascalCase variant name for responses, while requests get snake_case stems. Today's Display at rust/crates/truapi-server/src/host_logic/sso/messages/v1.rs:23-106 is uniformly snake_case (disconnected, sign_response). If #628 routes transcript events through name(), response and disconnect strings flip case for hosts that read them. The doc comment says this is intentional, but the enum will then carry two naming functions that disagree.

lore-bot-app · 2026-09-09T05:23:25Z

+        let stem: String = method_name
+            .split('_')
+            .map(|word| {
+                let mut chars = word.chars();


Minor: handler names are pinned to variant stems, not to today's wire names. rust/crates/truapi-macros/src/sso_service.rs:79-80 requires the handler for RingVrfAliasRequest to be ring_vrf_alias and sets NAME to the same string. The current Display for that variant is get_account_alias, and RingVrfProofRequest is create_account_proof. Either #628 renames the variants (the README example uses GetAccountAliasRequest, which suggests it does) or the action names change. Not visible from this diff.

lore-bot-app · 2026-09-09T05:23:26Z

+fn sso_handler_contracts() {
+    let cases = trybuild::TestCases::new();
+    cases.pass("tests/ui/sso/pass/*.rs");
+    cases.compile_fail("tests/ui/sso/fail/*.rs");


Minor: compile-fail snapshots against a floating toolchain. rust/crates/truapi-macros/tests/sso.rs:7 runs compile_fail cases whose .stderr files contain rustc help: suggestion blocks (missing_handler.stderr, request_without_variant.stderr, wrong_reply.stderr). CI installs unpinned stable (.github/workflows/ci.yml:35-37) and runs cargo test --workspace (line 68). This is the first trybuild use in the workspace, so each stable release that rewords a suggestion will fail CI for everyone.

Also: CONTRIBUTING.md:59 still describes truapi-macros as the #[wire(id = N)] proc-macro only. README and CLAUDE.md were updated, this one was not.

…elpers

feat(sso): add handler and wire contract macros

b21896f

pgherveou requested a review from a team September 8, 2026 20:57

pgherveou mentioned this pull request Sep 8, 2026

refactor(sso): serve inter-host requests through typed handlers #628

Draft

refactor(macros): keep each macro in its own module

c578ba2

refactor(macros): generate SSO conversions for shared responses

517e30c

pgherveou marked this pull request as draft September 9, 2026 04:29

refactor(sso): infer request variants from handler names

5cd0cff

lore-bot-app Bot reviewed Sep 9, 2026

View reviewed changes

pgherveou added 4 commits September 9, 2026 09:15

fix(macros): support raw identifiers in SSO handlers

54b420d

refactor(macros): accept Rust parameter patterns and inline private h…

003e660

…elpers

test(macros): consolidate SSO fixtures and remove mock runtime behavior

f361cdb

test(macros): refresh SSO diagnostics for CI Rust toolchain

ccbfaa0

Navigation Menu

Uh oh!

feat(sso): add typed handler and wire macros - #651

feat(sso): add typed handler and wire macros#651
pgherveou wants to merge 8 commits into
mainfrom
sso-rpc-macros

pgherveou commented Sep 8, 2026 •

edited

Loading

Uh oh!

socket-security Bot commented Sep 8, 2026 •

edited

Loading

Uh oh!

TarikGul commented Sep 9, 2026

Uh oh!

lore-bot-app Bot commented Sep 9, 2026

Uh oh!

lore-bot-app Bot left a comment

Uh oh!

lore-bot-app Bot Sep 9, 2026

Uh oh!

lore-bot-app Bot Sep 9, 2026

Uh oh!

lore-bot-app Bot Sep 9, 2026

Uh oh!

lore-bot-app Bot Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pgherveou commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TarikGul commented Sep 9, 2026

Uh oh!

lore-bot-app Bot commented Sep 9, 2026

Uh oh!

lore-bot-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

lore-bot-app Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

lore-bot-app Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

lore-bot-app Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

lore-bot-app Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pgherveou commented Sep 8, 2026 •

edited

Loading

socket-security Bot commented Sep 8, 2026 •

edited

Loading