feat: fix logic and refactor app install

[chrisduma-ledger]

✅ Checklist

• npx changeset was attached.
• Covered by automatic tests.
• Impact of the changes:
  • ...

📝 Description

We had an issue where if the app was not installed the sell/card would crash. This PR fixes it.
Also, i have refactored the logic a little.

❓ Context

Ticket

---

🧐 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.
• Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-ui-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 25, 2024 8:48am
web-tools	❌ Failed (Inspect)			Nov 25, 2024 8:48am

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
ledger-live-github-bot	⬜️ Ignored (Inspect)	Visit Preview		Nov 25, 2024 8:48am
native-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview		Nov 25, 2024 8:48am

[CremaFR]

I've done the QA for swaps no issue ;)

[sarneijim]

We need applied the same change for FUND.
If we can merge today, tomorrow you need to deliver a new exchangeSDK version (patch) and send it to Mercuryo and Moonpay.

[Justkant]

Looks like you removed a recent fix I did here, any reason we removed it ?

[chrisduma-ledger]

Hello. Yes, I have removed that and just added this part:
dependencies.push({ appName: mainFromAccount?.currency?.managerAppName });

It makes the code easier to read, and adds any app as needed. @CremaFR tested it for swaps, and it works well.

[Justkant]

Yes but we need to keep the above logic to add ethereum in case of avax as it's needed for swap
And instead of doing a specific logic for only one coin we do a semi-generic thing here to add ethereum in the deps for evm families
https://ledgerhq.atlassian.net/browse/LIVE-14666

[CremaFR]

Could you please add a comment next time to explain such changes? This was incredibly obscure, and no one could have guessed it without context.
Even just linking the JIRA ticket would have been enough, as we often do for similar quirks in swaps https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-desktop/src/renderer/screens/exchange/Swap2/Form/index.tsx#L388

[CremaFR]

Or put in a function like addAvaxAppDep

[CremaFR]

And I'll make a PR to add PTX owner of this file too from now on

[Justkant]

Yes sure, the name of the variable and the logic was not that hard to follow IMO and I understand that you probably wanted to remove this but if something is obscure or doesn't make sense I usually find the author of the change and try to get answers before doing a change if none are provided in the source code
I'll keep this in mind if I end up in a similar situation, as I usually add context if needed for strange changes

[CremaFR]

You're right, could’ve reached out first. Now that I know about managerAppName not being reliable for swap, it makes sense.
but seeing appManagerName to use directly instead of getting accounts like

const dependencies: AppRequest["dependencies"] = [
          {
            account: mainFromAccount,
          },
          {
            account: mainToAccount,
          },
        ];

I assume your fix was an issue with accounts not installing correct apps and you just didn't refacto the thing, and that we should just have used managerAppName directly...

anyway invited you to #ptx-pr-review , it's the kind of changes I like to follow and know about 😹