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

ParaTimes: take into account fee amount when calc max button value #1353

Merged
merged 6 commits into from
Mar 29, 2023

Conversation

buberdds
Copy link
Contributor

@buberdds buberdds commented Mar 27, 2023

Fix "insufficient balance to pay fees" error when using max button.

@github-actions
Copy link

github-actions bot commented Mar 27, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 15 0 0.07s
✅ JSON eslint-plugin-jsonc 1 0 0 0.83s
✅ JSON jsonlint 1 0 0.17s
✅ JSON npm-package-json-lint yes no 0.51s
✅ JSON prettier 1 0 0 0.53s
✅ JSON v8r 1 0 1.96s
✅ REPOSITORY checkov yes no 16.47s
✅ REPOSITORY git_diff yes no 0.0s
✅ TSX eslint 5 0 0 5.39s
✅ TYPESCRIPT eslint 9 0 0 5.7s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@cloudflare-pages
Copy link

cloudflare-pages bot commented Mar 27, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3e5d01
Status: ✅  Deploy successful!
Preview URL: https://83f98973.oasis-wallet.pages.dev
Branch Preview URL: https://mz-paratimemaxvalue.oasis-wallet.pages.dev

View logs

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #1353 (d3e5d01) into master (4a91562) will increase coverage by 0.19%.
The diff coverage is 97.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1353      +/-   ##
==========================================
+ Coverage   84.25%   84.44%   +0.19%     
==========================================
  Files         142      142              
  Lines        3632     3652      +20     
  Branches      667      668       +1     
==========================================
+ Hits         3060     3084      +24     
+ Misses        572      568       -4     
Flag Coverage Δ
cypress 50.96% <11.11%> (-0.20%) ⬇️
jest 79.70% <97.61%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/app/state/paratimes/index.ts 13.33% <ø> (ø)
src/app/state/paratimes/saga.ts 59.64% <0.00%> (-1.07%) ⬇️
src/app/components/ErrorFormatter/index.tsx 100.00% <100.00%> (ø)
src/app/components/Footer/index.tsx 96.29% <100.00%> (+0.14%) ⬆️
src/app/lib/helpers.ts 79.45% <100.00%> (+3.33%) ⬆️
src/app/lib/transaction.ts 68.81% <100.00%> (+1.81%) ⬆️
...pp/pages/ParaTimesPage/TransactionAmount/index.tsx 96.82% <100.00%> (+0.99%) ⬆️
src/app/pages/ParaTimesPage/useParaTimes.ts 92.15% <100.00%> (ø)
.../app/pages/ParaTimesPage/useParaTimesNavigation.ts 92.30% <100.00%> (+0.30%) ⬆️
src/config.ts 100.00% <100.00%> (ø)

src/consensus.ts Outdated Show resolved Hide resolved
@@ -12,6 +12,7 @@ export const initialState: ParaTimesState = {
confirmTransfer: false,
confirmTransferToValidator: false,
confirmTransferToForeignAccount: false,
defaultFeeAmount: '',
Copy link
Member

Choose a reason for hiding this comment

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

How come the default is in state and in TransactionForm? I only expected feeAmount

Copy link
Contributor Author

@buberdds buberdds Mar 28, 2023

Choose a reason for hiding this comment

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

feeAmount is provided by user. defaultFee value should not be visible in UI. With one prop feeAmount we could have scenario like: user is not interacting with fee amount (advance field), advance to next step, click previous, and at this point feeAmount input would have a value of default fee, but user did not interact with an input at all.

Copy link
Member

Choose a reason for hiding this comment

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

ow

src/app/lib/helpers.ts Outdated Show resolved Hide resolved
src/app/pages/ParaTimesPage/TransactionAmount/index.tsx Outdated Show resolved Hide resolved
@buberdds buberdds force-pushed the mz/paraTimeMaxValue branch 3 times, most recently from bc7e73a to d5c4e98 Compare March 28, 2023 17:16
@buberdds buberdds requested a review from lukaw3d March 28, 2023 17:25
@buberdds buberdds merged commit 33087d4 into master Mar 29, 2023
@buberdds buberdds deleted the mz/paraTimeMaxValue branch March 29, 2023 08:14
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.

None yet

2 participants