Skip to content
This repository has been archived by the owner on Jul 15, 2022. It is now read-only.

[Celo integration] Fix amount calculation in send max when fees exceed spendable amount #1900

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

pawelnguyen
Copy link
Contributor

Context (issues, jira)

This fixes amount calculation in send max when fees exceed spendable amount. In this case we just set amount to 0.

Before:

CleanShot 2022-04-19 at 12 07 31

After:

CleanShot 2022-04-19 at 12 10 03

Expectations

  • Test coverage: The changes of this PR are covered by test. Unit test were added with mocks when depending on a backend/device.
  • No impact: The changes of this PR have ZERO impact on the userland. Meaning, we can use these changes without modifying LLD/LLM at all. It will be a "noop" and the maintainers will be able to bump it without changing anything.

@pawelnguyen pawelnguyen requested a review from a team as a code owner April 19, 2022 10:16
@vercel
Copy link

vercel bot commented Apr 19, 2022

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

Name Status Preview Updated
ledger-live-common ❌ Failed (Inspect) Apr 19, 2022 at 8:54PM (UTC)

@pawelnguyen
Copy link
Contributor Author

@haammar-ledger @henrily-ledger 🙇

@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #1900 (19d9db2) into develop (8208e75) will decrease coverage by 9.12%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #1900      +/-   ##
===========================================
- Coverage    68.54%   59.42%   -9.13%     
===========================================
  Files          545      510      -35     
  Lines        23506    21105    -2401     
  Branches      6184     5687     -497     
===========================================
- Hits         16113    12542    -3571     
- Misses        7352     8542    +1190     
+ Partials        41       21      -20     
Impacted Files Coverage Δ
src/families/celo/js-getTransactionStatus.ts 16.66% <0.00%> (-69.55%) ⬇️
src/families/crypto_org/api/sdk.ts 16.85% <0.00%> (-78.66%) ⬇️
src/families/celo/hw-app-celo.ts 3.38% <0.00%> (-77.97%) ⬇️
src/families/bitcoin/js-signOperation.ts 21.53% <0.00%> (-75.39%) ⬇️
src/families/bitcoin/networks.ts 5.88% <0.00%> (-74.51%) ⬇️
src/families/celo/js-getFeesForTransaction.ts 26.66% <0.00%> (-73.34%) ⬇️
src/hw/signTransaction/ripple.ts 27.27% <0.00%> (-72.73%) ⬇️
src/families/celo/js-buildTransaction.ts 27.27% <0.00%> (-72.73%) ⬇️
src/families/elrond/api/sdk.ts 22.64% <0.00%> (-71.70%) ⬇️
src/families/solana/js-broadcast.ts 28.57% <0.00%> (-71.43%) ⬇️
... and 169 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8208e75...19d9db2. Read the comment docs.

@henri-ly
Copy link
Contributor

@pawelnguyen is it possible to sign the commit otherwise we can't merge it :(

@pawelnguyen
Copy link
Contributor Author

@henrily-ledger sure, commit signed :)

@henri-ly
Copy link
Contributor

Ok we are now doing QA on our side to be sure everything is alright ! Once it done we will merged it

@henri-ly henri-ly merged commit 478ff47 into LedgerHQ:develop Apr 20, 2022
@pawelnguyen pawelnguyen deleted the fix-celo-send-max branch April 20, 2022 14:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants