Target confirmations not intuitive in listsinceblock #14338

The listsinceblock command's second argument

2. target_confirmations:    (numeric, optional, default=1) Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value

does not seem to be very useful in its current form. What are the actual use cases here? (Harding pointed out the below on IRC)

Oh, maybe its just for parsing history backwards.  E.g. you're looking an the transactions
since block x and you want to get the block hash for x-1?
Doing that in a single RPC call over calling getblockhash keeps it atomic so that you don't
have to worry about a reorg between the listsinceblock call and the getblockhash call, I
guess.

It feels like it would make sense if this

1. did filter out txs that were below the target
2. did show txs that were below the target for arg 1 (blockhash), but were above the target for current block.

The above two changes would let someone create a simple starting point for only dealing with e.g. 6 confirmation transactions and not even have to bother with confirmation count since they'd never even see any transactions < target confs.

If not, the documentation is not very descriptive right now, and (despite having worked on the code for listsinceblock personally) I assumed things that weren't true about it, so it definitely needs a fix-up.

It's actually a great API, it's just poorly understood.

Let's say you consider 100 confirmations to be set-in-stone (probably pretty reasonable). So really what you want to do is ask core "Tell me everything that's happened in the last 100 blocks".

But that makes for a terrible API, because if your software goes down for a while -- you'll miss shit. So what you really want to do is say:

"Give me all transactions that have happened since $SET_IN_STONE" and then after you process that, you want to update $SET_IN_STONE, which should be 100 confirmations before the current best block.

From an application point of view, it's much nicer to do that atomically. And that's exactly what listsinceblock does.

We can write some psuedo code:

set_in_stone :=  "genesis block hash"
while true {
       transactions, new_set_in_stone :=  listsinceblock(set_in_stone, 100)
       process(transactions)
       set_in_stone = new_set_in_stone
}

Now the actual problem with listsinceblock is just how badly tested and undefined it is (especially around things like unconfirmed/double-spends). I forgot the exact details but I once had a processing system (including double-spend detection) with it working perfectly, then the behaviors silently broke, and then I fixed that and then it irreparably broke etc. I couldn't rely on something so unstable, so I had to just implement the exact same feature myself -- so I can't tell you ow usable it is or not.

I'm not sure if you're commending the RPC or condemning it. You seem to like the idea but you then say you rewrote what you needed separately as the current system is not working.

I think we need to fix the docs for this (at least) and perhaps even change it to be sensible. What you describe sounds sensible, I think.

Closing this stale issue. If someone else finds the wording offensive they will surely open an issue/PR and/or comment here.