Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoupling business layer from hardware layer in Signer component #177

Conversation

amendelzon
Copy link
Collaborator

@amendelzon amendelzon commented May 21, 2024

  • Added HAL headers
  • Added HAL implementation for Ledger
  • Refactored necessary common sources
  • Refactored necessary Signer sources
  • Updated Signer Makefile to include HAL code
  • Removed HSM_SIMULATOR define in favor of HSM_PLATFORM_* defines
  • Included X86 (partial) implemenations of some HAL modules (exceptions, logging, hashing)
  • Renamed UI communication module to avoid name clashing with HAL
  • Changed platform define in UI Makefile
  • Added linting to HAL

All physical Ledger tests currently passing.

- Added HAL headers
- Added HAL implementation for Ledger
- Refactored necessary common sources
- Refactored necessary Signer sources
- Updated Signer Makefile to include HAL code
- Removed HSM_SIMULATOR define in favor of HSM_PLATFORM_* defines
- Included X86 (partial) implemenations of some HAL modules (exceptions, logging, hashing)
@amendelzon amendelzon requested a review from italo-sampaio May 21, 2024 13:38
@amendelzon amendelzon force-pushed the improvement/decouple-business-hardware--signer branch from a10ea6a to 791382b Compare May 21, 2024 13:51
- Renamed UI communication module to avoid name clashing with HAL
- Changed platform define in Makefile
@amendelzon amendelzon force-pushed the improvement/decouple-business-hardware--signer branch from 791382b to a376ddd Compare May 21, 2024 13:53
- Also fixing a broken symlink error
@amendelzon amendelzon force-pushed the improvement/decouple-business-hardware--signer branch from 3432e6e to 197bc1f Compare May 21, 2024 14:17
Copy link
Collaborator

@italo-sampaio italo-sampaio left a comment

Choose a reason for hiding this comment

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

LGTM!

@amendelzon amendelzon merged commit 91d99cf into improvement/decouple-business-hardware May 23, 2024
2 of 4 checks passed
@amendelzon amendelzon deleted the improvement/decouple-business-hardware--signer branch May 23, 2024 14:13
amendelzon added a commit that referenced this pull request Jun 18, 2024
* Decoupling business layer from hardware layer in Signer component (#177)

- Added HAL headers
- Added HAL implementation for Ledger
- Refactored necessary common sources
- Refactored necessary Signer sources
- Updated Signer Makefile to include HAL code
- Removed HSM_SIMULATOR define in favor of HSM_PLATFORM_* defines
- Included X86 (partial) implemenations of some HAL modules (exceptions, logging, hashing)
- Renamed UI communication module to avoid name clashing with HAL
- Changed platform define in Makefile

* Decoupling business layer from hardware layer in TCPSigner (#179)

- Implementing HAL for x86 based on the existing TCPSigner implementation
- Additional x86-only functions for HAL implementation
- Updating TCPSigner implementation to use HAL
- Added TCPSigner-specific UI dependencies to implement functions needed by UI only code
- Factoring HAL constants into a constants file
- New BIP32 parser for TCPSigner seed file
- Ensuring Firmware tests pass both on Ledger Nano S and TCPSigner
- Linting

* Updating directory structure to accomodate for multiple platforms (#180)

- Moved 'ledger' directory to 'firmware'
- Unified Makefile, source and tests directory structure
- Updated Makefiles to account for new (unified) directory structure
- Updated paths within scripts
- Updated github workflows
- Updated script names to account for underlying platform
- Updated documentation references
- New 'run all' script for tcpsigner unit tests
- Incidentally fixing cross-module responsibility includes
- Incidentally fixing version number in Ledger app deployment scripts

* Firmware unit test fixing (#181)

- Fixing existing unit tests
- New unit tests for the TCPSigner BIP32 library
- Moving TCPSigner unit tests to HAL (TCPSigner is now basically a shell)
- Moving Signer unit tests to PowHSM
- Fixing code coverage generation to:
  - Forcibly terminate the TCPSigner after tests run
  - Ignore unit test sources
- Unifying firmware .gitignore files
- Organising common unit test Makefile rules into common.mk files
- Removed unnecessary testing comments
- Incidentally fixing some include style (angle bracket x quotes)

* Updating documentation to reflect the possibility of multiple platform implementations (#182)
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