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

New Release #84

Merged
merged 40 commits into from
Jul 10, 2024
Merged

New Release #84

merged 40 commits into from
Jul 10, 2024

Conversation

overcat
Copy link
Contributor

@overcat overcat commented May 8, 2024

Checklist

  • App update process has been followed
  • Target branch is develop
  • Application version has been bumped

You can find the changelog here.

Resolve #82

@ledger-wiz-cspm-secret-detection
Copy link

ledger-wiz-cspm-secret-detection bot commented May 8, 2024

Wiz Scan Summary

IaC Misconfigurations 0C 0H 2M 0L 2I
Sensitive Data 0C 0H 0M 0L 0I
Total 0C 0H 2M 0L 2I
Secrets 0🔑

@overcat overcat mentioned this pull request May 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2024

Codecov Report

Attention: Patch coverage is 87.81955% with 162 lines in your changes are missing coverage. Please review.

Project coverage is 89.34%. Comparing base (3d5896b) to head (e9ca974).

Files Patch % Lines
libstellar/parser.c 89.70% 83 Missing ⚠️
libstellar/printer.c 84.61% 78 Missing ⚠️
libstellar/base64.c 94.11% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop      #84       +/-   ##
============================================
+ Coverage    76.87%   89.34%   +12.47%     
============================================
  Files           14        5        -9     
  Lines         2378     2676      +298     
============================================
+ Hits          1828     2391      +563     
+ Misses         550      285      -265     
Flag Coverage Δ
unittests 89.34% <87.81%> (+12.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@overcat
Copy link
Contributor Author

overcat commented May 16, 2024

Perhaps we need to disable the default CodeQL scan configuration. Otherwise, the CodeQL CI will not pass.

https://docs.github.com/en/code-security/code-scanning/troubleshooting-sarif-uploads/default-setup-enabled

This commit does not include any modifications to any functionality.
@tdejoigny-ledger
Copy link
Contributor

Hello @overcat , ty for your PR

Could you please open a PR to update the ledger database (https://github.com/LedgerHQ/ledger-app-database) for Stellar in order to have the guideline CI green ?
Is there any test for the new device ? I didn't find any snapshots..
Last remark, you can probably:

  • use useCaseHomeAndSettings instead of useCaseHome + useCaseSettings
  • use useCaseAddressReview instead of nbgl_useCaseReviewStart + nbgl_useCaseAddressConfirmation

We have a good example in our boilerplate app. It would be more simple & easier to maintain (with the guarantee to be aligned with the design guidelines)

@overcat
Copy link
Contributor Author

overcat commented Jul 10, 2024

Hi @tdejoigny-ledger

Could you please open a PR to update the ledger database (https://github.com/LedgerHQ/ledger-app-database) for Stellar in order to have the guideline CI green ?

I added it a few days ago, and now CI has passed.

Is there any test for the new device ? I didn't find any snapshots.

We use zemu for e2e testing, but it does not yet support Flex, so we may need to wait some time before adding it.
The other four devices all have end-to-end testing.

Last remark, you can probably:
use useCaseHomeAndSettings instead of useCaseHome + useCaseSettings
use useCaseAddressReview instead of nbgl_useCaseReviewStart + nbgl_useCaseAddressConfirmation
We have a good example in our boilerplate app. It would be more simple & easier to maintain (with the guarantee to be aligned with the design guidelines)

I have made the corresponding modifications. Because I also want to make some changes in the UI aspect, originally I planned to submit them together at that time, but now let's first submit this part.

CodeQL test failure is likely due to repository configuration issues: #84 (comment), it is running well in my repository: https://github.com/lightsail-network/app-stellar/actions/runs/9869736057

@tdejoigny-ledger
Copy link
Contributor

Hi @tdejoigny-ledger

Could you please open a PR to update the ledger database (https://github.com/LedgerHQ/ledger-app-database) for Stellar in order to have the guideline CI green ?

I added it a few days ago, and now CI has passed.

Is there any test for the new device ? I didn't find any snapshots.

We use zemu for e2e testing, but it does not yet support Flex, so we may need to wait some time before adding it. The other four devices all have end-to-end testing.

Last remark, you can probably:
use useCaseHomeAndSettings instead of useCaseHome + useCaseSettings
use useCaseAddressReview instead of nbgl_useCaseReviewStart + nbgl_useCaseAddressConfirmation
We have a good example in our boilerplate app. It would be more simple & easier to maintain (with the guarantee to be aligned with the design guidelines)

I have made the corresponding modifications. Because I also want to make some changes in the UI aspect, originally I planned to submit them together at that time, but now let's first submit this part.

CodeQL test failure is likely due to repository configuration issues: #84 (comment), it is running well in my repository: https://github.com/lightsail-network/app-stellar/actions/runs/9869736057

Ok ty @overcat I will approve & merge this PR

@tdejoigny-ledger tdejoigny-ledger merged commit 499a9e2 into LedgerHQ:develop Jul 10, 2024
131 of 134 checks passed
@overcat overcat deleted the publish branch July 22, 2024 02:12
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.

Cannot sign InvokeHostFunction operations with Ledger Nano S
3 participants