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

LIVE-1292 Cover Polkadot existential deposit edge case #1953

Conversation

alexalouit
Copy link
Contributor

@alexalouit alexalouit commented May 12, 2022

Context (issues, jira)

https://ledgerhq.atlassian.net/browse/LIVE-1292

Description / Usage

When trying to emptied an account, it doesn't work.
This happens when the balance is at ~1 DOT, this is a limitation of the Polkadot blockchain.
The logic of Ledger Live Common was not adapted, this PR fix this.

This PR also contains a fix on the optimistic operation which does not contain the correct type when it is a send max.

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.

Send max must be balances.transfer palletMethod instead of balances.transferKeepAlive
On the Polkadot network, an address is only active when it holds a minimum amount, currently set at 1 DOT.
There's a limitation when you hold ~ 1 DOT.
Use recommended 1.1 DOT value.
@alexalouit alexalouit requested a review from a team as a code owner May 12, 2022 08:32
@vercel
Copy link

vercel bot commented May 12, 2022

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

Name Status Preview Updated
ledger-live-common ✅ Ready (Inspect) Visit Preview May 12, 2022 at 9:36AM (UTC)

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please put the link to the JIRA ticket in the description of the PR + cleanup the title a bit it looks weird 😄

src/families/polkadot/logic.ts Outdated Show resolved Hide resolved
src/families/polkadot/js-signOperation.ts Outdated Show resolved Hide resolved
Don't use ternary operator
Restore EXISTENTIAL_DEPOSIT, use new constant EXISTENTIAL_DEPOSIT_RECOMMENDED_MARGIN to handle recommended margin
@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #1953 (26de976) into develop (5b8d2fc) will decrease coverage by 9.67%.
The diff coverage is 12.50%.

❗ Current head 26de976 differs from pull request most recent head 437622f. Consider uploading reports for the commit 437622f to get more accurate results

@@             Coverage Diff             @@
##           develop    #1953      +/-   ##
===========================================
- Coverage    69.14%   59.46%   -9.68%     
===========================================
  Files          545      510      -35     
  Lines        23534    21132    -2402     
  Branches      6194     5693     -501     
===========================================
- Hits         16273    12567    -3706     
- Misses        7219     8543    +1324     
+ Partials        42       22      -20     
Impacted Files Coverage Δ
src/families/polkadot/js-getTransactionStatus.ts 7.89% <0.00%> (-0.11%) ⬇️
src/families/polkadot/js-signOperation.ts 24.28% <0.00%> (-9.52%) ⬇️
src/families/polkadot/logic.ts 34.34% <100.00%> (-15.66%) ⬇️
src/families/bitcoin/networks.ts 5.88% <0.00%> (-82.36%) ⬇️
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%> (-76.93%) ⬇️
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%) ⬇️
... and 173 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 5b8d2fc...437622f. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

👌 👍

@ghost ghost changed the base branch from develop to family/polkadot May 12, 2022 11:54
@ghost ghost changed the title LIVE-1292 WITH-1 - LIVE-1292-failed-to-submit-transaction-when-trying-to-empty-account-with-1-dot LIVE-1292 Cover Polkadot existential deposit edge case May 12, 2022
@ghost ghost merged commit 01e160a into family/polkadot May 12, 2022
@ghost ghost deleted the LIVE-1292-failed-to-submit-transaction-when-trying-to-empty-account-with-1-dot branch May 12, 2022 11:56
ghost pushed a commit that referenced this pull request May 16, 2022
* LIVE-1292 Cover Polkadot existential deposit edge case (#1953)

* Fix transaction mode mapping

Send max must be balances.transfer palletMethod instead of balances.transferKeepAlive

* Fix 1 DOT Polkadot limitation

On the Polkadot network, an address is only active when it holds a minimum amount, currently set at 1 DOT.
There's a limitation when you hold ~ 1 DOT.
Use recommended 1.1 DOT value.

* Improve code readability

Don't use ternary operator

* Split Existential Deposit and margin

Restore EXISTENTIAL_DEPOSIT, use new constant EXISTENTIAL_DEPOSIT_RECOMMENDED_MARGIN to handle recommended margin

* Polkabot

* trigger bot

* trigger bot

* Change Polkadot bot to 'Mooncake' seed

* Change Polkadot bot to 'Mere Denis' seed

* Change Polkadot bot to 'Silicium' seed

* Update Polkadot bot spec to withdraw staking funds sometimes

* Fix bot condition to do bond transactions

* trigger bot

* trigger bot

* trigger bot

* trigger bot

* trigger bot

* lint - remove unused import

Co-authored-by: Alexandre Alouit <[email protected]>
This pull request was closed.
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