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.

Sign up for GitHub

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

Jump to bottom

Generic clear-signing #699

Merged
merged 45 commits into from
Dec 19, 2024
Merged

Generic clear-signing #699

merged 45 commits into from
Dec 19, 2024

Conversation

apaillier-ledger
Copy link
Contributor

Description

Added new feature for clear-signing that will replace existing plugins (for the most part).

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Tests
  • Documentation
  • Other (for changes that might not fit in any category)

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 16, 2024
View reviewed changes
src_features/signTx/cmd_signTx.c Dismissed Show dismissed Hide dismissed
src_features/signTx/cmd_signTx.c Dismissed Show dismissed Hide dismissed
@apaillier-ledger apaillier-ledger force-pushed the feat/apa/gcs branch 10 times, most recently from 0f7c689 to b8262b4 Compare December 16, 2024 16:25
cedelavergne-ledger
cedelavergne-ledger requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@cedelavergne-ledger cedelavergne-ledger left a comment

Choose a reason for hiding this comment

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

Seems that there is some duplicates in the different handlers (like handle_version, handle_id, maybe handle_struct...)
I think the Info icon could be used from the SDK ?

Sorry, but I didn't go deeper in each file, this PR is quite complex 😜

client/src/ledger_app_clients/ethereum/command_builder.py Show resolved Hide resolved
src_features/generic_tx_parser/cmd_field.c Show resolved Hide resolved
src_features/generic_tx_parser/cmd_tx_info.c Show resolved Hide resolved
src_features/generic_tx_parser/cmd_tx_info.c Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 17, 2024
View reviewed changes
src_features/generic_tx_parser/gtp_field.c Dismissed Show dismissed Hide dismissed
src_features/generic_tx_parser/gtp_tx_info.c Dismissed Show dismissed Hide dismissed
@apaillier-ledger apaillier-ledger force-pushed the feat/apa/gcs branch 2 times, most recently from 4675744 to 345dc32 Compare December 18, 2024 15:51
@apaillier-ledger apaillier-ledger marked this pull request as ready for review December 18, 2024 17:02
bboilot-ledger
bboilot-ledger approved these changes Dec 19, 2024
View reviewed changes
Copy link

@bboilot-ledger bboilot-ledger left a comment

Choose a reason for hiding this comment

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

🎉

@apaillier-ledger apaillier-ledger merged commit 3a2354b into develop Dec 19, 2024
174 checks passed
@apaillier-ledger apaillier-ledger deleted the feat/apa/gcs branch December 19, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bboilot-ledger bboilot-ledger bboilot-ledger approved these changes

@cedelavergne-ledger cedelavergne-ledger cedelavergne-ledger left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apaillier-ledger @cedelavergne-ledger @bboilot-ledger