nochowderforyou / clams

Clam Project
MIT License
62 stars 58 forks source link

Replace json_spirit with UniValue #236

Closed l0rdicon closed 8 years ago

l0rdicon commented 8 years ago

ref: https://github.com/bitcoin/bitcoin/pull/6121

Includes all the recent commits to UniValue up the bitcoin master tree as of 10/09/2015

dooglus commented 8 years ago

I can't create a raw transaction:

$ clamd createrawtransaction '[{"txid":"deadbeef","vout":1}]' '{}'
error code: -8
error message: Invalid parameter, missing vout key

Note how rpcmisc.cpp has:

    const UniValue& vout_v = find_value(o, "vout");
    if (!vout_v.isNum())

but rpcrawtransaction.cpp has:

    const UniValue& vout_v = find_value(o, "vout");
    if (vout_v.isNum())

I think they should probably both have the ! at the start of the 2nd line there.

dooglus commented 8 years ago

In signrawtransaction:

        if (p.isObject())
            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "expected object with {\"txid'\",\"vout\",\"scriptPubKey\"}");

That should be !p.isObject(), right?

l0rdicon commented 8 years ago

I updated https://github.com/l0rdicon/clams/commit/7bc97ed3937bc09f5d6089d609f83b0fefac7d64 with the mistakes you picked out.

dooglus commented 8 years ago

signrawtransaction seems OK if I add the ! I mentioned in my previous comment:

$ cc signrawtransaction $(cc createrawtransaction '[{"txid":"1234","vout":4}]' '{"xJDCLAMZkcp7fQ3ieHfZA4SLu3aTy2Y1mr":1}') '[{"txid":"1234","vout":4,"scriptPubKey":"76a91491e596f8e4620be793964af8979e76073907c5b188ac"}]' '["'$p'","'$p'"]'
{
  "hex": "020...",
  "complete": true
}
dooglus commented 8 years ago

I just got 'bit' by this change, in testing.

I have a script which uses awk '{print $3}' to pull the results from an RPC command.

Previously the input to the awk looked like this:

$ cc getblock $(cc getblockhash 684635) | grep difficulty
    "difficulty" : 65093.99454380,

but with this pull request it has changed to look like this:

$ cc getblock $(cc getblockhash 684635) | grep difficulty
  "difficulty": 65093.99454380115,

Note there's no longer a space before the colon, and so the awk fails.

I don't know if anyone else is processing "clamd" output with shell scripts, but it would be better if this spirit -> univalue change was as transparent as possible for the end user.

l0rdicon commented 8 years ago

Absolutely. There shouldn't be any changes in the output in my opinion either. I didn't catch that.

I'll look into this

On Tue, Oct 13, 2015 at 10:20 PM, Chris Moore notifications@github.com wrote:

I just got 'bit' by this change, in testing.

I have a script which uses awk '{print $3}' to pull the results from an RPC command.

Previously the input to the awk looked like this:

$ cc getblock $(cc getblockhash 684635) | grep difficulty "difficulty" : 65093.99454380,

but with this pull request it has changed to look like this:

$ cc getblock $(cc getblockhash 684635) | grep difficulty "difficulty": 65093.99454380115,

Note there's no longer a space before the colon, and so the awk fails.

I don't know if anyone else is processing "clamd" output with shell scripts, but it would be better if this spirit -> univalue change was as transparent as possible for the end user.

— Reply to this email directly or view it on GitHub https://github.com/nochowderforyou/clams/pull/236#issuecomment-147898439 .

l0rdicon commented 8 years ago

I updated the response to include the separator

dooglus commented 8 years ago

listunspent is showing stuff like:

{
  "txid": "10e6e13f0eb340d07217cfec2b3bce1c02efbfb5906d22bd9aa1f8aa85cf8c8d",
  "vout": 13,
  "address": "xVH85MSAaUppSN5sYWDvikvu8e4zkVkcwW",
  "account": "1172957",
  "scriptPubKey": "76a914e61155624669f5615a9d6b730b61d1278397cc9188ac",
  "amount": 1.934e-05,
  "confirmations": 53004
}, 

Note the amount is in scientific notation. I'm pretty sure it always used to be in decimal notation, even if it was for a single satoshi.

l0rdicon commented 8 years ago

I'm unsure why your listupsent doesn't have the spacing between the separator.. I'll need to look deeper into that if it persists.

The update to ValueToAmount should fix any issues with the numbers being reported wrong.

dooglus commented 8 years ago

Oh, that's because I haven't rebuilt since I first saw your pull request. I was referring to the 'amount' values, not the spacing.

On Thu, Oct 15, 2015 at 11:26 PM, l0rdicon notifications@github.com wrote:

I'm unsure why your listupsent doesn't have the spacing between the separator.. I'll need to look deeper into that if it persists.

The update to ValueToAmount should fix any issues with the numbers being reported wrong.

— Reply to this email directly or view it on GitHub https://github.com/nochowderforyou/clams/pull/236#issuecomment-148628139 .

l0rdicon commented 8 years ago

Makes sense, I was wondering if that was the case about you not having rebuilt.

That amount should be fixed with that most recent update, and the spacing for your AWK print failure should be there as well

On Fri, Oct 16, 2015 at 4:46 AM, Chris Moore notifications@github.com wrote:

Oh, that's because I haven't rebuilt since I first saw your pull request. I was referring to the 'amount' values, not the spacing.

On Thu, Oct 15, 2015 at 11:26 PM, l0rdicon notifications@github.com wrote:

I'm unsure why your listupsent doesn't have the spacing between the separator.. I'll need to look deeper into that if it persists.

The update to ValueToAmount should fix any issues with the numbers being reported wrong.

— Reply to this email directly or view it on GitHub < https://github.com/nochowderforyou/clams/pull/236#issuecomment-148628139>

.

— Reply to this email directly or view it on GitHub https://github.com/nochowderforyou/clams/pull/236#issuecomment-148640163 .

dooglus commented 8 years ago

Looks good now I've rebuilt.

Before:

$ clamd listunspent | grep amount
    "amount": 2.079e-05,
    "amount": 1.934e-05,
    "amount": 2.662e-05,
    "amount": 0.00020465,
    "amount": 1.97e-05,
    "amount": 5.408e-05,
    "amount": 2.752e-05,
    "amount": 3.408e-05,
    "amount": 1.645e-05,
    "amount": 2.475e-05,
    "amount": 0.00021877,
    "amount": 0.0002062,

After:

$ clamd listunspent | grep amount
    "amount" : 0.00002079,
    "amount" : 0.00001934,
    "amount" : 0.00002662,
    "amount" : 0.00020465,
    "amount" : 0.00001970,
    "amount" : 0.00005408,
    "amount" : 0.00002752,
    "amount" : 0.00003408,
    "amount" : 0.00001645,
    "amount" : 0.00002475,
    "amount" : 0.00021877,
    "amount" : 0.00020620,