Backport 14734, 15457, 16344, 16352, 16366#4532
Closed
PastaPastaPasta wants to merge 5 commits intodashpay:developfrom
Closed
Backport 14734, 15457, 16344, 16352, 16366#4532PastaPastaPasta wants to merge 5 commits intodashpay:developfrom
PastaPastaPasta wants to merge 5 commits intodashpay:developfrom
Conversation
0f459d8 fix an undefined behavior in uint::SetHex (Kaz Wesley) Pull request description: Decrementing psz beyond the beginning of the string is UB, even though the out-of-bounds pointer is never dereferenced. I don't think any clang sanitizer covers this, so I don't see any way a test could catch the original behavior. ACKs for top commit: promag: utACK 0f459d8. l2a5b1: utACK 0f459d8 Tree-SHA512: 388223254ea6e955f643d2ebdf74d15a3d494e9f0597d9f05987ebb708d7a1cc06ce64bd25d447d75b5f5561bdae9630dcf25adb7bd75f7a382298b95d127162 # Conflicts: # src/uint256.cpp
f874e14 [build]: check std::system for -[alert|block|wallet]notify (Sjors Provoost) cc3ad56 [build] MSVC: set HAVE_SYSTEM for desktop apps (Sjors Provoost) c1c91bb [build] detect std::system or ::wsystem (Sjors Provoost) Pull request description: Platforms such as iOs and Universal Windows Platform do not support launching a process through system(). ACKs for top commit: laanwj: code review ACK f874e14 Tree-SHA512: 16bb4a8fa1896046ccb22a46c8985e1aa45f5b11ecf5539eb2299e9a58f1a5b085c0c12cb6939c7493d93abce7e84fadcbfc73374c887db63da6d00c08aa476d # Conflicts: # build_msvc/bitcoin_config.h # src/init.cpp # src/wallet/init.cpp
…VE_SYSTEM) 976b034 [build]: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM) (Sjors Provoost) Pull request description: It seems that `AC_DEFINE([HAVE_SYSTEM], [HAVE_STD__SYSTEM || HAVE_WSYSTEM]` causes `HAVE_SYSTEM` to always be defined, so we need to use `#if HAVE_SYSTEM` instead of `#if defined(HAVE_SYSTEM)`. Followup for bitcoin#15457, can be tested with bitcoin#12557. ACKs for top commit: dongcarl: ACK bitcoin@976b034. promag: ACK 976b034. fanquake: ACK 976b034 Tree-SHA512: b8cdd04c2ec399fd15638aef5d75ea0886ec1572d3cf4fcea27c193e1e6390344315908262cad8981a9b0a905ab9520619ce2ffe9a717f4ee6bfa8b028ebbdc6 # Conflicts: # src/init.cpp
e8fabd9 build: prune dbus from depends (fanquake) Pull request description: Since bitcoin#8210 (59d063d), we've been passing `-dbus-runtime` when configuring Qt. ``` qtbase-opensource-src-5.9.7 $ ./configure -h | grep -i dbus -no-dbus ............. Do not build the Qt D-Bus module -dbus-linked ......... Build Qt D-Bus and link to libdbus-1 [auto] -dbus-runtime ........ Build Qt D-Bus and dynamically load libdbus-1 [no] ``` This means we don't actually seem to be using the `D-Bus` we build in depends. This was pointed out by theuni at the time, [here](bitcoin#7993 (comment)) and [here](bitcoin#8210 (comment)), but was never followed up. dongcarl also bought it up as part of bitcoin#16150. I've tested building and running `bitcoin-qt` using depends on Debian. Needs further testing. ACKs for top commit: laanwj: code review ACK e8fabd9 Tree-SHA512: 164e6e52b6f97c04aef42bd185e2a157bc1a42103840f9404c5a795749f45a8c2c35f35873395a3a56398b3cd5955496b90d9c885d929b434c9bc871695abe20 # Conflicts: # depends/packages/dbus.mk # depends/packages/packages.mk # doc/dependencies.md
fa6f402 Call node->initError instead of InitError from GUI code (Russell Yanofsky) fad2502 init: Use InitError for all errors in bitcoind/qt (MarcoFalke) Pull request description: Using the same InitError for startup error in the daemon and the gui makes it possible to run the tests with the gui again: ```sh BITCOIND=bitcoin-qt ./test/functional/test_runner.py feature_includeconf feature_config_args ACKs for top commit: hebasto: ACK fa6f402 ryanofsky: utACK fa6f402. Only changes since last review are removing more includes and adding Node::initError method to avoid accessing node `InitError` function and global variables from GUI code. Tree-SHA512: bd19e08dcea4019dfe40356bc5c63cb583cefed54b6c9dcfb82f1b5b00308d8e2b363549afcaea5e93bf83864dbe0917400c3b70f43a8a5bdff45c9cd34cc294
UdjinM6
requested changes
Oct 22, 2021
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
should apply 15457 and 16344 to -instantsendnotify too
| if (!gArgs.ReadConfigFiles(error, true)) { | ||
| fprintf(stderr, "Error reading configuration file: %s\n", error.c_str()); | ||
| return false; | ||
| return InitError(strprintf("Error reading configuration file: %s\n", error)); |
There was a problem hiding this comment.
16366: should apply it below too, line 114
| #pragma GCC diagnostic ignored "-Wdeprecated-declarations" | ||
| #endif | ||
| fprintf(stdout, "Dash Core server starting\n"); | ||
| tfm::format(std::cout, PACKAGE_NAME "daemon starting\n"); |
| SetupUIArgs(); | ||
| std::string error; | ||
| if (!node->parseParameters(argc, argv, error)) { | ||
| node->initError(strprintf("Error parsing command line arguments: %s\n", error)); |
There was a problem hiding this comment.
16366: should probably apply it near QMessageBox::critical for printcrashinfo, various font and css settings below too
|
This pull request has conflicts, please rebase. |
|
This Pull Request may conflict if the Pull Requests below are merged first. #4649 |
This was referenced Dec 30, 2021
|
This Pull Request may conflict if the Pull Requests below are merged first. #4650 |
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
4 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.
No description provided.