fa04eb7d78 test: Document why no symbol names can be used for suppressions (MarcoFalke)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 02e5d9cb3ca299b5e7f1f0f780af514692147f5f0846e21e5375a99e90d3aed3a3489c674ac0eb90ae34d60f576410ba1d926d5ca70cdce4f9c7a111cd516adf
585854ac66 test: Replace blanket UBSan signed integer overflow suppression for txmempool.cpp with specific suppression (practicalswift)
Pull request description:
Replace file level (`txmempool.cpp`) signed integer overflow suppression with function level suppression (`CTxMemPool::PrioritiseTransaction`). The suppression was added yesterday in #21586.
Rationale: To avoid risk hiding other signed integer overflows in `txmempool.cpp`.
Obviously it would be better if this signed integer overflow fixed instead of suppressed - see details #20626. Any taker? :)
To hit the issue via fuzzing:
```
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=validation_load_mempool src/test/fuzz/fuzz
INFO: Seed: 1184244493
INFO: Loaded 1 modules (634418 inline 8-bit counters): 634418 [0x55a09fdfbf98, 0x55a09fe96dca),
INFO: Loaded 1 PC tables (634418 PCs): 634418 [0x55a09fe96dd0,0x55a0a08450f0),
INFO: 1264 files found in mempool/
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1040698 bytes
INFO: seed corpus: files: 1264 min: 1b max: 1040698b total: 15997133b rss: 197Mb
txmempool.cpp:847:15: runtime error: signed integer overflow: -7211388903327006720 + -7211353718954917888 cannot be represented in type 'long'
#0 0x55a09c3ce2d8 in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /home/thomas/bitcoin/src/txmempool.cpp:847:15
```
ACKs for top commit:
JeremyRubin:
utACK 585854a
hebasto:
ACK 585854ac66, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 5a343f028c1e1a1aba3b51a0eced605849184891ffafecb3cd2b424c6cfea01afd7c2672274936b0bac646075ec066408a570bf6b34bc9b87399a53ce20d8a23
fa8192f42e rpc: Fail to return undocumented return values (MarcoFalke)
Pull request description:
Currently a few return values are undocumented. This is causing confusion at the least. See for example #18476
Fix this by treating it as an internal bug to return undocumented return values.
ACKs for top commit:
ryanofsky:
Code review ACK fa8192f42e. Only changes: rebase, no const_cast suggestion, and tostring cleanups needed after suggestion
Tree-SHA512: c006905639bafe3045de152b00c34d9864731becb3c4f468bdd61a392f10d7e7cd89a54862c8daa8c11ac4eea0eb5f13b0f647d21e21a0a797b54191cff7238c
735610940c build: set --build when configuring packages in depends (fanquake)
Pull request description:
After reading [#Specifying-Target-Triplets](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#Specifying-Target-Triplets) in the autoconf manuel, my understanding is that this change should mostly be a no-op, as `--build` defaults to the output of [`config.guess`](https://github.com/bitcoin/bitcoin/blob/master/depends/config.guess), however, this may be slightly more correct
> For historical reasons, whenever you specify --host, be sure to specify --build too; this will be fixed in the future.
and will quell some warnings in depends (#16354), i.e:
> configure: WARNING: If you wanted to set the --build type, don't use --host.
If a cross compiler is detected then cross compile mode will be used.
If anything, this also explicitly enables cross-compilation mode when `--host` differs from `--build`. As for "fixed in the future", this is the case for Autoconf 2.70+.
If we don't make this change, I think we should consider closing #16354. We've now got a good handle on our depends configure flags, and we can just leave the last few warnings as they are. It's also unlikely that we are going to add the `xorg-macros` package to depends.
Guix builds at 735610940c:
```bash
bash-5.1# find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
afabf99650d57abd3df2c7e6253a3281f9814ada77d01c28ffb57c740b14fd8e output/bitcoin-735610940c0c-aarch64-linux-gnu-debug.tar.gz
6e9dedced61489f7734c5387ee05823db68eb4b890fa05f11541c1b966576446 output/bitcoin-735610940c0c-aarch64-linux-gnu.tar.gz
5333a3c5978a61966251e56fedab8e059d9d8a1a7231a0908095bb373204a81f output/bitcoin-735610940c0c-arm-linux-gnueabihf-debug.tar.gz
d28e1c166b96e58cf40b3f9801c3050c2785b25eeb261d44eeb31a32a75e134c output/bitcoin-735610940c0c-arm-linux-gnueabihf.tar.gz
e00b0149c52e9207be1cfb56ec3b715b55f41691401095f7a7038ac124f08d80 output/bitcoin-735610940c0c-osx-unsigned.dmg
ae5052ad1a05916bc6656b034c169097471fd9bdba16b606e2bcb592c9395544 output/bitcoin-735610940c0c-osx-unsigned.tar.gz
8586a9b8b82a8832397fcd0756aa9649adc3e61ae579fb370b99fe406298199f output/bitcoin-735610940c0c-osx64.tar.gz
d560a8781bae856b6593e9b731fa99de0a4ad7bd3e03084166dc98904ece9e7b output/bitcoin-735610940c0c-powerpc64-linux-gnu-debug.tar.gz
a24cf28b7efd5ebdc892e7e8809a05b3405bb4222f7d2eaa32de265214c78f28 output/bitcoin-735610940c0c-powerpc64-linux-gnu.tar.gz
ad29421842dbbe96526eb597a7ff2970f0acae8c3e75dc6c2100716a2ba20606 output/bitcoin-735610940c0c-powerpc64le-linux-gnu-debug.tar.gz
ee7a8ccfd68cc2297bb96eaaacf7b3eb939a5e39cecc3c710dbe828518f1bfbd output/bitcoin-735610940c0c-powerpc64le-linux-gnu.tar.gz
185bf4ea3b35072a8dd54aedbafc0892ec1e486c8ebd311f05f0fb47a19058cf output/bitcoin-735610940c0c-riscv64-linux-gnu-debug.tar.gz
803078b15a9bbaa567176827639895e762edbe80844b80400f8a9581e5311d32 output/bitcoin-735610940c0c-riscv64-linux-gnu.tar.gz
d920327a32a42a5abadd247f0d3f3ee0ab0488a28892c11838d99bede6456723 output/bitcoin-735610940c0c-win-unsigned.tar.gz
9f1741a37e294af153d57ad4a6dad3e5f1cdf75f1588c86cb467abf0c177ae27 output/bitcoin-735610940c0c-win64-debug.zip
2d1a91552c0dd7bb400ce6c71d2e98de702743a97afe5f4b0945865f251f3aea output/bitcoin-735610940c0c-win64-setup-unsigned.exe
8a70020bb0cb68516acf4b2cc89832d72373f31611bd075826a30a817a071cd2 output/bitcoin-735610940c0c-win64.zip
93b03837679474905538e96c44ba81ee5653795ad333dc00569b92c82dc6ea31 output/bitcoin-735610940c0c-x86_64-linux-gnu-debug.tar.gz
6ccd046a83ef5089667ad426875319beb83e2b2a5fb4d4459d5c5ce8d75f1a31 output/bitcoin-735610940c0c-x86_64-linux-gnu.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc output/src/bitcoin-735610940c0c.tar.gz
```
Gitian builds:
```bash
# macOS:
7d087749f610e5e3eca91c730d41afd4d404808e276242d05cedb89403de247f bitcoin-735610940c0c-osx-unsigned.dmg
96d68b4f0042dc40f5e8e362fa84dcf8e122dcb566889feb48ad8a982847dd8b bitcoin-735610940c0c-osx-unsigned.tar.gz
2dea72cbe5c3e228babd88b516a6f43b0e040e9510ea95b62f07fc0815bf8d22 bitcoin-735610940c0c-osx64.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
478dbda9d551a8bed985c61c625df3f543afab26989a52bc44d5a4745715af81 bitcoin-core-osx-22-res.yml
# Windows:
c31a5118dfa21259b9868e2c4858c635becb786c2ee8af92302da7811a1efe9b bitcoin-735610940c0c-win-unsigned.tar.gz
5c171ec3aee830abc8ae843d674d5fe7a57420e782ee765b6992914ae5b7671a bitcoin-735610940c0c-win64-debug.zip
ca010ced982f75d111e01f3759b961deb83e7bf511b1ee9c9041f4830570f2a0 bitcoin-735610940c0c-win64-setup-unsigned.exe
62514b01281f81af6057fb12cc5e3813c1d8f6532d0d8ac8f6f2909a595aa1dd bitcoin-735610940c0c-win64.zip
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
e26b55d8302a419f594396db6ff441aaeb5e275e02fde7b1b4eb092270f902e7 bitcoin-core-win-22-res.yml
# Linux:
e5be4deb91cb372de484468c0575a04f907f982e35a9841659bb213540f770d0 bitcoin-735610940c0c-aarch64-linux-gnu-debug.tar.gz
6169e822679ece2bb6af7027362a6f373eb939eef9d89edc58c21190b4083c7d bitcoin-735610940c0c-aarch64-linux-gnu.tar.gz
aee5308767c5c930853a80ab94325b2a4ab3dbe9a7a20c7a9d60d57af2b5383f bitcoin-735610940c0c-arm-linux-gnueabihf-debug.tar.gz
a9cf6e56d859fffd5a2b5f1e6360515bc6b06b048776b93cc08954859457a251 bitcoin-735610940c0c-arm-linux-gnueabihf.tar.gz
7617d85c358e6614fd40026a8d346f9edba5b5139e3daade2ec2aec72aed3a1f bitcoin-735610940c0c-powerpc64-linux-gnu-debug.tar.gz
f76e46a8b130812541ebcaaba173b4cbe2ec1781e348579b44f96e9244cc2475 bitcoin-735610940c0c-powerpc64-linux-gnu.tar.gz
19ada7abb801422bd7772d7ee559004cab11a46141a747aabd27085e8c3d200d bitcoin-735610940c0c-powerpc64le-linux-gnu-debug.tar.gz
f3a6bbefe9f47b3960e3b9eeb3f740ecddf8773ae76961ced6a3cdf82503e37f bitcoin-735610940c0c-powerpc64le-linux-gnu.tar.gz
42ca333e8f82b4ec0b2354efc7c4f1c27ed32d60c35b88e33d26833419f19e69 bitcoin-735610940c0c-riscv64-linux-gnu-debug.tar.gz
dde930d9134fb5be1374014db3f6f503b614ae801a1d77f6f4b82a5f11eb2518 bitcoin-735610940c0c-riscv64-linux-gnu.tar.gz
82e1b4cc8aac309a99458cc27fd0285215a8af53780094c4990303ce8948cf0c bitcoin-735610940c0c-x86_64-linux-gnu-debug.tar.gz
ee8caad2ccddd8eaf8aa2d19040d859130168183355b09a2a0f77a59a97fcaee bitcoin-735610940c0c-x86_64-linux-gnu.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
d90545b13226a338c994d9e538f468a6fafd694273bf71e14614feeaf5727ad4 bitcoin-core-linux-22-res.yml
```
ACKs for top commit:
dongcarl:
Code Review ACK 735610940c
hebasto:
ACK 735610940c, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: 13ce05269309a12d03b81a539ba5585003785f7b1bb7894a9353385a6e9c709f920cc4f79322f28d7a256809754f201e70be6b917915c7b14e1461530075781d
This bug doesn't have any effects currently because it only affects
external signer RPCs which aren't currently using the wallet context,
but it does cause an appveyor failure in a upcoming PR:
https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882
This bug is subtle and could have been avoided if JSONRPCRequest didn't
have constructors that were so loose with type checking. Suggested
change
https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351
eliminates these and would be a good followup for a future PR.
This is a non-functional change that replaces the `CNode` on-stack
variables with `CNode` pointers.
The reason for this is that it would allow us to add those `CNode`s
to `CConnman::vNodes[]` which in turn would allow us to check that they
are disconnected properly - a `CNode` object must be in
`CConnman::vNodes[]` in order for its `fDisconnect` flag to be set.
If we store pointers to the on-stack variables in `CConnman` then it
would crash at the end, trying to `delete` them.
e309646db6 Doc: Copyright: Fix embedded font file location (wodry)
Pull request description:
ACKs for top commit:
hebasto:
ACK e309646db6, it was overlooked in 89e421918e.
Tree-SHA512: 016035b82b587fd195a53bcea938b2d1258b8e908ac545477e2002d713c063b3f18f01cd546aaac3ce0543aec029502fcde3b552ab3e837125a1c0f8a983996a
2e5f7def22 wallet, rpc: update listdescriptors response format (Ivan Metlushko)
Pull request description:
Update `listdescriptors` response format according to [RPC interface guidelines](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#rpc-interface-guidelines).
This is a follow up for #20226
**Before:**
```
Result:
[ (json array) Response is an array of descriptor objects
{ (json object)
"desc" : "str", (string) Descriptor string representation
"timestamp" : n, (numeric) The creation time of the descriptor
"active" : true|false, (boolean) Activeness flag
"internal" : true|false, (boolean, optional) Whether this is internal or external descriptor; defined only for active descriptors
"range" : [ (json array, optional) Defined only for ranged descriptors
n, (numeric) Range start inclusive
n (numeric) Range end inclusive
],
"next" : n (numeric, optional) The next index to generate addresses from; defined only for ranged descriptors
},
...
]
```
**After:**
```
Result:
{ (json object)
"wallet_name" : "str", (string) Name of wallet this operation was performed on
"descriptors" : [ (json array) Array of descriptor objects
{ (json object)
"desc" : "str", (string) Descriptor string representation
"timestamp" : n, (numeric) The creation time of the descriptor
"active" : true|false, (boolean) Activeness flag
"internal" : true|false, (boolean, optional) Whether this is internal or external descriptor; defined only for active descriptors
"range" : [ (json array, optional) Defined only for ranged descriptors
n, (numeric) Range start inclusive
n (numeric) Range end inclusive
],
"next" : n (numeric, optional) The next index to generate addresses from; defined only for ranged descriptors
},
...
]
}
```
ACKs for top commit:
achow101:
re-ACK 2e5f7def22
meshcollider:
utACK 2e5f7def22
jonatack:
re-ACK 2e5f7def22
Tree-SHA512: 49bf73e46e2a61003ce594a4bfc506eb9592ccb799c2909c43a1a527490a4b4009f78dc09f3d47b4e945d3d7bb3cd2632cf48c5ace5feed5066158cc010dddc1
./windeploy is a "working directory", and therefore belongs inside
distsrc-*. Many people have noticed their Guix builds failing after
hours simply because they did not remove windeploy (but did remove the
distsrc-* directories).
In Guix, there are two flags for controlling parallelism:
Note: When I say "derivation," think "package"
--cores=n
- controls the number of CPU cores to build each derivation. This is
the value passed to `make`'s `--jobs=` flag.
- defaults to 0: as many cores as is available
--max-jobs=n
- controls how many derivations can be built in parallel
- defaults to 1
Therefore, if set --max-jobs=$MAX_JOBS and don't set --cores, Guix could
theoretically spin up $MAX_JOBS * $(nproc) number of threads, and that's
no good.
So we could either default to --cores=1, --max-jobs=$MAX_JOBS
- Pro: --cores=1 means that `make` will be invoked with `-j1`,
avoiding problems with package whose build systems and test
suites break when running multi-threaded.
- Con: There will be times when only 1 or 2 derivations can be built
at a time, because the rest of the dependency graph all depend
on those 1 or 2 derivations. During these times, the machine
will be severely under-utilized.
or --cores=$MAX_JOBS, --max-jobs=1
- Pro: We don't encounter prolonged periods of
severe under-utilization mentioned above.
- Con: Many packages' build systems and test suites break when running
multi-threaded.
or --cores=1, --max-jobs=1 and let the user override with
$ADDITIONAL_GUIX_COMMON_FLAGS
4eca20d6f7 [doc] correct comment about ATMPW (glozow)
8fa74aeb5b [doc] correct comment in chainparams (glozow)
2f8272c2a4 [doc] GetBestBlock() doesn't do nothing (gzhao408)
Pull request description:
Came across a few misleading comments, wanted to fix them
ACKs for top commit:
jnewbery:
ACK 4eca20d6f7
MarcoFalke:
ACK 4eca20d6f7
laanwj:
Code review ACK 4eca20d6f7
Tree-SHA512: 5bef1f1e7703f304128cf0eb8945e139e031580c99062bbbe15bf4db8443c2ba5a8c65844833132e6646c8980c678fc1d2ab0c63e17105585d583570ee350fd0
ccd976dd3d test: use 327 fewer blocks in feature_nulldummy (Jon Atack)
68c280f197 test, refactor: abstract the feature_nulldummy blockheight values (Jon Atack)
Pull request description:
The resolved timeout issue seen in the CI can be reproduced locally by running `test/functional/feature_nulldummy.py --valgrind --loglevel=debug`
Speeds up the normal test runtime for me from 3.8 to 2.2 seconds (debug build). Thanks to Marco Falke for the approach suggestion.
ACKs for top commit:
AnthonyRonning:
reACK ccd976dd3d - ran a few times with the rest of the tests and still passing for me with just the fewer block change.
MarcoFalke:
review ACK ccd976dd3d 🏝
Tree-SHA512: 38339dca4276d1972e3a5a5ee436da64e9e58fd3b50b186e34b07ade9523ac4c03f6c3869c5f2a59c23b43c44f87e712f8297a01a8d83202049c081e3eeb4445
3a0446fad4 script: Add explanatory comment to tc.sh (dscotese)
Pull request description:
This is a replacement for #21289
tc.sh is used to limit bandwidth. I ran it and it is limiting my bandwidth. When I ran it, I got one error. I have not found an explanation anywhere of what the error means, but my best guess is consistent with the result, so I propose the explanatory comment to save others time when they use it and also get the error.
ACKs for top commit:
laanwj:
that said, LGTM ACK 3a0446fad4
Tree-SHA512: 5403a2a0fec3724625c20402a96334c3c7a620324a930c5fd828017da8911d2867aecb7a2ad94a23d1f189009d3eb197a67eb59c8e4531fd215d9b1edb600440
