Fix documentations of rpc listsinceblock

[FrozenPrincess]

The second param of listsinceblock

From issue 8752

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]

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

[FrozenPrincess]

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.

[RHavar]

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

[RHavar]

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]

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]

There's also missing documentation for the field replaced_by_txid.

replaced_by_txid is documented here:

bitcoin/src/wallet/wallet.h

Line 280 in 7a74f88

* "replaced_by_txid" - txid (as HexStr) of transaction created by

[RHavar]

replaced_by_txid is documented here:

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

[laanwj]

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]

@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]

@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]

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:

• confirmations
• target-confirmations
• bip125-replaceable