Skip to content

Add display impl for errors variants in apps - #648

Open
bit-aloo wants to merge 5 commits into
stratum-mining:mainfrom
bit-aloo:2026-07-27-add-display-impl-for-errors
Open

Add display impl for errors variants in apps#648
bit-aloo wants to merge 5 commits into
stratum-mining:mainfrom
bit-aloo:2026-07-27-add-display-impl-for-errors

Conversation

@bit-aloo

@bit-aloo bit-aloo commented Jul 27, 2026

Copy link
Copy Markdown
Member

I don't really like seeing phantom types in error. Its an internal detail, and shouldn't be exposed in logs. This PR adds impl display trait for error variants across apps.

@bit-aloo
bit-aloo force-pushed the 2026-07-27-add-display-impl-for-errors branch from 60e7f44 to 3487c7e Compare July 27, 2026 04:02

@par1ram par1ram left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the four error hierarchies and the logging call-site conversions; I found no blocking correctness issues. On the current merge with main, the 33 existing jd-client/translator tests pass, the pool/JDS crates build and test successfully, and four temporary contract tests confirmed that representative Display outputs preserve owner/action/cause context without leaking PhantomData.

Non-blocking: please consider retaining downstream_id as a dedicated tracing field in the Action::Disconnect branches even though it is included in %e; this keeps operational filtering intact. A small table-driven Display test would also make this new logging contract explicit, especially since pool and JDS currently have no unit tests.

@GitGab19
GitGab19 self-requested a review July 28, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants