fa538813b1c382cf135cbf2a0cc3fa01f36964d8 scripted-diff: Replace ::mempool with m_node.mempool in tests (MarcoFalke)
8888ad02e204b0fa7a2ea2cfed2fc3f298cf1623 test: Replace recursive lock with locking annotations (MarcoFalke)
fac07f2038a3ccd5edadc6e6122c02fa30e697bd node: Add reference to mempool in NodeContext (MarcoFalke)
Pull request description:
This is the first step toward making the mempool a global that is not initialized before main.
#### Motivation
Currently the mempool is a global that is initialized before the `main` function. This is confusing and easy to get wrong. E.g. the mempool constructor queries state that has not been initialized, like randomness (fixed), or command line arguments (not an issue last time I checked). Also without having the chainstate (chain tip) initialized first, it doesn't make conceptually sense to have a mempool, since the mempool builds txs on top of the utxo set (chain tip).
Finally, in the future someone might want to run a consensus-only full node (`-nowallet -noblockfilter -no... -nomempool` command line options) that only verifies blocks and updates the utxo set.
This is conceptually the same change that has already been done for the connection manager `CConnman`.
ACKs for top commit:
jnewbery:
utACK fa538813b1c382cf135cbf2a0cc3fa01f36964d8
ariard:
Tested ACK fa53881.
Tree-SHA512: 2c446a8a51476354aad7126c2b833500d36b24490caa94f847b2bdc622054de0dae28980f23e3d91b1b492dc32931656d98dbd019af9e4e58f2f8c5375aac694
The function IsStandardTx() returns rejection reason "bare-multisig" if the
transaction has a bare multisig output and the policy flag fIsBareMultisigStd
is false (set by the boolean command-line argument "-permitbaremultisig" -- for
the unit test, we simply set the global flag variable directly).
Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting
COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.
Update the help string to remove "flags", which is not specified in
BIP 22.
faffa7f0dcc9971cb20534816eccdf75bebc853a wallet: Avoid showing GUI popups on RPC errors (take 2) (MarcoFalke)
Pull request description:
Commit 8b0d82bb428de9e7f1da7c61574e7a8376a62d43 claims "This commit does not change behavior." However, it re-introduced the bug I tried to fix in #17070
ACKs for top commit:
ryanofsky:
Code review ACK faffa7f0dcc9971cb20534816eccdf75bebc853a
Tree-SHA512: 99987f80c76414dca40c7d76b2fe4ea853debbe3c49e7acdeab2596c726a2935c468f4484d49212e65ecc9c8b0d861c0c2b83c1ddfc07670540699199dbfecb0
30fb598737f6efb7802d707a1fa989872e7f8b7b Fix segfault in allocator_tests/arena_tests (Jeffrey Czyz)
15c84f53f47bf6e6a9c4c9dfe50c78d98f7ec07f Define ARENA_DEBUG in Travis test runs (Jeffrey Czyz)
ad715488222f2f2ce2e2cff632eae94fd49ea9c5 Fix compilation errors in support/lockedpool.cpp (Jeffrey Czyz)
Pull request description:
Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.
Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.
Reproduced with:
make CPPFLAGS=-DARENA_DEBUG
ACKs for top commit:
laanwj:
ACK 30fb598737f6efb7802d707a1fa989872e7f8b7b
fanquake:
ACK 30fb598737f6efb7802d707a1fa989872e7f8b7b - thanks for following up jkczyz.
Tree-SHA512: 4eec368a4e9c67e4e2a27bc05608a807c2892d50c60d06ed21490cd274c0369f9671bc05b3006acc2a193316caf4896454c9c299603bfed29bd488f1987ec446
8944c1d340ac2be45b505ada7e187b770b0b036f Changed tooltips of receive form to highlight difference between Label and Message (dannmat)
Pull request description:
I have changed the tooltips for 'Label' & 'Message' text fields to be more clear, stating the difference between the two (#17173)
ACKs for top commit:
MarcoFalke:
ACK 8944c1d340ac2be45b505ada7e187b770b0b036f
laanwj:
ACK 8944c1d340ac2be45b505ada7e187b770b0b036f
Tree-SHA512: 7fbea4d3c4416264ae6c146d51d29958c418a278bdd6744133db0b684ad7a9413178c005592aa21a81d127f3f3a8583fc5de00078239db08e6f101f657a5dd3a
Also remove a needless loop in DecodeBase58 to prune zeroes in the base256
output of the conversion. The number of zeroes is implied by keeping track
explicitly of the length during the loop.
Invalid PSBTs need to be re-created, so the next role is the
Creator (new PSBTRole). Additionally, we need to know what went
wrong so an error field was added to PSBTAnalysis.
A PSBTAnalysis indicating invalid will have empty everything,
next will be set to PSBTRole::CREATOR, and an error message.
e5a0bece6e84402fcb1fe4f25fd24da1d21ec077 doc: add OpenSSL removal to release-notes.md (fanquake)
397dbae070dca9a635ff3d1d61add09db004661e ci: remove OpenSSL installation (fanquake)
a4eb83961965347792e9ac75928aae359d5f7405 doc: remove OpenSSL from build instructions and licensing info (fanquake)
648b2e3c3288ee0b83d4089d27fa7f84a73d118e depends: remove OpenSSL package (fanquake)
8983ee3e6dd8ab658bd2caf97c326cc53ea50818 build: remove OpenSSL detection and libs (fanquake)
b49b6b0f7090cc15860d815fb0ef306ddfc718ba random: Remove remaining OpenSSL calls and locking infrastructure (fanquake)
4fcfcc294e7cb17956e283d09050cb997093a35d random: stop retrieving random bytes from OpenSSL (fanquake)
5624ab0b4f844dc7c17aeb1b009f002c33c38fb3 random: stop feeding RNG output back into OpenSSL (fanquake)
Pull request description:
Now that #17165 has been merged, removing our remaining OpenSSL usage is possible.
That remaining usage was a call to [`RAND_bytes`](https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html) during the ::SLOW path of [ProcRand](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L616). As well as feeding output from our RNG back into OpenSSL via [`RAND_add`](https://www.openssl.org/docs/manmaster/man3/RAND_add.html) during the ::SLOW and ::SLEEP paths.
Optimistically tagged for `0.20.0`. Needs discussion, potentially in an upcoming weekly meeting?
Closes#12530.
ACKs for top commit:
MarcoFalke:
ACK e5a0bece6e84402fcb1fe4f25fd24da1d21ec077
laanwj:
ACK e5a0bece6e84402fcb1fe4f25fd24da1d21ec077
Tree-SHA512: 02fce08ec91d20e0da51e9314eec53dcf8699cded02f0a005417d627520c20b826332cb42bdae132af283d4903aa3088a9f613f3aea915d655a51532a4d4796c
Easier to review ignoring whitespace:
git log -p -n1 -w
This commit does not change behavior. It passes new CScript arguments to
signing functions, but the arguments aren't currently used.
SignTransaction will be called multiple times in the future. Pass
it a result UniValue so that it can accumulate the results of multiple
SignTransaction passes.
49f4c7f0699e5e19ac6e41ef5b607392dd7a2983 tests: Add fuzzing harness for various PSBT related functions (practicalswift)
Pull request description:
Add fuzzing harness for various PSBT related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/psbt
```
ACKs for top commit:
MarcoFalke:
re-ACK 49f4c7f0699e5e19ac6e41ef5b607392dd7a2983 🐟
Tree-SHA512: 4cebe62bd8c244ee40a43e829f5bd175ab40e1dfbbab1affb1529374858225820d6c9fa9ba45862bf56c1522845422fd96d620cedbdec52a67ac1449dec4e1b2
e161bc74d24a381c313aecb950d3b8411e0ed19d doc: Remove bitness from bitcoin-qt help message and manpage (Wladimir J. van der Laan)
Pull request description:
Remove the `(64-bit)` from the bitcoin-qt help message.
Since removing the Windows 32-bit builds, it is no longer information that is often useful for troubleshooting. This never worked for other architectures than x86, and the only 32-bit x86 build left is the Linux one. Linux users tend to know what architecture they are using.
It also accidentally ends up in the bitcoin-qt manpage (if you happen to be generating them on a x86 machine), which gets checked in. See for example 1bc9988993 (diff-e4b84be382c8ea33b83203ceb8c85296)
ACKs for top commit:
practicalswift:
ACK e161bc74d24a381c313aecb950d3b8411e0ed19d -- rationale makes sense and diff looks correct :)
MarcoFalke:
Tested ACK e161bc74d24a381c313aecb950d3b8411e0ed19d 🔮
Tree-SHA512: d38754903252896dc86fac6c12ad6615d322c2744db7c02b18574a08c69e8876b2c905e1f09b324002236b111ee93479f89769c562e7b3b2e6eb2992d76464ef
On the ::SLOW path we would use OpenSSL as an additional source of
random bytes. This commit removes that functionality. Note that this was
always only an additional source, and that we never checked the return
value
RAND_bytes(): https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html
RAND_bytes() puts num cryptographically strong pseudo-random bytes into buf.
On the ::SLOW or ::SLEEP paths, we would feed our RNG output back into
OpenSSL using RAND_add. This commit removes that functionality.
RAND_add(): https://www.openssl.org/docs/manmaster/man3/RAND_add.html
RAND_add() mixes the num bytes at buf into the internal state of the
random generator. This function will not normally be needed, as
mentioned above. The randomness argument is an estimate of how much
randomness is contained in buf, in bytes, and should be a number
between zero and num.
2f5f7d6b135e4eab368bbafd9e6e979aa72398de GuessVerificationProgress: cap the ratio to 1 (darosior)
Pull request description:
Noticed `getblockchaininfo` would return a `verificationprogress` > 1, especially while generating. This caps the verification progress to `1`.
Tried to append a check to functional tests but this would pass even without the patch, so it seems better to not add a superfluous check (but this can easily be reproduced by trying to generate blocks in the background and `watch`ing `getblockchainfo`).
ACKs for top commit:
laanwj:
ACK 2f5f7d6b135e4eab368bbafd9e6e979aa72398de
promag:
ACK 2f5f7d6b135e4eab368bbafd9e6e979aa72398de.
Tree-SHA512: fa3aca12acab9c14dab3b2cc94351082f548ea6e6c588987cd86e928a00feb023e8112433658a0e85084e294bfd940eaafa33fb46c4add94146a0901bc1c4f80
d1c02775aa74a0610809ac54bb241ddad61d2d8c Report amount of data gathered from environment (Pieter Wuille)
64e1e022cedf6776c5dffd488ca2e766adca5dc3 Use thread-safe atomic in perfmon seeder (Pieter Wuille)
d61f2bb076d8f17840a8e79f1583d7f6e3e6d09a Run background seeding periodically instead of unpredictably (Pieter Wuille)
483b94292e89587e5ab40a30b8a90e2f56e847f3 Add information gathered through getauxval() (Pieter Wuille)
11793ea22e1298fa7d3b44a5b6d20830248d8cf4 Feed CPUID data into RNG (Pieter Wuille)
a81c494b4c9a8c2f1a319a03375826f12863706f Use sysctl for seeding on MacOS/BSD (Pieter Wuille)
2554c1b81bb8c40e1989025c6f18e7935720b156 Gather additional entropy from the environment (Pieter Wuille)
c2a262a78c3bcc4d5e13612ab0214874abe15de0 Seed randomness with process id / thread id / various clocks (Pieter Wuille)
723c79666770b30cce9f962bed5ece8cc7d74580 [MOVEONLY] Move cpuid code from random & sha256 to compat/cpuid (Pieter Wuille)
cea3902015185adc88adbd031d919f91bc844fd7 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille)
b51bae1a5a4fa8ef7825dd1bb09e3f47f96d7a5a doc: minor corrections in random.cpp (fanquake)
Pull request description:
This introduces a new `randomenv` module that queries varies non-cryptographic (and non-RNG) sources of entropy available on the system; things like user IDs, system configuration, time, statistics, CPUID data.
The idea is that these provide a fallback in scenarios where system entropy is somehow broken (note that if system entropy *fails* we will abort regardless; this is only meant to function as a last resort against undetected failure). It includes some data sources OpenSSL currently uses, and more.
The separation between random and randomenv is a bit arbitrary, but I felt that all this "non-essential" functionality deserved to be separated from the core random module.
ACKs for top commit:
TheBlueMatt:
utACK d1c02775aa74a0610809ac54bb241ddad61d2d8c. Certainly no longer measuring the time elapsed between a 1ms sleep (which got removed in the latest change) is a fair tradeoff for adding about 2 million other actually-higher-entropy bits :).
laanwj:
ACK d1c02775aa74a0610809ac54bb241ddad61d2d8c
Tree-SHA512: d290a8db6538a164348118ee02079e4f4c8551749ea78fa44b2aad57f5df2ccbc2a12dc7d80d8f3e916d68cdd8e204faf9e1bcbec15f9054eba6b22f17c66ae3
Remove the `(64-bit)` from the bitcoin-qt help message.
Since removing the Windows 32-bit builds, it is no longer information
that is often useful for troubleshooting. This never worked for other
architectures than x86, and the only 32-bit x86 build left is the Linux
one. Linux users tend to know what architecture they are using.
It also accidentally ends up in the bitcoin-qt manpage.
The test uses reinterpret_cast<void*> on unallocated memory. Using this
memory in printchunk as char* causes a segfault, so have printchunk take
void* instead.
Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.
Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.
Reproduced with:
make CPPFLAGS=-DARENA_DEBUG
5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e test: add unit test for non-standard txs with too large scriptSig (Sebastian Falbesoner)
Pull request description:
Approaches the first missing test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason `"scriptsig-size"` if any one the inputs' scriptSig is larger than 1650 bytes.
ACKs for top commit:
MarcoFalke:
ACK 5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e
instagibbs:
ACK 5e8a56348b
Tree-SHA512: 79977b12ddea9438a37cefdbb48cc551e4ad02a8ccfaa2d2837ced9f3a185e2e07cc366c243b9e3c7736245e90e315d7b4110efc6b440c63dbef7ee2c9d78a73
-BEGIN VERIFY SCRIPT-
# tx pool member access (mempool followed by dot)
sed --regexp-extended -i -e 's/(::)?\<mempool\>\.([a-zA-Z])/m_node.mempool->\2/g' $(git grep -l mempool ./src/test)
# plain global (mempool not preceeded by dot, but followed by comma)
sed --regexp-extended -i -e 's/([^\.])(::)?\<mempool\>,/\1*m_node.mempool,/g' $(git grep -l mempool ./src/test)
-END VERIFY SCRIPT-
Currently it is an alias to the global ::mempool and should be used as
follows.
* Node code (validation and transaction relay) can use either ::mempool
or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
makes sure the mempool exists before use. This prepares the RPC code
to a future where the mempool might be disabled at runtime or compile
time.
* Test code should use m_node.mempool directly, as the mempool is always
initialized for tests.
edb6b768a4185a4aaa6281ee50a6538f7426cb1e fix uninitialized variable nMinerConfirmationWindow (NullFunctor)
Pull request description:
It is used for the computation of `BIP9WarningHeight`, and by that time it isn't initialized.
ACKs for top commit:
jnewbery:
utACK edb6b768a
promag:
ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, commit description could be cleaned up though.
MarcoFalke:
ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, used python3 to do the addition locally 📍
practicalswift:
ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, used `clang++ -O2` on the previous version^W^W^W^W^W^W`bc` to verify the addition locally 🏓
Sjors:
Code review ACK edb6b76. Nit: commit description has duplicate text.
Tree-SHA512: 6fa0be0ecfbfd5d537f2c5b4a9333c76530c1f3182f777330cc7939b0496e37b75d8f8810cdaf471a9bd3247b425f2e239578300dfa0d5a87cd14a6ccfafa619
