478695982b88fd48a277a9eb197ce819688487f0 bench: Add a benchmark for descriptor expansion (Ben Woosley)
Pull request description:
Taken from https://github.com/bitcoin/bitcoin/pull/16116 , as requested here: https://github.com/bitcoin/bitcoin/pull/25748#issuecomment-1205441706
ACKs for top commit:
achow101:
ACK 478695982b88fd48a277a9eb197ce819688487f0
Tree-SHA512: f2efdf8f84e1783c7c298abe65123191d25cab0a9da2d0ff5957a60acc2d10e356151d7ecec0d98d28c456f42ddef50efd70c7edc0c9012df2a977e080515b9d
daabd4121114fe6f780bccab311a522c0717c5b8 net: simplify GetLocalAddress() (Vasil Dimov)
Pull request description:
There is no need to use two variables `ret` and `addr` of the same type
`CService` and assign one to the other in a strange way like
`ret = CService{addr}`.
ACKs for top commit:
jarolrod:
ACK daabd4121114fe6f780bccab311a522c0717c5b8
aureleoules:
ACK daabd4121114fe6f780bccab311a522c0717c5b8.
w0xlt:
ACK daabd41211
Tree-SHA512: 4bbd3746bc30fbc05bb32b58bb122c938acd849c0f72f1d3e8170557c1999ec26a888e06e874c3fc22562a2becddc7d817db7d174e0e1b383e8d74c39aa1e898
9376a6dae41022874df3b9302667796a9fb7b81d refactor: make active_chain_tip a reference (Aurèle Oulès)
Pull request description:
This PR fixes a TODO introduced in #21055.
Makes `active_chain_tip` argument in `CheckFinalTxAtTip` function a reference instead of a pointer.
ACKs for top commit:
dongcarl:
ACK 9376a6dae41022874df3b9302667796a9fb7b81d
Tree-SHA512: c36d1769e0b9598b7f79334704b26b73e958d54caa3bd7e4eff954f3964fcf3f5e3a44a5a760497afad51b76e1614c86314fe035e4083c855e3574a620de7f4d
acda7e8686a1f7a967d6331a2f6a3a01389c3048 [coin selection] consolidate m_change_target and m_min_change_target (glozow)
Pull request description:
These values are both intended for the same thing. Their divergence seems to be the result of an incomplete rename.
ACKs for top commit:
achow101:
ACK acda7e8686a1f7a967d6331a2f6a3a01389c3048
Xekyo:
ACK acda7e8686a1f7a967d6331a2f6a3a01389c3048
furszy:
ACK acda7e86
aureleoules:
ACK acda7e8686a1f7a967d6331a2f6a3a01389c3048.
Tree-SHA512: 4b86171af5d893f7172373bb404bad12c49588ad1e22eb0544c242173f4bc4dede2ff1270c93c9f02f503ab8d9f66b841a8319d0ecb5e896d0fe8727cf03dbf4
2e79fb6585c802813f80080fc2cadc5b54ddebfb validation tests: Use existing {Chainstate,Block}Man (Carl Dong)
Pull request description:
This is split up because it is needed for two changes:
* https://github.com/bitcoin/bitcoin/pull/25781
* https://github.com/bitcoin/bitcoin/pull/25623
ACKs for top commit:
adam2k:
ACK tested 2e79fb6585c802813f80080fc2cadc5b54ddebfb
aureleoules:
ACK 2e79fb6585c802813f80080fc2cadc5b54ddebfb.
Tree-SHA512: 2cd6a2fec19545f8ffc77e37ccb793aa6cb5815bb1b5e560c0345af6e0f890fd500ae3297b044d3f6f613b8dd7fd4553f5fc2824013342b9e25af1fe2b624967
b16f93caddcd3254eaf3dc43e09adf2142a9c40a script/sign: remove needless IsSolvable() utility (Antoine Poinsot)
c232ef20c0fd2e3b55355e52684091cad3af5247 outputtype: remove redundant check for uncompressed keys in AddAndGetDestinationForScript (Antoine Poinsot)
Pull request description:
Now that we have descriptors there is no need to try to sign for a scriptPubKey using dummy signatures, and using a mocked verification of this witness against the interpreter, just to make sure we know how to spend such a Script. Just try to infer a solvable descriptor: any scriptPubKey that we can sign for can be inferred as such.
This came up in #24149 but i think it's worth it on its own.
ACKs for top commit:
instagibbs:
ACK b16f93cadd
achow101:
re-ACK b16f93caddcd3254eaf3dc43e09adf2142a9c40a
furszy:
ACK b16f93ca, only change is the `IsSolvable` helper function removal.
Tree-SHA512: 137068157ce90210b710b1bf9ac3c400e2ff5af1112f892094b69875ea473d6a899f52adb51e5030cb907dee517602059cd1661107808558efa5de842ba12b41
70a55c059b014c7a687de7a4813a90c65148aed4 psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATION (Andrew Chow)
Pull request description:
Fixes#25749
ACKs for top commit:
instagibbs:
ACK 70a55c059b014c7a687de7a4813a90c65148aed4
darosior:
re-utACK 70a55c059b014c7a687de7a4813a90c65148aed4
jonatack:
Review ACK 70a55c059b014c7a687de7a4813a90c65148aed4, this should avoid the issue reported in https://github.com/bitcoin/bitcoin/issues/25749
Tree-SHA512: 6bb58e1cda9a5baa50fcd24f818b5b27ed94f0d33da3f71f6e457618176611bf2a84e1864e9a48d9303c301252bc4c1dee8b19a67dd713e849fb9442851ca341
fb9faffae3a26b8aed8b671864ba679747163019 extended keys: fail to derive too large depth instead of wrapping around (Antoine Poinsot)
8dc6670ce159c2b080e9f735c6603a601d40b6ac descriptor: don't assert success of extended key derivation (Antoine Poinsot)
50cfc9e7613d6cf6b534df6e551238b80678c70d (pubk)key: mark Derive() as nodiscard (Antoine Poinsot)
0ca258a5ace798c4e54308aa8a09b1ab3302cd7e descriptor: never ignore the return value when deriving an extended key (Antoine Poinsot)
d3599c22bd4c6b3cfaaadd675e95ebe3b3cb1749 spkman: don't ignore the return value when deriving an extended key (Antoine Poinsot)
Pull request description:
We would previously silently wrap the derived child's depth back to `0`. Instead, explicitly fail when trying to derive an impossible depth, and handle the error in callers.
An extended fuzzing corpus of `descriptor_parse` triggered this behaviour, which was reported by MarcoFalke.
Fixes#25751.
ACKs for top commit:
achow101:
re-ACK fb9faffae3a26b8aed8b671864ba679747163019
instagibbs:
utACK fb9faffae3
Tree-SHA512: 9f75c23572ce847239bd15e5497df2960b6bd63c61ea72347959d968b5c4c9a4bfeee284e76bdcd7bacbf9eeb70feee85ffd3e316f353ca6eca30e93aafad343
d8b26abed91c421e8517a2c9a60b57d988121b3a build: move raw rule into Makefile.am (fanquake)
Pull request description:
The same rule is used by the tests and benchmarks to generate headers,
and currently causes #25501. Just deduplicate the code into Makefile.am.
Fixes: #25501.
ACKs for top commit:
hebasto:
ACK d8b26abed91c421e8517a2c9a60b57d988121b3a, tested on Ubuntu 22.04, the moved code was verified using `git diff --color-moved=dimmed-zebra HEAD~1..HEAD`.
jarolrod:
tACK d8b26abed91c421e8517a2c9a60b57d988121b3a
Tree-SHA512: 249813318c92f992a89002fb9b96e70fca6ca97b2136ba0a7f5cc312e9abe24fbbe9a8faddb3bc1c0d775ae901bc91eab63ba564810bb2e3b9d56a2b1a107eb1
Use {Chain,}TestingSetup's existing {Chainstate,Block}Manager and avoid
unnecessarily creating a local one.
This also helps reduce the code diff for a later commit where we change
{Chainstate,Block}Manager's constructor signature.
There is no need to use two variables `ret` and `addr` of the same type
`CService` and assign one to the other in a strange way like
`ret = CService{addr}`.
76b3c37fcb93b4bcb047e0500fdaa605160e25d5 refactor: wallet: return util::Result from `GetReservedDestination` methods (Sebastian Falbesoner)
Pull request description:
This PR is a follow-up to #25218, as suggested in comment https://github.com/bitcoin/bitcoin/pull/25218#discussion_r907710067. The interfaces of the methods `ReserveDestination::GetReservedDestination`, `{Legacy,Descriptor,}ScriptPubKeyMan::GetReservedDestination` are improved by returning `util::Result<CTxDestination>` instead of `bool` in order to get rid of the two `CTxDestination&` and `bilingual_str&` out-parameters.
ACKs for top commit:
furszy:
ACK 76b3c37f
Tree-SHA512: bf15560a88d645bcf8768024013d36012cd65caaa4a613e8a055dfd8f29cb4a219c19084606992bad177920cdca3a732ec168e9b9526f9295491f2cf79cc6815
544b4332f0e122167bdb94dc963405422faa30cb Add wallet tests for spending rawtr() (Pieter Wuille)
e1e3081200a71b6c9b0dcf236bc2a37ed1aa7552 If P2TR tweaked key is available, sign with it (Pieter Wuille)
8d9670ccb756592bddb2a269bf5078d62658537b Add rawtr() descriptor for P2TR with unknown tweak (Pieter Wuille)
Pull request description:
It may be useful to be able to represent P2TR outputs in descriptors whose script tree and/or internal key aren't known. This PR does that, by adding a `rawtr(KEY)` descriptor, where the KEY represents the output key directly. If the private key corresponding to that output key is known, it also permits signing with it.
I'm not convinced this is desirable, but presumably "tr(KEY)" sounds more intended for direct use than "rawtr(KEY)".
ACKs for top commit:
achow101:
ACK 544b4332f0e122167bdb94dc963405422faa30cb
sanket1729:
code review ACK 544b4332f0e122167bdb94dc963405422faa30cb
w0xlt:
reACK 544b4332f0
Tree-SHA512: 0de08de517468bc22ab0c00db471ce33144f5dc211ebc2974c6ea95709f44e830532ec5cdb0128c572513d352120bd651c4559516d4500b5b0a3d257c4b45aca
fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke)
Pull request description:
It has been pointed out that a bug in this function can prevent block template creation. ( https://github.com/bitcoin/bitcoin/pull/24080#issuecomment-1065148776 ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35.
ACKs for top commit:
ajtowns:
ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d - looks fine to me
glozow:
ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d
Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
b5a762a35368ad5ab07018e5da14229291a54b94 wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` log (w0xlt)
Pull request description:
This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code.
Master:
```
Setting spkMan to active: id = 9f..04, type = 3, internal = 0
Setting spkMan to active: id = 3d..21, type = 2, internal = 0
Setting spkMan to active: id = 69..d4, type = 0, internal = 1
Setting spkMan to active: id = 97..ea, type = 1, internal = 1
```
PR:
```
Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false
Setting spkMan to active: id = 83..dc, type = legacy, internal = true
Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true
Setting spkMan to active: id = bd..d2, type = bech32, internal = true
Setting spkMan to active: id = 13...7c, type = bech32m, internal = true
```
ACKs for top commit:
S3RK:
Code review ACK b5a762a35368ad5ab07018e5da14229291a54b94
achow101:
ACK b5a762a35368ad5ab07018e5da14229291a54b94
theStack:
Code-review ACK b5a762a35368ad5ab07018e5da14229291a54b94
Tree-SHA512: 5a79706d5452e523b0456fb8435545c6c8e550b6722c0d7966af79011275a97ed97cab297562e031d601aa855118082c5b770af118783b1faaaec0cba9f9ee6a
bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf Change mapWallet to be a std::unordered_map (Andrew Chow)
272356024db978c92112167f8d8e4cc62adad63d Change getWalletTxs to return a set instead of a vector (Andrew Chow)
97532867cf51db3e941231fbdc60f9f4fa0012a0 Change mapTxSpends to be a std::unordered_multimap (Andrew Chow)
1f798fe85ba952273005f68e36ed48cfc36f4c9d wallet: Cache SigningProviders (Andrew Chow)
8a105ecd1aeff15f84c3883e2762bf71ad59d920 wallet: Use CalculateMaximumSignedInputSize to indicate solvability (Andrew Chow)
Pull request description:
While running my coin selection simulations, I noticed that towards the end of the simulation, the wallet would become slow to make new transactions. The wallet generally performs much more slowly when there are a large number of transactions and/or a large number of keys. The improvements here are focused on wallets with a large number of transactions as that is what the simulations produce.
Most of the slowdown I observed was due to `DescriptorScriptPubKeyMan::GetSigningProvider` re-deriving keys every time it is called. To avoid this, it will now cache the `SigningProvider` produced so that repeatedly fetching the `SigningProvider` for the same script will not result in the same key being derived over and over. This has a side effect of making the function non-const, which makes a lot of other functions non-const as well. This helps with wallets with lots of address reuse (as my coin selection simulations are), but not if addresses are not reused as keys will end up needing to be derived the first time `GetSigningProvider` is called for a script.
The `GetSigningProvider` problem was also exacerbated by unnecessarily fetching a `SigningProvider` for the same script multiple times. A `SigningProvider` is retrieved to be used inside of `IsSolvable`. A few lines later, we use `GetTxSpendSize` which fetches a `SigningProvider` and then calls `CalculateMaximumSignedInputSize`. We can avoid a second call to `GetSigningProvider` by using `CalculateMaximumSignedInputSize` directly with the `SigningProvider` already retrieved for `IsSolvable`.
There is an additional slowdown where `ProduceSignature` with a dummy signer is called twice for each output. The first time is `IsSolvable` checks that `ProduceSignature` succeeds, thereby informing whether we have solving data. The second is `CalculateMaximumSignedInputSize` which returns -1 if `ProduceSignature` fails, and returns the input size otherwise. We can reduce this to one call of `ProduceSignature` by using `CalculateMaximumSignedInputSize`'s result to set `solvable`.
Lastly, a lot of time is spent looking in `mapWallet` and `mapTxSpends` to determine whether an output is already spent. The performance of these lookups is slightly improved by changing those maps to use `std::unordered_map` and `std::unordered_multimap` respectively.
ACKs for top commit:
Xekyo:
ACK bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf
furszy:
diff re-reACK bc886fcb
Tree-SHA512: fd710fe1224ef67d2bb83d6ac9e7428d9f76a67f14085915f9d80e1a492d2c51cb912edfcaad1db11c2edf8d2d97eb7ddd95bfb364587fb1f143490fd72c9ec1
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm)
701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm)
Pull request description:
(note: this was originally titled "add analyzerawtransaction RPC")
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream.
There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument.
ACKs for top commit:
jonatack:
ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
achow101:
re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
acbea66589100fe6ef726f4b2a92ec26132ef17b rest: clean-up for `mempool` endpoints (brunoerg)
Pull request description:
The functions `rest_mempool_info` and `rest_mempool_contents` are similar, the only difference between them is:
`rest_mempool_info` uses `MempoolInfoToJSON` to get the mempool informations and `rest_mempool_contents` uses `MempoolToJSON`, for this reason this PR creates a new function to handle it and reduce duplicated code.
Also,
1. Rename `strURIPart` to `str_uri_part`.
2. Rename `strJSON` to `str_json`.
ACKs for top commit:
stickies-v:
re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b - verified that just the error message was updated since da0c612c3d
theStack:
re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b
Tree-SHA512: 35f6f0732a573fe8a6cdcc782f89ae3427a1de19f069a68c9c51bb525118c2b07e20303cbe19b9d4b7d1ad055d69c32def2d0fb8f886c851da562dd9ce33ad6a
a23cca56c0a7f4a267915b4beba3af3454c51603 refactor: Replace BResult with util::Result (Ryan Ofsky)
Pull request description:
Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in https://github.com/bitcoin/bitcoin/pull/25665.
This change makes the following improvements originally implemented in https://github.com/bitcoin/bitcoin/pull/25665:
- More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value.
- Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors.
- More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent.
- Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?)
- Has unit tests.
ACKs for top commit:
MarcoFalke:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603 🏵
jonatack:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603
Tree-SHA512: 2769791e08cd62f21d850aa13fa7afce4fb6875a9cedc39ad5025150dbc611c2ecfd7b3aba8b980a79fde7fbda13babdfa37340633c69b501b6e89727bad5b31
fadd8b2676f6d68ec87189871461c9a6a6aa3cac addrman: Use system time instead of adjusted network time (MarcoFalke)
Pull request description:
This changes addrman to use system time for address relay instead of the network adjusted time.
This is an improvement, because network time has multiple issues:
* It is non-monotonic, even if the system time is monotonic.
* It may be wrong, even if the system time is correct.
* It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...)
This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS.
ACKs for top commit:
dergoegge:
Code review ACK fadd8b2676f6d68ec87189871461c9a6a6aa3cac
Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
b01f336708019f8c8274ea701d3446e4123e7af2 util, refactor: Drop explicit conversion to fs::path (Hennadii Stepanov)
138c668e2b4d64279ddefbe07c1d9b7c3d3c537c util, refactor: Use GetPathArg to read "-rpccookiefile" value (Hennadii Stepanov)
1276090705060fcc97072481c2383bbaaa556194 util, refactor: Use GetPathArg to read "-conf" value (Hennadii Stepanov)
Pull request description:
This PR is a continuation of bitcoin/bitcoin#24265 and bitcoin/bitcoin#24306.
Now the following command-line arguments / configure options been read with the `GetPathArg` method:
- `-conf`, also `includeconf` values been normalized
- `-rpccookiefile`
ACKs for top commit:
jarolrod:
Code Review ACK b01f336708019f8c8274ea701d3446e4123e7af2
ryanofsky:
Code review ACK b01f336708019f8c8274ea701d3446e4123e7af. Changes since last review: just dropping first commit (NormalizedPathFromString) as suggested
Tree-SHA512: 2d26d50b73542acdbcc63a32068977b2a49a017d31ca337471a0446f964eb0a6e3e4e3bb1ebe6771566a260f2cae3bc2ebe93b4b523183cea0d51768daab85c9
faab8dceb37a944d0763fcca390342111e6a9fcc Remove unused SetTip(nullptr) code (MacroFake)
Pull request description:
Now that this path is no longer used after commit b51e60f91472da5216116626afc032acd5616e85, we can remove it.
Future code should reset `CChain` by simply discarding it and constructing a fresh one.
ACKs for top commit:
ryanofsky:
Code review ACK faab8dceb37a944d0763fcca390342111e6a9fcc. Just moved an assert statement since last review
Tree-SHA512: 7dc273b11133d85d32ca2a69c0c7c07b39cdd338141ef5b51496e7de334a809864d5459eb95535497866c8b1e468aae84ed8f91b543041e6ee20130d5622874e
For some reason, the primary consumer of getWalletTxs requires the
transactions to be in hash order when it is processing them. std::map
will iterate in hash order so the transactions end up in that order when
placed into the vector. To ensure this order when mapWallet is no longer
ordered, the vector is replaced with a set which will maintain the hash
order.
In order to avoid constantly re-deriving the same keys in
DescriptorScriptPubKeyMan, cache the SigningProviders generated inside
of GetSigningProvider.
Also:
- Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr
DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer
arithmetic overflow checking available to constexpr.
- Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES.
- Pass in max_size_bytes parameter to InitS*Cache(), modify log line to
no longer allude to maxsigcachesize being split evenly between the two
validation caches.
- Fix possible integer truncation and add a comment.
[META] I've kept the integer types as int64_t in order to not introduce
unintended behaviour changes, in the next commit we will make
them size_t.
This fixes an potential overflow which existed prior to this patchset.
If CuckooCache::cache<Element, Hash>::setup_bytes is called with a
`size_t bytes` which, when divided by sizeof(Element), does not fit into
an uint32_t, the implicit conversion to uint32_t in the call to setup
will result in an overflow.
At least on x86_64, this overflow is possible:
static_assert(std::numeric_limits<size_t>::max() / 32 <= std::numeric_limits<uint32_t>::max());
static_assert(std::numeric_limits<size_t>::max() / 4 <= std::numeric_limits<uint32_t>::max());
This commit detects such cases and signals to callers that the `size_t
bytes` input is too large.
1. -maxsigcachesize is a DEBUG_ONLY option
2. Almost 7 years has passed since its semantics change in
830e3f3d027ba5c8121eed0f6a9ce99961352572 from "number of entries" to
"number of mebibytes"
3. A std::new_handler was added to the codebase after the original PR
which introduced this limit, which will terminate immediately instead
of causing trouble by being caught somewhere unexpected.
