Skip to content

Conversation

felipemadero
Copy link
Contributor

Why this should be merged

This refactoring improves code organization by separating ledger-specific implementation from the generic keychain interfaces. The current structure mixes generic keychain interfaces with ledger-specific implementation details in the same package, making it harder to maintain and extend.

How this works

  • Package reorganization: All ledger-related functionality (ledgerKeychain, ledgerSigner, ledger interfaces, tests, and mocks) is moved to utils/crypto/keychain/ledger/
  • Interface preservation: The core Signer and Keychain interfaces remain in the main keychain package unchanged
  • Ledger device constructor now returns a concrete *Ledger struct while the ledger keychain package defines a Ledger interface for its dependency needs, enabling clean testing and dependency injection

How this was tested

  • unit tests

Need to be documented in RELEASES.md?

- Move ledger keychain from utils/crypto/keychain to utils/crypto/keychain/ledger/
@Copilot Copilot AI review requested due to automatic review settings September 25, 2025 17:21
Copy link
Contributor

@Copilot 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 codebase by extracting ledger-specific keychain functionality from the generic keychain package into a dedicated utils/crypto/keychain/ledger/ package, improving code organization and separation of concerns.

Key changes:

  • Moves all ledger-related implementations (keychain, signer, interfaces, tests, mocks) to dedicated ledger package
  • Updates ledger device constructor to return concrete type while maintaining interface-based design in the ledger package
  • Renames functions to remove "Ledger" prefix since they're now in a ledger-specific package

Reviewed Changes

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

Show a summary per file
File Description
utils/crypto/ledger/ledger.go Updates constructor to return concrete *Ledger type and removes keychain interface dependency
utils/crypto/keychain/ledger/mocks_generate_test.go Updates package declaration for moved mock generation
utils/crypto/keychain/ledger/ledgermock/ledger.go Updates generated mock to reflect new package structure
utils/crypto/keychain/ledger/ledger_keychain_test.go Updates tests to use new package and function names
utils/crypto/keychain/ledger/ledger_keychain.go New implementation file containing moved ledger keychain logic
utils/crypto/keychain/ledger/ledger.go Defines Ledger interface for the ledger package with verification
utils/crypto/keychain/keychain.go Removes ledger-specific implementations, keeping only generic interfaces

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants