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

Add return value to WithdrawBalance #26

Closed
steven004 opened this issue Nov 3, 2020 · 5 comments
Closed

Add return value to WithdrawBalance #26

steven004 opened this issue Nov 3, 2020 · 5 comments
Labels
FIP0020 Links an existing discussion item to an existing FIP.

Comments

@steven004
Copy link
Contributor

Background
Both miner and market actors have WithdrawBalance methods.

  • WithdrawBalance in Miner actor is for owner to withdraw the vested block rewards, which is the only way the owner could get FIL from a miner actor;
  • WithdrawBalance in Market actor is for clients or providers to withdraw a specified amount from the balance held in escrow.

However, either method is actually an attempt to withdraw a specified amount, it could be successful even when the available balance is less than required amount, that means, the actual amount withdrawn is equal to or less than the amount specified in the method parameters, and the method always return nil, so that, there is no way to get how much amount actually withdrawn if we check the chain status and message info, e.g. from CLI or explorer.

Proposal
Simply add a return value to the method to indicate the actual withdrawn amount.

This will improve the visibility and traceability of FIL flow. This is important especially for miners who need to have a very clear balance sheet and financial report.

@anorth
Copy link
Member

anorth commented Nov 3, 2020

This sounds great to me.

It's part of an open discussion, but this also sounds like a change that's inconsequential enough that we might not need to use the FIP process for it. Since it changes the actors API it will need to wait for the next major release (v3) of actors, but we're planning for that later this year.

@whyrusleeping @dignifiedquire @momack2 should we use the FIP process for this, or just go straight to implementation.

@dignifiedquire
Copy link
Contributor

I would error on the side of creating a FIP for now, even if it is a simple one.

@kaitlin-beegle
Copy link
Collaborator

Hi @anorth! I'm currently working to audit all open FIP discussion items so that we can triage existing issues and flag priorities that may have been missed.

It looks like this issue was lost in the bundle of FIPs that were implemented prior to the Actors v5 launch. Is this the case? If so, we should reintroduce the idea and write a FIP draft.

@anorth
Copy link
Member

anorth commented Aug 8, 2021 via email

@kaitlin-beegle kaitlin-beegle added FIP0020 Links an existing discussion item to an existing FIP. and removed Potential FIP labels Sep 28, 2021
@kaitlin-beegle
Copy link
Collaborator

This was implemented as FIP0020!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIP0020 Links an existing discussion item to an existing FIP.
Projects
None yet
Development

No branches or pull requests

4 participants