[whiskyrelaxing]

listtransactions declares two arrays and only fills one:

UniValue ret(UniValue::VARR);
UniValue retReversed(UniValue::VARR);

The loop fills retReversed. ret is never written to, but both clamps read its
size, rpcwallet.cpp:2943:

if (nFrom > (int)ret.size())
    nFrom = retReversed.size();
if ((nFrom + nCount) > (int)ret.size())
    nCount = retReversed.size() - nFrom;

ret.size() is always 0. So any skip above zero puts nFrom past the end and nCount
at zero, and returns nothing. With skip at zero, nCount is replaced by the whole
collected size, which overshoots because the loop breaks on >= and
ListTransactions appends one row per detail entry.

On a wallet whose eight newest transactions have one row each and whose next two
have four and nineteen:

asked  9  ->  12 rows
asked 22  ->  31 rows
skip 1    ->   0 rows
skip 5    ->   0 rows

The patch points both clamps at retReversed and drops the dead declaration and
the stale comment. The right hand sides already said retReversed.size().

Checked by lifting the post-collection half of the function into a harness
compiled against the project's own UniValue. Unpatched it matches a live node on
sixteen cases; patched it returns the requested count on all of them and
successive pages join up.

[ohcee]

utACK a09e0e1

ret was never filled, so the clamps compared against an empty array, skip ≥ 1 returned nothing, skip = 0 returned the whole collection. Both right-hand sides already said retReversed; this just makes the comparisons match. Checked that the pattern doesn't exist anywhere else in the file.