Age | Commit message (Collapse) | Author |
|
Co-Authored-By: Andrew Chow <github@achow101.com>
|
|
561848aaf2d67791e92754f3d11813bc53959a8f Exercise non-DIRTY spent coins in caches in fuzz test (Pieter Wuille)
59e6828bb5b56a2354a80059d3f660f551f3e207 Add deterministic mode to CCoinsViewCache (Pieter Wuille)
b0ff31084006ac7d4a7afba3190ca75f5f8441af Add CCoinsViewCache::SanityCheck() and use it in fuzz test (Pieter Wuille)
3c9cea1340fd1358d6854209d782922864945eb0 Add simulation-based CCoinsViewCache fuzzer (Pieter Wuille)
Pull request description:
The fuzzer goes through a sequence of operations that get applied to both a real stack of `CCoinsViewCache` objects, and to simulation data, comparing the two at the end.
ACKs for top commit:
jamesob:
re-ACK https://github.com/bitcoin/bitcoin/pull/27011/commits/561848aaf2d67791e92754f3d11813bc53959a8f
dergoegge:
Code review ACK 561848aaf2d67791e92754f3d11813bc53959a8f
Tree-SHA512: 68634f251fdb39436b128ecba093f651bff12ac11508dc9885253e57fd21efd44edf3b22b0f821c228175ec507df7d46c7f9f5404fc1eb8187fdbd136a5d5ee2
|
|
At the expense of more complexity, this target generates a valid
Miniscript node at every iteration.
This target will at first run populate a list of recipe (a map from
desired type to possible ways of creating such type) and curate it
(remove the unavailable or redundant recipes).
Then, at each iteration it will pick a type, choose a manner to create a
node of such type from the available recipes, and then
pseudo-recursively do the same for the type constraints of the picked
recipe.
For instance, if it is instructed based on the fuzzer output to create a
Miniscript node of type 'Bd', it could choose to create an 'or_i(subA, subB)'
nodes with type constraints 'B' for subA and 'Bd' for subB. It then
consults the recipes for creating subA and subB, etc...
Here is the list of all the existing recipes, by type constraint:
B: 0()
B: 1()
B: older()
B: after()
B: sha256()
B: hash256()
B: ripemd160()
B: hash160()
B: c:(K)
B: d:(Vz)
B: j:(Bn)
B: n:(B)
B: and_v(V,B)
B: and_b(B,W)
B: or_b(Bd,Wd)
B: or_d(Bdu,B)
B: or_i(B,B)
B: andor(Bdu,B,B)
B: thresh(Bdu)
B: thresh(Bdu,Wdu)
B: thresh(Bdu,Wdu,Wdu)
B: multi()
V: v:(B)
V: and_v(V,V)
V: or_c(Bdu,V)
V: or_i(V,V)
V: andor(Bdu,V,V)
K: pk_k()
K: pk_h()
K: and_v(V,K)
K: or_i(K,K)
K: andor(Bdu,K,K)
W: a:(B)
W: s:(Bo)
Bz: 0()
Bz: 1()
Bz: older()
Bz: after()
Bz: n:(Bz)
Bz: and_v(Vz,Bz)
Bz: or_d(Bzdu,Bz)
Bz: andor(Bzdu,Bz,Bz)
Bz: thresh(Bzdu)
Vz: v:(Bz)
Vz: and_v(Vz,Vz)
Vz: or_c(Bzdu,Vz)
Vz: andor(Bzdu,Vz,Vz)
Bo: sha256()
Bo: hash256()
Bo: ripemd160()
Bo: hash160()
Bo: c:(Ko)
Bo: d:(Vz)
Bo: j:(Bon)
Bo: n:(Bo)
Bo: and_v(Vz,Bo)
Bo: and_v(Vo,Bz)
Bo: or_d(Bodu,Bz)
Bo: or_i(Bz,Bz)
Bo: andor(Bzdu,Bo,Bo)
Bo: andor(Bodu,Bz,Bz)
Bo: thresh(Bodu)
Vo: v:(Bo)
Vo: and_v(Vz,Vo)
Vo: and_v(Vo,Vz)
Vo: or_c(Bodu,Vz)
Vo: or_i(Vz,Vz)
Vo: andor(Bzdu,Vo,Vo)
Vo: andor(Bodu,Vz,Vz)
Ko: pk_k()
Ko: and_v(Vz,Ko)
Ko: andor(Bzdu,Ko,Ko)
Bn: sha256()
Bn: hash256()
Bn: ripemd160()
Bn: hash160()
Bn: c:(Kn)
Bn: d:(Vz)
Bn: j:(Bn)
Bn: n:(Bn)
Bn: and_v(Vz,Bn)
Bn: and_v(Vn,B)
Bn: and_b(Bn,W)
Bn: multi()
Vn: v:(Bn)
Vn: and_v(Vz,Vn)
Vn: and_v(Vn,V)
Kn: pk_k()
Kn: pk_h()
Kn: and_v(Vz,Kn)
Kn: and_v(Vn,K)
Bon: sha256()
Bon: hash256()
Bon: ripemd160()
Bon: hash160()
Bon: c:(Kon)
Bon: d:(Vz)
Bon: j:(Bon)
Bon: n:(Bon)
Bon: and_v(Vz,Bon)
Bon: and_v(Von,Bz)
Von: v:(Bon)
Von: and_v(Vz,Von)
Von: and_v(Von,Vz)
Kon: pk_k()
Kon: and_v(Vz,Kon)
Bd: 0()
Bd: sha256()
Bd: hash256()
Bd: ripemd160()
Bd: hash160()
Bd: c:(Kd)
Bd: d:(Vz)
Bd: j:(Bn)
Bd: n:(Bd)
Bd: and_b(Bd,Wd)
Bd: or_b(Bd,Wd)
Bd: or_d(Bdu,Bd)
Bd: or_i(B,Bd)
Bd: or_i(Bd,B)
Bd: andor(Bdu,B,Bd)
Bd: thresh(Bdu)
Bd: thresh(Bdu,Wdu)
Bd: thresh(Bdu,Wdu,Wdu)
Bd: multi()
Kd: pk_k()
Kd: pk_h()
Kd: or_i(K,Kd)
Kd: or_i(Kd,K)
Kd: andor(Bdu,K,Kd)
Wd: a:(Bd)
Wd: s:(Bod)
Bzd: 0()
Bzd: n:(Bzd)
Bzd: or_d(Bzdu,Bzd)
Bzd: andor(Bzdu,Bz,Bzd)
Bzd: thresh(Bzdu)
Bod: sha256()
Bod: hash256()
Bod: ripemd160()
Bod: hash160()
Bod: c:(Kod)
Bod: d:(Vz)
Bod: j:(Bon)
Bod: n:(Bod)
Bod: or_d(Bodu,Bzd)
Bod: or_i(Bz,Bzd)
Bod: or_i(Bzd,Bz)
Bod: andor(Bzdu,Bo,Bod)
Bod: andor(Bodu,Bz,Bzd)
Bod: thresh(Bodu)
Kod: pk_k()
Kod: andor(Bzdu,Ko,Kod)
Bu: 0()
Bu: 1()
Bu: sha256()
Bu: hash256()
Bu: ripemd160()
Bu: hash160()
Bu: c:(K)
Bu: d:(Vz)
Bu: j:(Bnu)
Bu: n:(B)
Bu: and_v(V,Bu)
Bu: and_b(B,W)
Bu: or_b(Bd,Wd)
Bu: or_d(Bdu,Bu)
Bu: or_i(Bu,Bu)
Bu: andor(Bdu,Bu,Bu)
Bu: thresh(Bdu)
Bu: thresh(Bdu,Wdu)
Bu: thresh(Bdu,Wdu,Wdu)
Bu: multi()
Bzu: 0()
Bzu: 1()
Bzu: n:(Bz)
Bzu: and_v(Vz,Bzu)
Bzu: or_d(Bzdu,Bzu)
Bzu: andor(Bzdu,Bzu,Bzu)
Bzu: thresh(Bzdu)
Bou: sha256()
Bou: hash256()
Bou: ripemd160()
Bou: hash160()
Bou: c:(Ko)
Bou: d:(Vz)
Bou: j:(Bonu)
Bou: n:(Bo)
Bou: and_v(Vz,Bou)
Bou: and_v(Vo,Bzu)
Bou: or_d(Bodu,Bzu)
Bou: or_i(Bzu,Bzu)
Bou: andor(Bzdu,Bou,Bou)
Bou: andor(Bodu,Bzu,Bzu)
Bou: thresh(Bodu)
Bnu: sha256()
Bnu: hash256()
Bnu: ripemd160()
Bnu: hash160()
Bnu: c:(Kn)
Bnu: d:(Vz)
Bnu: j:(Bnu)
Bnu: n:(Bn)
Bnu: and_v(Vz,Bnu)
Bnu: and_v(Vn,Bu)
Bnu: and_b(Bn,W)
Bnu: multi()
Bonu: sha256()
Bonu: hash256()
Bonu: ripemd160()
Bonu: hash160()
Bonu: c:(Kon)
Bonu: d:(Vz)
Bonu: j:(Bonu)
Bonu: n:(Bon)
Bonu: and_v(Vz,Bonu)
Bonu: and_v(Von,Bzu)
Bdu: 0()
Bdu: sha256()
Bdu: hash256()
Bdu: ripemd160()
Bdu: hash160()
Bdu: c:(Kd)
Bdu: d:(Vz)
Bdu: j:(Bnu)
Bdu: n:(Bd)
Bdu: and_b(Bd,Wd)
Bdu: or_b(Bd,Wd)
Bdu: or_d(Bdu,Bdu)
Bdu: or_i(Bu,Bdu)
Bdu: or_i(Bdu,Bu)
Bdu: andor(Bdu,Bu,Bdu)
Bdu: thresh(Bdu)
Bdu: thresh(Bdu,Wdu)
Bdu: thresh(Bdu,Wdu,Wdu)
Bdu: multi()
Wdu: a:(Bdu)
Wdu: s:(Bodu)
Bzdu: 0()
Bzdu: n:(Bzd)
Bzdu: or_d(Bzdu,Bzdu)
Bzdu: andor(Bzdu,Bzu,Bzdu)
Bzdu: thresh(Bzdu)
Bodu: sha256()
Bodu: hash256()
Bodu: ripemd160()
Bodu: hash160()
Bodu: c:(Kod)
Bodu: d:(Vz)
Bodu: j:(Bonu)
Bodu: n:(Bod)
Bodu: or_d(Bodu,Bzdu)
Bodu: or_i(Bzu,Bzdu)
Bodu: or_i(Bzdu,Bzu)
Bodu: andor(Bzdu,Bou,Bodu)
Bodu: andor(Bodu,Bzu,Bzdu)
Bodu: thresh(Bodu)
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
|
|
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>
|
|
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.
|
|
Preimages must be externally provided (typically, via a PSBT).
|
|
|
|
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
|
|
This introduces the logic to "sign for" a Miniscript.
Co-Authored-By: Pieter Wuille <pieter.wuille@gmail.com>
|
|
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.
|
|
|
|
|
|
2d955ff006b8949017fe617c35cfc1cfe117db6e 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](https://github.com/bitcoin/bitcoin/commit/2d955ff006b8949017fe617c35cfc1cfe117db6e)
Tree-SHA512: 0beb7125312168a3df130c1793a1412ab423ef0f46023bfe2a121630c79df7e55d3d143fcf053bd09e2d96e9385a7a04594635da3e5c6be0c5d3a9cafbe3b631
|
|
Use ChainstateManagerOpts struct to remove ArgsManager uses from validation.cpp.
This commit does not change behavior.
|
|
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.
|
|
|
|
to generate semi-random CAmounts up to MAX_MONEY rather
than only uint32, and use it in the unit tests.
|
|
it adds `Ensure{any}Banman` functions to avoid
code repetition and make it cleaner. Similar
approach as done with argsman, chainman, connman
and others.
|
|
4de02def844102c08b65bf1311a333e7aca482b9 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 https://github.com/bitcoin-core/gui/commit/4de02def844102c08b65bf1311a333e7aca482b9
jarolrod:
tACK 4de02def844102c08b65bf1311a333e7aca482b9
pablomartin4btc:
> tACK [4de02de](https://github.com/bitcoin-core/gui/commit/4de02def844102c08b65bf1311a333e7aca482b9)
john-moffett:
tACK 4de02def844102c08b65bf1311a333e7aca482b9
Tree-SHA512: 247deb78df4911516625bf8b25d752feb480ce30eb31335cf9baeb07b7c6c225fcc37d5c45de62d6e6895ec10c7eefabb15527e3c9723a3b8ddda1e12ebbf46b
|
|
bar label
faff2ba4f80fad5af63a31559fd4065e631a8166 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 faff2ba4f80fad5af63a31559fd4065e631a8166
hebasto:
ACK faff2ba4f80fad5af63a31559fd4065e631a8166, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 7f110c4beb1451d26f32da3a60150dac91c8a7b8d1c01749017204712b73cc1b77578af492930e4b6704097a73ed051f77bc39d8f60e0ff15a797a201805312e
|
|
and `wallets/` subdir
c9ba4f9ecb1a282d98e7456a84ca84362b161757 test: Add test for file system permissions (Hennadii Stepanov)
581f16ef3404274cb5c1a79dd3d6ee7b584f9844 Apply default umask in `SetupEnvironment()` (Hennadii Stepanov)
8a6219e54379911605aed860519e0194f1433b72 Remove `-sysperms` option (Hennadii Stepanov)
Pull request description:
On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) 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
- fixes bitcoin/bitcoin#15902
- fixes bitcoin/bitcoin#22595
- closes bitcoin/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 c9ba4f9ecb1a282d98e7456a84ca84362b161757
willcl-ark:
ACK c9ba4f9ecb1a282d98e7456a84ca84362b161757
Tree-SHA512: 96c745339e6bd0e4d7bf65daf9a721e2e1945b2b0ab74ca0f66576d0dc358b5de8eb8cdb89fe2160f3b19c39d2798bb8b291784316085dc73a27102d3415bd57
|
|
|
|
as many of the unit tests don't use this code
|
|
935acdcc79d1dc5ac04a83b92e5919ddbfa29329 refactor: modernize the implementation of uint256.* (pasta)
Pull request description:
- Constructors of uint256 to utilize Span instead of requiring a std::vector
- converts m_data into a std::array
- Prefers using `WIDTH` instead of `sizeof(m_data)`
- make all the things constexpr
- replace C style functions with c++ equivalents
- memset -> std::fill
This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable.
- memcpy -> std::copy
Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy)
This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm
- memcmp -> std::memcmp
ACKs for top commit:
achow101:
ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
hebasto:
Approach ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329.
aureleoules:
reACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
john-moffett:
ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
stickies-v:
Approach ACK 935acdcc7
Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650
|
|
This change makes all filesystem artifacts--files and directories--being
created with the default umask.
|
|
b8032293e67a3b61ecad531412be5330f7cb39e3 Remove use of snprintf and simplify (John Moffett)
Pull request description:
These are the only remaining uses of `snprintf` in our project, and they can cause unexpected issues -- for example, see https://github.com/bitcoin/bitcoin/issues/27014. Change them to use our `ToString` (which uses a locale-independent version of `std::to_string`) to convert an `int` to `std::string`. Also remove resulting unused parts of `StringContentsSerializer`.
Closes https://github.com/bitcoin/bitcoin/issues/27014
ACKs for top commit:
Sjors:
tACK b8032293e67a3b61ecad531412be5330f7cb39e3, fixes #27014.
Tree-SHA512: c903977e654711929decafe8887d0de13b38a340d7082875acc5d41950d834dcfde074e9cabecaf5f9a760f62c34322297b4b156af29761650ef5803b1a54b59
|
|
82f895d7b540ae421f80305a4f7cbb42905fb2c6 Update nanobench to version v4.3.10 (Martin Leitner-Ankerl)
Pull request description:
Nothing has changed that would affect Bitcoin's usage of nanobench.
Here is a detailed list of the changes
* Plenty of clang-tidy updates
* documentation updates
* faster Rng::shuffle
* Enable perf counters on older kernels
* Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage)
* Add support for custom information per benchmark
ACKs for top commit:
hebasto:
ACK 82f895d7b540ae421f80305a4f7cbb42905fb2c6, I've reviewed the code, all related changes from #26642 have been implemented.
Tree-SHA512: 942518398809a2794617a347ab8182b784a8e822e84de5af078b2531eabb438412d687cac22a21936585e60e07138a89b41c28c9750744c05a3d1053f55cad01
|
|
if dbcache is too small
fe683f352480245add0b27fe7efef5fef4c1e8c3 log: Log VerifyDB Progress over multiple lines (Martin Zumsande)
61431e3a57b5613d8715c93c6eae0058e0217eaa validation: Skip VerifyDB checks of level >=3 if dbcache is too small (Martin Zumsande)
Pull request description:
This is the first two commits from #25574, leaving out all changes to `-verifychain` error-handling :
- The Problem of [25563](https://github.com/bitcoin/bitcoin/issues/25563) is that when we skip blocks at level 3 due to an insufficient dbcache (skipping some `DisconnectBlock()` calls), we would still attempt the level 4 checks, attempting to reconnect a block that was never disconnected, leading to an assert in `ConnectBlock()`.
Fix this by not attempting level 4 checks in this case.
- Logging of verification progress is now split over multiple lines. This is more verbose, but now each update has its own timestamp, and other threads logging concurrently will no longer lead to mangled output.
This can be tested with a small `dbcache` value, for example:
`bitcoind -signet -dbcache=10`
`bitcoin-cli -signet verifychain 4 1000`
Fixes #25563
ACKs for top commit:
MarcoFalke:
review ACK fe683f352480245add0b27fe7efef5fef4c1e8c3 🗄
john-moffett:
ACK fe683f352480245add0b27fe7efef5fef4c1e8c3
Tree-SHA512: 3e2e0f8b73cbc518a0fa17912c1956da437787aab95001c110b01048472e0dfe4783c44df22bd903d198069dd2f6b02bfdf74e0b934c7a776f144c2e86cb818a
|
|
This change effectively reverts commits from
https://github.com/bitcoin/bitcoin/pull/4286.
Users, who rely on non-default access permissions, should use `chmod`
command.
|
|
fb1c6c14c11ccd4833c1a24f77c507f098d369ad test: Remove redundant test (yancy)
Pull request description:
I can't think of any reason to keep this test case around labeled [fix me](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L242). The test was originally added [here](https://github.com/bitcoin/bitcoin/commit/4566ab75f277612425337bf7786c1d3a410d894a) however there was never an assertion about the coins that should be selected, only that a solution is found (which is a redundant solution to the test [above](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L222)). The comment was later added here to [fix](https://github.com/bitcoin/bitcoin/commit/384273260a6ccbcf79dade0830011f528e5a1581) it, however it's unclear what exactly it's testing. A test was later added [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L366) where if the [long term fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L357) is less than the current [fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L356), then select fewer UTXOs, which may have been the original intent.
ACKs for top commit:
S3RK:
ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad
Zero-1729:
Concept ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad
achow101:
ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad
Tree-SHA512: bce2cdae669c144ffaa130237a1643e3b6728e13d603cebf5d9493c4c7c68b3635868e4d93d210783c2ded2a871f185ca09a2053168c05b26a1e056ff6edf68f
|
|
in decodescript
6699d850e466a65ddd0802be747188ef40cf9de0 doc: release notes for #27037 (Antoine Poinsot)
dfc9acbf0170bde6f2abb879b5584dabd1266531 rpc: decode Miniscript descriptor when possible in decodescript (Antoine Poinsot)
Pull request description:
The descriptor inference logic would previously always use a dummy signing provider and would never analyze the witness script of a P2WSH scriptPubKey.
It's often not possible to infer a Miniscript only from the onchain Script, but it was such a low hanging fruit that it's probably worth having it?
Fixes https://github.com/bitcoin/bitcoin/issues/27007. I think it also closes https://github.com/bitcoin/bitcoin/issues/25606.
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/27037/commits/6699d850e466a65ddd0802be747188ef40cf9de0
achow101:
ACK 6699d850e466a65ddd0802be747188ef40cf9de0
sipa:
utACK 6699d850e466a65ddd0802be747188ef40cf9de0
Tree-SHA512: e592bf1ad45497e7bd58c26b33cd9d05bb3007f1e987bee773d26013c3824e1b394fe4903809d80997d5ba66616cc79d77850cd7e7f847a0efb2211c59466982
|
|
fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e gui: Show watchonly balance only for Legacy wallets (Andrew Chow)
Pull request description:
Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets.
The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC"
ACKs for top commit:
johnny9:
tACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
furszy:
ACK fdb8dc8a
hebasto:
ACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
Tree-SHA512: e5c0703a62d25c881c8dadfb9cffd482791f3d437a4ec5ae0088ce1a2069c2455ad6d3ec6c95a4404a3b55fbd727f92694529c35052236951553ca90c4ed31b5
|
|
c497a198db6f417d2612078a9fbc101e259fab33 Fix comment about how wallet txs are sorted (John Moffett)
Pull request description:
The wallet transactions in the node are not sorted by txid (or any hash) since https://github.com/bitcoin/bitcoin/pull/24699.
This is how they're stored in memory now:
https://github.com/bitcoin-core/gui/blob/835212cd1d8f8fc7f19775f5ff8cc21c099122b2/src/wallet/wallet.h#L397-L399
ACKs for top commit:
achow101:
ACK c497a198db6f417d2612078a9fbc101e259fab33
jarolrod:
ACK c497a198db6f417d2612078a9fbc101e259fab33
Tree-SHA512: e72559991688452ef254474d4235dc75fac655bce04909c3a0eece907360f4c6f57707db9b4373a4bd2271b23c57e863684c33e0728adf48e477c5499cdfdad7
|
|
One test case uses snprintf to convert an
int to a string. Change it to use ToString
(which uses a locale-independent version of
std::to_string). Also remove unnecessary
parts of StringContentsSerializer.
|
|
The descriptor inference logic would previously always use a dummy
signing provider and would never analyze the witness script of a P2WSH
scriptPubKey.
Note even a valid Miniscript might not always be decodable from Script
without more contextual information (for instance the key preimage for a
pk_h).
|
|
|
|
types except bare multisig
b093f5619f8f9b7d63ee60ff04de00b907b13d64 Fill out dust limit unit test for known types except bare multisig (Greg Sanders)
Pull request description:
Having the constants checked explicitly in a single spot helps with possible regressions and also useful for documentation.
In addition, add a check for undefined v1 witness programs.
ACKs for top commit:
theStack:
Code-review ACK b093f5619f8f9b7d63ee60ff04de00b907b13d64
MarcoFalke:
review ACK b093f5619f8f9b7d63ee60ff04de00b907b13d64 🥉
Tree-SHA512: 1421f75471739d29b9ef59b0a925b6b07e4e9af92822dbe56eedfb590be9a00fb0c34312146c7c1b5211906461ed00bfa2eb53c88595c6e5a27694b2dc21df38
|
|
Nothing has changed that would affect Bitcoin's usage of nanobench. Here is a detailed list of the changes
* Plenty of clang-tidy updates
* documentation updates
* faster Rng::shuffle
* Enable perf counters on older kernels
* Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage)
* Add support for custom information per benchmark
|
|
|
|
coins_tests
fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d test: Use std::unique_ptr over manual delete in coins_tests (MarcoFalke)
Pull request description:
Makes the code smaller and easier to read
ACKs for top commit:
stickies-v:
ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d
john-moffett:
ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d
Tree-SHA512: 30d2d2097906e61fdef47a52fc6a0c5ce2417bc41c3c82dafc1b216c655f31dabf9c1c13759575a696f61bbdfdba3f442be032d5e5145b7a54fae2a927824621
|
|
|
|
576f7b86147447215566f0b15ef0b56cd1282929 Fix misleading RPC console wallet message (John Moffett)
Pull request description:
## Misleading message from RPCConsole window ##
In certain circumstances, the GUI console will display the message 'Executing command without any wallet' when it is, in fact, using the currently loaded wallet. For instance:

