GetBalance problem against blockchain RPC about btcrpcclient HOT 18 CLOSED

Contrary to their claims, the blockchain API is not completely compatible with the bitcoind API. I'd have to check this particular call to know if it's a case of that, but it's quite likely that is what's going on.

from btcrpcclient.

I was afraid of that. :(
I know it's not your problem, but do you think you could add support for blockchain here? It's really annoying relying on this "standard Bitcoind RPC API", and stumble upon differences like this.

from btcrpcclient.

I'll take a look when I have a chance, but one of the biggest issues with the Bitcoin RPC API (there are many of them) is that there is no versioning. That makes it rather difficult to handle inconsistencies like this.

from btcrpcclient.

Ok, thanks.

If it is a type difference, I guess it would be rather safe just to check the type, or do a type test. Like if it is a s string vs a json array and handle it that way.

from btcrpcclient.

Had any chance to look at this yet? I contacted blockchain.info support and asked if they could tell me the difference between what they do and what bitcond does. Waiting reply...

from btcrpcclient.

I haven't as I'm pretty deep in a fairly major wallet deliverable right now. If you can paste the raw reply from bc.i for the command, that would help as I don't have an account there currently to see the output.

You can output the raw response by adding a print statement (or log if you're using the logging facilities) to the (r FutureGetBalanceResult) Receive() function here.

fmt.Println(string(res))

You'll also need to add "fmt" to the imports section at the top of the same file. After that, do your request and you should get a print of the raw JSON. Please paste that here.

from btcrpcclient.

Also, don't forget that if you get stuck on anything like this you can temporarily hack around it by issuing commands directly by using RawRequest or RawRequestAsync.

That way you aren't held up waiting for changes upstream.

// Ignoring the error handling and optional args here for brevity...
account, err := json.Marshal("*")

params := []json.RawMessage{account}
response, err := client.RawRequest("getbalance", params)

// Modify according to whatever response you're getting back from bc.i.
var balance float64
if err := json.Unmarshal(response, &balance); err != nil {
    // handle error
}

from btcrpcclient.

Ah, thanks for that! Also, I added that, and noticed they return literally "0". Not just 0, but using "" too it seems. Might explain the error message.

from btcrpcclient.

Ok, so they're returning a string instead of a numeric value. Thanks.

from btcrpcclient.

Yea, seems like it. I might just use RawMessage as a temporary workaround for now. Is this something you will add "support" for?

from btcrpcclient.

Yeah, I think I can add it without too much hassle. I'll just have to examine the bytes before doing the unmarshal to choose which one to use. I should be able to get to it this weekend.

Ideally though, bc.i should fix their API if they want to claim Bitcoin RPC API compatibility.

from btcrpcclient.

I'm of the opinion that we shouldn't special case broken APIs, even if they are popular. A small wrapper around this using RawMessage and RawRequest is the "best" solution to this, and it won't hide future bugs when servers erroneously start replying with balances as strings.

Of course, it's entirely possible that bitcoind did in fact use to reply with strings rather than JSON numbers. I haven't picked through the repo history closely, but if that was the case, I would be more sympathetic to adding a workaround.

from btcrpcclient.

How would you guys feel about an additional option in the ConnConfig setup struct? I tend to agree with @jrick that I would like to avoid API specific hacks in the general case, but I also see a benefit in being able to simply switch back and forth between bc.i and a local btcd.

Given that, I think a reasonable compromise is to provide the caller with an opt-in flag that allows them to knowingly enable special casing for a broken API. Existing code which doesn't opt-in will then still fail on the future bugs case you call out.

Something like EnableBCInfoHacks bool. Thus it would be false and disabled by default unless the caller specifically sets it to true to enable it.

from btcrpcclient.

How are things going with this?

from btcrpcclient.

Oh. Thanks for the reminder. I was waiting for a response on my proposal and totally forgot about this with everything else going on. I'll try to get to it this weekend.

from btcrpcclient.

Just an update that I have not forgot about this. I wasn't able to get to it over the weekend, but it's high on my list.

from btcrpcclient.

Is this one resolved now?

from btcrpcclient.

This is resolved by commit 331f25f. In particular, a new config option named EnableBCInfoHacks has been added to the ConnConfig struct which, when specified, enables the alternative behavior for GetBalanceMinConf.

from btcrpcclient.