refactor: consolidate Rust boilerplate and move runtime modules - #617
refactor: consolidate Rust boilerplate and move runtime modules#617pgherveou wants to merge 10 commits into
Conversation
Reuse derive_more Debug, Display and Error, standard Default, zeroization derives, and thiserror conversions already established in the workspace. Field annotations retain secret redaction and selective wiping; optional mnemonic Debug still distinguishes None from a redacted Some. Session Debug now redacts entropy/private keys. Derived wrapped errors expose their sources. No session, authorization, storage, or transport control flow changes.
Extract the common pallet-call, constant and storage-value macro expansion used by metadata V14, V15 and V16. Keep version-specific extrinsic extensions and V16 view functions explicit. This removes repeated token expansion without changing collection order or decoded metadata.
Move the existing Rust bridge traversal into platform_callbacks and reuse it from the TypeScript callback emitter. Preserve the fixed-point walk over local aliases, fields, enum variants, stream items, and generic arguments; only the TypeScript boundary converts borrowed names into owned strings. Protocol normalization and frame-ID policy remain unchanged.
Keep owned argument vectors for asynchronous bridge calls and emit stack-backed slices for synchronous calls. Both use the same argument-expression builder. Update the generated bridge and golden fixture; callback payload encoding and invocation order stay the same.
Replace the unnamed platform/chat/permission-status tuple with a struct and destructure it at the two callers. Remove a redundant borrow and flatten an equivalent error-extraction conditional. Optional-adapter detection and runtime construction are unchanged.
Move existing product-facing trait implementations into capability modules and move the shared runtime fixture and signing tests into test modules. Preserve method bodies, validation order, visibility, and error mapping. No authorization helper extraction or lifecycle change is included in this commit.
e03045d to
8e0f50f
Compare
pgherveou
changed the title
|
@lore-bot-app review |
|
Reading the diff and checking what the record says. Back in a few minutes. |
![lore-bot-app[bot]](https://avatars.githubusercontent.com/u/14176906?s=60&v=4){kind=small}
There was a problem hiding this comment.
TL;DR
Mechanical refactor: handwritten Debug/Display/Error/Drop/Default impls become derives, duplicated codegen and metadata-macro code is shared, and the 14 ProductRuntimeHost trait impls plus their tests move out of runtime.rs into runtime/capabilities/* and runtime/tests*. 0 blocking, 4 minor.
Summary
The diff replaces manual trait impls with derive_more (Debug with per-field #[debug("\"<redacted>\"")] for secret fields, Display, Error), zeroize::ZeroizeOnDrop, thiserror #[from], and #[derive(Default)]; extracts collect_local_bridge_payload_types into platform_callbacks.rs so the Rust WASM bridge and TS host-callback generators share one traversal; borrows sync JS callback args as &[..] instead of &vec![..]; factors a collect_pallets! macro out of the two metadata macros; and moves every product-facing capability impl into its own module under runtime/capabilities/, with tests moved to runtime/tests.rs and runtime/tests/signing.rs. I verified the move mechanically against the merge base (696fd73): all 14 impl blocks are byte-identical apart from module headers and one blank line in payment.rs, and test count is unchanged (120 before, 98 + 22 after). Four files were truncated from the diff I was given: runtime/tests.rs, runtime/tests/signing.rs, wasm.rs, wasm/generated_bridge.rs. I did not review the test bodies. From the local checkout, wasm.rs replaces a 3-tuple return with a WasmPlatformAdapters struct plus two clippy-style cleanups, and generated_bridge.rs is the 2-line &vec! to &[..] regeneration. I could not run cargo check/clippy in this sandbox, so compile and lint claims rest on the PR's own CI.
What the record says
- This is PR #617, which Lore records as merged, with PR #618 stacked on it for the actual logic changes. The PR's claim that the 14 moved bodies are byte-for-byte identical checks out.
- The move invalidates line anchors in the open issue #343 ("short-circuit confirmation gates at runtime.rs:1316, :1369, :1422"). Those gates now live in
runtime/capabilities/signing.rs. - The repo's
AGENTS.mdasks forderive_more::Displayover handwrittenfmt::Display. This PR applies the same rule toDebugandError, which is consistent with it. - Lore has no prior discussion of the Debug-redaction or zeroize design in
truapi-host-cli, or of splittingruntime.rs. The record is silent there.
Concerns
rust/crates/truapi-server/src/runtime.rs:828,:1006,:1236. Comments describing the Account, Chain, "Deferred product surfaces", and Notifications impls stayed behind after the impls moved and now sit above unrelated code (connected_session_ui_info, thePAYMENTS_NOT_IMPLEMENTEDconst,mod tests;). CLAUDE.md says comments describe only the current code. Move each to its impl in the capability module or delete it.rust/crates/truapi-server/src/host_logic/session.rs:32and:38.#[debug("\"<redacted>\"")]onOption<[u8; 32]>prints the same text forNoneandSome, so a log line can no longer show whether a session has an entropy source or chat key. The same PR keeps theOptionshape forResolveSignerConfig::mnemonicinaccounts.rs:88. Suggest the same pattern here so "Session secret missing" stays diagnosable.docs/issue-drafts/bulletin-preimage-in-core.md:306still pointsPreimage::submitattruapi-server/src/runtime.rs; it is now inruntime/capabilities/preimage.rs. CLAUDE.md asks for anrgof old paths on every move.rust/crates/truapi-codegen/src/platform_callbacks.rs:210.collect_local_bridge_payload_typesbecamepub(crate)without a doc comment. Its siblingsto_camel_caseandsnake_casehave one, and CLAUDE.md requires one on everypubitem.
Questions for the author
- The capability modules import the parent with
use super::super::*;, so they depend onruntime.rskeeping every type import alive. Is that the intended long-term shape, or a stopgap until #618 lands and the modules get explicit imports?
Next: fix the three stale comment blocks in runtime.rs (about 5 minutes), then the Option redaction in session.rs.
🤖 Reviewed by Lore (Parity knowledge base) · 68 agent turns · 656.1s · knowledge as of 2026-09-07
Keep capability descriptions beside their implementations, remove the obsolete deferred-surface narrative, and update the preimage draft and runtime layout documentation. Document the shared bridge traversal and its transitive payload closure. A repository-wide search found no other references to runtime.rs that need updating.
Use the existing derive_more Option redaction pattern for session entropy and chat keys. Debug retains None versus Some while redacting secret bytes, matching the CLI mnemonic diagnostic behavior. Test all four presence combinations in compact and alternate Debug output; all session tests pass.
Import protocol envelopes, platform types, and authority requests directly in each capability module. Shared runtime helpers remain explicit imports from their owner; the runtime no longer serves as an implicit prelude for every adapter. Move test-only protocol imports beside the fixtures. All 14 capability implementation bodies remain byte-for-byte unchanged. The server all-features suite passes 833 tests, and workspace Clippy passes for every target and feature with warnings denied.
|
@copilot resolve the merge conflicts in this pull request |
…fixes # Conflicts: # rust/crates/truapi-server/src/wasm/generated_bridge.rs Co-authored-by: pgherveou <521091+pgherveou@users.noreply.github.com>
Resolved the merge conflict by accepting main’s removal of the generated WASM bridge, then finalized merge commit |
Replace handwritten declarative trait implementations with existing derives, share metadata macro expansion and platform-codegen traversal, and move runtime implementations/tests into modules. The nine commits separate each mechanical change; all 14 moved capability implementation bodies are byte-for-byte unchanged.
Validation:
cargo test --workspace --all-features --locked --offline: 1,260 passed, 20 ignored.cargo +nightly clippy --workspace --all-targets --all-features -- -D warnings: passed.The derive commit explicitly calls out redacted session Debug output and wrapped error-source propagation. The WASM generator cleanup changes only temporary argument allocation. No wire IDs or payload layouts change.
Review follow-up validation: all 833 server tests pass with all features, including the optional-secret redaction regression. Workspace Clippy (all targets/features), WASM server Clippy, and nightly formatting pass. Capability dependencies are explicit imports; all 14 implementation bodies remain unchanged.