In RPC calls, if no wallet is explicitly selected and there is exactly one wallet loaded, the [default](https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/wallet/rpc/util.cpp#L71-L93) is to act on that loaded wallet.
The GUI console acts that way in reality, but sometimes erroneously reports that it's not acting on any particular wallet. The root issue is due to the logic that prevents changing the selected wallet if the RPCConsole is visible:
https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/qt/rpcconsole.cpp#L783-L786
This PR removes that unnecessary logic. This does have some ramifications. Prior to this PR, if a user opened the console window without any wallets loaded, then opened two or more wallets, the RPC console would select "None" of the wallets and any wallet-specific RPCs would fail. However, the behavior was different if the user hadn't had the console window open. In that case, if they opened the RPC Console window _after_ loading at least the first wallet, it would select the first-loaded wallet. This context-dependent behavior is (IMO) undesirable, and this PR changes it to be consistent.
ACKs for top commit:
hebasto:
ACK 576f7b86147447215566f0b15ef0b56cd1282929, tested on Ubuntu 22.04 (Qt 5.15.3).
Tree-SHA512: 627da186025ba4f4e8df7fdd1b10363f923c4ecc50f023bbf2aece6e2593da65c45147c933effaca9040f813a6e46f034fc2d1ee2fb0f401000a3a6221a0e36e
|
|
08209c039ff4ca5be4982da7a2ab7a624117ce1a Correctly limit overview transaction list (John Moffett)
Pull request description:
Fixes #703
The way the main overview page limits the number of transactions displayed (currently 5) is not an appropriate use of Qt. Our subclassed transaction sort/filter proxy model returns a maximum of `5` in `rowCount()`. However, the model itself actually may hold significantly more. While this has _worked_, it breaks the contract of `rowCount()`.
If `bitcoin-qt` is run with a DEBUG build of Qt, it'll result in an assert-crash in certain relatively common situations (see #703 for details). Instead of artificially limiting the `rowCount()` in the subclassed filter, we can hide/unhide the rows in the displaying `QListView` upon any changes in the sorted proxy filter.
I loaded a wallet with 20,000 transactions and did not notice any performance differences between master and this branch.
For reference, this is the list I'm referring to:
<img width="934" alt="image" src="https://user-images.githubusercontent.com/116917595/214947304-3f289380-3510-487b-80e8-d19428cf2f0f.png">
ACKs for top commit:
Sjors:
tACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a
hebasto:
ACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a, tested on Ubuntu 22.04.
Tree-SHA512: c2a7b1a2a6e6ff30694830d7c722274c4c47494a81ce9ef25f8e5587c24871b02343969f4437507693d4fd40ba7a212702b159cf54b3357d8d76c02bc8245113
|
|
SerializeMany constructor
fa47b28dfc2a6577519e10da68ebd8da93568434 refactor: Remove unused CDataStream SerializeMany constructor (MarcoFalke)
Pull request description:
Seems odd to have an unused method. Moreover, the function is fragile and dangerous, because one could have a `std::vector vec_a` and type `CDataStream{vec_a, 0, 0}.size()` and `CDataStream{0, 0, vec_a}.size()`, assuming they are the same thing, when in fact they are not. (The first takes over the memory as is, the second serializes the vector).
So my suggestion would be to remove the unused method and introduce a new method when this functionality is needed. For example: `static DataStream FromMany(Args&&... args)`.
ACKs for top commit:
stickies-v:
ACK fa47b28dfc2a6577519e10da68ebd8da93568434
Tree-SHA512: 9593a034b997e33a0794f779f76f02425b1097b218cf8cb1facb7f874fa69da328ce567a79138015baeebe004ae7d103dda4f64f83e8ad375b6dae6b66d3d950
|
|
fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 Use steady clock for logging timer (MarcoFalke)
Pull request description:
The logging timer has many issues:
* The underlying clock is mockable, meaning that benchmarks are useless when mocktime was set at the beginning or end of the benchmark.
* The underlying clock is not monotonic, meaning that benchmarks are useless when the system time was changed during the benchmark.
Fix all issues in this patch.
ACKs for top commit:
stickies-v:
Approach ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950
john-moffett:
ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950
Tree-SHA512: bec8da0f338ed4611e1807937575e1b2afda25139d88015b1c29fa7d13946fbfbc4ee589b576c0508d505df5e5fafafcbc07d63ce4bab4b01475260d9d5d2107
|
|
|