fa8e01a5b8 doc: Remove outdated scriptChange TODO comment (MarcoFalke)
Pull request description:
This was added in commit bf798734db (raw multisig). Raw multisig isn't a thing, so remove the TODO.
ACKs for top commit:
S3RK:
ACK fa8e01a5b8
achow101:
ACK fa8e01a5b8
Tree-SHA512: 01d521ca3605ab130c43531da4922ea85461ca5e7436267a34fb5df348009e086b3c66d85532c62255d9a0ba43db56424884808e773d0ef0177035dfb25d6a6c
efab28b06b Add FastRange32 function and use it throughout the codebase (Pieter Wuille)
96ecd6fa3e scripted-diff: rename MapIntoRange to FastRange64 (Pieter Wuille)
c6d15c45d9 [moveonly] Move MapIntoRange() to separate util/fastrange.h (Pieter Wuille)
Pull request description:
Several places in the codebase use the fast range mapping technique described in https://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/, some for 32-bit ranges, some for 64-bit ones.
Move all of these to `util/fastrange.h`, and give them a consistent name.
ACKs for top commit:
Sjors:
ACK efab28b06b
shaavan:
reACK efab28b06b
MarcoFalke:
review ACK efab28b06b🍸
Tree-SHA512: 3190a25ef21d17f0ab2afcd9b8d5a1813fdfac0d93996878017e84ff62eee412c823d6149ae8e92cfc3214458641e83ace4b022b4a0fe0679f78dbaee21c6227
6200fbf54f build: rename --enable-ebpf to --enable-usdt (0xb10c)
e158a2a7aa build: add systemtap's sys/sdt.h as depends (0xb10c)
Pull request description:
There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don't have to be switched out. This is possible because we don't do [expensive computations](https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#no-expensive-computations-for-tracepoints) only needed for the tracepoints. The tracepoints are NOPs when not used.
Systemtap's `sys/sdt.h` header is required to build Bitcoin Core with USDT support. The header file defines the `DTRACE_PROBE` macros used in [`src/util/trace.h`](https://github.com/bitcoin/bitcoin/blob/master/src/util/trace.h). This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints.
Closes https://github.com/bitcoin/bitcoin/issues/23297.
ACKs for top commit:
fanquake:
ACK 6200fbf54f - tested enabling / disabling and with/without SDT from depends. We can follow up with #23819, #23907 and #23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free.
Tree-SHA512: 0263f44892bf8450e8a593e4de7a498243687f8d81269e1c3283fa8354922c7cf93fddef4b92cf5192d33798424aa5812e03e68ef8de31af078a32dd34021382
742918c8ef qt: hide Create Unsigned button behind an expert mode option (Andrew Chow)
5c3b800acd qt: Add Create Unsigned button to SendConfirmationDialog (Andrew Chow)
Pull request description:
Instead of having different buttons or changing button behavior for making a PSBT, just have SendConfirmationDialog return whether the user wants a PSBT or a broadcasted transaction. Since this dialog is used by both the bumpFeeAction and the SendCoinsDialog, changes to both to support the different behavior is needed. They will check the return value of the SendConfirmationDialog for whether a PSBT needs to be created instead of checking whether private keys are disabled.
Strings used in this dialog are being slightly modified to work with both private keys enabled and disabled wallets.
Moved from https://github.com/bitcoin/bitcoin/pull/18789
ACKs for top commit:
jarolrod:
ACK 742918c
ryanofsky:
Code review ACK 742918c8ef. Just suggested changes since last review. Looks great!
hebasto:
ACK 742918c8ef, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: dd29f4364c7b4f15befe8fe63257b26187918786b005e0f8336183270b1a162680b93f6ced60f0285c6e607c084cc0d24950fc68a8f9c056521ede614041be66
1823766fc6 refactor: add thread safety lock assertion to WriteBlockIndexDB() (Jon Atack)
Pull request description:
New helper function `BlockManager::WriteBlockIndexDB()` added in #23974 has a thread safety lock annotation in its declaration but is missing the corresponding run-time lock assertion in its definition.
Per doc/developer-notes.md: "Combine annotations in function declarations with run-time asserts in function definitions."
ACKs for top commit:
MarcoFalke:
cr ACK 1823766fc6
Tree-SHA512: b915e6b105c38b8bbe04ad810aefa68e940a13b8dd265e79563a2aaefc93ffa031d56a7f3c481a5ada90de7c2ddd3b419dcfa46c22fa26c22f95eda15cd243bc
2fda0c7851 doc: Drop no longer required notes for Windows builds (Hennadii Stepanov)
f09ed92be1 build: Try posix-specific CXX first for mingw32 host (Hennadii Stepanov)
Pull request description:
On master (1186910b6b), when cross-compiling for Windows using our depends build system, we must manually choose the POSIX thread model for the `x86_64-w64-mingw32-g++` compiler.
This PR improves the build system to make this choice automa**g**ically.
ACKs for top commit:
jarolrod:
re-ACK 2fda0c7851
shaavan:
tACK 2fda0c7851
Tree-SHA512: ad3bbdfe84b3c8eb56e102aa0abdc76fd864b2724ac59c9a12a798c57d1c5a1172682172588019d5d1715801a9f201fd9424effbd858e8dc30607c23c0d5941f
The new helper function, BlockManager::WriteBlockIndexDB(),
has a thread safety lock annotation in its declaration but is
missing the corresponding run-time lock assertion in its definition.
Per doc/developer-notes.md: "Combine annotations in function
declarations with run-time asserts in function definitions."
84f9931cb4 guix: use upstream python-requests (2.26.0) (fanquake)
187dc1ec0c build: use python-asn1crypto from upstream (fanquake)
b1e8f0b96e guix: use uptream nsis-x86_64 (fanquake)
3ccfba1c7f guix: use GCC 10 (over GCC 8) to build releases (fanquake)
Pull request description:
Guix's `core-updates-frozen` branch has been merged back into `master`, and a [`version-1.4.0`](https://git.savannah.gnu.org/cgit/guix.git/log/?h=version-1.4.0) branch has been created. This is great, as it means the next Guix release is on the horizon, and it contains a number of changes I'd like to take advantage of. In particular, is migrating the version of GCC we use for releases from GCC 8 to GCC 10.3.0 (which is also the new Guix default GCC). This is my preferred method of unblocking progress in #20744, which is currently stalled due to support for `std::filesystem` for Windows not arriving in GCC until version 9, whereas it's usable on Linux starting with GCC 8. The current set of changes in that PR [attempt to backport support](9604eda1ab) for `std::filesystem`, for Windows, to GCC 8, similar to what is currently done by Debian, however that is somewhat convoluted, and using GCC 10 with our current Guix version would require updating at least one Guix patch to GCC, so is not completely straightforward either.
Other changes included here:
* Dropping our `--no-*` patch for mingw binutils ld, as we can take advantage of the `--disable-*` flags that are now available in binutlils 2.37. The security check tests are updated accordingly.
* Dropping our current patch for NSIS, as it's been integrated upstream, however given we are building v3.05, we need a different one (229b6136c4) for compiling against GCC 10.
* Removing our `python-asn1crypto` package definition, as an identical package is available in Guix. Over time we should look at trying to get the rest of the python packages we define here upstreamed.
* Adding a patch for `python-elfsteem` to fix an issue in the example code when using Python 3.9+.
* Our base glibc (`2.24`) now inherits from glibc-2.31. Guix has removed packages of glibc < 2.29, and the current version of glibc is `2.33`. However glibc-2.31 is the newest version that still contains a workaround for installing sunrpc data, which we need, so inheriting from that version seemed like the most straightforward solution.
* As mentioned, Guix has removed glibcs < 2.29, so we add our own package definition for glibc 2.27, which we use for our RISC-V toolchain (also inheriting from 2.31).
The guix commit hash currently points to the head of the `version-1.4.0` branch. This can be updated to an official release tag when one is available.
Looking for Concept ACKs on migrating to using GCC 10.3 for building releases. Keeping in mind issues like #20005, however that particular bug should be fixed in GCC 10.3.0+, according to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189.
Guix Builds:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
ea56ef38bd94dbcb11b9d10e2f10c205109daad03fea4313f79892fc497ba68d guix-build-84f9931cb449/output/aarch64-linux-gnu/SHA256SUMS.part
01123ab23e5a09dc06a897837389e859d302ba2b18fbe827936ec8983765e7df guix-build-84f9931cb449/output/aarch64-linux-gnu/bitcoin-84f9931cb449-aarch64-linux-gnu-debug.tar.gz
7a24e25c2237e5aeb14508b91c5c6954572814e1767e892c164494f32d73b0c0 guix-build-84f9931cb449/output/aarch64-linux-gnu/bitcoin-84f9931cb449-aarch64-linux-gnu.tar.gz
0e1dba0233da1f487222b128964980d50393e61a6971bcf4c71951c29fdf3993 guix-build-84f9931cb449/output/arm-linux-gnueabihf/SHA256SUMS.part
8cd4c6f42abc81427f1d2500f86daced2a4ee78882dd9d03b5a0211a1d96306e guix-build-84f9931cb449/output/arm-linux-gnueabihf/bitcoin-84f9931cb449-arm-linux-gnueabihf-debug.tar.gz
c180db6bffb1a54b6dc65929d86d5eba9adf876a28ad320590ed230233e57299 guix-build-84f9931cb449/output/arm-linux-gnueabihf/bitcoin-84f9931cb449-arm-linux-gnueabihf.tar.gz
4efcda7b63646eb46dabea7122fb026f2c063d2919a9dcbbffbc0929b9c56ced guix-build-84f9931cb449/output/dist-archive/bitcoin-84f9931cb449.tar.gz
1e35e96034fed00674f362d6471fb402dd2758cec2860ded4fd7e37c38935a44 guix-build-84f9931cb449/output/powerpc64-linux-gnu/SHA256SUMS.part
96a0b7f54d3b3935c134f8c2aaaf11a314b54c9d7924ba751503caa16bd1c840 guix-build-84f9931cb449/output/powerpc64-linux-gnu/bitcoin-84f9931cb449-powerpc64-linux-gnu-debug.tar.gz
ae05137b6fb3494120f5413bf8a94ca3c1b0c047e1f512e6c2c5a0b1f122f075 guix-build-84f9931cb449/output/powerpc64-linux-gnu/bitcoin-84f9931cb449-powerpc64-linux-gnu.tar.gz
c22e5fbcdcdbfa5d385537e2c1dab55004d9e94396ebccef0bc3d216edfacbbe guix-build-84f9931cb449/output/powerpc64le-linux-gnu/SHA256SUMS.part
52602b41e81a921435d93f2a3ae29549aa65a4147cdbf1ed7d9e4a44c4dc902a guix-build-84f9931cb449/output/powerpc64le-linux-gnu/bitcoin-84f9931cb449-powerpc64le-linux-gnu-debug.tar.gz
a2cc7e9385452163a7bda99f6f9aa630fd35d4ba13d4fd9a4dd7e8062206650d guix-build-84f9931cb449/output/powerpc64le-linux-gnu/bitcoin-84f9931cb449-powerpc64le-linux-gnu.tar.gz
e75fadf1b1c7e4ae3d52e7a8051a881de17bd4d9d32c1ca29ca0ddbb8028ee51 guix-build-84f9931cb449/output/riscv64-linux-gnu/SHA256SUMS.part
3b643c33842a15befb5d36d13b598a5e628c11b95671336c8dea51b5eed9c79a guix-build-84f9931cb449/output/riscv64-linux-gnu/bitcoin-84f9931cb449-riscv64-linux-gnu-debug.tar.gz
e9a1ee7451502508cde73dc300aca8a421e379ac08c3f4adaf8c768fbfa942ac guix-build-84f9931cb449/output/riscv64-linux-gnu/bitcoin-84f9931cb449-riscv64-linux-gnu.tar.gz
c0508a0872cf1415a47983d2ebbc9e5a46282ce7b6453afac544e0d1315b7bf9 guix-build-84f9931cb449/output/x86_64-apple-darwin/SHA256SUMS.part
7c02267cb91e2649088af5e96f81142beaad67f6a1a0588355174a4157b31458 guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx-unsigned.dmg
46dbf5a911abfa63e3c5aa8440289da5fdea89da013253c08768ce58b798a99d guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx-unsigned.tar.gz
ab2e2360f18cb1b80bfd37f1a9508a938e89237767120472f932402cc809f0eb guix-build-84f9931cb449/output/x86_64-apple-darwin/bitcoin-84f9931cb449-osx64.tar.gz
f58aa000692f7ea09ab8e7ec159a806d3a665f0f70558e62a53d56afb361eb02 guix-build-84f9931cb449/output/x86_64-linux-gnu/SHA256SUMS.part
78a76aef8469b07a41588e019a6dfa890c36fd5becf2c8d73a71c9e72bcabde6 guix-build-84f9931cb449/output/x86_64-linux-gnu/bitcoin-84f9931cb449-x86_64-linux-gnu-debug.tar.gz
5e6e0040b37ff035de41c8fcfee5d498bd19fa489024704dd4caa0ab9f566450 guix-build-84f9931cb449/output/x86_64-linux-gnu/bitcoin-84f9931cb449-x86_64-linux-gnu.tar.gz
d6e6af70f277d9c9ef9b4773ec05920355ac07ebec71ff3e179676047329964b guix-build-84f9931cb449/output/x86_64-w64-mingw32/SHA256SUMS.part
37f24f6899e7803ed07bd0f5eb3f0fb6237ac1254dd72f446e9e4e488a927c8e guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win-unsigned.tar.gz
14f7d1c14a5fc3b4c336d301f936c5578d6e31d61ec720dfc9d4129445d1e2a2 guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64-debug.zip
c8049dcc0308a76f21dd781e8561ebbafa84034fbf8e3afa7d4017866d7fd195 guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64-setup-unsigned.exe
fb1e6580c25b073118f121aabaa04aa09643bc97cfeaea7c9a24bbe65c33cbb6 guix-build-84f9931cb449/output/x86_64-w64-mingw32/bitcoin-84f9931cb449-win64.zip
```
ACKs for top commit:
hebasto:
re-ACK 84f9931cb4
Tree-SHA512: 2f5f4f6bb1f55a048dba88523f235320e51c4af963355abf6a86b7035623b2100ae3dc44396c76fbeea89ae9cfbc5342abd3e2c41760ede8b689d7757d6e7f25
fa68a6c2fc scripted-diff: Rename touched member variables (MarcoFalke)
facd3df21f Make blockstorage globals private members of BlockManager (MarcoFalke)
faa8c2d7d7 doc: Clarify nPruneAfterHeight for signet (MarcoFalke)
fad381b2f8 test: Load genesis block to allow flush (MarcoFalke)
fab262174b Move blockstorage-related unload to BlockManager::Unload (MarcoFalke)
fa467f3913 move-only: Create WriteBlockIndexDB helper (MarcoFalke)
fa88cfd3f9 Move functions to BlockManager (MarcoFalke)
Pull request description:
Globals aren't too nice because they hide dependencies, also they make testing harder.
Fix that by removing some.
ACKs for top commit:
Sjors:
ACK fa68a6c2fc
ryanofsky:
Code review ACK fa68a6c2fc. Nice changes!
Tree-SHA512: 6abc5929a5e43a05e238276721d46a64a44f23dca18c2caa9775437a32351d6815d88b88757254686421531d0df13861bbd3a202e13a3192798d87a96abef65d
c9374af102 test: set ban after mocking time (brunoerg)
Pull request description:
Fixes#23988
Set ban after mocking time to avoid intermittent failures related to the assertion of ban_duration and time_remaining.
See: https://cirrus-ci.com/task/6754020390862848?logs=ci#L4652
ACKs for top commit:
mzumsande:
Tested ACK c9374af102
vincenzopalazzo:
ACK c9374af102
Tree-SHA512: fac3ac91a045bb46334d7c568f6a53a3b0a45b306914a54ea13bcc845734eaaad1ff295ff3ab158037fd9d08df77344058331336110b8f7888832b16b0589be5
0754e9c01b test: run feature_pruning.py without wallet compiled (Sebastian Falbesoner)
Pull request description:
Only one small part of the pruning test (sub-test `wallet_test`) is wallet-related, hence we can run all other parts without wallet compiled.
ACKs for top commit:
MarcoFalke:
cr ACK 0754e9c01b
Tree-SHA512: 856856903d21d64953ed0102cc2a96f55975c4b7d8e93e57b82c266024967160df64df2b6068be089efc05e883e8d6d12e7327053420d4c640b9d8cc5bcb1c58
172096e9dd scripted-diff: Rename libbitcoin_server.a to libbitcoin_node.a (Russell Yanofsky)
Pull request description:
Goal along with namespacing PR #23497 is to make code organization more obvious and have `src/node/` code in `node::` namespace in `libbitcoin_node.a` library
ACKs for top commit:
MarcoFalke:
cr ACK 172096e9dd
Tree-SHA512: a2e787eeaa3ab769b0f5376473072cae584d237aa8b67b677bea833bb36b0134a0eca17eb01722389639473b8463f4953bc3a5e4801a6b2c8965ac1075cba005
e3544c864e init: Use clang-tidy named args syntax (Carl Dong)
3401630417 style-only: Rename *Chainstate return values (Carl Dong)
1dd582782d docs: Make LoadChainstate comment more accurate (Carl Dong)
6b83576388 node/chainstate: Use MAX_FUTURE_BLOCK_TIME (Carl Dong)
Pull request description:
There are 2 proposed fixups in discussions in #23280 which I have not implemented:
1. An overhaul to return types and an option type for the two `*Chainstate` functions: https://github.com/bitcoin/bitcoin/pull/23280#issuecomment-984149564
- The change reintroduces stringy return types and is quite involved. It could be discussed in a separate PR.
2. Passing in the unix time to `VerifyChainstate` instead of a callback to get the time: https://github.com/bitcoin/bitcoin/pull/23280#discussion_r765051533
- I'm not sure it matters much whether it's a callback or just the actual unix time. Also, I think `VerifyDB` can take quite a while, and I don't want to impose that the function have to "run quickly" in order to have it be correct.
If reviewers feel strongly about either of the two fixups listed above, please feel free to open a PR based on mine and I'll close this one!
ACKs for top commit:
ryanofsky:
Code review ACK e3544c864e
MarcoFalke:
ACK e3544c864e🐸
Tree-SHA512: dd1de0265b6785eef306e724b678ce03d7c54ea9f4b5ea0ccd7af59cce2ea3aba73fd4af0c15e2dca9265807dc4075f9afa2ec103672677b6638b1a4fc090904
fe86eb50c9 Refactor: Uses c++ init convention for time variables (Shashwat)
6111b0d7fa Refactor: Changes remaining time variable type from int to chrono (Shashwat)
Pull request description:
This PR is a follow-up to #23801.
This PR aims to make the following changes to all the time variables in **net_processing.cpp** wherever possible.
- Convert all time variables to `std::chrono.`
- Use `chorno::literals` wherever possible.
- Use `auto` keywords wherever possible.
- Use `var{val}` convention of initialization.
This PR also minimizes the number of times, serialization of time `count_seconds(..)` occurs.
ACKs for top commit:
MarcoFalke:
re-ACK fe86eb50c9 🏕
Tree-SHA512: c8684c0c60a11140027e36b6e9706a45ecdeae6b5ba0bf267e50655835daee5e5410e34096a8c4eca005f327caae1ac258cc7b8ba663eab58abf131f6d2f4d42
60ae1161a4 qa: replace assert with test framework assertion helpers in fee estimation test (Antoine Poinsot)
e50213967b qa: fee estimation with RBF test cleanups (Antoine Poinsot)
15f5fd62af qa: don't mine non standard txs in fee estimation test (Antoine Poinsot)
eae52dd6ab qa: pass scriptsig directly to txins constructor in fee estimation test (Antoine Poinsot)
1fc03155e5 qa: split coins in a single tx in fee estimation test (Antoine Poinsot)
cc204b8be7 qa: use a single p2sh script in fee estimation test (Antoine Poinsot)
19dd91a9be qa: remove a redundant condition in fee estimation test (Antoine Poinsot)
Pull request description:
Some cleanups that i noticed would be desirable while working on #23074 and #22539, which are intentionally not based on it. Mainly simplifications and a slight speedup.
- Use a single tx to create the `2**9` coins instead of creating `2**8` 2-outputs transactions
- Use a single P2SH script
- Avoid the use of non-standard transactions
- Misc style nits (happy to take more)
ACKs for top commit:
pg156:
I ACK all commits up to 60ae1161a4 (except 1fc03155e5, where I have a question more for my own learning than actually questioning the PR). I built and ran the test successfully. I agree after the changes, the behavior is kept the same and the code is shorter and easier to reason.
glozow:
utACK 60ae1161a4
Tree-SHA512: 57ae2294eb68961ced30f32448c4a530ba1cdee17881594eecb97e1b9ba8927d58c25022b847eb07fb67d676bf436108c416c2f2174864d258fcca5b528b8bbd
eBPF is a Linux kernel technology used to "extend the capabilities
of the kernel without requiring to change kernel source code or
load kernel modules". While Userspace, Statically Defined Tracing
(USDT) uses eBPF under the hood, --enable-usdt better resembles that
support for USDT is enabled, and tracepoints will be included in the
binary.
9bd46e24a6 reviewers: add tracing (William Casarin)
Pull request description:
Adding myself and 0xB10C to USDT tracing reviews
~~cc 0xB10C lmk if you want in on this~~
ACKs for top commit:
0xB10C:
ACK 9bd46e24a6
Tree-SHA512: 8c6674bcf0fc70422163d353ad22d4bb3d747af9d17de069d39b4576ec3e1baf883605774683f9caf84d138ae1dc1424d1a9afdd115abba9d947cbc31a309144
fa9f4554ca refactor: Remove pointless and confusing shift in RelayAddress (MarcoFalke)
Pull request description:
The second argument written to the siphash is already quantized to 24 hours, so it seems confusing to quantize the first argument to 32 bits (out of 64 bits).
> The shifting is pointless, we should get rid of it. It seems to be a silly evolution of this 2010 Satoshi code: 5cbf753 (where it made sense because everything was XORed together, and the address used the high bits, while the time used the low ones).
(Copied from https://github.com/bitcoin/bitcoin/pull/18642#issuecomment-613773120)
(The original code was `uint256 hashRand = hashSalt ^ (((int64)addr.ip)<<32) ^ ((GetTime()+addr.ip)/(24*60*60));`)
This also allows to remove a integer sanitizer suppression for the whole file.
ACKs for top commit:
laanwj:
Code review ACK fa9f4554ca
sipa:
utACK fa9f4554ca
promag:
Code review ACK fa9f4554ca.
Tree-SHA512: f5fd107464ccd839d6749aed6914b4935e39ab42906546b3f3810a7339fc4633fef931a1783a287572af5ec64525626fa91d147d8ff52eb076740465bf5cf839
ac617cc141 wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)
Pull request description:
After parsing the checksum, make sure that it is the size that we expect it to be.
This issue was reported by Pedro Baptista.
ACKs for top commit:
laanwj:
Code review ACK ac617cc141
Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
8bd34dc774 test: check that bitcoin-tx detects missing input amount for segwit transactions (Sebastian Falbesoner)
c337b27d7c Require that input amount is provided for bitcoin-tx witness transactions (Ben Woosley)
Pull request description:
This PR picks up the obviously abandoned PR #13608 (last activity was three and a half years ago) by rebasing it on master and adding missing tests. Original PR description: "_Applies fix from #12458 / #13547 to bitcoin-tx._"
The private key is the compressed version of the one used in most other util tests (5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf, corresponds to the scalar value k=1 in big endian), since segwit signing refuses uncompressed keys.
The error message from the picked up PR is changed to not include the amount, as showing any value would be just confusing.
ACKs for top commit:
josibake:
ACK 8bd34dc774
Tree-SHA512: 334b418f89527363ad7e3326b4126e86a05fd64876c49a8280de38e64cfac52cb62c4b24b83603dd68b6bcebbe57c64161832edffb1cac7e9c68426f6b6eae1f
975097f424 Let test_runner.py start multiple jobs per timeslot (Pieter Wuille)
Pull request description:
test_runner.py currently only checks every 0.5s whether any job has finished, and if so, starts at most one new job. At higher parallellism it becomes increasingly likely that multiple jobs have finished at the same time. Fix this by always noticing *all* finished jobs every timeslot, and starting as many new ones.
ACKs for top commit:
laanwj:
Code review and lightly tested ACK 975097f424
prayank23:
ACK 975097f424
Tree-SHA512: b70c51f05efcde9bc25475c192b86e86b4c399495b42dee20576af3e6b99e8298be8b9e82146abdabbaedb24a13ee158a7c8947518b16fc4f33a3b434935b550
This is needed to turn globals into member variables. Otherwise, this
will lead to issues:
runtime error: reference binding to null pointer of type 'CBlockFileInfo'
#0 in std::vector<CBlockFileInfo, std::allocator<CBlockFileInfo> >::operator[](unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1046:2
#1 in BlockManager::FlushBlockFile(bool, bool) src/node/blockstorage.cpp:540:47
#2 in CChainState::FlushStateToDisk(BlockValidationState&, FlushStateMode, int) src/validation.cpp:2262:28
#3 in CChainState::ResizeCoinsCaches(unsigned long, unsigned long) src/validation.cpp:4414:15
#4 in validation_chainstate_tests::validation_chainstate_resize_caches::test_method() src/test/validation_chainstate_tests.cpp:66:12
This is a refactor and safe to do because:
* UnloadBlockIndex calls ChainstateManager::Unload, which calls
BlockManager::Unload
* Only unit tests call Unload directly
fab16415ba doc: Fix typo in getmempoolinfo (MarcoFalke)
Pull request description:
Also, remove whitespace. Can be reviewed with `--ignore-all-space --word-diff-regex=.`.
ACKs for top commit:
laanwj:
Good catch. ACK fab16415ba
shaavan:
ACK fab16415ba
Tree-SHA512: 9d51ef4a4eccfcf437c99a9f84f48e4f090d75715332ad2b4cf10ad77c3691de03255b4817e9fd203fad9baf338066982304dc62fab93dd605e735943f8ca346
6bf6e9fd9d net: change CreateNodeFromAcceptedSocket() to take Sock (Vasil Dimov)
9e3cbfca7c net: use Sock in CConnman::ListenSocket (Vasil Dimov)
f8bd13f85a net: add new method Sock::Accept() that wraps accept() (Vasil Dimov)
Pull request description:
_This is a piece of https://github.com/bitcoin/bitcoin/pull/21878, chopped off to ease review._
Introduce an `accept(2)` wrapper `Sock::Accept()` and extend the usage of `Sock` in `CConnman::ListenSocket` and `CreateNodeFromAcceptedSocket()`.
ACKs for top commit:
laanwj:
Code review ACK 6bf6e9fd9d
jamesob:
ACK 6bf6e9fd9d ([`jamesob/ackr/21879.2.vasild.wrap_accept_and_extend_u`](https://github.com/jamesob/bitcoin/tree/ackr/21879.2.vasild.wrap_accept_and_extend_u))
jonatack:
ACK 6bf6e9fd9d per `git range-diff ea989de 976f6e8 6bf6e9f` -- only change since my last review was `s/listen_socket.socket/listen_socket.sock->Get()/` in `src/net.cpp: CConnman::SocketHandlerListening()` -- re-read the code changes, rebase/debug build/ran units following my previous full review (https://github.com/bitcoin/bitcoin/pull/21879#pullrequestreview-761251278)
w0xlt:
tACK 6bf6e9f
Tree-SHA512: dc6d1acc4f255f1f7e8cf6dd74e97975cf3d5959e9fc2e689f74812ac3526d5ee8b6a32eca605925d10a4f7b6ff1ce5e900344311e587d19786b48c54d021b64
c03cf38a16 doc: Fix typo in LoadBlockIndex (brunoerg)
Pull request description:
Instad -> Instead
Top commit has no ACKs.
Tree-SHA512: 37dcdd34e2bd985619daecc9d1072ac002f3bc7b47db413432d027a2b8cce32501f93a57ee85869755bb0eedecedfab6ecac9ce2a591341c8f011118390a5b18
fa996c58e8 refactor: Avoid integer overflow in ApplyStats when activating snapshot (MarcoFalke)
fac01888d1 Move AdditionOverflow to util, Add CheckedAdd with unit tests (MarcoFalke)
fa526d8fb6 Add dev doc to CCoinsStats::m_hash_type and make it const (MarcoFalke)
faff051560 style: Remove unused whitespace (MarcoFalke)
Pull request description:
A snapshot contains the utxo set, including the out value. To activate the snapshot, the hash needs to be calculated. As a side-effect, the total amount in the snapshot is calculated (as the sum of all out values), but never used. Instead of running into an integer overflow in an unused result, don't calculate the result in the first place.
Other code paths (using the active utxo set) can not run into an integer overflow, since the active utxo set is valid.
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39716
ACKs for top commit:
shaavan:
reACK fa996c58e8
vasild:
ACK fa996c58e8
Tree-SHA512: 4f207f634841f6f634fd02ae1e5907e343fd767524fd0e8149aa99fa9a1834fe50167d14874834d45236e9c325d567925f28129bacb7d80be29cf22277a16a14
7746606cfa test: use MiniWallet for mining_basic.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (mining_basic.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.
ACKs for top commit:
brunoerg:
crACK 7746606cfa
Tree-SHA512: 4455b8b764413b0fc3ef388e3c5d5758f9e6b6d3193ac660269a9ba1c988022e6b7bc148549c2167942ea472c5aaddd2b6b3b9d99790d0654b089af975b86e11
96eb0093d0 test: wait rather than assert presence of file in startupnotify test (fanquake)
Pull request description:
Should fix#23967.
ACKs for top commit:
brunoerg:
crACK 96eb0093d0
kristapsk:
utACK 96eb0093d0
Tree-SHA512: 9107970e45c027cfc6c6cbfcfd5a7d9f5956259bbbb11f5b180c3947126e42e62c0f8ffd69cf7b39b51c9c5b4fedbb753839d59aebe876be68c1484bb6065819
e09773d20a build: use a static .tiff for macOS .dmg over generating (fanquake)
Pull request description:
For demonstration, after [discussion in #23778](https://github.com/bitcoin/bitcoin/pull/23778#issuecomment-1003005503), and the question as to why we can't just have a `background.tiff` that we copy into the macOS DMG, and do away with the somewhat convoluted image generation steps.
From my understanding, the only reason we have this image generation as part of our build system is so that forks of Core can adapt the imagery for their own branding via `PACKAGE_NAME`. It don't think it provides much value to us, and could just have a static .tiff that we copy into the dmg (replacing the .svg that currently lives in macdeploy/).
Doing this would eliminate the following build dependencies:
For native macOS:
* `sed` (usage in Makefile.am)
* `librsvg` (rsvg-convert)
* `tiffutil`
Linux macOS cross-compile:
* `sed` (usage in Makefille.am)
* `librsvg`
* `tiffcp`
* `convert` (imagemagick)
* `font-tuffy`
Guix Build:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c98d67796863f4b1bab0ad600d46bd74e744d94072cbd4bc856a6aeaba3bb329 guix-build-e09773d20a92/output/dist-archive/bitcoin-e09773d20a92.tar.gz
3336f90bab312798cb7665e2b4ae24d1a270fb240647d5fed8dbfcd83e3ed37e guix-build-e09773d20a92/output/x86_64-apple-darwin/SHA256SUMS.part
8fd680c7ee158c64bad212385df7b0b302c6c2143d4e672b4b0eb5da41f9256d guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.dmg
34f54177c2f0700e8cfaf5d85d91e404807cd9d411e22006cdff82653e5f4af2 guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.tar.gz
da6b8f54ef755d40330c8eac4f5bd0329637e827be9ee61318600d5d0bdcc3dc guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx64.tar.gz
```
ACKs for top commit:
hebasto:
ACK e09773d20a
jarolrod:
ACK e09773d20a
Zero-1729:
ACK e09773d20a
Tree-SHA512: 0ad06699a5451daa8cfaaa46759eb7bd85254a72e23f857f70d433a2ffb1a4bf6dd464d9c4ac9f8c20aab045f4e2b61c6dcdcbcceef96ce515b1a0c501665b1f
