Skip to content

Conversation

@aleflm
Copy link
Contributor

@aleflm aleflm commented Nov 28, 2025

PR intention

Needed for bip155 port.

Depends on: #1691

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Walkthrough

The PR raises the C++ standard to C++20 across build files and dependency packages, updates Boost to 1.83.0 (with hash), adjusts allocator public typedefs for C++20 compatibility, and fixes a genesis callback disconnect call in initialization.

Changes

Cohort / File(s) Summary
Top-level CMake
CMakeLists.txt, src/secp256k1/CMakeLists.txt
Bumped CMAKE_CXX_STANDARD from 17 to 20
Dependency build flags & versions
depends/packages/bdb.mk, depends/packages/boost.mk, depends/packages/qt.mk, depends/packages/zeromq.mk
Changed package C++ flags to C++20; boost.mk also bumps Boost from 1.81.01.83.0 and updates SHA256
Allocator typedefs
src/support/allocators/mt_pooled_secure.h, src/support/allocators/pooled_secure.h
Replaced inherited allocator typedefs (pointer, const_pointer, reference, const_reference) with explicit T*, const T*, T&, const T& aliases
Initialization fix
src/init.cpp
Use &BlockNotifyGenesisWait when disconnecting the genesis wait callback to match the connected callback form

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to Boost ABI/compatibility implications after version bump (depends/packages/boost.mk).
  • Verify allocator typedef changes don't break containers or allocator-aware code expecting std::allocator traits.
  • Confirm all build scripts and package recipes consistently use C++20 flags.

Poem

🐰 C++ twenty hops into sight,

Boost refreshed and typedefs made right,
Callbacks fixed with a tidy ampersand,
Builds leap forward across the land —
A rabbit’s cheer for the updated flight 🌿✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The PR description is incomplete. While it provides minimal intent ('Needed for bip155 port'), it lacks the mandatory 'PR intention' section detail and omits the optional 'Code changes brief' section explaining the architectural rationale. Expand the description to explain why C++20 is needed for bip155 support and document any architectural implications of the upgrade.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'enable C++20 support' accurately and concisely summarizes the main change across all modified files: upgrading C++ standard from 17 to 20 in CMake and build configurations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 d795531 and e088c85.

📒 Files selected for processing (9)
  • CMakeLists.txt (1 hunks)
  • depends/packages/bdb.mk (1 hunks)
  • depends/packages/boost.mk (2 hunks)
  • depends/packages/qt.mk (1 hunks)
  • depends/packages/zeromq.mk (1 hunks)
  • src/init.cpp (1 hunks)
  • src/secp256k1/CMakeLists.txt (1 hunks)
  • src/support/allocators/mt_pooled_secure.h (1 hunks)
  • src/support/allocators/pooled_secure.h (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • CMakeLists.txt
  • src/secp256k1/CMakeLists.txt
  • src/init.cpp
  • depends/packages/qt.mk
  • depends/packages/zeromq.mk
  • src/support/allocators/pooled_secure.h
🔇 Additional comments (4)
src/support/allocators/mt_pooled_secure.h (1)

24-27: LGTM! Correct C++20 allocator compatibility fix.

These explicit typedef definitions are necessary because std::allocator removed pointer, const_pointer, reference, and const_reference in C++20 (they were deprecated in C++17 per P0174R2). Custom allocators that inherit from std::allocator must now explicitly define these types if downstream code depends on them.

depends/packages/boost.mk (2)

24-24: Boost 1.83.0 officially supports C++20 compilation. Boost's 1.83.0 release (August 2023) is usable with C++20, with many libraries adopting or interoperating with C++20 facilities. Tested C++20-capable compilers include GCC 8+, Clang 11+, and recent MSVC versions.


2-2: Verify Boost 1.83.0 hash is correct.

Boost 1.83.0 is a legitimate release (August 2023) with no reported security vulnerabilities. However, independently verify the SHA256 hash 6478edfe2f3305127cffe8caf73ea0176c53769f4bf1585be237eb30798c3b8e against the official Boost release archives to ensure correctness before merging.

depends/packages/bdb.mk (1)

12-12: BDB 4.8 C++ bindings cannot compile with -std=c++20 without compatibility patches

Berkeley DB 4.8's C++ API (db_cxx.h) uses std::auto_ptr and other deprecated C++ constructs that were removed in C++17. Building with -std=c++20 requires either:

  • Keeping BDB compiled with an older standard (e.g., -std=c++11 or -std=gnu++14), or
  • Patching the BDB sources to replace std::auto_ptr with std::unique_ptr and remove other deprecated constructs.

Confirm that any required compatibility patches have been applied to the BDB sources in depends/patches/ and that builds pass on all supported toolchains (Linux/Windows, gcc/clang/MSVC).


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aleflm aleflm force-pushed the dev/aleflm/cpp20-support branch 2 times, most recently from 3d32eea to d795531 Compare December 3, 2025 10:42
@aleflm aleflm marked this pull request as ready for review December 3, 2025 10:47
@coderabbitai coderabbitai bot requested a review from firstcryptoman December 3, 2025 10:48
Update Boost 1.83.0 since 1.81.0 does not work with cpp20.
@aleflm aleflm force-pushed the dev/aleflm/cpp20-support branch from d795531 to e088c85 Compare December 8, 2025 10:19
@levonpetrosyan93 levonpetrosyan93 merged commit 8a821fc into firoorg:master Dec 15, 2025
11 checks passed
aleflm added a commit to aleflm/firo that referenced this pull request Dec 15, 2025
Update Boost 1.83.0 since 1.81.0 does not work with cpp20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants