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

feat(coin:xrp): apply api change on operations #8987

Merged
merged 13 commits into from
Jan 31, 2025
Merged

Conversation

jprudent
Copy link
Contributor

@jprudent jprudent commented Jan 23, 2025

Sorry, this is a big PR.

I changed the API of "operations" so, it impacted the code on all coins that implements the coin framework. Specifically I changed the semantic of "operations" so that it returns all operations, without pagination. In the future we may paginate using a proper token described in this document https://ledgerhq.atlassian.net/wiki/x/XIGeRAE

For stellar/tezos/xrp coins: I also changed signature of "logic" layer so that the token used for iteration is string not a number. The value of the token is a stringified json representation of the token returned by the underlying "network" layer. It's returned by the listOperations so that it could be reused as input in future implementation.

For stellar and tezos, the "api" is adapted for compilation but not yet ready to use, we'll have 2 respective separate PR for that:

For XRP specifically,

  • the "api" loop over the "logic" using the dedicated token provided by the explorer instead of the block height (that was buggy). There is a hard coded limit over the number of iteration to avoid infinite loop (if there is a bug or something) that could result in freezing users. There is a log when that limit is reached.
  • introduced a order parameter as a workaroud for LIVE-16705

integrations tests: https://github.com/LedgerHQ/ledger-live/actions/runs/13056449704

Copy link

vercel bot commented Jan 23, 2025

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

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2025 8:28am
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2025 8:28am
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2025 8:28am
web-tools ⬜️ Ignored (Inspect) Visit Preview Jan 31, 2025 8:28am

@live-github-bot live-github-bot bot added desktop Has changes in LLD mobile Has changes in LLM common Has changes in live-common ui Has changes in the design system library ledgerjs Has changes in the ledgerjs open source libs tools Has changes in tools automation CI/CD stuff translations Translation files have been touched labels Jan 23, 2025
@jprudent jprudent changed the base branch from main to develop January 23, 2025 14:47
@live-github-bot live-github-bot bot removed desktop Has changes in LLD mobile Has changes in LLM common Has changes in live-common ui Has changes in the design system library ledgerjs Has changes in the ledgerjs open source libs tools Has changes in tools automation CI/CD stuff translations Translation files have been touched labels Jan 28, 2025
@jprudent jprudent changed the title refix xrp feat(coin:xrp): apply api change on operations Jan 28, 2025
@jprudent jprudent marked this pull request as ready for review January 29, 2025 08:42
@jprudent jprudent requested a review from a team as a code owner January 29, 2025 08:42
Copy link
Member

@hedi-edelbloute hedi-edelbloute left a comment

Choose a reason for hiding this comment

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

Thank you ! minor improvements possible but not mandatory

@jprudent
Copy link
Contributor Author

jprudent commented Jan 29, 2025

@hedi-edelbloute I fixed the comments
There are tests that are failing in https://github.com/LedgerHQ/ledger-live/actions/runs/13032147026/job/36353665865#step:7:4009
looks like there are more tx than before, so is it ok to merge ?

@hedi-edelbloute hedi-edelbloute merged commit 5969aaa into develop Jan 31, 2025
54 of 55 checks passed
@hedi-edelbloute hedi-edelbloute deleted the refix_xrp branch January 31, 2025 09:13
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