Skip to content

🎨 (clear-sign-tester) [DSDK-1062]: Make clear-sign-tester chain-agnostic#1374

Open
fAnselmi-Ledger wants to merge 3 commits intodevelopfrom
feat/dsdk-1062-make-clear-sign-tester-agnostic
Open

🎨 (clear-sign-tester) [DSDK-1062]: Make clear-sign-tester chain-agnostic#1374
fAnselmi-Ledger wants to merge 3 commits intodevelopfrom
feat/dsdk-1062-make-clear-sign-tester-agnostic

Conversation

@fAnselmi-Ledger
Copy link
Contributor

@fAnselmi-Ledger fAnselmi-Ledger commented Mar 20, 2026

πŸ“ Description

  • Split SigningService interface into TransactionSigningService and TypedDataSigningService, removing all SignerEth imports from the domain layer.
  • Unify input models with a SignableInput discriminated union so shared infrastructure handles any signing input generically.
  • Replace DeviceRepository.performSignTransaction() / performSignTypedData() with a single performSign() that dispatches by input kind.
  • Make FlowOrchestrator configurable by injecting which user interactions trigger signing via DI, instead of hardcoding them.
  • Parameterise the Speculos app name so it can load any Ledger app, not just Ethereum.

This is the first epic related to this study https://ledgerhq.atlassian.net/wiki/spaces/WXP/pages/6922174534/Solana+Signer+Support+for+Clear-Sign-Tester

❓ Context

βœ… Checklist

Pull Requests must pass CI checks and undergo code review. Set the PR as Draft if it is not yet ready for review.

  • Covered by automatic tests
  • Changeset is provided
  • Documentation is up-to-date
  • Impact of the changes:
    • list of the changes

🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.

Copilot AI review requested due to automatic review settings March 20, 2026 13:47
@fAnselmi-Ledger fAnselmi-Ledger requested a review from a team as a code owner March 20, 2026 13:47
@vercel
Copy link

vercel bot commented Mar 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
device-sdk-ts-sample Ready Ready Preview, Comment Mar 20, 2026 5:10pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
doc-device-management-kit Ignored Ignored Mar 20, 2026 5:10pm

Request Review

@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2026

Danger Check Results

Messages

⚠️

No changeset file found. Please make sure this is intended or add a changeset file.

βœ…

Danger: All checks passed successfully! πŸŽ‰

Generated by 🚫 dangerJS against 93d75d5

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the clear-signing-tester app to be chain-agnostic by splitting the signing abstractions, introducing a discriminated SignableInput union, and making orchestration/signing dispatch configurable via DI rather than hardcoded Ethereum assumptions.

Changes:

  • Split SigningService into TransactionSigningService and TypedDataSigningService, and propagate SignableInput through the domain/application/infrastructure layers.
  • Replace DeviceRepository.performSignTransaction() / performSignTypedData() with a single performSign() that dispatches by input.kind.
  • Parameterize Speculos app loading via SpeculosConfig.appName and inject β€œsignable interactions” into DefaultFlowOrchestrator.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
apps/clear-signing-tester/src/infrastructure/state-handlers/StateHandler.ts Updates handler input type to SignableInput.
apps/clear-signing-tester/src/infrastructure/state-handlers/SignTransactionStateHandler.ts Updates handler signature to accept SignableInput.
apps/clear-signing-tester/src/infrastructure/state-handlers/OptOutStateHandler.ts Updates handler signature to accept SignableInput.
apps/clear-signing-tester/src/infrastructure/state-handlers/ErrorStateHandler.ts Updates handler signature to accept SignableInput.
apps/clear-signing-tester/src/infrastructure/state-handlers/CompleteStateHandler.ts Updates handler signature to accept SignableInput.
apps/clear-signing-tester/src/infrastructure/services/DefaultSigningService.ts Implements new signing interfaces; keeps concrete signer wiring.
apps/clear-signing-tester/src/infrastructure/services/DefaultFlowOrchestrator.ts Accepts SignableInput and injects signable interactions set.
apps/clear-signing-tester/src/infrastructure/service-controllers/SpeculosServiceController.ts Adds configurable app name resolution for Speculos app loading.
apps/clear-signing-tester/src/infrastructure/service-controllers/DMKServiceController.ts Switches signing service injection to concrete DefaultSigningService.
apps/clear-signing-tester/src/infrastructure/repositories/TypedDataFileRepository.ts Emits TypedData inputs with kind discriminant.
apps/clear-signing-tester/src/infrastructure/repositories/TransactionFileRepository.ts Emits transaction inputs with kind discriminant.
apps/clear-signing-tester/src/infrastructure/repositories/SpeculosDeviceRepository.ts Consolidates signing entrypoint to performSign() and dispatches by kind.
apps/clear-signing-tester/src/index.ts Reorders/extends public exports to include signable models.
apps/clear-signing-tester/src/domain/types/TestStatus.ts Updates TestResult.input to SignableInput.
apps/clear-signing-tester/src/domain/services/TypedDataSigningService.ts Introduces typed-data signing interface.
apps/clear-signing-tester/src/domain/services/TransactionSigningService.ts Introduces transaction signing interface + shared result type.
apps/clear-signing-tester/src/domain/services/SigningService.ts Removes old combined signing interface.
apps/clear-signing-tester/src/domain/services/FlowOrchestrator.ts Updates orchestrator to accept SignableInput and new result type import.
apps/clear-signing-tester/src/domain/repositories/DeviceRepository.ts Replaces two sign methods with single performSign(SignableInput, ...).
apps/clear-signing-tester/src/domain/models/TypedDataInput.ts Adds kind discriminant to typed data input.
apps/clear-signing-tester/src/domain/models/TransactionInput.ts Adds kind discriminant to transaction input.
apps/clear-signing-tester/src/domain/models/SignableInputKind.ts Adds enum discriminant for signable inputs.
apps/clear-signing-tester/src/domain/models/SignableInput.ts Adds discriminated union type for signable inputs.
apps/clear-signing-tester/src/domain/models/config/SpeculosConfig.ts Adds appName config option for selecting Ledger app to load.
apps/clear-signing-tester/src/di/types.ts Adds DI tokens for split signing services + signable interactions set.
apps/clear-signing-tester/src/di/modules/infrastructureModuleFactory.ts Rebinds signing services and provides signable interactions set via DI.
apps/clear-signing-tester/src/cli/EthereumTransactionTesterCli.ts Adds kind when constructing signable inputs.
apps/clear-signing-tester/src/application/usecases/TestTypedDataUseCase.ts Switches to deviceRepository.performSign().
apps/clear-signing-tester/src/application/usecases/TestTransactionUseCase.ts Switches to deviceRepository.performSign().
apps/clear-signing-tester/src/application/usecases/TestContractUseCase.ts Switches to performSign() and adds transaction kind discriminant.
apps/clear-signing-tester/src/application/usecases/TestBatchTypedDataFromFileUseCase.ts Switches to performSign() in batch runner.
apps/clear-signing-tester/src/application/usecases/TestBatchTransactionFromFileUseCase.ts Switches to performSign() in batch runner.
apps/clear-signing-tester/src/application/usecases/TestBatchFromFileUseCase.ts Generalizes batch runner generic bound to SignableInput.
apps/clear-signing-tester/src/application/usecases/TestBatchContractFromFileUseCase.ts Adds kind to synthesized TestResult.input rows.

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@aussedatlo aussedatlo left a comment

Choose a reason for hiding this comment

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

Don't forget to run nightly jobs to see if everything is working as intended

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.

3 participants