| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
git-subtree-dir: src/minisketch
git-subtree-split: 89629eb2c7e262b39ba489b93b111760baded4b3
|
|
17ae2601c786e6863cee1bd62297d79521219295 build: remove build stubs for external leveldb (Cory Fields)
Pull request description:
Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages.
For context, this was reported on IRC:
> \<Talkless> bitcoind fails to start with undefined symbol: _ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to 1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486
ACKs for top commit:
fanquake:
ACK 17ae2601c786e6863cee1bd62297d79521219295
hebasto:
ACK 17ae2601c786e6863cee1bd62297d79521219295. I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2f1ac2cb30dac64791933a245a2b66ce237bde3955e6f4a6b7ec181248f77a9b1b10597d865d3e2c2b6def696af70de40e905ec274e4ae7cccd1daf461473957
|
|
b65a25a84666d41a0af4ad98ffadfa4ac802d1bb log: improve addrman logging (Martin Zumsande)
Pull request description:
The addrman helper functions `GetNewBucket()` and `GetTriedBucket()`
1) log into the wrong category (`BCLog::NET` instead of `BCLog::ADDRMAN`)
2) log too unspecifically - especially `GetTriedBucket()` gets called from many different places (e.g. `Check_()`, `Serialize()`), it seems sufficient to me logging these when moving an address from new to tried. Running a node with `-checkaddrman=1`and net logging currently results in a lot of repetitive log entries.
This PR moves these log entries to `Add_()` and `Good_()` and also adds logging for `Select_()` (allowing statistics about New/Tried success probabilities), `GetAddr_()`, `ClearNew()` and `MakeTried()`.
ACKs for top commit:
jnewbery:
ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb
vasild:
ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb
Tree-SHA512: 90ab0f64eb44b7388a198efccb613577b74989fea73194bda7de8bfbd50bdb19127cb12f5ec645c7859afdb89290614a79e255f3af0a63a58d4f21aa8fe7b696
|
|
faf13e272cad44917c4e5516172617fe8d68c00a Add missing gettimeofday to syscall sandbox (MarcoFalke)
Pull request description:
Fixes:
```
2021-10-18T09:12:31Z [init] [httpserver.cpp:181] [InitHTTPAllowList] Allowing HTTP connections from: 127.0.0.0/8 ::1/128
2021-10-18T09:12:31Z [init] [util/syscall_sandbox.cpp:487] [SyscallSandboxDebugSignalHandler] ERROR: The syscall "gettimeofday" (syscall number 96) is not allowed by the syscall sandbox in thread "init". Please report.
ACKs for top commit:
practicalswift:
cr ACK faf13e272cad44917c4e5516172617fe8d68c00a
Tree-SHA512: fb7b56124e3c9b04fc03224e421f54e9b3a28992e03500a23c465819d5f7c0700b1c04eb0e4cf8e2378ef69694d9f068f4356a85245289b04cb02e08c58b7c9b
|
|
due to SegWit
ef72e9bd4124645fe2d00521a71c1c298d760225 doc: nChainTx needs to become a 64-bit earlier due to SegWit (Sjors Provoost)
Pull request description:
As of block 597,379 txcount is 460,596,047 (see `chainparams.cpp`), while `uint32` can handle up to 4,294,967,296.
Pre segwit the [minimum transaction size](https://en.bitcoin.it/wiki/Maximum_transaction_rate) was 166 bytes, so the worst case number of transactions per block was ~6000. As the original source comment for `unsigned int nChainTx` says, that should last until the year 2030.
With SegWit the smallest possible transaction is 60 bytes (potentially increased to 65 with a future soft fork, see #15482), without a witness:
```
4 bytes version
1 byte input count
36 bytes outpoint
1 byte scriptSigLen (0x00)
0 bytes scriptSig
4 bytes sequence
1 byte output count
8 bytes value
1 byte scriptPubKeyLen
1 byte scriptPubKey (OP_TRUE)
4 bytes locktime
```
That puts the maximum number of transactions per block at 16,666 so we might have to deal with this as early as a block 827,450 in early 2024.
Given that it's a memory-only thing and we want to allow users many years to upgrade, I would suggest fixing this in v0.20 and back-porting it.
ACKs for top commit:
practicalswift:
re-ACK ef72e9bd4124645fe2d00521a71c1c298d760225
jarolrod:
ACK ef72e9bd4124645fe2d00521a71c1c298d760225
theStack:
ACK ef72e9bd4124645fe2d00521a71c1c298d760225
Tree-SHA512: d8509ba7641796cd82af156354ff3a12ff7ec0f7b11215edff6696e95f8ca0e3596f719f3492ac3acb4b0884ac4e5bddc76f107b656bc2ed95a8ef1b2b5d4f71
|
|
::ChainActive()
a0efe529e4fd053b890450413b9ca5e1bcd8f2c2 Fix outdated comments referring to ::ChainActive() (Samuel Dobson)
Pull request description:
After #21866 there are a few outdated comments referring to `::ChainActive()`, which should instead refer to `ChainstateManager::ActiveChain()`.
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23258/commits/a0efe529e4fd053b890450413b9ca5e1bcd8f2c2
Tree-SHA512: 80da19c105ed29ac247e6df4c8e916c3bf3f37230b63f07302114eef9c115add673e9649f0bbe237295be0c6da7b1030b5b93e14daf6768f17ce5de7cf2c9ff2
|
|
`--with-system-univalue`
0f95247246344510c9a51810c14c633abb382e95 Integrate univalue into our buildsystem (Cory Fields)
9b49ed656fb2b687fbbe8a3236d18285957eee16 Squashed 'src/univalue/' changes from 98fadc0909..a44caf65fe (fanquake)
Pull request description:
This PR more tightly integrates building Univalue into our build system. This follows the same approach we use for [LevelDB](https://github.com/bitcoin-core/leveldb/), ([`Makefile.leveldb.include`](https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.leveldb.include)), and [CRC32C](https://github.com/bitcoin-core/crc32c) ([`Makefile.crc32c.include`](https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include)), and will be the same approach we use for [minisketch](https://github.com/sipa/minisketch); see #23114.
This approach yields a number of benefits, including:
* Faster configuration due to one less subconfigure being run during `./configure` i.e 22s with this PR vs 26s
* Faster autoconf i.e 13s with this PR vs 17s
* Improved caching
* No more issues with compiler flags i.e https://github.com/bitcoin/bitcoin/pull/12467
* More direct control means we can build exactly the objects we want
There might be one argument against making this change, which is that builders should have the option to use "proper shared/system libraries". However, I think that falls down for a few reasons. The first being that we already don't support building with a number of system libraries (secp256k1, leveldb, crc32c); some for good reason. Univalue is really the odd one out at the moment.
Note that the only fork of Core I'm aware of, that actively patches in support for using system libs, also explicitly marks them as ["DANGEROUS"](https://github.com/bitcoinknots/bitcoin/blob/a886811721ce66eb586871706b3f5dd27518ac3e/configure.ac#L1430) and ["NOT SUPPORTED"](https://github.com/bitcoinknots/bitcoin/blob/a886811721ce66eb586871706b3f5dd27518ac3e/configure.ac#L1312). So it would seem they exist more to satisfy a distro requirement, as opposed to something that anyone should, or would actually use in practice.
PRs like #22412 highlight the "issue" with us operating with our own Univalue fork, where we actively fix bugs, and make improvements, when upstream (https://github.com/jgarzik/univalue) may not be taking those improvements, and by all accounts, is not currently actively maintained. Bitcoin Core should not be hamstrung into not being able to fix bugs in a library, and/or have to litter our source with "workarounds", i.e #22412, for bugs we've already fixed, based on the fact that an upstream project is not actively being maintained. Allowing builders to use system libs is really only exacerbating this problem, with little benefit to our project. Bitcoin Core is not quite like your average piece of distro packaged software.
There is the potential for us to give the same treatment to libsecp256k1, however it seems doing that is currently less straightforward.
ACKs for top commit:
dongcarl:
ACK 0f95247246 less my comment above, always nice to have an include-able `sources.mk` which makes integration easier.
theuni:
ACK 0f95247246344510c9a51810c14c633abb382e95. Thanks fanquake for keeping this going.
Tree-SHA512: a7f2e41ee7cba06ae72388638e86b264eca1b9a8b81c15d1d7b45df960c88c3b91578b4ade020f8cc61d75cf8d16914575f9a78fa4cef9c12be63504ed804b99
|
|
35e814c1cfedf17e7751b730ae1e36099801be4c qt: never disable HD status icon (=)
Pull request description:
[#8517](https://github.com/bitcoin/bitcoin/pull/8517) introduced the HD status icon in the bottom menu bar. At the time, it may have made sense to disable the HD status icon (which would display the icon at 50% opacity) when the wallet is not HD or watch-only.
Since then, we have had many changes to our UI. Namely, the ability to switch between dark and light mode on macOS. Additionally, since version 0.16, the Bitcoin Core client only [generates HD wallets](https://github.com/bitcoin/bitcoin/issues/12547). Non-HD Wallets cannot be created anymore, only imported.
This PR proposes never to disable the HD status icon in the bottom menu bar. Instead, we always leave the HD status icon enabled for a better user interface. There’s no good reason for it to remain disabled, and it’s hard to see the watch-only icon in macOS dark mode. If a user has imported a non-HD wallet, we also want the icon to be as clear as possible in the bottom menu bar.
- on macOS
| | Master | PR |
| ------------- | ------------- | ------------- |
| Dark Mode |  |  |
| Light Mode || |
- on ubuntu 20.04
| Master | PR |
| ------------- | ------------- |
|  | |
ACKs for top commit:
jarolrod:
ACK 35e814c1cfedf17e7751b730ae1e36099801be4c
hebasto:
ACK 35e814c1cfedf17e7751b730ae1e36099801be4c
Tree-SHA512: ba7d1c1f2a439bbb32f7755c10cadc5d61892add3abca2a286a30a0fbbbb077e2190365c9e8dee6a10b4feed249656c7bd58ea14a319f5b28cd56182d4ddbdc1
|
|
(#21245 modified)
5c34507ecbbdc29c086276d1c62835b461823507 core_write: Rename calculate_fee to have_undo for clarity (fyquah)
8edf6204a87057a451160d1e61e79d8be112e81f release-notes: Add release note about getblock verbosity level 3. (fyquah)
459104b2aae6eeaadfa5a7e47944f1a34780dacd rest: Add test for prevout fields in getblock (fyquah)
4330af6f72172848f5971a052a8f325ed50eb576 rpc: Add test for level 3 verbosity getblock rpc call. (fyquah)
51dbc167e98daab317baa80cf80bfda337672dab rpc: Add level 3 verbosity to getblock RPC call. (fyquah)
3cc95345ca49b87e8caca9a0e6418c63ae1e463a rpc: Replace boolean argument for tx details with enum class. (fyquah)
Pull request description:
Author of #21245 expressed [time issues](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-902332088) in the original PR. Given that #21245 has received a lot of review*, I have decided to open this new pull request with [modifications required to get ACK from luke-jr ](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-905150806) and a few nits of mine.
### Original PR description
> Display the prevout in transaction inputs when calling getblock level 3 verbosity. This PR affects the existing `/rest/block` API by adding a `prevout` fields to tx inputs. This is mentioned in the change to the release notes.
>
> I added some functional tests that
>
> * checks that the RPC call still works when TxUndo can't be found
>
> * Doesn't display the "value" or "scriptPubKey" of the previous output when at a lower verbosity level
>
>
> This "completes" the issue #18771
### Possible improvements
* https://github.com/kiminuo/bitcoin/commit/b0bf4f255f86aeaddce68889087c22f9068f4d97 - I can include even this commit to this PR if deemed useful or I can leave it for a follow-up PR. See https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-894853784 for more context.
### Examples
Examples of the `getblock` output with various verbose levels. Note that `000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5` contains only 2 transactions.
#### Verbose level 0
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 0
```
##### Verbose level 1
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 1
```
##### Verbose level 2
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 2
```
##### Verbose level 3
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 3
```
#### REST
```bash
curl -H "content-type:text/plain;" http://127.0.0.1:18332/rest/block/000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5.json
```
<sub>* ... and my everyday obsessive checking of my email inbox whether the PR moves forward.</sub>
Edit laanwj: Removed at symbol from message, and large example output to prevent it from all ending up in the commit message.
ACKs for top commit:
0xB10C:
ACK 5c34507ecbbdc29c086276d1c62835b461823507
meshcollider:
utACK 5c34507ecbbdc29c086276d1c62835b461823507
theStack:
ACK 5c34507ecbbdc29c086276d1c62835b461823507 👘
promag:
Concept ACK 5c34507ecbbdc29c086276d1c62835b461823507
Tree-SHA512: bbff120d8fd76e617b723b102b0c606e0d8eb27f21c631d5f4cdab0892137c4bc7c65b1df144993405f942c91be47a26e80480102af55bff22621c19f518aea3
|
|
Also, sort entries. Can be reviewed with: --color-moved=dimmed-zebra
|
|
|
|
The tracepoint `validation:block_connected` was introduced in #22006.
The first argument was the hash of the connected block as a pointer
to a C-like String. The last argument passed the hash of the
connected block as a pointer to 32 bytes. The hash was only passed as
string to allow `bpftrace` scripts to print the hash. It was
(incorrectly) assumed that `bpftrace` cannot hex-format and print the
block hash given only the hash as bytes.
The block hash can be printed in `bpftrace` by calling
`printf("%02x")` for each byte of the hash in an `unroll () {...}`.
By starting from the last byte of the hash, it can be printed in
big-endian (the block-explorer format).
```C
$p = $hash + 31;
unroll(32) {
$b = *(uint8*)$p;
printf("%02x", $b);
$p -= 1;
}
```
See also: https://github.com/bitcoin/bitcoin/pull/22902#discussion_r705176691
This is a breaking change to the block_connected tracepoint API, however
this tracepoint has not yet been included in a release.
|
|
ca2c313aa291ae44adc1b7148ed49125bdc77bf4 Use absolute FQDN for DNS seed domains (Prayank)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/23193 by using absolute FQDN for domains used by DNS seeds.
It improves security and should not break anything based on my research and testing. Few things about absolute FQDN are shared in https://superuser.com/questions/1467958/why-does-putting-a-dot-after-the-url-remove-login-information
Master branch:
```
DNS seed x9.dnsseed.bitcoin.dashjr.org. responded with IP 127.0.0.1
```
PR branch:
```
DNS seed x9.dnsseed.bitcoin.dashjr.org. responded with IP 159.89.108.149
```
Reviewers can follow the steps mentioned in Issue to test: https://github.com/bitcoin/bitcoin/issues/23193#issuecomment-937717145
ACKs for top commit:
practicalswift:
cr ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4
laanwj:
code review ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4
promag:
Code review ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4.
Tree-SHA512: 9818227332282a78c45b4470c2fc80bf899ed78aed76644ebf014e0fff1b139402ea023acdea162363a478b6f6613dbf1da57e214d2240ea0f04310473f57cca
|
|
d873db7f8ff37c37f9c72482d8ecd52282f4438e policy: document we intentionally don't lower the dust threshold for Taproot (Antoine Poinsot)
Pull request description:
Following discussions in #22779 .
ACKs for top commit:
benthecarman:
ACK d873db7f8ff37c37f9c72482d8ecd52282f4438e
ariard:
Code Review ACK d873db7
theStack:
ACK d873db7f8ff37c37f9c72482d8ecd52282f4438e
Tree-SHA512: 1f5d20dce767f8a74d57ece47a7f6b881741f508896131b8433600cccf9e4262892603b46521d1bb69d5c83b450f24a16731341072a471c1f2c9adad682af895
|
|
fs::path(std::string) constructor and fs::path::string() method
6544ea5035268025207d2402db2f7d90fde947a6 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec69a51b2016d3a8c70c0c77670f87f2b refactor: Add fs::PathToString, fs::PathFromString, u8string, u8path functions (Russell Yanofsky)
Pull request description:
The `fs::path` class has a `std::string` constructor which will implicitly convert from strings. Implicit conversions like this are not great in general because they can hide complexity and inefficiencies in the code, but this case is especially bad, because after the transition from `boost::filesystem` to `std::filesystem` in #20744 the behavior of this constructor on windows will be more complicated and can mangle path strings. The `fs::path` class also has a `.string()` method which is inverse of the constructor and has the same problems.
Fix this by replacing the unsafe method calls with `PathToString` and `PathFromString` function calls, and by forbidding unsafe method calls in the future.
ACKs for top commit:
kiminuo:
ACK 6544ea5035268025207d2402db2f7d90fde947a6
laanwj:
Code review ACK 6544ea5035268025207d2402db2f7d90fde947a6
hebasto:
re-ACK 6544ea5035268025207d2402db2f7d90fde947a6, only added `fsbridge_stem` test case, updated comment, and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22937#pullrequestreview-765503126) review. Verified with the following command:
Tree-SHA512: c36324740eb4ee55151146626166c00d5ccc4b6f3df777e75c112bcb4d1db436c1d9cc8c29a1e7fb96051457d317961ab42e6c380c3be2771d135771b2b49fa0
|
|
Presumably these stubs indicate to packagers that external leveldb is meant to
be supported in some way. It is not. Remove the stubs to avoid sending any
mixed messages.
|
|
when upgrading non-HD to HD
6531599f422524fbbcc43816121e7536cf79d66c test: Add check that newkeypool flushes change addresses too (Samuel Dobson)
84fa19c77a2c8d0d01add2daf18b42af07c17710 Add release notes for keypool flush changes (Samuel Dobson)
f9603ee4e05d7f0bd7d81f5cf24168c1aec8e5b0 Add test for flushing keypool with newkeypool (Samuel Dobson)
6f6f7bb36c492fa76aeda6513be58ca822ea1968 Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson)
2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Fix outdated keypool size default (Samuel Dobson)
22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Add newkeypool RPC to flush the keypool (Samuel Dobson)
Pull request description:
This PR makes two main changes:
1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool.
2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys.
This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call.
ACKs for top commit:
laanwj:
re-ACK 6531599f422524fbbcc43816121e7536cf79d66c
meshcollider:
Added new commit 6531599f422524fbbcc43816121e7536cf79d66c to avoid invalidating previous ACKs.
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/23093/commits/6531599f422524fbbcc43816121e7536cf79d66c
Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774
|
|
eb04badcd6108e17c8ec78e9c316775c334030cb scripted-diff: Prefix makefile variables with QT_ (João Barbosa)
Pull request description:
Improves consistency and readability if future QML variables are added.
ACKs for top commit:
hebasto:
re-ACK eb04badcd6108e17c8ec78e9c316775c334030cb, only suggested changes, and script-diff used.
shaavan:
Code Review ACK eb04badcd6108e17c8ec78e9c316775c334030cb
Tree-SHA512: 71e5f835edbb36d6749773e63ef5f4ce040cc576f1c302e371ae8715b874df0810b8a1ca2329e581880168c8ca95375cb84856a7ac5311bc8a059425da113341
|
|
|
|
b5950dd59ca3e144721a5f15568a65be43bd2f20 validation: put coins cache write log into bench debug log (Anthony Towns)
31b2b802b5b8f214e91ec47fbc953c6ca95c7cf9 blockstorage: use debug log category (Anthony Towns)
da94ebc2facd75c6105a7bd31765c6d2b37fc73b validation: move header validation error logging to VALIDATION debug category (Anthony Towns)
1d7d835ec3a7cc17924f80222784fe3afb567b67 validation: include block hash when reporting prev block not found errors (Anthony Towns)
Pull request description:
Moves the following log messages into debug log categories:
* "AcceptBlockHeader: ..." to validation
* "Prune: deleted blk/rev" to new blockstorage log category
* "Leaving block file" moves from validation to blockstorage
* "write coins cache to disk" to bench
Also adds the hash of the block to the log message when AcceptBlockHeader is rejecting because of problems with the prev block.
ACKs for top commit:
practicalswift:
cr ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
Empact:
Code review ACK https://github.com/bitcoin/bitcoin/pull/23235/commits/b5950dd59ca3e144721a5f15568a65be43bd2f20
laanwj:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
promag:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20.
meshcollider:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
Tree-SHA512: a73fdbfe8d36da48a3e89c2d5e0b6a3c5045d280c1a57f61c38d0d21f4f198aece4bd85155be3439e179d5dabdb523bf15fa0395e0e3ceff19c878ba3112c840
|
|
Improves consistency and readability if future QML variables are added.
-BEGIN VERIFY SCRIPT-
sed -i \
-e 's/RES_ANIMATION/QT_RES_ANIMATION/g' \
-e 's/RES_FONTS/QT_RES_FONTS/g' \
-e 's/RES_ICONS/QT_RES_ICONS/g' \
-e 's/BITCOIN_RC/BITCOIN_QT_RC/g' \
src/Makefile.qt.include
-END VERIFY SCRIPT-
|
|
ce69e18947dd917d54eb036e732ac7f7bceede76 scripts: remove pixie.py (fanquake)
00b85d0b13e419c1b48f47a90236cb4ed7fab491 scripts: only parse the binary once in security-check.py (fanquake)
cad40a5b167c5300564bd7b4505cf6650780084b scripts: use LIEF for ELF checks in security-check.py (fanquake)
8242ae230e1ff362a4e16f1503e503317471c790 scripts: only parse the binary once in symbol-check.py (fanquake)
309eac9019c224dfd79a78e381cfcb70fee190f3 scripts: use LIEF for ELF checks in symbol-check.py (fanquake)
610a8a8e39c1d94839dfb7e1c66c01f13f946657 test-*-check: Pass in *FLAGS and compile with them (Carl Dong)
Pull request description:
This finishes the transition to using LIEF for the ELF symbol and security checks.
Note that there's currently a work around used for identifying RISCV binaries (just checking the interpreter). I've sent a PR upstream, https://github.com/lief-project/LIEF/pull/562, and we should be able to drop that when using LIEF 0.12.0 and onwards.
ACKs for top commit:
dongcarl:
Code Review ACK ce69e18947dd917d54eb036e732ac7f7bceede76
laanwj:
Code review ACK ce69e18947dd917d54eb036e732ac7f7bceede76
Tree-SHA512: 911ba693cd9777ad1fc1f66dff6c4d3630a907351215380cbde5b14a4bbf5cf7eebf52eafa7e86b27deabd2d93d1b403f34aabd356b5ceaab3cc6e9941a01dd4
|
|
range int strings
fa6f29de516c7af5206b91b59ada466032329250 bitcoin-tx: Reject non-integral and out of range multisig numbers (MarcoFalke)
fafab8ea5e6ed6b87fac57a5cd16a8135236cdd6 bitcoin-tx: Reject non-integral and out of range sequence ids (MarcoFalke)
fa53d3d8266ad0257315d07b71b4f8a711134622 test: Check that bitcoin-tx accepts whitespace around sequence id and multisig numbers (MarcoFalke)
Pull request description:
Seems odd to silently accept arbitrary strings that don't even represent integral values.
Fix that.
ACKs for top commit:
practicalswift:
cr ACK fa6f29de516c7af5206b91b59ada466032329250
laanwj:
Code review ACK fa6f29de516c7af5206b91b59ada466032329250
Empact:
Code review ACK https://github.com/bitcoin/bitcoin/pull/23253/commits/fa6f29de516c7af5206b91b59ada466032329250
promag:
Code review ACK fa6f29de516c7af5206b91b59ada466032329250.
Tree-SHA512: e31f7f21fe55ac069e755557bdbcae8d5d29e20ff82e441ebdfc65153e3a31a4edd46ad3e6dea5190ecbd1b8ea5a8f94daa5d59a3b7558e46e794e30db0e6c79
|
|
interfaces::Chain::isTaprootActive non-const
7e88f61b285e6a356ea4f6ba384858f109559985 multiprocess: Make interfaces::Chain::isTaprootActive non-const (Russell Yanofsky)
Pull request description:
`interfaces::Chain` is an abstract class, so declaring the method const would be exposing internal implementation details of subclasses to interface callers. And specifically this doesn't work because the multiprocess implementation of the `interfaces::Chain::isTaprootActive` method can't be const because IPC connection state and request state is not constant during the call.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23003/commits/7e88f61b285e6a356ea4f6ba384858f109559985
Tree-SHA512: 1c5ed89870aeb7170b9048c41299ab650dfa3d0978088e08c4c866fa0babb292722710b16f25540f26667220cb4747b1c256c4bd42893c552291eccc155346a3
|
|
|
|
|
|
|
|
reference in validation_chainstate_tests
fa4d0aacf2bbddaf1709660ffd8d520570533aa8 test: * -> & (MarcoFalke)
Pull request description:
This changes background_cs from being a pointer to a reference to work
around a gcc false warning. Also, this makes the test easier to read.
Fixes bitcoin#23101
Can be reviewed with --ignore-all-space.
ACKs for top commit:
practicalswift:
cr ACK fa4d0aacf2bbddaf1709660ffd8d520570533aa8
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23132/commits/fa4d0aacf2bbddaf1709660ffd8d520570533aa8
hebasto:
ACK fa4d0aacf2bbddaf1709660ffd8d520570533aa8, tested on Linux Mint 20.2 (x86_64) by merging this PR on top of the current master.
Tree-SHA512: 93a0d8859201f7074bea52fab8f6701409148bc50cfbb142cacfa6c991fc12c07584df04fead645f11703883df99535423d154f9945202e1c5aff49540d9b607
|
|
fa43e7c2d9dc5e2df70acd2019bdd24023c1d333 bitcoin-tx: Avoid treating overflow as OP_0 (MarcoFalke)
fa053c0019bc8b2174c485f4885f894f2b5de472 style: Fix whitespace in Parse* functions (MarcoFalke)
fa03dec7e98bdda8aa596ef7943cf0a8d0bcb127 refactor: Use C++11 range based for loop in ParseScript (MarcoFalke)
fad55e79ca18a5894a8da6db6309c323eecbb178 doc: Fixup ToIntegral docs (MarcoFalke)
Pull request description:
Seems odd to treat integer overflow as `OP_0`, so fix that.
ACKs for top commit:
theStack:
re-ACK fa43e7c2d9dc5e2df70acd2019bdd24023c1d333
shaavan:
ACK fa43e7c2d9dc5e2df70acd2019bdd24023c1d333
Tree-SHA512: 1bbe2de62d853badc18d57d169c6e78ddcdff037e5a85357995dead11c8e67a4fe35087e08a181c60753f8ce91058b7fcc06f5b7901afedc78fbacea8bc3ef4f
|
|
|
|
Co-authored-by: Carl Dong <contact@carldong.me>
|
|
|
|
This addresses issues like the one in #12467, where some of our compiler flags
end up being dropped during the subconfigure of Univalue. Specifically, we're
still using the compiler-default c++ version rather than forcing c++17.
We can drop the need subconfigure completely in favor of a tighter build
integration, where the sources are listed separately from the build recipes,
so that they may be included directly by upstream projects. This is
similar to the way leveldb build integration works in Core.
Core benefits of this approach include:
- Better caching (for ex. ccache and autoconf)
- No need for a slow subconfigure
- Faster autoconf
- No more missing compile flags
- Compile only the objects needed
There are no benefits to Univalue itself that I can think of. These changes
should be a no-op there, and to downstreams as well until they take advantage
of the new sources.mk.
This also removes the option to use an external univalue to avoid similar ABI
issues with mystery binaries.
Co-authored-by: fanquake <fanquake@gmail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
BIP32 nChild (de)serialization
7fc487afd1249eee97651a0eec0630d7e421924f refactor: use `{Read,Write}BE32` helpers for BIP32 nChild (de)serialization (Sebastian Falbesoner)
Pull request description:
This small refactoring PR replaces manual bit-fiddling (de)serialization of the BIP32 child number (nChild) by the helpers `ReadBE32`/`WriteBE32`. Note that those were first introduced in #4100, almost one year _after_ the BIP32 derivation implementation has been merged (#2829, eb2c9990).
ACKs for top commit:
sipa:
utACK 7fc487afd1249eee97651a0eec0630d7e421924f
laanwj:
Code review ACK 7fc487afd1249eee97651a0eec0630d7e421924f
Tree-SHA512: bbe3e411fb0429fa74c8a5705a91f4d6ed704dac9d6623ecb633563f22acf8e21f3189a16f1d0cf1aeedfc56a5b695df54ae51e9577e34eb6d7dc335de2da6de
|
|
fa165e954579436fe4b636e4222d8ce0c1269786 Replace stoul with ToIntegral in dbwrapper (MarcoFalke)
Pull request description:
The string is created with `%llu`. See: https://github.com/bitcoin/bitcoin/blob/7fcf53f7b4524572d1d0c9a5fdc388e87eb02416/src/leveldb/db/db_impl.cc#L1436-L1437
So it seems odd to silently accept when parsing: whitespace, a sign character, trailing chars, overflow, ....
Fix that by using the stricter ToIntegral.
ACKs for top commit:
laanwj:
Code review ACK fa165e954579436fe4b636e4222d8ce0c1269786
practicalswift:
cr ACK fa165e954579436fe4b636e4222d8ce0c1269786
theStack:
Code-review ACK fa165e954579436fe4b636e4222d8ce0c1269786
Tree-SHA512: b87f01431ca0b971ff84610022da8679d3c33470b88cfc3f4a337e6e176a0455715588aefd40e8e2bbe7459d902dc89d7bfe34e7fd66755f631cc18dc039fa2f
|
|
d54ec27bac388d7b84cf7b6cb4506bb0c25f2f88 qt: Add helper to load font (João Barbosa)
Pull request description:
Originally submitted as https://github.com/bitcoin-core/gui-qml/pull/49.
ACKs for top commit:
hebasto:
re-ACK d54ec27bac388d7b84cf7b6cb4506bb0c25f2f88
stratospher:
Tested ACK d54ec27. Refactoring the code and defining `loadFont()` in `src/qt/guiutil.cpp` reduces redundant imports of the `QFontDatabase` and is a better design.
shaavan:
ACK d54ec27bac388d7b84cf7b6cb4506bb0c25f2f88
Tree-SHA512: b156bb6ffb08dd57476f383a29bbb0a1108b62794d430debb77252f7d09df1409a7532b09d17d8836d1c2ab7c126a6618231164b9d0def1b8f361a81ef22d107
|
|
|
|
|
|
|
|
fa1477e706504f45a7a0f51b9f4b8014ee6a7d8d test: Add ParseMoney and ParseScript tests (MarcoFalke)
Pull request description:
Add missing tests
ACKs for top commit:
practicalswift:
cr ACK fa1477e706504f45a7a0f51b9f4b8014ee6a7d8d
shaavan:
tACK fa1477e706504f45a7a0f51b9f4b8014ee6a7d8d
Tree-SHA512: e57b4e8da4abe075b4ad7e7abd68c4d0eecf0c805acd2c72076aac4993d3ec5748fd02b721c4c110494db56fdbc199301e5cfd1dc0212f2002f355b47f70e539
|
|
44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c [fuzz] Update comment in FillAddrman() (John Newbery)
640476eb0e17fd4c64d4840ceab229642f1d79d9 [fuzz] Make Fill() a free function in fuzz/addrman.cpp (John Newbery)
90ad8ad61a38dbb1f247a5f3d5f649a856d9938a [fuzz] Make RandAddr() a free function in fuzz/addrman.cpp (John Newbery)
491975c596ebce93ae8de192c9ef171f002fac7c [fuzz] Pass FuzzedDataProvider& into Fill() in addrman fuzz tests (John Newbery)
56303e382e26ac7096c09152c66894dc3bb4d1fd [fuzz] Create a FastRandomContext in addrman fuzz tests (John Newbery)
Pull request description:
#22974 improved the performance of `CAddrMan::Good()` significantly so that it could be used directly in the fuzz tests, instead of those tests reaching directly into addrman's internal data structures.
This PR continues the work of making the fuzz tests only use `CAddrMan`'s public methods and pulls the `Fill()` and `RandAddr()` methods from `CAddrManDeterministic` into free functions, since they no longer need access to `CAddrMan` internals.
ACKs for top commit:
theuni:
utACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c. Agree the failure seems unrelated, looks like some startup race.
mzumsande:
ACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c
vasild:
ACK 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c
Tree-SHA512: fcf994e1dedd0012b77f632720b6423d51ceda4eb85c9efe572f2a1150117f9e511114a5206738dd94409137287577f3b01a9998f5237de845410d3d96e7cb7f
|
