During Initial Block Download, the mempool is usually empty, but `CTxMemPool::removeForBlock` is still called for every connected block where we:
* iterate over every transaction in the block even though none will be found in the empty `mapTx`, always leaving `txs_removed_for_block` empty...
* which is pre-allocated regardless with 40 bytes * vtx.size(), even though it will always remain empty.
This change introduces a minor performance optimization by only executing the loop if any of the core mempool maps have any contents.
The call to `MempoolTransactionsRemovedForBlock` and the updates to the rolling fee logic remain unchanged.
The `removeForBlock` was also updated stylistically to match the surrounding methods and a clarification was added to clarify that it affects fee estimation as well.
96da68a38f qa: functional test a transaction running into the legacy sigop limit (Antoine Poinsot)
367147954d qa: unit test standardness of inputs packed with legacy sigops (Antoine Poinsot)
5863315e33 policy: make pathological transactions packed with legacy sigops non-standard. (Antoine Poinsot)
Pull request description:
The Consensus Cleanup soft fork proposal includes a limit on the number of legacy signature
operations potentially executed when validating a transaction. If this change is to be implemented
here and activated by Bitcoin users in the future, we should make transactions that are not valid
according to the new rules non-standard first because it would otherwise be a trivial DoS to
potentially unupgraded miners after the soft fork activates.
ML post: https://gnusha.org/pi/bitcoindev/49dyqqkf5NqGlGdinp6SELIoxzE_ONh3UIj6-EB8S804Id5yROq-b1uGK8DUru66eIlWuhb5R3nhRRutwuYjemiuOOBS2FQ4KWDnEh0wLuA=@protonmail.com/T/#u
ACKs for top commit:
instagibbs:
reACK 96da68a38f
maflcko:
review ACK 96da68a38f🚋
achow101:
ACK 96da68a38f
glozow:
light code review ACK 96da68a38f, looks correct to me
Tree-SHA512: 106ffe62e48952affa31c5894a404a17a3b4ea8971815828166fba89069f757366129f7807205e8c6558beb75c6f67d8f9a41000be2f8cf95be3b1a02d87bfe9
50024620b9 [bench] worst case LimitOrphans and EraseForBlock (glozow)
45c7a4b56d [functional test] orphan resolution works in the presence of DoSy peers (glozow)
835f5c77cd [prep/test] restart instead of bumpmocktime between p2p_orphan_handling subtests (glozow)
b113877545 [fuzz] Add simulation fuzz test for TxOrphanage (Pieter Wuille)
03aaaedc6d [prep] Return the made-reconsiderable announcements in AddChildrenToWorkSet (Pieter Wuille)
ea29c4371e [p2p] bump DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE to 3,000 (glozow)
24afee8d8f [fuzz] TxOrphanage protects peers that don't go over limit (glozow)
a2878cfb4a [unit test] strengthen GetChildrenFromSamePeer tests: results are in recency order (glozow)
7ce3b7ee57 [unit test] basic TxOrphanage eviction and protection (glozow)
4d23d1d7e7 [cleanup] remove unused rng param from LimitOrphans (glozow)
067365d2a8 [p2p] overhaul TxOrphanage with smarter limits (glozow)
1a41e7962d [refactor] create aliases for TxOrphanage Count and Usage (glozow)
b50bd72c42 [prep] change return type of EraseTx to bool (glozow)
3da6d7f8f6 [prep/refactor] make TxOrphanage a virtual class implemented by TxOrphanageImpl (glozow)
77ebe8f280 [prep/test] have TxOrphanage remember its own limits in LimitOrphans (glozow)
d0af4239b7 [prep/refactor] move DEFAULT_MAX_ORPHAN_TRANSACTIONS to txorphanage.h (glozow)
51365225b8 [prep/config] remove -maxorphantx (glozow)
8dd24c29ae [prep/test] modify test to not access TxOrphanage internals (glozow)
44f5327824 [fuzz] add SeedRandomStateForTest(SeedRand::ZEROS) to txorphan (glozow)
15a4ec9069 [prep/rpc] remove entry and expiry time from getorphantxs (glozow)
08e58fa911 [prep/refactor] move txorphanage to node namespace and directory (glozow)
bb91d23fa9 [txorphanage] change type of usage to int64_t (glozow)
Pull request description:
This PR is part of the orphan resolution project, see #27463.
This design came from collaboration with sipa - thanks.
We want to limit the CPU work and memory used by `TxOrphanage` to avoid denial of service attacks. On master, this is achieved by limiting the number of transactions in this data structure to 100, and the weight of each transaction to 400KWu (the largest standard tx) [0]. We always allow new orphans, but if the addition causes us to exceed 100, we evict one randomly. This is dead simple, but has problems:
- It makes the orphanage trivially churnable: any one peer can render it useless by spamming us with lots of orphans. It's possible this is happening: "Looking at data from node alice on 2024-09-14 shows that we’re sometimes removing more than 100k orphans per minute. This feels like someone flooding us with orphans." [1]
- Effectively, opportunistic 1p1c is useless in the presence of adversaries: it is *opportunistic* and pairs a low feerate tx with a child that happens to be in the orphanage. So if nothing is able to stay in orphanages, we can't expect 1p1cs to propagate.
- This number is also often insufficient for the volume of orphans we handle: historical data show that overflows are pretty common, and there are times where "it seems like [the node] forgot about the orphans and re-requested them multiple times." [1]
Just jacking up the `-maxorphantxs` number is not a good enough solution, because it doesn't solve the churnability problem, and the effective resource bounds scale poorly.
This PR introduces numbers for {global, per-peer} {memory usage, announcements + number of inputs}, representing resource limits:
- The (constant) **global latency score limit** is the number of unique (wtxid, peer) pairs in the orphanage + the number of inputs spent by those (deduplicated) transactions floor-divided by 10 [2]. This represents a cap on CPU or latency for any given operation, and does not change with the number of peers we have. Evictions must happen whenever this limit is reached. The primary goal of this limit is to ensure we do not spend more than a few ms on any call to `LimitOrphans` or `EraseForBlock`.
- The (variable) **per-peer latency score limit** is the global latency score limit divided by the number of peers. Peers are allowed to exceed this limit provided the global announcement limit has not been reached. The per-peer announcement limit decreases with more peers.
- The (constant) **per-peer memory usage reservation** is the amount of orphan weight [3] reserved per peer [4]. Reservation means that peers are effectively guaranteed this amount of space. Peers are allowed to exceed this limit provided the global usage limit is not reached. The primary goal of this limit is to ensure we don't oom.
- The (variable) **global memory usage limit** is the number of peers multiplied by the per-peer reservation [5]. As such, the global memory usage limit scales up with the number of peers we have. Evictions must happen whenever this limit is reached.
- We introduce a "Peer DoS Score" which is the maximum between its "CPU Score" and "Memory Score." The CPU score is the ratio between the number of orphans announced by this peer / peer announcement limit. The memory score is the total usage of all orphans announced by this peer / peer usage reservation.
Eviction changes in a few ways:
- It is triggered if either limit is exceeded.
- On each iteration of the loop, instead of selecting a random orphan, we select a peer and delete 1 of its announcements. Specifically, we select the peer with the highest DoS score, which is the maximum between its CPU DoS score (based on announcements) and Memory DoS score (based on tx weight). After the peer has been selected, we evict the oldest orphan (non-reconsiderable sorted before reconsiderable).
- Instead of evicting orphans, we evict announcements. An orphan is still in the orphanage as long as there is 1 peer announcer. Of course, over the course of several iteration loops, we may erase all announcers, thus erasing the orphan itself. The purpose of this change is to prevent a peer from being able to trigger eviction of another peer's orphans.
This PR also:
- Reimplements `TxOrphanage` as single multi-index container.
- Effectively bounds the number of transactions that can be in a peer's work set by ensuring it is a subset of the peer's announcements.
- Removes the `-maxorphantxs` config option, as the orphanage no longer limits by unique orphans.
This means we can receive 1p1c packages in the presence of spammy peers. It also makes the orphanage more useful and increases our download capacity without drastically increasing orphanage resource usage.
[0]: This means the effective memory limit in orphan weight is 100 * 400KWu = 40MWu
[1]: https://delvingbitcoin.org/t/stats-on-orphanage-overflows/1421
[2]: Limit is 3000, which is equivalent to one max size ancestor package (24 transactions can be missing inputs) for each peer (default max connections is 125).
[3]: Orphan weight is used in place of actual memory usage because something like "one maximally sized standard tx" is easier to reason about than "considering the bytes allocated for vin and vout vectors, it needs to be within N bytes..." etc. We can also consider a different formula to encapsulate more the memory overhead but still have an interface that is easy to reason about.
[4]: The limit is 404KWu, which is the maximum size of an ancestor package.
[5]: With 125 peers, this is 50.5MWu, which is a small increase from the existing limit of 40MWu. While the actual memory usage limit is higher (this number does not include the other memory used by `TxOrphanage` to store the outpoints map, etc.), this is within the same ballpark as the old limit.
ACKs for top commit:
marcofleon:
ReACK 50024620b9
achow101:
light ACK 50024620b9
instagibbs:
ACK 50024620b9
theStack:
Code-review ACK 50024620b9
Tree-SHA512: 270c11a2d116a1bf222358a1b4e25ffd1f01e24da958284fa8c4678bee5547f9e0554e87da7b7d5d5d172ca11da147f54a69b3436cc8f382debb6a45a90647fd
5fa34951ea test: avoid unneeded block header hash -> integer conversions (Sebastian Falbesoner)
2118301d77 test: rename CBlockHeader `.hash` -> `.hash_hex` for consistency (Sebastian Falbesoner)
23be0ec2f0 test: rename CBlockHeader `.rehash()`/`.sha256` -> `.hash_int` for consistency (Sebastian Falbesoner)
8b09cc350a test: remove bare CBlockHeader `.rehash()`/`.calc_sha256()` calls (Sebastian Falbesoner)
0716382c20 test: remove header hash caching in CBlockHeader class (Sebastian Falbesoner)
0f044e82bd test: avoid direct block header modification in feature_block.py (Sebastian Falbesoner)
f3c791d2e3 test: refactor: dedup `CBlockHeader` serialization (Sebastian Falbesoner)
Pull request description:
Similar to what #32421 did for `CTransaction` instances, this PR aims to improve the block hash determination of `CBlockHeader`/`CBlock` (the latter is a subclass of the former) instances by removing the block header caching mechanism and introducing consistent naming. Without the statefulness, sneaky testing bugs like #32742 and #32823 are less likely to happen in the future. Note that performance is even less of an issue here compared to `CTransaction`, as we only need to hash 80 bytes, which is less than typical standard transaction sizes [2].
The only instance where the testing logic was relying on caching (i.e. we want to return an outdated value) is tackled in the second commit, the rest should be straight-forward to review, especially for contributors who already reviewed #32421.
Summary table showing block hash determaination before/after this PR:
| Task | master | PR |
|:-----------------------------------|:-------------------------|:-------------|
| get block header hash (hex string) | `.hash`[1] | `.hash_hex` |
| get block header hash (integer) | `rehash()`, `.sha256`[1] | `.hash_int` |
[1] = returned value might be `None` or out-of-date, if rehashing function wasn't called after modification
[2] = the only exception I could think of are transaction with pay-to-anchor (P2A) outputs
ACKs for top commit:
rkrux:
re-ACK 5fa34951ea modulo failing CI due to silent merge conflict.
maflcko:
re-ACK 5fa34951ea🎩
danielabrozzoni:
reACK 5fa34951ea
Tree-SHA512: 3d13540012654effa063846958a3166d56c1bcb58e1321f52ca4d5c3bcb7abdea72c54d1fb566d04e636d84d06a41d293e16232dbe5d5e78a73c903bb6ffc80d
It's useful to have an end-to-end test in addition to the unit test to sanity check the RPC error as
well as making sure the transaction is otherwise fully standard.
The Consensus Cleanup soft fork proposal includes a limit on the number of legacy signature
operations potentially executed when validating a transaction. If this change is to be implemented
here and activated by Bitcoin users in the future, we should prevent the ability for someone to
broadcast a transaction through the p2p network that is not valid according to the new rules. This
is because if it was possible it would be a trivial DoS to potentially unupgraded miners after the
soft fork activates.
We do not know for sure whether users will activate the Consensus Cleanup. However if they do such
transactions must have been made non-standard long in advance, due to the time it takes for most
nodes on the network to upgrade. In addition this limit may only be run into by pathological
transactions which pad the Script with sigops but do not use actual signatures when spending, as
otherwise they would run into the standard transaction size limit.
Note that we unfortunately can't use a scripted diff here, as the
`sha256` symbol is also used for other instances (e.g. as function
in hashlib, or in the `UTXO` class in p2p_segwit.py).
Since the previous commit, CBlockHeader/CBlock object calls to the
methods `.rehash()` and `.calc_sha256()` are effectively no-ops
if the returned value is not used, so we can just remove them.
Rather than block hashes (represented by the fields `.sha256` and
`.hash`) being stateful, simply compute them on-the-fly. This ensures
that the correct values are always returned and takes the burden of
rehashing from test writers, making the code shorter overall. In a
first step, the fields are kept at the same name with @property
functions as drop-in replacements, for a minimal diff. In later commits,
the names are changed to be more descriptive and indicating the return
type of the block hash.
This is a preparatory commit for removing the header hash
caching in the CBlockHeader class. In order to not lose the
old block hash, necessary for updating the internal state of
the test (represented by `self.block_heights` and `self.blocks`),
we should only modify it within the `update_block` method.
Note that we can't call `.serialize()` directly in
the `.calc_sha256()` method, as this could wrongly lead
to the serialization of the derived class (CBlock) if
called from an instance there.
f5647c6c5a depends: fix libevent _WIN32_WINNT usage (fanquake)
Pull request description:
Starting with version 13.x, the mingw headers will define the value of
`NTDDI_VERSION`, based on the value of `_WIN32_WINNT`, if that version is <
Windows 10. Given that libevent was undefining our `_WIN32_WINNT`, and
redefining it to a value < Windows 10 (`0x0501`), `NTDDI_VERSION` was also
being defined to that value, leading to functions not being exposed in
the mingw-w64 headers; see here: 9c2668ef77/mingw-w64-headers/include/iphlpapi.h (L36-L41).
Imports a commit from usptream ([a14ff91254f40cf36e0fee199e26fb11260fab49](a14ff91254)).
Fixes#32707.
ACKs for top commit:
willcl-ark:
crACK f5647c6c5a
Tree-SHA512: eb429457a4af6191dd27ef3d1087667c5304ff0f49d4c6824883651e3c2dbab5d9784fa1f170402f23cd9238005c5214e0a71a4160562a59dfa35618dc702132
4bb4c86599 test: document HOST for get_previous_releases.py (Sjors Provoost)
609203d507 test: stop signing previous releases >= v28.2 (Sjors Provoost)
c6dc2c29f8 test: replace v28.0 with notarized v28.2 (Sjors Provoost)
5bd73d96a3 test: fix macOS detection (Sjors Provoost)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/31407 macOS guix builds are signed and notarized. This was included in v29 and backported to 28.x.
This PR bumps the v28.0 previous release binary to v28.2 and adjusts the test that uses it. Additionally it no longer manually code signs binaries >= v28.2.
While testing on an M4 mac and redownloading all the binaries, I noticed that `platform == "arm64-apple-darwin"` doesn't actually work. This initially used `args.platform` in #26694, but that was changed to just `platform` in #32219.
So the first commit switches this to use `args.host`. I manually tested on Intel macOS 13.7.6 that code-signing still isn't needed there (when downloading using a script).
Also documented that you can set `HOST`.
ACKs for top commit:
m3dwards:
ACK 4bb4c86599
maflcko:
review ACK 4bb4c86599🚏
Tree-SHA512: b4803d39a21cb622fd2388a0528b76d2b502956e2505385d3da201143b0afcf6f9d71c8c28937f27b70d2588fb6da677da058bdcd67b90fb53617acc3a727818
61e800e75c test: headers sync timeout (stringintech)
Pull request description:
When reviewing PR #32051 and considering which functional tests might need to be adapted/extended accordingly, I noticed there appears to be limited functional test coverage for header sync timeouts and thought it might be helpful to add one.
This test attempts to cover two scenarios:
1. **Normal peer timeout behavior:** When a peer fails to respond to initial getheaders requests within the timeout period, it should be disconnected and the node should attempt to sync headers from the next available peer.
2. **Noban peer behavior:** When a peer with noban privileges times out, it should remain connected while the node still attempts to sync headers from other peers.
ACKs for top commit:
maflcko:
re-ACK 61e800e75c 🗝
stratospher:
reACK 61e800e7.
Tree-SHA512: b8a867e7986b6f0aa00d81a84b205f2bf8fb2e6047a2e37272e0244229d1f43020e9031467827dabbfe7849a91429f2685e00a25356e2ed477fa1a035fa0b1fd
28416f367a test: fix intermittent failure in rpc_invalidateblock.py (stratospher)
Pull request description:
resolves#32965.
node1 (with 24 blocks) causes node0 (with 6 blocks + 1 extra header) to silently reorg. so move the subtest to a point before the 20 blocks are generated so that node1's state doesn't cause node0 to silently reorg.
ACKs for top commit:
maflcko:
lgtm ACK 28416f367a
mzumsande:
Code Review ACK 28416f367a
Tree-SHA512: f6cc682b8e5416125f887c094d5e291dd37f0bfc41d7c0de218f3e24fa1ea0cd642f7a1e362f3127f68cde725a67f3054501326b9bd25f0caa9a05de7d0052b0
This adds a large simulation fuzz test for all TxOrphanage public interface
functions, using a mix of comparison with expected behavior (in case it is
fully specified), and testing of properties exhibited otherwise.
This is preparation for the simulation fuzz test added in a later commit. Since
AddChildrenToWorkSet consumes randomness, there is no way for the simulator to
exactly predict its behavior. By returning the set of made-reconsiderable announcements
instead, the simulator can instead test that it is *a* valid choice, and then
apply it to its own data structures.
For the default number of peers (125), allows each to relay a default
descendant package (up to 25-1=24 can be missing inputs) of small (9
inputs or fewer) transactions out of order.
This limit also gives acceptable bounds for worst case LimitOrphans iterations.
Functional tests aren't changed to check for larger cap because it would
make the runtime too long.
Also deletes the now-unused DEFAULT_MAX_ORPHAN_TRANSACTIONS.
This is largely a reimplementation using boost::multi_index_container.
All the same public methods are available. It has an index by outpoint,
per-peer tracking, peer worksets, etc.
A few differences:
- Limits have changed: instead of a global limit of 100 unique orphans,
we have a maximum number of announcements (which can include duplicate
orphans) and a global memory limit which scales with the number of
peers.
- The maximum announcements limit is 100 to match the original limit,
but this is actually a stricter limit because the announcement count
is not de-duplicated.
- Eviction strategy: when global limits are reached, a per-peer limit
comes into play. While limits are exceeded, we choose the peer whose
“DoS score” (max usage / limit ratio for announcements and memory
limits) is highest and evict announcements by entry time, sorting
non-reconsiderable ones before reconsiderable ones. Since announcements
are unique by (wtxid, peer), as long as 1 announcement remains for a
transaction, it remains in the orphanage.
- This eviction strategy means no peer can influence the eviction of
another peer’s orphans.
- Also, since global limits are a multiple of per-peer limits, as long
as a peer does not exceed its limits, its orphans are protected from
eviction.
- Orphans no longer expire, since older announcements are generally
removed before newer ones.
- GetChildrenFromSamePeer returns the transactions from newest to
oldest.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
Move towards a model where TxOrphanage is initialized with limits that
it remembers throughout its lifetime.
Remove the param. Limiting by number of unique orphans will be removed
in a later commit.
Now that -maxorphantx is gone, this does not change the node behavior.
The parameter is only used in tests.
c18bf0bd9b refactor: cleanup index logging (Sjors Provoost)
Pull request description:
This PR removes the use of `__func__` from index logging, since we have `-logsourcelocations`.
It also improves readability by putting `GetName()` in a more logical place.
Before
> coinstatsindex: best block of the index not found. Please rebuild the index.
After:
> best block of coinstatsindex not found. Please rebuild the index.
I found myself maintaining this commit as part of https://github.com/Sjors/bitcoin/pull/86, but since that might never land here, it seemed better to split it into its own PR (or get rid of it).
ACKs for top commit:
l0rinc:
Lightweight code review ACK c18bf0bd9b
maflcko:
review ACK c18bf0bd9b🚣
Tree-SHA512: 755948371e3ff7a5515b63ce48075631ec7868d69c3c1469176d5be0e8b28e1c071e206ae3f7320f87d8c441f815894acfef61621f05795b5ff6b8a5a3031e3b
node1 (with 24 blocks) causes node0 (with 6 blocks) to silently
reorg. so move the subtest to a point before the 20 blocks are
generated so that node1's state doesn't cause node0 to silently
reorg.
44f3bae300 depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE` (Hennadii Stepanov)
Pull request description:
When using CMake policies 3.14 and below, the `export(PACKAGE)` command by default populates the user package registry, which is stored outside the build tree. Setting the `CMAKE_EXPORT_NO_PACKAGE_REGISTRY` variable disables this side effect.
In CMake 3.15 and later, this behavior is disabled by default, and the variable has no effect.
This PR forces `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE` globally, rather than managing it for each dependency package individually rather. It may be reverted once all CMake-based packages have updated to policies 3.15 or newer.
Fixes https://github.com/bitcoin/bitcoin/issues/32938.
ACKs for top commit:
fanquake:
ACK 44f3bae300
Tree-SHA512: 0aac398b7182e80185b064d59f81aece4d8477a609fad9cc3fee317da2aff43b66ef7db1efec0135b4f0feaad23b1db664e33bd035fe659712c5b2a9bf2d6fb6
fad191ff48 ci: Avoid cd into build dir (MarcoFalke)
Pull request description:
Changing into the build dir is confusing and brittle, because the following commands implicitly assume it. So they could break on unrelated changes.
The changes are required for stuff like:
* cmake presets (see https://github.com/bitcoin/bitcoin/pull/30871#issuecomment-2344031208)
* meta ci tests (like https://github.com/bitcoin/bitcoin/pull/32874)
So remove the `cd` and just make the build dir explicit.
ACKs for top commit:
hebasto:
ACK fad191ff48, I have reviewed the code and it looks OK.
Tree-SHA512: a88a9341445ffe28a0dac3815f235ec8eb0459d10a91a80829fd3184762d3c807d0f68c56243b20c04a6efa5becd8a7fad568f43c2b1e6af1ff8ba07b140ef87
12a6959892 cmake: Drop no longer necessary "cmakeMinimumRequired" object (Hennadii Stepanov)
Pull request description:
The minimum required CMake version is 3.22:6a13a6106e/CMakeLists.txt (L10)
ACKs for top commit:
fanquake:
ACK 12a6959892 - has been unneeded since it was introduced (minimum was already 3.22).
Tree-SHA512: 26f97662bfe52986e19e38dbf4ab8e1e7558bc78c3a65593cbecd1f35887bba7a9f7d8a3d08ccfab8396f41c2334cdad5b0e503999a759cfa158d3bb8d0d14d7
84ef5524d5 fix spelling in tor.md docs (stutxo)
Pull request description:
This PR is to fix some spelling mistakes i found of the word occurrences! there are two occurrences of this mistake.
thanks!
ACKs for top commit:
maflcko:
lgtm ACK 84ef5524d5
willcl-ark:
ACK 84ef5524d5
delta1:
ACK 84ef5524d5
Tree-SHA512: 4ba71b772fdc8cf36ada7493d29fb5b312a7a6085099347162eb3495db4de984b0417b7861f2927c617cbd552741356e26688479601bdf7e835c15e097aa28f3
8f766f39df ci: enable -Werror=dev (fanquake)
7b420ca834 guix: configure with -Werror=dev (fanquake)
44097ddb19 cmake: enable -Werror=dev in dev-mode preset (fanquake)
Pull request description:
Pass `-Werror=dev` in the CI, Guix and the `dev-mode` preset.
https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-Werror:
> Make developer warnings errors.
> Make warnings that are meant for the author of the CMakeLists.txt files errors. By default this will also turn on deprecated warnings as errors.
Pulled out of #32865.
ACKs for top commit:
Sjors:
re-ACK 8f766f39df
hebasto:
ACK 8f766f39df, tested on Ubuntu 24.04.
Tree-SHA512: 0fa321b77d2519b5249d90590664c4e5938ac86209b068658647adf97ab55ea4d54c913aae2f622385fe2f41d7c851cd5d7371905fdad38b66cb124371e16ac7
Expiry is going away in a later commit.
This is only an RPC change. Behavior of the orphanage does not change.
Note that getorphantxs is marked experimental.
