Sync with the main repo#59
Merged
Merged
Conversation
Revert to the state prior to e507acbe and allow debugging from project root dir with `gdb src/bitcoind`.
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 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/bitcoin#23101 Can be reviewed with --ignore-all-space.
No longer special case a set of warnings, to make up our own -Werror, just use -Werror outright. This shouldn't really have any effect on existing builders, who were already using --enable-werror, and is more inline with what they would expect --enable-werror to be, which is erroring on any/all warnings. We keep -Wno-error=return-type because we know that is broken when using mingw-w64. It should only be applied when cross-compiling for Windows. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Inline with moving to descriptor (sqlite) wallets by default for 0.23, this adapts the build system so that a default `./configure` invocation no-longer fails if BDB isn't present. Currently, if configure is run with no options, and no BDB is present, we'll fail with: ```bash checking for Berkeley DB C++ headers... no configure: error: libdb_cxx headers missing, Bitcoin Core requires this library for BDB wallet support (--without-bdb to disable BDB wallet support) ``` If descriptor wallets are to be the default, this behaviour no longer makes sense, as a builder should be able to configure and build, to use a wallet, without BDB installed, and without passing additional arguments, i.e `--without-bdb` or `--with-incompatible-bdb`, to configure. With this change, running configure will no-longer fail, and will instead print: ```bash checking for Berkeley DB C++ headers... no configure: WARNING: libdb_cxx headers missing configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support configure: WARNING: Passing --without-bdb will suppress this warning checking for sqlite3 >= 3.7.17... yes checking whether to build wallet with support for sqlite... yes ```
…if BDB isn't available 747cd17 build: no-longer fail default configure if BDB isn't available (fanquake) Pull request description: Inline with moving to descriptor (sqlite) wallets by default for 0.23, this adapts the build system so that a default `./configure` invocation no-longer fails if BDB isn't present. Currently, if configure is run with no options, and no BDB is present, we'll fail with: ```bash checking for Berkeley DB C++ headers... no configure: error: libdb_cxx headers missing, Bitcoin Core requires this library for BDB wallet support (--without-bdb to disable BDB wallet support) ``` If descriptor wallets are to be the default, this behaviour no longer makes sense, as a builder should be able to configure and build, to use a wallet, without BDB installed, and without passing additional arguments, i.e `--without-bdb` or `--with-incompatible-bdb`, to configure. With this change, running configure will no-longer fail, but will instead print: ```bash checking for Berkeley DB C++ headers... no configure: WARNING: libdb_cxx headers missing configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support configure: WARNING: Passing --without-bdb will suppress this warning checking for sqlite3 >= 3.7.17... yes checking whether to build wallet with support for sqlite... yes ``` ACKs for top commit: hebasto: ACK 747cd17, tested on Linux Mint 20.2 (x86_64) with the (un)installed system packages `libdb-dev` and `libdb++-dev`. Tree-SHA512: ae316d71ad0803c9d4b02a5fedcade08242650d987cc047840493ba4a881e71ff48b099075bb7c325307d44744fcdeccb57f7fa8db4135c81a5835841f562afa
fa165e9 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 fa165e9 practicalswift: cr ACK fa165e9 theStack: Code-review ACK fa165e9 Tree-SHA512: b87f01431ca0b971ff84610022da8679d3c33470b88cfc3f4a337e6e176a0455715588aefd40e8e2bbe7459d902dc89d7bfe34e7fd66755f631cc18dc039fa2f
… for BIP32 nChild (de)serialization 7fc487a 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, eb2c999). ACKs for top commit: sipa: utACK 7fc487a laanwj: Code review ACK 7fc487a Tree-SHA512: bbe3e411fb0429fa74c8a5705a91f4d6ed704dac9d6623ecb633563f22acf8e21f3189a16f1d0cf1aeedfc56a5b695df54ae51e9577e34eb6d7dc335de2da6de
…info 0bc666b doc: add info for debugging with relative paths (S3RK) a8b515c configure: keep relative paths in debug info (S3RK) Pull request description: This is a follow-up for #20353 that fixes #21885 It also adds a small section to assist debugging without absolute paths in debug info. ACKs for top commit: kallewoof: Tested ACK 0bc666b Zero-1729: Light crACK 0bc666b Tree-SHA512: d4b75183c3d3a0f59fe786841fb230581de87f6fe04cf7224e4b89c520d45513ba729d4ad8c0e62dd1dbaaa7a25741f04d036bc047f92842e76c9cc31ea47fb2
…c_invalid_address_message.py test c2fbdca Add BECH32_INVALID_VERSION test (lsilva01) b142f79 skip test_getaddressinfo() if wallet is disabled (lsilva01) Pull request description: Most of `test/functional/rpc_invalid_address_message.py` does not requires wallet. But if the project is compiled in disable-wallet mode, the entire test will be skipped. This PR changes the test to run the RPC tests first and then checks if the wallet is compiled. ACKs for top commit: stratospher: tested ACK c2fbdca Tree-SHA512: 11fa2fedf4a15aa45e3f12490df8e22290a867d5de594247211499533c32289c68c0b60bd42dbf8305e43dbcc042789e7139317ef5c9f8cf386f2d84c91b9ac2
These test-*-check scripts should compile "test" binaries in a way that is as close to what autotools would do, since the goal is to make sure that if we run the *-check script, they can correctly detect flaws in binaries which are compiled by our autotools-based system. Therefore, we should emulate what happens when the binary is linked in autotools, meaning that for C binaries, we need to supply the CFLAGS, CPPFLAGS, and LDFLAGS flags in that order. Note to future developers: perhaps it'd be nice to have these test-*-check scripts be part of configure.ac to avoid having to manually replicate autoconf-like behaviour every time we find a discrepancy. Of course, that would also mean you'd have to write more m4...
Co-authored-by: Carl Dong <contact@carldong.me>
…sandbox fab4073 util: Add mincore and clone3 to syscall sandbox (MarcoFalke) Pull request description: Closes #23248 ACKs for top commit: practicalswift: cr ACK fab4073 laanwj: ACK fab4073 fanquake: ACK fab4073 Tree-SHA512: a8eb6b1844e40880163d2c52022dc4f84a53fae4d82fc651e456f527eca455dec32bbf960dac4366915c8a73d57b546b0b18f11b4da031962f7f775f2ca8c112
…low as OP_0 fa43e7c bitcoin-tx: Avoid treating overflow as OP_0 (MarcoFalke) fa053c0 style: Fix whitespace in Parse* functions (MarcoFalke) fa03dec refactor: Use C++11 range based for loop in ParseScript (MarcoFalke) fad55e7 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 fa43e7c shaavan: ACK fa43e7c Tree-SHA512: 1bbe2de62d853badc18d57d169c6e78ddcdff037e5a85357995dead11c8e67a4fe35087e08a181c60753f8ce91058b7fcc06f5b7901afedc78fbacea8bc3ef4f
…to reference in validation_chainstate_tests fa4d0aa 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 fa4d0aa jamesob: ACK bitcoin/bitcoin@fa4d0aa hebasto: ACK fa4d0aa, tested on Linux Mint 20.2 (x86_64) by merging this PR on top of the current master. Tree-SHA512: 93a0d8859201f7074bea52fab8f6701409148bc50cfbb142cacfa6c991fc12c07584df04fead645f11703883df99535423d154f9945202e1c5aff49540d9b607
… multisig numbers
…not integral fa8d492 rest: Return error when header count is not integral (MarcoFalke) Pull request description: Seems odd to interpret a hash (or any other string) as integer when it contains more than the digits 0 to 9. ACKs for top commit: practicalswift: cr ACK fa8d492 promag: Code review ACK fa8d492. shaavan: Code Review ACK fa8d492 Tree-SHA512: d6335b132ca2010fb8cae311dd936b2dea99a5bd0e6b2556a604f93698b8456df9190c5151345a03344243ede4aad0e2526cedc2aa8b5b1b8e8ce786cb3b6e50
38fd709 build: make --enable-werror just -Werror (fanquake) Pull request description: No longer special case a set of warnings, to make up our own -Werror, just use -Werror outright. This shouldn't really have any effect on existing builders, who were already using `--enable-werror`, and is more inline with what they would expect `--enable-werror` to be, which is erroring on any/all warnings. We keep `-Wno-error=return-type` because we know that is broken when using mingw-w64. It should only be applied when cross-compiling for Windows. Similar to the change in #20544, but with (hopefully) less work-arounds, and other bundled changes. A step towards some configure "cleanups". ACKs for top commit: hebasto: ACK 38fd709 (also see bitcoin/bitcoin#23149 (comment)), tested: MarcoFalke: Concept ACK 38fd709 Tree-SHA512: 37f1857d9408442cab63e40f9280427b73e09cdf03146b19c1339d7e44abd78e93df7f270ca1da0e83b79343cd3ea915f7b9e4e347488b5bc5ceaaa7540e5926
…TaprootActive non-const 7e88f61 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 bitcoin/bitcoin@7e88f61 Tree-SHA512: 1c5ed89870aeb7170b9048c41299ab650dfa3d0978088e08c4c866fa0babb292722710b16f25540f26667220cb4747b1c256c4bd42893c552291eccc155346a3
…of range int strings fa6f29d bitcoin-tx: Reject non-integral and out of range multisig numbers (MarcoFalke) fafab8e bitcoin-tx: Reject non-integral and out of range sequence ids (MarcoFalke) fa53d3d 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 fa6f29d laanwj: Code review ACK fa6f29d Empact: Code review ACK bitcoin/bitcoin@fa6f29d promag: Code review ACK fa6f29d. Tree-SHA512: e31f7f21fe55ac069e755557bdbcae8d5d29e20ff82e441ebdfc65153e3a31a4edd46ad3e6dea5190ecbd1b8ea5a8f94daa5d59a3b7558e46e794e30db0e6c79
…bol checks ce69e18 scripts: remove pixie.py (fanquake) 00b85d0 scripts: only parse the binary once in security-check.py (fanquake) cad40a5 scripts: use LIEF for ELF checks in security-check.py (fanquake) 8242ae2 scripts: only parse the binary once in symbol-check.py (fanquake) 309eac9 scripts: use LIEF for ELF checks in symbol-check.py (fanquake) 610a8a8 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, lief-project/LIEF#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 ce69e18 laanwj: Code review ACK ce69e18 Tree-SHA512: 911ba693cd9777ad1fc1f66dff6c4d3630a907351215380cbde5b14a4bbf5cf7eebf52eafa7e86b27deabd2d93d1b403f34aabd356b5ceaab3cc6e9941a01dd4
I used Guix's values for the powerpc64(le) dynamic linkers, and the /lib-prefix seems to be a Guix-ism rather than standard. The standard path for the linker-loaders start with /lib64. I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file. For future reference, loader path values can also be found on glibc's website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16
It is important that binaries request a standard interpreter location
where most distros would place the linker-loader. Otherwise, the user
would be met with a very confusing message:
bash: <path>/<to>/bitcoind: No such file or directory
When really it's the interpreter that's not found.
…dd check_ELF_interpreter 1527b7e symbol-check: Check requested ELF interpreter (Carl Dong) b96adcb guix: Fix powerpc64(le) dynamic linker name (Carl Dong) Pull request description: Fixes bitcoin/bitcoin#23111 It would seem that I got the wrong default glibc-dynamic-linker path for PowerPC platforms. This means that for our currently released v22.0 binaries to be run on powerpc platforms, users would have to either: 1. Move `/lib64/ld64.so.?` to `/lib`, or 2. Invoke their linker-loader directly to start our binaries, e.g. `/lib64/ld64.so.? bitcoind` This is my bad. I've fixed the paths in this patchset, and also added a test to `symbol-check.py` so that this does not ever slip past our checks again. ACKs for top commit: laanwj: Code review ACK 1527b7e Tree-SHA512: bc520c35f72a9d4a3804b53d211138724560bd2405bf2f592ef755d19073e72f114fc4b8a3747e0c8724ac46a60b6ca86ea7766d66acb88eed1ebe2abc2678b8
Member
|
To merge this PR, I used the @MarcoFalke
Do I understand correctly, that |
Author
|
The tree sha is the hash of the contents of the repo (files) of the commit itself (HEAD) |
promag
pushed a commit
to promag/gui-qml
that referenced
this pull request
Oct 29, 2021
…during shutdown 451ca24 qt, refactor: Drop intermediate BitcoinApplication::shutdownResult slot (Hennadii Stepanov) f3a17bb qt: Do not exit and re-enter main event loop during shutdown (Hennadii Stepanov) b4e0d2c qt, refactor: Allocate SendConfirmationDialog instances on heap (Hennadii Stepanov) 332dea2 qt, refactor: Keep HelpMessageDialog in the main event loop (Hennadii Stepanov) c8bae37 qt, refactor: Keep PSBTOperationsDialog in the main event loop (Hennadii Stepanov) 7fa91e8 qt, refactor: Keep AskPassphraseDialog in the main event loop (Hennadii Stepanov) 6f6fde3 qt, refactor: Keep EditAddressDialog in the main event loop (Hennadii Stepanov) 59f7ba4 qt, refactor: Keep CoinControlDialog in the main event loop (Hennadii Stepanov) 7830cd0 qt, refactor: Keep OptionsDialog in the main event loop (Hennadii Stepanov) 13f6188 qt: Add GUIUtil::ShowModalDialogAndDeleteOnClose (Hennadii Stepanov) Pull request description: On master (1ef34ee) during shutdown `QApplication` exits the main event loop, then re-enter again. This PR streamlines shutdown process by removing the need to interrupt the main event loop, that is required for bitcoin-core#59. Also, blocking [`QDialog::exec()`](https://doc.qt.io/qt-5/qdialog.html#exec) calls are replaced with safer [`QDialog::show()`](https://doc.qt.io/qt-5/qwidget.html#show), except for `SendConfirmationDialog` as that change is not trivial (marked as TODO). The [`QDialog::open()`](https://doc.qt.io/qt-5/qdialog.html#open) was not used because the actual modality mode (application modal or window modal) of a dialog depends on whether it has a parent. This PR does not change behavior, and all touched dialogs are still application modal. As a follow up, a design research could suggest to make some dialogs window modal. NOTE for reviewers: quitting app while a dialog is open (e.g., via systray icon menu) must work fine. ACKs for top commit: laanwj: Code review and lighly tested ACK 451ca24 promag: ACK 451ca24, just changed signal to `quitRequested`. Tree-SHA512: ef01ab6ed803b202e776019a4e1f592e816f7bc786e00574b25a0bf16be2374ddf9db21f0a26da08700df7ef0ab9e879550df46dcfe3b6d940f5ed02ca5f8447
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Created with: