backport: trivial 2025 02 12

[PastaPastaPasta]

Issue being fixed or feature implemented

Batch of trivial back ports

What was done?

How Has This Been Tested?

Built locally; haven't reviewed commits yet.

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[coderabbitai]

Walkthrough

This pull request includes a range of updates across configuration, build scripts, tests, and core functionality. The continuous integration configuration has been modified to update container images and remove a default environment variable. Dependency and package management files have been refactored to change input definitions, remove certain packages, and adjust commit references. Several patches have been removed from both build and source code files to streamline processing and address compatibility issues with specific compilers and language versions. Changes in core modules involve updates to documentation, logging, and handling of network peer properties, as well as modifications to serialization and transaction operations. Testing frameworks have also been enhanced with additional assertions and revised wallet initialization processes, while new SQL tracing functionality was introduced to improve database logging.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a93a65 and c34e7ab.

📒 Files selected for processing (6)

• contrib/guix/manifest.scm (9 hunks)
• contrib/macdeploy/gen-sdk (0 hunks)
• depends/packages/qt.mk (0 hunks)
• depends/patches/qt/fast_fixed_dtoa_no_optimize.patch (0 hunks)
• src/leveldb/include/leveldb/status.h (2 hunks)
• src/psbt.h (1 hunks)

💤 Files with no reviewable changes (3)

• contrib/macdeploy/gen-sdk
• depends/packages/qt.mk
• depends/patches/qt/fast_fixed_dtoa_no_optimize.patch

🚧 Files skipped from review as they are similar to previous changes (3)

• src/leveldb/include/leveldb/status.h
• src/psbt.h
• contrib/guix/manifest.scm

⏰ Context from checks skipped due to timeout of 90000ms (1)

• GitHub Check: Build container / Build container

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

🔭 Outside diff range comments (1)

src/net_processing.cpp (1)

6299-6317: 🛠️ Refactor suggestion

Add safety check for expiry timer overflow

The expiry timer calculation should be protected against potential overflow.

-            state.m_object_download.m_check_expiry_timer = current_time + GetObjectExpiryInterval(MSG_TX)/2 + GetRandMicros(GetObjectExpiryInterval(MSG_TX));
+            // Ensure we don't overflow when calculating next check time
+            const auto max_delay = std::chrono::microseconds::max() - current_time;
+            const auto expiry_interval = GetObjectExpiryInterval(MSG_TX);
+            const auto random_delay = GetRandMicros(expiry_interval);
+            const auto total_delay = std::min(expiry_interval/2 + random_delay, max_delay);
+            state.m_object_download.m_check_expiry_timer = current_time + total_delay;

🧹 Nitpick comments (13)

src/psbt.h (1)

253-253: Consider using const references for unknown map iteration.

For consistency with the partial_sigs change and to prevent accidental modifications, consider using const references when iterating over the unknown map in the serialization methods:

