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

ton jettons integration #7513

Closed
wants to merge 23 commits into from
Closed

ton jettons integration #7513

wants to merge 23 commits into from

Conversation

ayelenmurano
Copy link
Contributor

✅ Checklist

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

📝 Description

This PR adds support for TON jetton transfers into ledger live.

❓ Context

  • JIRA or GitHub link:

🧐 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)

@ayelenmurano ayelenmurano requested review from a team as code owners August 5, 2024 18:48
@vercel Vercel
Copy link

vercel bot commented Aug 5, 2024 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
web-tools ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 6, 2024 3:54pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-docs ⬜️ Ignored (Inspect) Visit Preview Sep 6, 2024 3:54pm
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Sep 6, 2024 3:54pm
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Sep 6, 2024 3:54pm

@vercel Vercel
Copy link

vercel bot commented Aug 5, 2024

@ayelenmurano is attempting to deploy a commit to the LedgerHQ Team on Vercel.

A member of the Team first needs to authorize it.

@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 translations Translation files have been touched labels Aug 5, 2024
@live-github-bot live-github-bot bot added the fork Pull request base branch comes from a fork. label Aug 5, 2024
@ayelenmurano ayelenmurano force-pushed the feat/jetton-integration branch 4 times, most recently from 31a81b3 to e9a6382 Compare August 12, 2024 16:19
@hzheng-ledger hzheng-ledger mentioned this pull request Aug 19, 2024
Merged
3 tasks
@hzheng-ledger hzheng-ledger mentioned this pull request Aug 19, 2024
Closed
3 tasks
hzheng-ledger
hzheng-ledger reviewed Aug 19, 2024
View reviewed changes
libs/coin-modules/coin-ton/src/synchronisation.ts
spendableBalance: new BigNumber(balance),
operations,
operationsCount: operations.length,
pendingOperations: maybeExistingSubAccount ? maybeExistingSubAccount.pendingOperations : [],
Copy link
Contributor

@hzheng-ledger hzheng-ledger Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we don't use pendingOperations in getAccountShape now.
operations with blockheight = null are considered as pending operations

you can just

pendingOperations: [],

I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested and it didn't work as expected if I change the code. When the account is synchronized again (and the transaction wasn't received yet), the pending operation is no longer displayed.
We receive the pendingOperations in getAccountShape and we use them in postSync too.

hzheng-ledger
hzheng-ledger suggested changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@hzheng-ledger hzheng-ledger left a comment

Choose a reason for hiding this comment

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

see my comments. Thanks

This was referenced Aug 26, 2024
Merged
Merged
hzheng-ledger
hzheng-ledger previously approved these changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@hzheng-ledger hzheng-ledger left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@github-actions GitHub Actions
Copy link

There as been no activity on this PR for the last 14 days. Please consider closing this PR.

@github-actions github-actions bot added the Stale label Sep 22, 2024
@github-actions github-actions bot closed this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzheng-ledger hzheng-ledger hzheng-ledger left review comments

Assignees

@ayelenmurano ayelenmurano

Labels
common Has changes in live-common desktop Has changes in LLD fork Pull request base branch comes from a fork. ledgerjs Has changes in the ledgerjs open source libs mobile Has changes in LLM Stale translations Translation files have been touched ui Has changes in the design system library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ayelenmurano @hzheng-ledger