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

fix: ux: exclude negative available balance from spendable amount #9182

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

jennijuju
Copy link
Member

@jennijuju jennijuju commented Aug 18, 2022

Related Issues

resolves #9169

Proposed Changes

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • All commits have a clear commit message.
  • The PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
    • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
  • This PR has tests for new functionality or change in behaviour
  • If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
  • CI is green

@jennijuju jennijuju requested a review from a team as a code owner August 18, 2022 13:33
Comment on lines +296 to +298
if availBalance.GreaterThan(big.Zero()) {
spendable = big.Add(spendable, availBalance)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right spot to make this change. We still want to display a negative miner balance if it exists, we just don't want to add a negative miner balance to the worker balance on line 322.

Copy link
Member Author

@jennijuju jennijuju Aug 18, 2022

Choose a reason for hiding this comment

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

this doesnt impact the miner balance tho, like we are not changing the value of availBalance?
spendable is only printed out on line 337 - the total balance spendable

Copy link
Contributor

@geoff-vball geoff-vball Aug 18, 2022

Choose a reason for hiding this comment

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

Oh, you're right, this does work. I just hate the way this function it was originally coded - very confusing.

@jennijuju jennijuju merged commit e165731 into master Aug 18, 2022
@jennijuju jennijuju deleted the jen/9169 branch August 18, 2022 18:44
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.

lotus-miner info total spendable amount calculated wrong
2 participants