-    for (auto& entry : unknown) {
+    for (const auto& entry : unknown) {

This change would be applicable in three locations:

1. Line 253 in PSBTInput::Serialize
2. Line 507 in PSBTOutput::Serialize
3. Line 665 in PartiallySignedTransaction::Serialize

Also applies to: 507-507, 665-665

test/functional/p2p_outbound_eviction.py (1)

87-87: Rename unused loop variable.

The loop control variable i is never used within the loop. For clarity and to comply with static analysis suggestions, rename it to _ or _i.

- for i in range(10):
+ for _ in range(10):

🧰 Tools

🪛 Ruff (0.8.2)

87-87: Loop control variable i not used within loop body

Rename unused i to _i

(B007)

src/net_processing.cpp (8)

549-552: Improve comment clarity for service flags handling

The comment explains the behavior of service flags handling during version handshake, but could be clearer about the distinction between inbound and non-inbound connections.

-            // Overwrites potentially existing services. In contrast to this,
-            // unvalidated services received via gossip relay in ADDR/ADDRV2
-            // messages are only ever added but cannot replace existing ones.
+            // For non-inbound connections, overwrites existing services in AddrMan.
+            // This differs from ADDR/ADDRV2 gossip messages where services can only
+            // be added but not replaced, to prevent service flag manipulation.

---

1428-1440: Improve comment clarity for chain work comparison

The comment could be clearer about the logic for resetting the timeout when a peer has sent a block with sufficient work.

-            // The outbound peer has sent us a block with at least as much work as our current tip, so reset the timeout if it was set
+            // Reset the timeout if:
+            // 1. A timeout was previously set (m_timeout != 0s)
+            // 2. The peer has sent us a block with work >= our current tip
+            // This ensures we don't disconnect peers that are actively helping us sync

---

1435-1440: Improve comment clarity for timeout setting conditions

The comment explaining when timeouts are set could be more precise.

-            // At this point we know that the outbound peer has either never sent us a block/header or they have, but its tip is behind ours
-            // AND
-            // we are noticing this for the first time (m_timeout is 0)
-            // OR we noticed this at some point within the last CHAIN_SYNC_TIMEOUT + HEADERS_RESPONSE_TIME seconds and set a timeout
-            // for them, they caught up to our tip at the time of setting the timer but not to our current one (we've also advanced).
-            // Either way, set a new timeout based on our current tip.
+            // Set a new timeout when:
+            // 1. First time seeing this peer (m_timeout is 0) OR
+            // 2. Previous timeout exists but peer's best known block:
+            //    - Has caught up to the work target we set previously
+            //    - But is still behind our current tip
+            // This ensures peers make consistent forward progress.

---

6207-6221: Improve stalling detection and timeout handling

The stalling detection logic could be made more robust by adding logging of the actual stall duration.

 if (state.m_stalling_since.count() && state.m_stalling_since < current_time - stalling_timeout) {
+    const auto stall_duration = current_time - state.m_stalling_since;
     LogPrintf("Peer=%d%s is stalling block download, disconnecting\n", pto->GetId(), 
-              fLogIPs ? strprintf(" peeraddr=%s", pto->addr.ToStringAddrPort()) : "");
+              fLogIPs ? strprintf(" peeraddr=%s", pto->addr.ToStringAddrPort()) : "",
+              " stall_duration=", count_seconds(stall_duration), "s");

---

6227-6235: Add timeout duration to disconnection message

The block download timeout message could include more diagnostic information.

-                LogPrintf("Timeout downloading block %s from peer=%d%s, disconnecting\n", queuedBlock.pindex->GetBlockHash().ToString(), pto->GetId(), fLogIPs ? strprintf(" peeraddr=%s", pto->addr.ToStringAddrPort()) : "");
+                const auto timeout = std::chrono::seconds{consensusParams.nPowTargetSpacing} * 
+                    (BLOCK_DOWNLOAD_TIMEOUT_BASE + BLOCK_DOWNLOAD_TIMEOUT_PER_PEER * nOtherPeersWithValidatedDownloads);
+                const auto elapsed = current_time - state.m_downloading_since;
+                LogPrintf("Timeout downloading block %s from peer=%d%s, elapsed=%ds timeout=%ds, disconnecting\n",
+                    queuedBlock.pindex->GetBlockHash().ToString(), pto->GetId(),
+                    fLogIPs ? strprintf(" peeraddr=%s", pto->addr.ToStringAddrPort()) : "",
+                    count_seconds(elapsed), count_seconds(timeout));

---

6319-6360: Improve object request handling and logging

The object request handling could benefit from more detailed logging and clearer error handling.

1. Add more detailed logging:

-            LogPrint(BCLog::NET, "%s -- GETDATA skipping inv=(%s), peer=%d\n", __func__, inv.ToString(), pto->GetId());
+            LogPrint(BCLog::NET, "%s -- GETDATA skipping erased request inv=(%s), peer=%d\n", __func__, inv.ToString(), pto->GetId());

2. Add error handling for invalid object types:

+            if (!inv.IsKnownType()) {
+                LogPrint(BCLog::NET, "%s -- GETDATA invalid type inv=(%s), peer=%d\n", __func__, inv.ToString(), pto->GetId());
+                state.m_object_download.m_object_announced.erase(inv);
+                continue;
+            }

3. Add timing information to request logs:

-            LogPrint(BCLog::NET, "Requesting %s peer=%d\n", inv.ToString(), pto->GetId());
+            LogPrint(BCLog::NET, "Requesting %s peer=%d last_request=%ds ago\n",
+                inv.ToString(), pto->GetId(),
+                count_seconds(current_time - last_request_time));

---

1047-1050: Consider using std::unordered_map for better performance

The g_already_asked_for and g_erased_object_requests maps could potentially benefit from using unordered_map for better lookup performance.

However, this change would require careful benchmarking to verify the performance improvement, as the actual benefit depends on the hash function quality and collision rates.

---

81-84: Consider making constants configurable

The peer object limits are currently hardcoded. Consider making them configurable via command line or configuration file.

-static constexpr int32_t MAX_PEER_OBJECT_IN_FLIGHT = 100;
-static constexpr int32_t MAX_PEER_OBJECT_ANNOUNCEMENTS = 2 * MAX_INV_SZ;
+static constexpr int32_t DEFAULT_MAX_PEER_OBJECT_IN_FLIGHT = 100;
+static constexpr int32_t DEFAULT_MAX_PEER_OBJECT_ANNOUNCEMENTS = 2 * MAX_INV_SZ;
+static int32_t MAX_PEER_OBJECT_IN_FLIGHT = DEFAULT_MAX_PEER_OBJECT_IN_FLIGHT;
+static int32_t MAX_PEER_OBJECT_ANNOUNCEMENTS = DEFAULT_MAX_PEER_OBJECT_ANNOUNCEMENTS;

Then add command line options:

gArgs.AddArg("-maxpeerobjectinflight=<n>", strprintf("Maximum number of in-flight objects from a peer (default: %u)", DEFAULT_MAX_PEER_OBJECT_IN_FLIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
gArgs.AddArg("-maxpeerobjectannouncements=<n>", strprintf("Maximum number of announced objects from a peer (default: %u)", DEFAULT_MAX_PEER_OBJECT_ANNOUNCEMENTS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

src/validation.cpp (2)

460-461: Enhance cache size handling for level 3 verifications.

The code now properly handles cases where the database cache size is insufficient for level 3 verifications by:

1. Checking cache size before attempting level 3 checks
2. Skipping the checks if cache size is insufficient
3. Setting a flag to track skipped verifications

This prevents potential memory issues while maintaining verification integrity.

Consider adding metrics to track how often L3 checks are skipped to help tune cache sizes appropriately.

Also applies to: 503-518

---

525-527: Consider adding more context to the cache size recommendation message.

The message could be more helpful by including the current cache size and recommended minimum size.

-        LogPrintf("Skipped verification of level >=3 (insufficient database cache size). Consider increasing -dbcache.\n");
+        LogPrintf("Skipped verification of level >=3 (current cache size: %zu bytes is insufficient). Consider increasing -dbcache to at least %zu bytes.\n", 
+                  curr_coins_usage, chainstate.m_coinstip_cache_size_bytes);

.cirrus.yml (1)

51-51: YAML Formatting: Reduce Extra Spaces in Merge Key

The static analysis tool flagged extra spaces before the colon on line 51. Consider tightening the syntax to improve readability and meet YAML linting expectations. For example:

-  << : *BASE_TEMPLATE
+  <<: *BASE_TEMPLATE

🧰 Tools

🪛 YAMLlint (1.35.1)

[warning] 51-51: too many spaces before colon

(colons)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d29b6c and c85063f.

📒 Files selected for processing (23)

• .cirrus.yml (1 hunks)
• ci/test/00_setup_env.sh (0 hunks)
• contrib/guix/libexec/build.sh (2 hunks)
• contrib/guix/manifest.scm (9 hunks)
• contrib/macdeploy/gen-sdk (0 hunks)
• depends/hosts/darwin.mk (1 hunks)
• depends/packages/qt.mk (2 hunks)
• depends/patches/qt/darwin_no_libm.patch (1 hunks)
• depends/patches/qt/fast_fixed_dtoa_no_optimize.patch (0 hunks)
• src/addrman.h (1 hunks)
• src/init/common.cpp (1 hunks)
• src/leveldb/include/leveldb/status.h (2 hunks)
• src/net_processing.cpp (2 hunks)
• src/psbt.h (1 hunks)
• src/qt/test/util.cpp (1 hunks)
• src/qt/test/util.h (1 hunks)
• src/test/addrman_tests.cpp (2 hunks)
• src/validation.cpp (3 hunks)
• src/wallet/scriptpubkeyman.cpp (0 hunks)
• src/wallet/sqlite.cpp (2 hunks)
• test/functional/p2p_outbound_eviction.py (1 hunks)
• test/functional/test_runner.py (1 hunks)
• test/functional/wallet_backup.py (1 hunks)

💤 Files with no reviewable changes (4)

• ci/test/00_setup_env.sh
• contrib/macdeploy/gen-sdk
• depends/patches/qt/fast_fixed_dtoa_no_optimize.patch
• src/wallet/scriptpubkeyman.cpp

✅ Files skipped from review due to trivial changes (2)

• src/qt/test/util.h
• src/qt/test/util.cpp

🧰 Additional context used

🪛 YAMLlint (1.35.1)

.cirrus.yml

[warning] 51-51: too many spaces before colon

(colons)

🪛 Ruff (0.8.2)

test/functional/p2p_outbound_eviction.py

87-87: Loop control variable i not used within loop body

Rename unused i to _i

(B007)

⏰ Context from checks skipped due to timeout of 90000ms (9)

• GitHub Check: win64-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: linux64_nowallet-build / Build source

🔇 Additional comments (31)

src/psbt.h (1)

195-195: LGTM! Good improvement in const-correctness.

The change to use const auto& instead of auto for iterating over partial_sigs is a good practice as it:

1. Prevents accidental modifications to the map elements during iteration
2. Makes the intent clearer that the iteration is for reading only
3. Can potentially improve performance by avoiding unnecessary copying

test/functional/p2p_outbound_eviction.py (11)

1-5: Header and license check.

The shebang and MIT license header conform to typical project standards.

---

6-15: Comprehensive docstring appreciated.

The multi-line docstring provides a clear explanation of the test's purpose and covers the core functionality.

---

16-25: Imports align with project structure.

These imports from the test framework keep the code self-contained and leverage existing utilities.

---

26-29: Timeout constants seem appropriately named.

CHAIN_SYNC_TIMEOUT and HEADERS_RESPONSE_TIME are descriptive; maintaining them as top-level constants improves readability.

---

31-34: Straightforward test parameter setup.

num_nodes = 1 is consistent with a single-node environment for easier control over outbound connections.

---

35-122: Robust logic for outbound eviction test (unprotected peers).

1. Mock time manipulations and sequential checks simulate realistic eviction conditions accurately.
2. The step-by-step approach to induce timeouts and verify evictions ensures thorough coverage of edge cases.
3. Return value checks after block generation verify the code properly handles chain synchronization.

🧰 Tools

🪛 Ruff (0.8.2)

87-87: Loop control variable i not used within loop body

Rename unused i to _i

(B007)

---

123-151: Solid coverage for protected peer eviction checks.

1. The logic accurately verifies that peers sending a valid tip block remain protected.
2. Timeouts test whether protected peers remain connected despite delayed chain updates.

---

152-214: Mixed scenario testing is well-structured.

1. Splitting peers into protected/unprotected and honest/misbehaving categories provides comprehensive coverage.
2. Logging distinguishes peer behaviors and confirms that only misbehaving unprotected peers are evicted.

---

215-242: Blocks-relay-only path tested thoroughly.

1. Testing block-relay peers ensures they do not receive the same protection as full-relay peers.
2. The final eviction check confirms correct handling of peer roles.

---

243-250: Sequence of test calls is logical.

run_test orchestrates the various scenarios in a consistent manner, ensuring coverage of all eviction paths before concluding.

---

252-253: Main entry point is straightforward.

This block properly integrates the test with the framework's runner.

src/net_processing.cpp (2)

6207-6221:

✅ Verification successful

Verify stalling detection edge cases

The stalling detection logic should be verified for potential edge cases.

---

🏁 Script executed:

#!/bin/bash
# Search for all stalling-related code to verify timeout handling
rg "m_stalling_since" -A 5 -B 5
rg "BLOCK_STALLING_TIMEOUT" -A 5 -B 5

Length of output: 7380

---

Stalling Detection Edge Cases Verified

The review request was to scrutinize the stalling detection logic for potential edge cases. After reviewing the code and searching for related logic (e.g., the use of m_stalling_since and BLOCK_STALLING_TIMEOUT), the verification confirms that:

• The condition to detect a stalled peer triggers only when m_stalling_since has been set (i.e., non-zero) and the elapsed time exceeds the dynamic timeout.
• The dynamic timeout adjustment (both increase on stalling and gradual decrease) is properly bounded by BLOCK_STALLING_TIMEOUT_MAX and defaults, respectively.
• The atomic update using compare_exchange_strong ensures that the timeout is updated safely and only when a new appropriate value is computed.

No additional edge cases seem to be unaddressed, and the current implementation appears robust.

---

1428-1433:

✅ Verification successful

Verify chain work comparison logic

The chain work comparison logic for resetting timeouts needs careful verification to ensure it correctly handles edge cases.

---

🏁 Script executed:

#!/bin/bash
# Search for all chain work comparisons to verify consistency
rg "nChainWork" -A 5 -B 5

Length of output: 40041

---

Verified: Chain Work Comparison Logic Appears Correct

I’ve reviewed the chain work comparisons across the codebase—including those used in resetting the timeout logic in src/net_processing.cpp—and found them to be consistent with other parts of the code. In particular, the use of comparison operators (e.g., using >= when comparing the best-known block’s work with the active chain tip’s work) correctly handles edge cases without any discrepancies.

No changes are required.

depends/patches/qt/darwin_no_libm.patch (1)

1-18:

❓ Verification inconclusive

LGTM! Build configuration simplification looks good.

The removal of explicit -lm linking is correct since:

1. GCC with libstdc++ automatically includes the math library
2. This resolves macOS cross-build issues

Let's verify that the Qt build still works correctly after this change:

---

🏁 Script executed:

#!/bin/bash
# Description: Verify Qt build configuration and dependencies
# Check if libm is still available through automatic linking
nm $(find . -name "*.a" -o -name "*.so" -o -name "*.dylib") 2>/dev/null | grep -w sin

Length of output: 75

---

Build Verification: Manual Check Recommended

LGTM! The build configuration simplification looks good, and removal of the explicit -lm link is appropriate since GCC with libstdc++ automatically includes the math library, which also resolves the macOS cross-build issues.

However, the automated verification script intended to check for the presence of math symbols (e.g., sin) produced no output. This result is inconclusive. Please manually verify that the expected math symbols are available in the built libraries to ensure that the automatic linking of libm is functioning as intended.

depends/hosts/darwin.mk (1)

53-60: LGTM! Compiler configuration is well-structured and documented.

The compiler definitions are clear, maintain all necessary flags, and are properly documented with explanatory comments above.

src/leveldb/include/leveldb/status.h (1)

107-117: LGTM! Proper handling of self-assignment with clear suppression of linter warnings.

The implementation correctly:

1. Handles self-assignment case
2. Uses NOLINT directives appropriately
3. Maintains exception safety

src/init/common.cpp (1)

66-66: LGTM! Improved clarity in logging configuration documentation.

The updated description better explains:

1. Which log levels are always enabled (error, warning, info)
2. How to configure category-specific levels
3. The relationship with -debug option and logging RPC

src/addrman.h (1)

124-133: LGTM! Documentation improvements enhance clarity.

The updated documentation for the Add method now clearly explains:

• The behavior for existing entries (may be updated with additional service flags)
• The handling of addresses in the new table (may be added to more buckets)
• The return value semantics (true for successful additions or bucket additions, unaffected by updates)

test/functional/wallet_backup.py (1)

228-231: LGTM! Test improvements enhance coverage.

The addition of empty wallet creation before testing importwallet() improves test coverage by:

• Ensuring a clean state before wallet import
• Testing the import functionality with empty wallets
• Clearly documenting the test's intent

depends/packages/qt.mk (1)

28-28:

✅ Verification successful

Verify the new patch file and its purpose.

The addition of darwin_no_libm.patch suggests changes to the math library linking on Darwin systems.

Please confirm:

1. The existence and contents of the patch file
2. The specific build issues it addresses on Darwin systems

---

🏁 Script executed:

#!/bin/bash
# Description: Check the contents of the new patch file

# Verify patch file exists and show its contents
cat depends/patches/qt/darwin_no_libm.patch

Length of output: 581

---

Patch Verified: Darwin Build Issue Confirmed

The patch file at depends/patches/qt/darwin_no_libm.patch exists and its contents confirm that it removes the explicit -lm link from the Qt tools. This change addresses the build issues observed during the macOS cross build by relying on GCC's default linking behavior with libstdc++, thus eliminating the need for a manual -lm specification.

• File verification: The patch file exists and includes a diff that removes the explicit -lm link from qtbase/src/corelib/tools/tools.pri.
• Build issue addressed: Removing -lm resolves cross building issues on Darwin systems where this explicit link is unnecessary and can cause conflicts.

src/wallet/sqlite.cpp (2)

39-52: LGTM! Well-implemented SQL tracing with security considerations.

The TraceSqlCallback implementation:

• Safely handles memory with proper cleanup of expanded SQL
• Conservatively logs only read-only statements to prevent leaking sensitive data
• Includes proper error handling

---

250-256: LGTM! Proper integration of SQL tracing.

The tracing setup:

• Is correctly guarded by appropriate log category and level checks
• Includes error handling for trace setup failures
• Uses appropriate trace flags

contrib/guix/manifest.scm (1)

84-88: LGTM! Consistent list formatting.

The changes standardize the list format across package definitions, making the code more consistent and maintainable.

Also applies to: 151-154, 408-411

test/functional/test_runner.py (1)

278-278: LGTM! Added new P2P outbound eviction test.

The addition of p2p_outbound_eviction.py to the base test suite ensures that peer eviction functionality is tested by default, which is important for maintaining network health.

src/test/addrman_tests.cpp (2)

1051-1051: LGTM! Updated test description.

The comment now accurately reflects that the test also verifies service flag behavior during address updates via Add().

---

1079-1103: LGTM! Comprehensive service flag behavior tests.

The new test cases thoroughly verify service flag behavior in various scenarios:

1. Gossip relay updates can only add services, not remove them
2. Direct connections can overwrite service flags
3. Service flags persist during tried table promotion
4. Service flags can be updated for addresses in tried table

This ensures robust handling of service flags throughout the address manager's lifecycle.

src/validation.cpp (2)

459-460: LGTM! Good addition of tracking variable for skipped verifications.

The new skipped_l3_checks variable helps track when level 3 verifications are skipped due to insufficient cache size, improving observability.

---

551-552: LGTM! Improved logging format.

The modified logging format provides clearer progress indication by using a consistent format for all verification progress messages.

.cirrus.yml (1)

50-63: Update Lint Task Configuration

The new lint task now targets "lint [bookworm]" using the debian:bookworm image. This update appears correctly aligned with a move away from Ubuntu-based images. Please verify that downstream linting scripts are compatible with Debian as the base OS.

🧰 Tools

🪛 YAMLlint (1.35.1)

[warning] 51-51: too many spaces before colon

(colons)

---

[warning] 57-57: too many spaces before colon

(colons)

contrib/guix/libexec/build.sh (1)

183-190: Clean Environment for Darwin After Qt Build

The newly added case block for *darwin* correctly unsets the environment variables C_INCLUDE_PATH, CPLUS_INCLUDE_PATH, and LIBRARY_PATH after building Qt. This helps prevent any unwanted side effects in subsequent build steps. The implementation is clear and follows best practices.

[UdjinM6]

29650: incomplete (failed to drop $(package)_patches += fast_fixed_dtoa_no_optimize.patch line)

[PastaPastaPasta]

that's so weird... Wonder why it didn't "conflict" / require manual intervention

[PastaPastaPasta]

resolved in https://github.com/dashpay/dash/compare/9a93a653c3131190dbb971659330a78689bc8e32..c34e7ab5345c714078e10483adba6a186b6a7f4a

[kwvg]

• bitcoin#30451 is already included in dash#6421, consider dropping it
• bitcoin#29230 assumes bitcoin#28318 is merged, which isn't the case as of develop (c72cbbf), consider dropping it

[PastaPastaPasta]

Dropped those two in https://github.com/dashpay/dash/compare/0fc6cdd3ce8883ec21631a5530e8b3ca2da1acaf..9a93a653c3131190dbb971659330a78689bc8e32

[UdjinM6]

utACK c34e7ab

[kwvg]

utACK c34e7ab