Repository navigation
Revert "Rebrand to darkcoin" - #191
Merged
Merged
Conversation
eduffield222
pushed a commit
that referenced
this pull request
Feb 19, 2015
Revert "Rebrand to darkcoin"
knst
pushed a commit
to knst/dash
that referenced
this pull request
Sep 26, 2024
ecbd911 qt: Handle peer addition/removal in a right way (Hennadii Stepanov) 1b66f6e qt: Drop PeerTablePriv class (Hennadii Stepanov) efb7e5a qt, refactor: Use default arguments for overridden functions (Hennadii Stepanov) Pull request description: This PR makes `PeerTableModel` handle a peer addition/removal in a right way. See: - https://doc.qt.io/qt-5/model-view-programming.html#inserting-and-removing-rows - https://doc.qt.io/qt-5/model-view-programming.html#resizable-models Fixes dashpay#160. Fixes dashpay#191. ACKs for top commit: jarolrod: re-ACK ecbd911 promag: reACK ecbd911 just improvements to the comment since last review. Tree-SHA512: 074935d67f78561724218e8b33822e2de16749f873c29054926b720ffcd642f08249a222b563983cf65a9b716290aa14e2372c47fc04e5f401f759db25ca710f
knst
pushed a commit
to knst/dash
that referenced
this pull request
Sep 26, 2024
ecbd911 qt: Handle peer addition/removal in a right way (Hennadii Stepanov) 1b66f6e qt: Drop PeerTablePriv class (Hennadii Stepanov) efb7e5a qt, refactor: Use default arguments for overridden functions (Hennadii Stepanov) Pull request description: This PR makes `PeerTableModel` handle a peer addition/removal in a right way. See: - https://doc.qt.io/qt-5/model-view-programming.html#inserting-and-removing-rows - https://doc.qt.io/qt-5/model-view-programming.html#resizable-models Fixes dashpay#160. Fixes dashpay#191. ACKs for top commit: jarolrod: re-ACK ecbd911 promag: reACK ecbd911 just improvements to the comment since last review. Tree-SHA512: 074935d67f78561724218e8b33822e2de16749f873c29054926b720ffcd642f08249a222b563983cf65a9b716290aa14e2372c47fc04e5f401f759db25ca710f
2 of 3 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
Sep 28, 2026
… not_null precondition 00aabf5 fix: report the real source location for GetCreditPool's not_null precondition (UdjinM6) Pull request description: ## Issue being fixed or feature implemented `CCreditPoolManager::GetCreditPool()` records the wrong source location for its `not_null` precondition, so a failure reports a libc++ internal header instead of the code that produced the null pointer. 966f532, from #7693, changed `evo/creditpool.cpp` to call `emplace()` instead of `push()` on a `std::stack<gsl::not_null<const CBlockIndex*>>`, to satisfy clang-tidy's `modernize-use-emplace`. `gsl::not_null`'s converting constructor takes `nostd::source_location loc = nostd::source_location::current()` (`src/gsl/pointers.h:102`), backed by `__builtin_FILE()`, and forwards it to `Expects()`. A default argument is evaluated at the point of the call: `push()` constructs the `not_null` at the call site in `creditpool.cpp`, while `emplace()` forwards the raw pointer and constructs it inside libc++'s `std::construct_at`. That location is not merely stored, it is printed. `gsl::details::terminate()` (`src/gsl/assert.cpp:26-35`) writes `loc.file_name()`, `line()`, `column()` and `function_name()` to stderr and to `debug.log` via `LogPrintf` before calling `std::terminate()`, in release builds as well as debug ones. So a null `pprev` reaching `GetCreditPool` currently logs ERROR: error detected null not_null detected at .../__memory/construct_at.h:41:...:construct_at instead of naming `creditpool.cpp` and `GetCreditPool`. `modernize-use-emplace` is a false positive whenever the element type's constructor captures the caller's location this way. This was found while investigating a guix hash divergence on v24.0.0-rc.1 (dashpay/guix.sigs#190 vs #191), where the six macOS outputs differed between builders while all fifteen Linux, Windows and source outputs matched. On darwin the libc++ headers resolve through `-isysroot$(OSX_SDK)` (`depends/hosts/darwin.mk:75-81`), so the captured path is inside the macOS SDK, and `contrib/guix/guix-build:452` shares the SDK into the container at the builder's own host path — `--share="$SDK_PATH"`, with no `=/target` remap, unlike `--share="$PWD"=/dash`. Worth being precise about why this has not bitten earlier releases: the default `SDK_PATH` is repo-relative — `depends/Makefile:34` (`SDK_PATH ?= $(BASEDIR)/SDKs`), `contrib/containers/guix/scripts/setup-sdk:11`, `contrib/containers/guix/scripts/guix-start:13` — so it resolves to `/dash/depends/SDKs/…` inside the container for every builder regardless of where their checkout lives. The embedded path was a constant, not a variable. It only becomes builder-specific when a builder overrides `SDK_PATH` to a location outside the checkout, which is what happened for rc.1. ## What was done? Reverted the `emplace()` to `push()`, with a `NOLINT(modernize-use-emplace)` and a comment recording why, so the tidy rewrite is not reapplied. `gsl::not_null` is a thin wrapper over a raw pointer, so both forms emit the same store; `emplace()` bought nothing here beyond silencing the check. Deliberately out of scope: `SDK_PATH` is the only container share whose value reaches the compiler command line and is not remapped to a fixed path. `SOURCES_PATH` and `BASE_CACHE` are used solely for source and package-cache storage (`depends/funcs.mk:59,74,208`, `depends/Makefile:268`), while `$PWD` and `DISTSRC_BASE` are remapped precisely because they do reach it. Normalising the SDK mount is a separate guix fix and belongs in its own PR rather than riding along here. ## How Has This Been Tested? No test was added. The behaviour is a property of the emitted binary, and the only way to observe the logged location is to make the precondition fail, which requires a null `pprev` — a state that would mean the block index is already corrupt. Verified by compiling a translation unit that includes the real `src/gsl/pointers.h` and mirrors `GetCreditPool`'s stack, then inspecting the object file for the captured location: | variant | location recorded in the object file | |---|---| | `emplace()` (current `develop`) | `<sdk>/usr/include/c++/v1/__memory/construct_at.h` | | `push()` (this PR) | the call site in `creditpool.cpp` | Also audited the tree for the same pattern elsewhere. `gsl::not_null` appears in 145 places outside `src/gsl`, all parameters or locals except two stored ones: the stack fixed here, and `UtilParameters::m_base_index` (`src/llmq/utils.h:36`). Every `UtilParameters` construction site is braced aggregate initialisation at the call site, so none forwards a raw pointer through a std header. `src/gsl` is also the only user of `nostd::source_location` in the tree. `test/lint/lint-whitespace.py` passes. Not built or run through `make check` locally; CI covers both. ## Breaking Changes None. No consensus, P2P, RPC, wallet or on-disk behaviour changes. The only difference is the file, line and function reported if the `not_null` precondition in `GetCreditPool` ever fires. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 00aabf5 Tree-SHA512: 8e070e5489b922c5520e35f494022174a9d1c1ecf157e65afd39ec8471d2acb1059954f86b32416728c8b67b2441493a8726285016806e083344b3e2da8a01c2
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
1 participant
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.
Reverts #187