If -acceptstalefeeestimates option is passed stale fee estimates can now
be read when operating in regtest environments.
Additionally, this commit updates all declarations of the CBlockPolicyEstimator
class to include a the second constructor variable.
Github-Pull: #27622
Rebased-From: cf219f29f3c5b41070eaab9a549a476f01990f3a
`evhttp_uri_parse` can return a nullptr, for example when the URI
contains invalid characters (e.g. "%").
`GetQueryParameterFromUri` passes the output of `evhttp_uri_parse`
straight into `evhttp_uri_get_query`, which means that anyone calling
a REST endpoint in which query parameters are used (e.g. `rest_headers`)
can cause a segfault.
This bugfix is designed to be minimal and without additional behaviour change.
Follow-up work should be done to resolve this in a more general and robust way,
so not every endpoint has to handle it individually.
3153e7d779ac284f86e433af033d63f13f361b6f [fuzz] Add HeadersSyncState target (dergoegge)
53552affca381cdb5103ecdbcc7f3fb562e66ac4 [headerssync] Make m_commit_offset protected (dergoegge)
Pull request description:
This adds a fuzz target for the `HeadersSyncState` class.
I am unsure how well this is able to cover the logic since it is just processing unserialized CBlockHeaders straight from the fuzz input (headers are sometimes made continuous). However, it does manage to get to the redownload phase so i thought it is better then not having fuzzing at all.
It would also be nice to fuzz the p2p logic that is using `HeadersSyncState` (e.g. `TryLowWorkHeadersSync`, `IsContinuationOfLowWorkHeadersSync`) but that likely requires some more work (refactoring👻).
ACKs for top commit:
mzumsande:
ACK 3153e7d779ac284f86e433af033d63f13f361b6f
Tree-SHA512: 8a4630ceeeb30e4eeabaa8eb5491d98f0bf900efe7cda07384eaac9f2afaccfbcaa979cc1cc7f0b6ca297a8f5c17a7759f94809dd87eb87d35348d847c83e8ab
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan)
106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan)
b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan)
18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152.
#### Context
There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238.
#### Changes
Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files.
There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory.
Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed.
ACKs for top commit:
hebasto:
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3
Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
faf8dc496e761a15956f8226d727f4bbab8dff82 fuzz: Remove legacy int parse fuzz tests (MarcoFalke)
Pull request description:
The fuzz tests checked that the result of the new function was equal to the legacy function. (Side note: The checks were incomplete, as evident by the follow-up fix in commit b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822).
Given that they haven't found any issues in years (beside missing the above issue, that they couldn't catch), it seems time to remove them.
They may come in handy in the rare case that someone would want to modify `LocaleIndependentAtoi()` or `Parse*Int*()`, however that seems unlikely. Also, appropriate checks can be added then.
ACKs for top commit:
fanquake:
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
dergoegge:
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
Tree-SHA512: 4ec88b9fa8ba49a923b0604016f0f471b3c9b9e0ba6c5c3dc4e20503c6994789921e7221d9ec467a2a37a73f21a70ba51ba3370ed5ad311dee989e218290b29a
cd0c8eeb0940790b6ba83786d1c9e362d4dc4829 [net] Pass nRecvFloodSize to CNode (dergoegge)
860402ef2ed728ef096dda4e65e77d566782209f [net] Remove trivial GetConnectionType() getter (dergoegge)
b5a85b365a4abd98176b0935015dbb502cc3e6f6 [net] Delete CNetMessage copy constructor/assignment op (dergoegge)
Pull request description:
Follow-up PR for #27257
* Deletes the copy constructor/assignment operator of `CNetMessage`
* Removes trivial getter for the connection type
* Avoids passing `nRecvFloodSize` to CNode methods by passing it to `CNode` on creation
ACKs for top commit:
jnewbery:
utACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
theStack:
ACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
Tree-SHA512: 673a758668617f69fba77e61f0eaa1538da27a4849c82c98742436692baa2d7f001129af3e7a66b160e599d12109dac08137a146f10ff9b9ebdc5c2237311d41
3566aa7d495bb783bbd135726238d9f2a9e9f80e [net] Remove CNode friends (dergoegge)
3eac5e7cd1eda6ababb9af9cd72dae08d395993d [net] Add CNode helper for send byte accounting (dergoegge)
60441a3432df10f5d7c15c09c9569f27a793625b scripted-diff: [net] Rename CNode process queue members (dergoegge)
6693c499f7d48aaa1f527a1a860b943fc8442241 [net] Make cs_vProcessMsg a non-recursive mutex (dergoegge)
23d93526545c271b0eed2bd468681864c4213cce [net] Make CNode msg process queue members private (dergoegge)
897e342d6ec320c286753daef01d6eb9839e2c4d [net] Encapsulate CNode message polling (dergoegge)
cc5cdf877666c232a94f03faaf430cbeb6968372 [net] Deduplicate marking received message for processing (dergoegge)
ad44aa5c64d4ee5f31c867fda26350ab560575b7 [net] Add connection type getter to CNode (dergoegge)
Pull request description:
We should define clear interfaces between CNode, CConnman and PeerManager. This PR makes a small step in that direction by ending the friendship of CNode, CConnman and ConnmanTestMsg. CNode's message processing queue is made private in the process and its mutex is turned into a non-recursive mutex.
ACKs for top commit:
jnewbery:
utACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
vasild:
ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
theStack:
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
brunoerg:
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
Tree-SHA512: 26b87da5054e32401b693b2904e9c5f40e35a53937c0b6cf44b8597034ad07bacf27d87cdffc54d3e7ccfebde4231ef30a38d326f88cc18133bbb34688ead567
The fs.* files are already part of the libbitcoin_util library. With the
introduction of the fs_helpers.* it makes sense to move fs.* into the
util/ directory as well.
This is an extraction of filesystem related functions from util/system
into their own utility file.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving these functions out of system.h allows including them from a
separate source file without including the ArgsManager definitions from
system.h.
95ad70ab652ddde7de65f633c36c1378b26a313a test: Default initialize `should_freeze` to `true` (Hennadii Stepanov)
cea50521fe810111a8a3c84ad14f944eafb5b658 refactor: Drop no longer used `swap` member functions (Hennadii Stepanov)
a87fb6bee5a7fb0879b3adea9a29997f1331acb0 clang-tidy: Fix modernize-use-default-member-init in `CScriptCheck` (Hennadii Stepanov)
b4bed5c1f98c0eed18f52fdcea11a420c10ed98d refactor: Drop no longer used `CScriptCheck()` default constructor (Hennadii Stepanov)
d8427cc28e3a9ac3319fb452b16661957c812b8f refactor: Use move semantics in `CCheckQueue::Loop` (Hennadii Stepanov)
9a0b5241396efe3b3ceb3931717c30bb94f99bfb clang-tidy, test: Fix bugprone-use-after-move in `Correct_Queue_range()` (Hennadii Stepanov)
04831fee6dca3eb86cd1d6b9ef879b296263fe35 refactor: Make move semantics explicit for callers (Hennadii Stepanov)
6c2d5972f3544c4f3e987828a99e88f27b62cf87 refactor: Use move semantics in `CCheckQueue::Add` (Hennadii Stepanov)
06820032142a75cc3c5b832045058bc6f6f74786 test, refactor: Avoid `CScriptCheck::swap` in `transaction_tests` (Hennadii Stepanov)
15209d97c6aad7d5c199fe007ad39b91c8ee6562 consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts` (Hennadii Stepanov)
Pull request description:
This PR makes code more succinct and readable by using move semantics.
ACKs for top commit:
martinus:
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
achow101:
ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
TheCharlatan:
re-ACK 95ad70ab65
MarcoFalke:
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a 🚥
Tree-SHA512: adda760891b12d252dc9b823fe7c41eed660364b6fb1a69f17607d7a31eb0bbb82a80d154a7acfaa241b5de37d42a293c2b6e059f26a8e92d88d3a87c99768fb
fixes#22638
If we find a duplicate key and error, clear `values` before returning so that
WriteSettings will write an empty file, therefore clearing it.
This aligns with GUI behaviour added in 1ee6d0b.
2b373fe49d64f04ceab2309d3f40da7bac6b37d6 docs: update assumeutxo.md (James O'Beirne)
87a1108c81fe0cb15c3860e3a67dc1f43ffec705 test: add snapshot completion unittests (James O'Beirne)
d70919a88fc90a2662f9a844deb085d03ee7b5d8 refactor: make MempoolMutex() public (James O'Beirne)
7300ced9de22e6d1bff816e6538d3370cebe7501 log: add LoadBlockIndex() message for assumedvalid blocks (James O'Beirne)
d96c59cc5cd2f73f1f55c133c52208671fe75ef3 validation: add ChainMan logic for completing UTXO snapshot validation (James O'Beirne)
f2a4f3376f1476b38a79a549bd81ba3006225df6 move-only-ish: init: factor out chainstate initialization (James O'Beirne)
637a90b973f60555ea4fef4b845ffa7533dcb866 add Chainstate::HasCoinsViews() (James O'Beirne)
c29f26b47b8ef978d8689dc0222aa663361ee6cb validation: add CChainState::m_disabled and ChainMan::isUsable (James O'Beirne)
5ee22cdafd2562bcb8bf0ae6025e4b53c826382d add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}() (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606)
Part two of replacing https://github.com/bitcoin/bitcoin/pull/24232.
---
When a user activates a snapshot, the serialized UTXO set data is used to create an "assumed-valid" chainstate, which becomes active in an attempt to get the node to network tip as quickly as possible. Simultaneously in the background, the already-existing chainstate continues "conventional" IBD to both accumulate full block data and serve as a belt-and-suspenders to validate the assumed-valid chainstate.
Once the background chainstate's tip reaches the base block of the snapshot used, we set `m_stop_use` on that chainstate and immediately take the hash of its UTXO set; we verify that this matches the assumeutxo value in the source code. Note that while we ultimately want to remove this background chainstate, we don't do so until the following initialization process, when we again check the UTXO set hash of the background chainstate, and if it continues to match, we remove the (now unnecessary) background chainstate, and move the (previously) assumed-valid chainstate into its place. We then reinitialize the chainstate in the normal way.
As noted in previous comments, we could do the filesystem operations "inline" immediately when the background validation completes, but that's basically just an optimization that saves disk space until the next restart. It didn't strike me as worth the risk of moving chainstate data around on disk during runtime of the node, though maybe my concerns are overblown.
The final result of this completion process is a fully-validated chain, where the only evidence that the user synced using assumeutxo is the existence of a `base_blockhash` file in the `chainstate` directory.
ACKs for top commit:
achow101:
ACK 2b373fe49d64f04ceab2309d3f40da7bac6b37d6
Tree-SHA512: a204e1d6e6932dd83c799af3606b01a9faf893f04e9ee1a36d63f2f1ccfa9118bdc1c107d86976aa0312814267e6a42074bf3e2bf1dead4b2513efc6d955e13d
Also adjusts the previous snapshot chainstate init tests
to account for the fact that the init process is now attempting to
validate and complete background chainstates whose tip is at the
snapshot base block. We use a DisconnectTip() hack to preserve the
nature of the test.
802cc1ef536e11944608fe9ab782d3e962037703 Deduplicate bitcoind and bitcoin-qt init code (Ryan Ofsky)
d172b5c6718b69200c8ad211fe709860081bd692 Add InitError(error, details) overload (Ryan Ofsky)
3db2874bd71d2391747b7385cabcbfef67218c4c Extend bilingual_str support for tinyformat (Ryan Ofsky)
c361df90b9fd34e50bbf1db43b866930e5498357 scripted-diff: Remove double newlines after some init errors (Ryan Ofsky)
Pull request description:
Add common InitConfig function to deduplicate bitcoind and bitcoin-qt code reading config files and creating the datadir.
Noticed the duplicate code while reviewing #27073 and want to remove it because difference in bitcoin-qt and bitcoind behavior make it hard to evaluate changes like #27073
There are a few minor changes in behavior:
- In bitcoin-qt, when there is a problem reading the configuration file, the GUI error text has changed from "Error: Cannot parse configuration file:" to "Error reading configuration file:" to be consistent with bitcoind.
- In bitcoind, when there is a problem reading the settings.json file, the error text has changed from "Failed loading settings file" to "Settings file could not be read" to be consistent with bitcoin-qt.
- In bitcoind, when there is a problem writing the settings.json file, the error text has changed from "Failed saving settings file" to "Settings file could not be written" to be consistent with bitcoin-qt.
- In bitcoin-qt, if there datadir is not accessible (e.g. no permission to read), there is an normal error dialog showing "Error: filesystem error: status: Permission denied [.../settings.json]", instead of an uncaught exception.
ACKs for top commit:
Sjors:
Light review ACK 802cc1ef536e11944608fe9ab782d3e962037703
TheCharlatan:
ACK 802cc1ef536e11944608fe9ab782d3e962037703
achow101:
ACK 802cc1ef536e11944608fe9ab782d3e962037703
Tree-SHA512: 9c78d277e9ed595fa8ce286b97d2806e1ec06ddbbe7bd3434bd9dd7b456faf8d989f71231e97311f36edb9caaec645a50c730bd7514b8e0fe6e6f7741b13d981
545ff924ab6303ffabd91fdfc4f0a4962daf133c refactor: use string_view for RPC named argument values (stickies-v)
7727603e44f8f674e0fc8389e78047e2b56e6052 refactor: reduce unnecessary complexity in ParseNonRFCJSONValue (stickies-v)
1d02e599012721549d4c20b1b37fcc5ee7b961b6 test: add cases to JSON parsing (stickies-v)
Pull request description:
Inspired by MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/26506#discussion_r1036149426). Main purpose of this PR is to minimize copying (potentially large) RPC named arguments when calling `.substr()` by using `std::string_view` instead of `std::string`. Furthermore, cleans up the code by removing unnecessary complexity in `ParseNonRFCJSONValue()` (done first to avoid refactoring required to concatenate `string` and `string_view`), updates some naming and adds a few test cases. Should not introduce any behaviour change.
## Questions
- ~Was there actually any merit to `ParseNonRFCJSONValue()` surrounding the value with brackets and then parsing it as an array? I don't see it, and the new approach doesn't fail any tests. Still a bit suspicious about it though.~
- Cleared up by https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059
- If there are no objections to 7727603e44f8f674e0fc8389e78047e2b56e6052, I think we should follow up with a PR to rename `ParseNonRFCJSONValue()` to a local `Parse()` helper function (that throws if invalid), remove it from `client.h` and merge the test coverage we currently have on `ParseNonRFCJSONValue()` with the coverage we have on `UniValue::read()`.
ACKs for top commit:
ryanofsky:
Code review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c
MarcoFalke:
review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c 📻
Tree-SHA512: b1c89fb010ac9c3054b023cac1acbba2a539a09cf39a7baffbd7f7571ee268d5a6d98701c7ac10d68a814526e8fd0fe96ac1d1fb072f272033e415b753f64a5c
56e37e71a2538a240cc360678aeb752d17bd8f45 Make miniscript fuzzers avoid script size limit (Pieter Wuille)
bcec5ab4ff1039c0c309dbbb9953adbd0a4f3e88 Make miniscript fuzzers avoid ops limit (Pieter Wuille)
213fffa5138229eac2d4a9eda0f643fe90870378 Enforce type consistency in miniscript_stable fuzz test (Pieter Wuille)
e1f30414c6b9434048e089ccc3ec4f475f980c60 Simplify miniscript fuzzer NodeInfo struct (Pieter Wuille)
5abb0f5ac37e8a17072d5989a025227035fdc7e6 Do base type propagation in miniscript_stable fuzzer (Pieter Wuille)
Pull request description:
This adds a number of improvements to the miniscript fuzzers that all amount to rejecting invalid or overly big miniscripts early on:
* Base type propagation in the miniscript_stable fuzzers prevents constructing a large portion of miniscripts that would be illegal, with just a little bit of type logic in the fuzzer. The fuzzer input format is unchanged.
* Ops and script size tracking in GenNode means that too-large scripts (either due to script size limit or ops limit) will be detected on the fly during fuzz input processing, before actually constructing the scripts.
Closes#27147.
ACKs for top commit:
darosior:
re-ACK 56e37e71a2
dergoegge:
tACK 56e37e71a2538a240cc360678aeb752d17bd8f45
Tree-SHA512: 245584adf9a6644a35fe103bc81b619e5b4f5d467571a761b5809d08b1dec48f7ceaf4d8791ccd8208b45c6b309d2ccca23b3d1ec5399df76cd5bf88f2263280
Previous bilingual_str tinyformat::format accepted bilingual format strings,
but not bilingual arguments. Extend it to accept both. This is useful when
embedding one translated string inside another translated string, for example:
`strprintf(_("Error: %s"), message)` which would fail previously if `message`
was a bilingual_str.
75db62ba4cae048e742ca02dc6a52b3b3d6727de refactor: Move calculation logic out from `CheckSequenceLocksAtTip()` (Hennadii Stepanov)
3bc434f4590758db673e1bd4ebf1906ea632f593 refactor: Add `CalculateLockPointsAtTip()` function (Hennadii Stepanov)
Pull request description:
This PR is follow up for bitcoin/bitcoin#22677 and bitcoin/bitcoin#23683.
On master (013daed9acca1b723f599d63ab36b9c2a5c60e5f) it is not obvious that `CheckSequenceLocksAtTip()` function can modify its `LockPoints* lp` parameter which leads to https://github.com/bitcoin/bitcoin/pull/22677#discussion_r762040101.
This PR:
- separates the lockpoint calculate logic from `CheckSequenceLocksAtTip()` function into a new `CalculateLockPointsAtTip()` one
- cleans up the `CheckSequenceLocksAtTip()` function interface
- makes code easier to reason about (hopefully)
ACKs for top commit:
achow101:
ACK 75db62ba4cae048e742ca02dc6a52b3b3d6727de
stickies-v:
re-ACK 75db62b
Tree-SHA512: 072c3fd9cd1e1b0e0bfc8960a67b01c80a9f16d6778f374b6944ade03a020415ce8b8ab2593b0f5e787059c8cf90af798290b4c826785d41955092f6e12e7486
3141eab9c669488a2e7fef5f60d356ac92294922 test: add functional test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth)
e252909e561e47d75cb3a892657662a139f6532c test: add unit test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth)
77557dda4a123515d0fa2a545ee21d7c43a66988 prune: scan and unlink already pruned block files on startup (Andrew Toth)
Pull request description:
There are a few cases where we can mark a block and undo file as pruned in our block index, but not actually remove the files from disk.
1. If we call `FindFilesToPrune` or `FindFilesToPruneManual` and crash before `UnlinkPrunedFiles`.
2. If on Windows there is an open file handle to the file somewhere else when calling `fs::remove` in `UnlinkPrunedFiles` (https://en.cppreference.com/w/cpp/filesystem/remove, https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-deletefilew#remarks). This could be from another process, or if we are calling `ReadBlockFromDisk`/`ReadRawBlockFromDisk` without having a lock on `cs_main` (which has been allowed since ccd8ef65f9).
This PR mitigates this by scanning all pruned block files on startup after `LoadBlockIndexDB` and unlinking them again.
ACKs for top commit:
achow101:
ACK 3141eab9c669488a2e7fef5f60d356ac92294922
pablomartin4btc:
re-ACK with added functional test 3141eab9c669488a2e7fef5f60d356ac92294922.
furszy:
Code review ACK 3141eab9
theStack:
Code-review ACK 3141eab9c669488a2e7fef5f60d356ac92294922
Tree-SHA512: 6c73bc57838ad1b7e5d441af3c4d6bf4c61c4382e2b86485e57fbb74a61240710c0ceeceb8b4834e610ecfa3175c6955c81ea4b2285fee11ca6383f472979d8d
Use the same technique as is using in the FromString miniscript parser to
predict the final script size of the miniscript being generated in the
miniscript_stable and miniscript_smart fuzzers (by counting every unexplored
sub node as 1 script byte, which is possible because every leaf node always
adds at least 1 byte). This allows bailing out early if the script being
generated would exceed the maximum allowed size (before actually constructing
the miniscript, as that may happen only significantly later potentially).
Also add a self-check to make sure this predicted script size matches that
of generated scripts.
Keep track of the total number of ops the constructed script will have
during miniscript_stable and miniscript_smart fuzzers' GenNode, so it
can abort early if the 201 ops limit would be exceeded.
Also add a self-check that the final constructed node has the predicted
ops size limit, so we know the fuzzer's logic for keeping track of this
is correct.
Since we now keep track of all expected child node types (even if rudimentary)
in both miniscript_stable and miniscript_smart fuzzers, there is no need anymore
for the former shortcut NodeInfo constructors without sub types.
Keep track of which base type (B, K, V, or W) is desired in the miniscript_stable
ConsumeStableNode function. This allows aborting early if the constructed node
won't have the right type.
Note that this does not change the fuzzer format; the meaning of inputs in
ConsumeStableNode is unmodified. The only change is that often the fuzzer will
abort early.
The direct motivation is preventing recursing v: wrappers, which are the only
fragment type that does not otherwise increase the overall minimum possible script
size. In a later commit this will be exploited to prevent overly-large scripts from
being constructed.
faab273e060d27e166b5fb7fe7692614ec9e5c76 util: Return empty vector on invalid hex encoding (MarcoFalke)
fa3549a77bf6a15d8309d36056237f3126baf721 test: Add hex parse unit tests (MarcoFalke)
Pull request description:
Seems a bit confusing to happily accept random bytes and pretend they are hex encoded strings.
ACKs for top commit:
stickies-v:
re-ACK faab273e060d27e166b5fb7fe7692614ec9e5c76
Tree-SHA512: a808135f744f50aece03d4bf5a71481c7bdca1fcdd0d5b113abdb0c8b382bf81cafee6d17c239041fb49b59f4e19970f24a475378e7f711c3a47d6438de2bdab
