Fix documentations of rpc `listsinceblock`

FrozenPrincess commented at 1:28 pm on September 19, 2016: none

The second param of listsinceblock

The second parameter (target confirmation) is quite strange and should probably be better > documented in the RPC help of listsinceblock (check original comment: #199 (comment))

From the actual docs here is it is:

target-confirmations: (numeric, optional) The confirmations required, must be 1 or more

However, that’s misleading at best. I’ve seen at least one project thinking that target-confirmations means “filter to only show transactions of n target-confirmations”. It’s a weird param but I’d document it as

target-confirmations: (numeric, optional) Return the hash of the block that has target-confirmations of confirmations, for use in a further call to listsinceblock

Transactions confirmations

Says:

“confirmations”: n, (numeric) The number of confirmations for the transaction. Available for ‘send’ and ‘receive’ category of transactions.

It should also say when it’s < 0, it means the transaction conflicted that many blocks ago

bip125-replaceable

There’s an undocumented field of transactions “bip125-replaceable” which I believe has three possibilities “yes” “no” and “unknown” (under what circumstances it says unknown, should also be documented)

laanwj commented at 1:31 pm on September 19, 2016: member

Agree, though I don’t think the parameter should even be called target-confirmations if that is what it does.

MarcoFalke added the label Easy to implement on Sep 19, 2016

MarcoFalke added the label Docs and Output on Sep 19, 2016

FrozenPrincess renamed this:
~~Properly document the second parameter of `listsinceblock`~~
Fix documentations of second parameter of `listsinceblock`
on Sep 19, 2016

FrozenPrincess renamed this:
~~Fix documentations of second parameter of `listsinceblock`~~
Fix documentations of rpc `listsinceblock`
on Sep 19, 2016

FrozenPrincess commented at 6:15 pm on September 19, 2016: none

I updated the issue to include 2 other minor documentation issues for the same rpc call, as it probably should all be done as one thing.

accraze referenced this in commit 3f67972654 on Dec 22, 2016

laanwj closed this on Jan 4, 2017

RHavar commented at 4:32 pm on January 4, 2017: contributor

I don’t think this should be closed, #9396 only fixed 1 out of the three issues in the docs.

MarcoFalke reopened this on Jan 4, 2017

RHavar commented at 10:46 pm on June 22, 2017: contributor

Another issue with listsinceblock documentation, is that it’s missing documentation for walletconflicts (and it should explain exactly what that mean, if exclusively transactions from the wallet that conflicts or any transaction)

RHavar commented at 4:00 am on June 23, 2017: contributor

There’s also missing documentation for the field replaced_by_txid. I’m not sure I’m a fan of these “omit empty” style fields that only appear when the data exists. They make them rather hard to discover, especially if the docs aren’t comprehensive.

ryanofsky commented at 11:33 am on June 23, 2017: member

There’s also missing documentation for the field replaced_by_txid.

replaced_by_txid is documented here: https://github.com/bitcoin/bitcoin/blob/7a74f88a26cf251ba36b26f604f1ac9940fd9c92/src/wallet/wallet.h#L280

RHavar commented at 1:40 pm on June 23, 2017: contributor

replaced_by_txid is documented here:

That’s not exposed to end users when they via the listsinceblock help/docs :D

laanwj commented at 11:48 am on June 24, 2017: member

I’m not sure I’m a fan of these “omit empty” style fields that only appear when the data exists.

The alternative is to use a “special value”, which is fraught with more risk. E.g. the return value -1 for “no data” in estimatesmartfee has been criticized many times, because it’s too easy to use as a value. Leaving out the field completely avoids potential confusion. Of course the documentation needs to be complete.

gmaxwell commented at 6:39 pm on June 24, 2017: contributor

@RHavar errors are always hard to discover if they’re not documented correctly and even then; hiding the fields is about shaping how the caller fails when they get it wrong.

I think our coding standards should reflect something like: “Avoid returning special values for error conditions, especially for gauge outputs (like numbers of connections, btc amounts, etc.) because the error code can be mistaken for a value even if the error is negative and negative results don’t make much sense. For fields which are naturally categorical an in-band error can be more acceptable e.g. something that returns “apple” / “grape” / “cherry” could also return “error”, but is generally preferable to have a named result field and not return it when there is an error, and return an error field instead.”

It’s always sad if you mishandle an error– but I’d rather the caller fail cleanly rather than average a -1 into some data or something like that.

gmaxwell commented at 6:44 pm on June 24, 2017: contributor

@RHavar errors are always hard to discover if they’re not documented correctly and even then; hiding the fields is about shaping how the caller fails when they get it wrong.

I think our coding standards should reflect something like: “Avoid returning special values for error conditions, especially for gauge outputs (like numbers of connections, btc amounts, etc.) because the error code can be mistaken for a value even if the error is negative and negative results don’t make much sense. For fields which are naturally categorical an in-band error can be more acceptable e.g. something that returns “apple” / “grape” / “cherry” could also return “error”, but is generally preferable to have a named result field and not return it when there is an error, and return an error field instead.”

It’s always sad if you mishandle an error– but I’d rather the caller fail cleanly rather than average a -1 into some data or something like that.

jeffrade commented at 1:16 am on January 8, 2018: contributor

I believe this issue can be closed. The fields target-confirmations, confirmations and bip125-replaceable have either been updated or added since this issue was originally opened. The PRs weren’t referencing this issue number, so this issue stayed open. Below are the commits that have addressed them:

MarcoFalke closed this on Jan 13, 2018

lateminer referenced this in commit bfce5075cc on Jan 5, 2019

MarcoFalke locked this on Sep 8, 2021

Fix documentations of rpc listsinceblock #8758

The second param of listsinceblock

Transactions confirmations

bip125-replaceable

Fix documentations of rpc `listsinceblock` #8758