Concept ACK

Concept ACK.
Some concrete use-cases would be nice.
Nice to see the new RPC commands cs_main lock free.

Updated to remove dead code I inadvertently left in rpcblockchain.cpp.

Here's an example of a tool that would benefit from the getdescendants function:
https://github.com/sdaftuar/replace-by-fee-tools/blob/d0121b03d02c1a7e5e16425b55da1302ddf081aa/combine-transactions.py

This is a script that takes two txid's (that are signaling opt-in RBF) and combines them down to one -- consolidating duplicate output addresses (if any) and removing pay-to-self outputs, assumed for demo purposes to be change outputs -- and paying for all fees including those of descendants. The main idea is that this avoids over-paying for descendants -- it calls getdescendants on each transaction and takes the union of the two sets so that each descendant is paid for once. I don't know any simple or straightforward way to do this without this new RPC call.

There are similar benefits to having getancestors, such as if you wanted to understand why a stuck transaction wasn't confirming, then getancestors would give you the set of unconfirmed parents to look at (and you could write a replace-by-fee-tool that was smart enough to drop an input coming from a stuck unconfirmed parent, when possible, to replace a transaction with one likely to confirm sooner; or better still, replace such a stuck parent instead if possible).

@sipa True. Perhaps getmempoolancestors/getmempooldescendants? I'm open to suggestions.

@laanwj if this code is ok I think it would make sense to include this in 0.12

Updated with new RPC names (getmempoolancestors and getmempooldescendants).

Any reason not to just add the ancestors/descendants info to getrawmempool (and also the getmempoolentry)?

The goal was to avoid having to dump the entire mempool if you were only interested in a very small subset of it. But I suppose there's no reason we couldn't also show the txid's of ancestors and descendants when you call getrawmempool (with verbose=true). If that seems useful, I'm happy to add that.

(edited)
Oh, right, you are suggesting that, and then yes I agree it would also make sense to update getmempoolentry to match. Sure, I'll go ahead and do that.

@luke-jr On further thought, the downside to that is that doing the calculation could cause getrawmempool to get up to ~25x slower (default policy allows length 25 ancestor and descendant chains, and the calculation of ancestors and descendants requires walking those chains), which on a full mempool could be a meaningful -- and perhaps unacceptable? -- slowdown.

The other possibility would be to add a new option to getrawmempool to determine whether to do the calculation or not, but I think we try to avoid adding new arguments to existing RPC calls? So now I think it's probably better to leave as-is. Let me know if you disagree...

Concept ACK

Needs rebase.

Rebased. There weren't any actual conflicts though, so not sure why github thought it wouldn't merge cleanly?

Thanks!
I've noticed that before. Github is somewhat less good (or "more careful") at merging than git.

Rebase needed.

Rebased. Also added a commit that extends the mempool entry output to include the ancestor state (added by #7594), as well as a brief update to the release notes.

Unit tests are now failing on linux. I'll see if I can reproduce locally.

Oh, I suppose it could be this: #8069 (comment)

Looks like the travis issue was fixed by #8070, so I think this is ready now.

reACK 5379307

utACK 5379307

Can one of the admins verify this patch?

utACK 5379307a23177c33b5a869513d1d1d09790b0269

Is there anything holding this up?

Can you squash the typo?

@sipa done

utACK 176e19b (same tree as before)