Skip to content

fix: avoid static initialization of payment_address in tests#211

Merged
fpelliccioni merged 1 commit intomasterfrom
fix/test-static-init
Mar 26, 2026
Merged

fix: avoid static initialization of payment_address in tests#211
fpelliccioni merged 1 commit intomasterfrom
fix/test-static-init

Conversation

@fpelliccioni
Copy link
Copy Markdown
Contributor

@fpelliccioni fpelliccioni commented Mar 26, 2026

Summary

  • Convert test_address from a namespace-scope static constant to a function returning a local static reference, avoiding static initialization order issues with payment_address constructor

Problem

kth_domain_test segfaults during Catch2 test discovery on GCC 15 with sanitizers. The payment_address constructor parses a cashaddr string at static initialization time, which may depend on other statics not yet initialized.

Test plan

  • Verify GCC 15 sanitizers CI job passes
  • All coin_selection tests pass locally

Closes #210


Note

Low Risk
Low risk: test-only change that delays payment_address construction to runtime to avoid static initialization order crashes; no production wallet/coin-selection logic is modified.

Overview
Updates coin_selection.cpp tests to replace the namespace-scope payment_address constant with a test_address() accessor that returns a function-local static.

All test call sites are updated to use test_address() so the cashaddr parsing in payment_address happens on first use, preventing static initialization order issues during Catch2 test discovery.

Written by Cursor Bugbot for commit 1906b22. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Tests
    • Improved coin-selection tests by changing how test addresses are initialized, reducing flaky setup and making test execution more reliable.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f03c80f-a27b-4976-9bea-bc26452d9c62

📥 Commits

Reviewing files that changed from the base of the PR and between ab31d67 and 1906b22.

📒 Files selected for processing (1)
  • src/domain/test/wallet/coin_selection.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/domain/test/wallet/coin_selection.cpp

📝 Walkthrough

Walkthrough

Replaced a namespace-scope payment_address test constant with a function payment_address const& test_address() that returns a local static instance, deferring construction until first use. Updated all test call sites to call test_address() instead of using the previous global.

Changes

Cohort / File(s) Summary
Static init fix — coin selection tests
src/domain/test/wallet/coin_selection.cpp
Replaced anonymous-namespace payment_address const test_address{...} with payment_address const& test_address() returning a local static to avoid static-init ordering issues. Updated test call sites to use test_address() for destination and change address arguments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I waited in silence, no panic or mess,
A static no longer planted to stress.
Now called when needed, I stretch and I play,
Tests wake me gently and hop on their way. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting a static namespace-scope payment_address to a function returning a local static to avoid static initialization order issues.
Linked Issues check ✅ Passed The PR implements the exact fix specified in issue #210: replacing the namespace-scope static constant with a function returning a local static, eliminating the static initialization order problem.
Out of Scope Changes check ✅ Passed All changes are confined to the test helper function in coin_selection.cpp, directly addressing the static initialization issue identified in issue #210 with no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/test-static-init

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.

@fpelliccioni fpelliccioni force-pushed the fix/test-static-init branch from ab31d67 to 1906b22 Compare March 26, 2026 18:33
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/domain/test/wallet/coin_selection.cpp`:
- Line 985: The vector initializations use the function symbol test_address
instead of calling it, causing a type mismatch for std::vector<payment_address>;
replace bare test_address with calls to the function (test_address()) in the
change_addrs initializer at the occurrences (e.g., change_addrs =
{test_address(), test_address(), test_address()}), and do the same for the other
occurrence around line 1142 so each element is a payment_address instance rather
than a function pointer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 99ec2249-a144-4f2a-9204-6aada289f7c2

📥 Commits

Reviewing files that changed from the base of the PR and between 2e3f6fa and ab31d67.

📒 Files selected for processing (1)
  • src/domain/test/wallet/coin_selection.cpp

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.61%. Comparing base (2e3f6fa) to head (1906b22).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #211       +/-   ##
===========================================
+ Coverage    0.26%   47.61%   +47.35%     
===========================================
  Files         326      326               
  Lines       16066    16066               
  Branches     5826     5826               
===========================================
+ Hits           42     7650     +7608     
+ Misses      15999     6242     -9757     
- Partials       25     2174     +2149     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fpelliccioni fpelliccioni merged commit 561b3fb into master Mar 26, 2026
27 checks passed
@fpelliccioni fpelliccioni deleted the fix/test-static-init branch March 26, 2026 22:22
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.

Segfault in test discovery with GCC 15 sanitizers (static initialization)

1 participant