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

implement getbalance rpc call #26

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@evgeniy-scherbina
Copy link
Contributor

evgeniy-scherbina commented Sep 26, 2018

No description provided.

@thomaseizinger
Copy link
Member

thomaseizinger left a comment

Thank you for the contribution!

We want to represent the actual JSON-RPC API as close as possible. This includes exposing all the parameters of function calls.

@@ -164,6 +164,10 @@ impl BitcoinRpcApi for BitcoinCoreClient {
))
}

fn get_balance(&self) -> Result<Result<f32, RpcError>, HTTPError> {

This comment has been minimized.

@thomaseizinger

thomaseizinger Sep 26, 2018

Member

According to the API here: https://bitcoin.org/en/developer-reference#getbalance, getbalance has 3 parameters:

  • an account (although this one is deprecated, I think we should still add it for now) it is an optional parameter, so we should probably accept an Option<String> and default to * as described in the docs if it is missing.
  • minimum number of confirmations
  • whether to include watch-only addresses

Please add these 3 parameters to the method.

@thomaseizinger

This comment has been minimized.

Copy link
Member

thomaseizinger commented Oct 20, 2018

Please resolve the merge conflicts. In addition could you also please squash those changes together into one commit?

@bonomat bonomat added this to the Sprint 4 🎄🎅🏿 milestone Dec 20, 2018

@D4nte D4nte removed this from the Sprint 4 🎄🎅🏿 milestone Jan 8, 2019

@D4nte D4nte added this to the Sprint 5 📜🔓 milestone Jan 9, 2019

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Jan 10, 2019

Hi @evgeniy-scherbina, I'll take over your change due to lack of activity.

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Jan 10, 2019

Replaced with #45

@D4nte D4nte closed this Jan 10, 2019

@wafflebot wafflebot bot removed the sprint-backlog label Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment