This is a "dumb" way of randomly generating a Miniscript node from
fuzzer input. It defines a strict binary encoding and will always generate
a node defined from the encoding without "helping" to create valid nodes.
It will cut through as soon as it encounters an invalid fragment so
hopefully the fuzzer can tend to learn the encoding and generate valid
nodes with a higher probability.
On a valid generated node a number of invariants are checked, especially
around the satisfactions and testing them against the Script
interpreter.
The node generation and testing is modular in order to later introduce
other ways to generate nodes from fuzzer inputs with minimal code.
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
We'll need a better integration of the hash preimages PSBT fields to
satisfy Miniscript with such challenges from the RPC.
Thanks to Greg Sanders for his examples and suggestions to improve this
test.
This is a workaround for Miniscript descriptors containing hash
challenges. For those we can't mock the signature creator without making
OP_EQUAL mockable in the interpreter, so CalculateMaximumInputSize will
always return -1 and outputs for these descriptors would appear
unsolvable while they actually are.
Try to solve a script using the Miniscript satisfier if the legacy
solver fails under P2WSH context. Only solve public key and public key
hash challenges for now.
We don't entirely replace the raw solver and especially rule out trying to
solve CHECKMULTISIG-based multisigs with the Miniscript satisfier since
some features, such as the transaction input combiner, rely on the
specific behaviour of the former.
Cherry-picked and squashed from
https://github.com/sipa/bitcoin/commits/202302_miniscript_improve.
- Explain thresh() and multi() satisfaction algorithms
- Comment on and_v dissatisfaction
- Mark overcomplete thresh() dissats as malleable and explain
- Add comment on unnecessity of Malleable() in and_b dissat
When an encrypted wallet is locked (for instance via the
RPC `walletlock`), the docs indicate that the key is
removed from memory. However, the vector (with a secure
allocator) is merely cleared. This allows the key to persist
indefinitely in memory. Instead, manually fill the bytes with
zeroes before clearing.
To prepare for the removal of laanwj's key from trusted key, the trusted
git root needs to be newer than the most recent merge commit signed by
his key.
2d955ff006 net: add `Ensure{any}Banman` (brunoerg)
Pull request description:
This PR adds `Ensure{any}Banman` functions to avoid code repetition and make it cleaner. Same approach as done with argsman, chainman, connman and others.
ACKs for top commit:
davidgumberg:
ACK [2d955ff](2d955ff006)
Tree-SHA512: 0beb7125312168a3df130c1793a1412ab423ef0f46023bfe2a121630c79df7e55d3d143fcf053bd09e2d96e9385a7a04594635da3e5c6be0c5d3a9cafbe3b631
Use DBParams struct to remove ArgsManager uses from txdb.
To reduce size of this commit, this moves references to gArgs variable out of
txdb.cpp to calling code in chainstate.cpp. But these moves are temporary. The
gArgs references in chainstate.cpp are moved out to calling code in init.cpp in
later commits.
This commit does not change behavior.
Add CoinsViewOptions struct to remove ArgsManager uses from txdb.
To reduce size of this commit, this moves references to gArgs variable out of
txdb.cpp to calling code in validation.cpp. But these moves are temporary. The
gArgs references in validation.cpp are moved out to calling code in init.cpp in
later commits.
This commit does not change behavior.
Add DBParams and DBOptions structs to remove ArgsManager uses from dbwrapper.
To reduce size of this commit, this moves references to gArgs variable out of
dbwrapper.cpp to calling code in txdb.cpp. But these moves are temporary. The
gArgs references in txdb.cpp are moved out to calling code in init.cpp in later
commits.
This commit does not change behavior.
fa6f67837b test: Fix intermittent sync issue in wallet_pruning (721217.xyz)
Pull request description:
Setting the mocktime on each loop iteration will make net processing racy and cause a disconnect due to timeout.
Fix that by setting the mocktime only once.
Fixes https://github.com/bitcoin/bitcoin/issues/27065
ACKs for top commit:
brunoerg:
crACK fa6f67837b
Tree-SHA512: 128b962c05a6fa3caf3ce392e870fff6609ce2206a43bbae6661ecb45291df93bed77fe362a514d4472056f83fb6631df39a5170fa34e41a7577b9685dd26b1f
it adds `Ensure{any}Banman` functions to avoid
code repetition and make it cleaner. Similar
approach as done with argsman, chainman, connman
and others.
4de02def84 qt: Persist Mask Values option (Andrew Chow)
Pull request description:
The mask values option is memory only. If a user has enabled this option, it's reasonable to expect that they would want to have it enabled on the next start.
ACKs for top commit:
RandyMcMillan:
tACK 4de02def84
jarolrod:
tACK 4de02def84
pablomartin4btc:
> tACK [4de02de](4de02def84)
john-moffett:
tACK 4de02def84
Tree-SHA512: 247deb78df4911516625bf8b25d752feb480ce30eb31335cf9baeb07b7c6c225fcc37d5c45de62d6e6895ec10c7eefabb15527e3c9723a3b8ddda1e12ebbf46b
73a3b161b7 ci: Inline `MACOS_NATIVE_TASK_TEMPLATE` (Hennadii Stepanov)
8a61527cf6 ci: Re-introduce `depends_built` cache back in macOS and Android tasks (Hennadii Stepanov)
Pull request description:
This PR brings a `depends_built` cache back to the "macOS 10.15" and "ARM64 Android APK" CI tasks.
Fixes#27031.
ACKs for top commit:
MarcoFalke:
reACK 73a3b161b7🌻
Tree-SHA512: 2eb845f865ee2ee453c1fd284d5eeddbebb2653586b17946822fec03d46e73c5eb483499761a0de6c3c466b06623957664e22dee01f7312ad18e212f1c9c6439
887bb53b67 ci: Use the latest Ubuntu LTS for "ARM64 Android APK" task (Hennadii Stepanov)
Pull request description:
Suggested in https://github.com/bitcoin/bitcoin/pull/25797#discussion_r1100172227:
> I don't expect that anyone is building for android, and if they did, it should be fine to just require the latest Ubuntu LTS, which is Jammy
ACKs for top commit:
fanquake:
ACK 887bb53b67 - but I'd also suggest we remove this task entirely, and either replacing it with another task, maybe a *BSD, or delegating the resources to other jobs.
Tree-SHA512: 1f4b6155e5bbb8ca3580809c5999e3abf6b15b409d164a719b0a89205ca48c178aa6401039a22151ce464009adc48ba272a5a2ff05dc3ca06d3b2d64c99e3e22
b03a98291b build: set boost cppflags with --enable-fuzz (fanquake)
Pull request description:
Even though all other targets are disabled, we still need Boost CPPFLAGS (`use_boost`) to compile. This currently works everywhere, except on arm macOS (where the include path is non-standard), because generally, the Boost include path is generic, i.e `/usr/include`.
ACKs for top commit:
hebasto:
ACK b03a98291b
Tree-SHA512: 7544a903ce641fd4b994ae51423a7007de85628ae29be36362a5cbdd62f9b16ac0a62e9edadaaa998ad4c1e82c0fde0d8c53aba41f94ad30ffa9f10ba0984521
Even though all other targets are disabled, we still need Boost CPPFLAGS
(use_boost) to compile. This currently works everywhere, except on arm
macOS (where the include path is pretty non-standard), because
generally, the Boost include path is generic, i.e `/usr/include`.
0e02f72548 depends: define `__BSD_VISIBLE` for FreeBSD bdb build (fanquake)
Pull request description:
Required for additional definitions (`IPC_R` & friends), to be available, when compiling under C11, which would otherwise cause compile fails.
See: https://github.com/MarcoFalke/btc_nightly/pull/4.
ACKs for top commit:
hebasto:
ACK 0e02f72548, tested on FreeBSD 13.1:
Tree-SHA512: 885d4aa341d9668da360cf6dfafb97ce816803c54e76c0a06e448db39a723666d42cd14b3e713d17ecbe33163f5af69924567cf449d679a2db95b36357005d43
fa83005a26 doc: Document affected gcc versions for -fstack-reuse=none workaround (MarcoFalke)
Pull request description:
gcc version(s) 11 and prior won't be fixed, looking at the activity in the bug report. So it seems best to just document gcc 12.1+ as fixed, so that in the future the workaround can be removed once the minimum compiler is gcc12.1.
ACKs for top commit:
fanquake:
ACK fa83005a26
hebasto:
re-ACK fa83005a26
Tree-SHA512: a19723457eb1925196828a5fafd4e7f75a04f86ffae63cb86679d732c662fd1a22e17fe3c69195a97438ff189ba3ff681be3650cf99aa195d7a3e89cd8ee137c
b49e19ccd9 doc: use arch agnostic clang path in fuzzing doc (macOS) (fanquake)
Pull request description:
The current path will only work for clang installed via brew on x86_64 macOS.
ACKs for top commit:
hebasto:
ACK b49e19ccd9, similar to 702836530f.
Tree-SHA512: 8ae4845e1953d5a7178f2b422e2241af1057d8cce1ab79da65df0cd068456dbf85da3489355f81fc4ee09ba602a4b53e989e2dc02476b4abf6c5b3bc3e96473b
741c215b5f test: remove unused vars in `feature_block` (brunoerg)
Pull request description:
There is no need to assign `self.next_block` to variables if we're not using its return value. Most cases touched here, we're reassigning it right after with the value from `self.update_block`.
Top commit has no ACKs.
Tree-SHA512: 25bbea2a09f38c3a3483fa363f024d2a8edd06a00cccc93cef99e489b9a3485d58bbd6a1ed2dddc00f1cebec7e63aed8ad95701a2645ce20a0db9b69573c20a7
75347236f2 docs: document c-style cast prohibition (Pasta)
Pull request description:
In the words of practicalswift:
```
A C-style cast is equivalent to try casting in the following order:
const_cast(...)
static_cast(...)
const_cast(static_cast(...))
reinterpret_cast(...)
const_cast(reinterpret_cast(...))
By using static_cast<T>(...) explicitly we avoid the possibility of an unintentional and
dangerous reinterpret_cast. Furthermore static_cast<T>(...) allows for easier grepping of casts.
For a more thorough discussion, see "ES.49: If you must use a cast, use a named cast"
in the C++ Core Guidelines (Stroustrup & Sutter).
```
Modern tooling, specifically `-Wold-style-cast` can enable us to enforce never using C-style casts. I believe this is especially important due to the number of C-style casts the codebase is currently being used as a reinterpret_cast. reinterpret_casts are especially dangerous, and should never be done via C-style casts.
Update the docs to suggest the use of named cast or functional casts.
Top commit has no ACKs.
Tree-SHA512: 29a98de396f0c78e32d8a1831319162203c4405a670da5add5da956fcc7df200a1cec162ef1cfac4ddfb02714b66406081d40ed435c7f0f28581cfa24d94fac1
faff2ba4f8 Remove reindex special case from the progress bar label (MarcoFalke)
Pull request description:
The user knows which option they passed to the program, so it seems overly verbose to offer the user feedback whether or not they passed `-reindex`. Treat it as `DISK`, like all other cases that are treated as `DISK`:
* `-reindex-chainstate`
* `-loadblock`
ACKs for top commit:
john-moffett:
Re-ACK faff2ba4f8
hebasto:
ACK faff2ba4f8, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 7f110c4beb1451d26f32da3a60150dac91c8a7b8d1c01749017204712b73cc1b77578af492930e4b6704097a73ed051f77bc39d8f60e0ff15a797a201805312e
1914e470e3 build: copy config.{guess,sub} post autogen in zmq package (fanquake)
Pull request description:
Otherwise our config.guess and config.sub will be copied over. This problem has been masked by the fact that modern systems ship with versions that recognise all the triplets we use (namely arm64-apple-darwin). However building on ubuntu 20.04 surfaces the issue.
Fixes#26420.
ACKs for top commit:
hebasto:
ACK 1914e470e3, tested on Ubuntu 18.04.
Tree-SHA512: dff64c3c62d9f8fc205e5a4dffe8befd58838418d073a15dfe304a0f64b182dfffd9dcf98b53df44bfab905c12a62d03cd5c0f91fa7c4b246ac21ae5f20540fd
c9ba4f9ecb test: Add test for file system permissions (Hennadii Stepanov)
581f16ef34 Apply default umask in `SetupEnvironment()` (Hennadii Stepanov)
8a6219e543 Remove `-sysperms` option (Hennadii Stepanov)
Pull request description:
On master (1e7564eca8) docs say:
```
$ ./src/bitcoind -help | grep -A 3 sysperms
-sysperms
Create new files with system default permissions, instead of umask 077
(only effective with disabled wallet functionality)
```
Basing on that, one could expect that running `bitcoind` first time will create data directory and `wallets/` subdirectory with safe 0700 permissions.
But that is not the case:
```
$ stat .bitcoin | grep id
Access: (0775/drwxrwxr-x) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto)
$ stat .bitcoin/wallets | grep id
Access: (0775/drwxrwxr-x) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto)
```
Both directories, in fact, are created with system default permissions.
With this PR:
```
$ stat .bitcoin/wallets | grep id
Access: (0700/drwx------) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto)
$ stat .bitcoin/wallets | grep id
Access: (0700/drwx------) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto)
```
---
This PR:
- is alternative to bitcoin/bitcoin#13389
- fixesbitcoin/bitcoin#15902
- fixesbitcoin/bitcoin#22595
- closesbitcoin/bitcoin#13371
- reverts bitcoin/bitcoin#4286
Changes in behavior: removed `-sysperms` command-line argument / configure option. The related discussions are here:
- https://github.com/bitcoin/bitcoin/pull/13389#issuecomment-395306690
- https://github.com/bitcoin/bitcoin/pull/13389#issuecomment-539906114
- https://github.com/bitcoin/bitcoin/pull/13389#discussion_r279160472
If users rely on non-default access permissions, they could use `chmod`.
ACKs for top commit:
john-moffett:
ACK c9ba4f9ecb
willcl-ark:
ACK c9ba4f9ecb
Tree-SHA512: 96c745339e6bd0e4d7bf65daf9a721e2e1945b2b0ab74ca0f66576d0dc358b5de8eb8cdb89fe2160f3b19c39d2798bb8b291784316085dc73a27102d3415bd57
