close
Skip to content

Initialize trusted block providers only once - #8062

Open
majesticsky wants to merge 1 commit into
bisq-network:masterfrom
majesticsky:fix-trusted-bsq-block-provider-lazy-init
Open

Initialize trusted block providers only once#8062
majesticsky wants to merge 1 commit into
bisq-network:masterfrom
majesticsky:fix-trusted-bsq-block-provider-lazy-init

Conversation

@majesticsky

@majesticsky majesticsky commented Sep 5, 2026

Copy link
Copy Markdown

Fixes #7973 — the review comment it points to:
#7966 (comment)

getTrustedBsqBlockProviders() used the emptiness of the provider set as its lazy initialization guard. An empty set is a legitimate outcome: regtest bundles no providers, the resource file can be absent, and the deny list may filter out every entry. In those cases, lazyInitialize() ran again on every call, re-reading the resource on a path that DaoBlockSignatureVerifier walks per block.

Populating the set in place also handed callers a live reference to it, and left it briefly empty between clear() and addAll(). Every caller reaches this singleton on the user thread today, so that window is not observable in practice, but nothing in the class enforces it.

Track initialization in an explicit flag, synchronize the accessor, and publish the providers as an immutable set, so the guard is atomic and callers cannot mutate shared state.

Two new tests cover this; both fail without the production change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved trusted block provider loading so initialization occurs reliably and only once, including when no providers are available.
    • Prevented accidental modification of the trusted provider list after it has been loaded.
    • Improved consistency when accessing trusted block provider information from multiple operations.

getTrustedBsqBlockProviders() used the emptiness of the provider set as
its lazy initialization guard. An empty set is a legitimate outcome:
regtest bundles no providers, the resource file can be absent, and the
deny list may filter out every entry. In those cases lazyInitialize()
ran again on every call, re-reading the resource on a path that
DaoBlockSignatureVerifier walks per block.

Populating the set in place also handed callers a live reference to it,
and left it briefly empty between clear() and addAll(). Every caller
reaches this singleton on the user thread today, so that window is not
observable in practice, but nothing in the class enforces it.

Track initialization in an explicit flag, synchronize the accessor and
publish the providers as an immutable set, so the guard is atomic and
callers cannot mutate shared state.

Addresses the review comment on PR bisq-network#7966.
@boring-cyborg

boring-cyborg Bot commented Sep 5, 2026

Copy link
Copy Markdown

Thanks for opening this pull request!

Please check out our contributor checklist and check if CI or Codacy found any issues with your PR. Also make sure your commits are signed, and that you applied Bisq's code style and formatting.

A maintainer will add an is:priority label to your PR if it is up for compensation. Please see our Bisq Q1 2020 Update post for more details.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 849622b4-ca71-42ad-a81a-cb1460c42c20

📥 Commits

Reviewing files that changed from the base of the PR and between 3875ed4 and dcf9916.

📒 Files selected for processing (2)
  • core/src/main/java/bisq/core/dao/node/block_provider/TrustedBsqBlockProviderRepository.java
  • core/src/test/java/bisq/core/dao/node/block_provider/TrustedBsqBlockProviderRepositoryTest.java

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

TrustedBsqBlockProviderRepository now uses an explicit initialization flag, synchronized lazy initialization, and immutable provider storage. Tests verify one-time initialization when no providers load and reject collection mutation.

Changes

Trusted BSQ provider initialization

Layer / File(s) Summary
Explicit initialization and immutable storage
core/src/main/java/bisq/core/dao/node/block_provider/TrustedBsqBlockProviderRepository.java
The repository tracks initialization with a boolean flag, synchronizes provider access, and stores loaded providers with Set.copyOf(...).
Initialization behavior tests
core/src/test/java/bisq/core/dao/node/block_provider/TrustedBsqBlockProviderRepositoryTest.java
Tests verify that empty initialization runs once and that the returned provider collection is unmodifiable.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to dcf99

Trusted provider initialization now runs once even when no providers are available, and callers receive an immutable provider collection. The covered behavior has no remaining actionable merge risk.

Suggested reviewers: henrikjannsen

Poem

A rabbit checks the provider set,
Then marks the setup done.
Immutable leaves stay in their place,
No second load can run.
The tests hop twice and find it true,
While clear commands are shunned.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue [#7973] by adding an explicit initialized flag and using it to prevent repeated initialization. The tests cover empty-provider initialization and immutable provider access.
Out of Scope Changes check ✅ Passed The synchronization, immutable set handling, and focused tests support the initialization fix and the stated pull request objectives. No unrelated changes are present.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: ensuring that trusted block providers initialize only once.
  • Fix all pre-merge checks with AI

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.

@majesticsky

Copy link
Copy Markdown
Author

CI is green on my fork for this commit — ubuntu-24.04, macos-15 and
windows-2025 on Java 21.0.6:
https://github.com/majesticsky/bisq/actions/runs/33975169843

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.

Use an explicit initialized flag in TrustedBsqBlockProviderRepository

1 participant