Skip to content

Conversation

@alvasw
Copy link
Contributor

@alvasw alvasw commented Dec 1, 2025

Summary by CodeRabbit

  • Chores

    • Systematically addressed deprecation warnings throughout the codebase with appropriate annotations.
    • Updated Gradle build system to version 8.9 and refreshed dependency verification metadata.
    • Enhanced build infrastructure with updated verification schema and component tracking.
  • Refactor

    • Improved resource management using try-with-resources patterns.
    • Removed deprecated internal preferences field to streamline data structures.

✏️ Tip: You can customize this high-level summary in your review settings.

alvasw added 21 commits December 1, 2025 18:36
The recommendation is to switch to the UniformRandomProvider. This would
however add a new dependency that's only used in the unit tests.

[1] https://issues.apache.org/jira/browse/LANG-1604
…tAsLong

The buyerSecurityDepositAsLong field was superseded by the
buyerSecurityDepositAsPercent field.
It's too risky to refactor the code because it's used in the account age verification logic.
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

This PR adds @SuppressWarnings annotations across 60+ files to suppress deprecation and unchecked compiler warnings, particularly in proto-related methods. It removes the deprecated buyerSecurityDepositAsLong field from PreferencesPayload, refactors FileOutputStream resource management in DownloadTask, updates Gradle wrapper to version 8.9, and expands proto verification metadata.

Changes

Cohort / File(s) Summary
Payment Payload Classes
core/src/main/java/bisq/core/payment/payload/{AchTransferAccountPayload, AdvancedCashAccountPayload, AliPayAccountPayload, AmazonGiftCardAccountPayload, AustraliaPayidAccountPayload, BizumAccountPayload, CapitualAccountPayload, CashByMailAccountPayload, CashDepositAccountPayload, CelPayAccountPayload, ClearXchangeAccountPayload, CryptoCurrencyAccountPayload, DomesticWireTransferAccountPayload, F2FAccountPayload, FasterPaymentsAccountPayload, HalCashAccountPayload, ImpsAccountPayload, InstantCryptoCurrencyPayload, InteracETransferAccountPayload, JapanBankAccountPayload, MercadoPagoAccountPayload, MoneseAccountPayload, MoneyBeamAccountPayload, MoneyGramAccountPayload, NationalBankAccountPayload, NeftAccountPayload, NequiAccountPayload, PaxumAccountPayload, PayseraAccountPayload, PaytmAccountPayload, PerfectMoneyAccountPayload, PixAccountPayload, PopmoneyAccountPayload, PromptPayAccountPayload, RevolutAccountPayload, RtgsAccountPayload, SameBankAccountPayload, SatispayAccountPayload, SbpAccountPayload, SepaAccountPayload, SepaInstantAccountPayload, SpecificBanksAccountPayload, StrikeAccountPayload, SwiftAccountPayload, SwishAccountPayload, TikkieAccountPayload, TransferwiseAccountPayload, TransferwiseUsdAccountPayload, USPostalMoneyOrderAccountPayload, UpholdAccountPayload, UpiAccountPayload, VerseAccountPayload, WeChatPayAccountPayload, WesternUnionAccountPayload}.java
Added @SuppressWarnings("deprecation") to fromProto() static methods to suppress deprecation warnings from proto deserialization; some also added to toProtoMessage() methods.
Core Proto & Offer Classes
core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java, core/src/main/java/bisq/core/offer/OpenOfferManager.java, core/src/main/java/bisq/core/proto/CoreProtoResolver.java
Added @SuppressWarnings("deprecation") to toProtoMessage(), fromProto(), and maybeUpdatePersistedOffers() methods.
Payment Account Payload Base
core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java
Wrapped protobuf builder creation in getPaymentAccountPayloadBuilder() with @SuppressWarnings("deprecation") annotation.
Trade Statistics
core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java, core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java
Added @SuppressWarnings("deprecation") to proto-conversion and statistics republishing methods.
Desktop UI Classes
desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java, desktop/src/main/java/bisq/desktop/components/MenuItem.java, desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java, desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java, desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java, desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java, desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java, desktop/src/test/java/bisq/desktop/ComponentsDemo.java
Added @SuppressWarnings("deprecation") and/or @SuppressWarnings("unchecked") to various methods and constructors.
Test Classes
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java, p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java, p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java, p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java
Added @SuppressWarnings("deprecation") and @SuppressWarnings("unchecked") annotations to test methods and constructors.
Resource Management Refactoring
desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java
Refactored copyInputStreamToFileNew() to use try-with-resources for FileOutputStream, removing explicit close in finally block.
Field Removal
core/src/main/java/bisq/core/user/PreferencesPayload.java
Removed deprecated buyerSecurityDepositAsLong field, updated constructor signature, removed serialization/deserialization calls.
Proto Schema
proto/src/main/proto/pb.proto
Marked field index 30 as reserved in PreferencesPayload message after removing deprecated buyer_security_deposit_as_long field.
Build Configuration
gradle/wrapper/gradle-wrapper.properties, gradle/verification-metadata.xml
Updated Gradle wrapper from 7.6.3 to 8.9; updated verification metadata schema from 1.1 to 1.3 and added new dependency entries for Gradle plugins, Kotlin, and JaCoCo tooling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Repetitive annotation additions across 50+ payment payload classes require verification that each annotation is properly placed but follow a consistent pattern
  • PreferencesPayload field removal requires checking that serialization/deserialization logic is correctly updated and no call sites are broken
  • DownloadTask resource management refactoring needs careful review to ensure try-with-resources semantics are correct and no resource leaks exist
  • Gradle/verification metadata updates warrant verification of version compatibility and schema alignment
  • Most changes are low-risk (annotations) but broad scope necessitates spot-checking multiple files

Possibly related PRs

Suggested labels

refactor, cleanup, deprecation-warnings

Suggested reviewers

  • alejandrogarcia83

Poem

🐰 Warnings suppressed with a hop and a bound,
Deprecated fields tidied without a sound,
Resource management flows clean and bright,
Proto warnings silenced left and right!
Build scripts updated, gradle jumps ahead,
Clean code emerging from the clutter, thread by thread!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is empty despite a clear template being available that requires meaningful description and issue references. Add a descriptive PR description including what was changed (Gradle upgrade), why (reasons for upgrade), any breaking changes or compatibility notes, and reference related issues if applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 3.13% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'Update to Gradle 8.9' clearly describes the primary change in the changeset, which is upgrading Gradle from version 7.6.3 to 8.9.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (1)

88-100: Deprecation suppression requires a migration plan—deprecated proto APIs are not stable long-term.

The @SuppressWarnings("deprecation") suppresses warnings but does not address the underlying risk. Per protobuf's deprecation policy, non-experimental deprecated Java APIs are commonly removed ~2 years after deprecation and may be removed in major or minor releases. Simply suppressing the warnings without a documented migration plan or timeline creates technical debt and risks build failures in future protobuf upgrades.

Either establish a tracking issue with a migration timeline to replace deprecated proto method calls, or document why the current approach is acceptable (e.g., if protoc/runtime versions are pinned indefinitely).

🧹 Nitpick comments (11)
desktop/src/test/java/bisq/desktop/ComponentsDemo.java (1)

52-123: Consider fixing the root cause instead of suppressing the warning.

The unchecked warning originates from the raw type usage of ObservableList on line 121. Rather than suppressing the warning, parameterize the list to eliminate it at the source.

Apply this diff to fix the root cause:

-        ObservableList list = FXCollections.observableArrayList();
+        ObservableList<String> list = FXCollections.observableArrayList();

Then remove the suppression annotation:

-    @SuppressWarnings("unchecked")
     @Override
     public void start(Stage primaryStage) throws Exception {
core/src/main/java/bisq/core/offer/OpenOfferManager.java (1)

959-960: Appropriate suppression for migration code.

The method-level scope is correct since maybeUpdatePersistedOffers() intentionally accesses deprecated OfferPayload fields (e.g., getArbitratorNodeAddresses(), isCurrencyForMakerFeeBtc()) to migrate legacy persisted offers.

Consider adding a brief comment documenting which deprecated APIs trigger this warning, making future cleanup easier:

+    // Suppress deprecation: accesses legacy OfferPayload fields for migration (e.g., arbitratorNodeAddresses, isCurrencyForMakerFeeBtc)
     @SuppressWarnings("deprecation")
     private void maybeUpdatePersistedOffers() {
desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java (1)

93-104: Consider using ArrayList to avoid the unchecked cast.

The @SuppressWarnings("unchecked") annotation is appropriate for this Gradle 8.9 upgrade, and the cast at line 104 is safe since buffer only contains T elements. However, you can eliminate the warning entirely by using:

action.accept(new ArrayList<>(buffer).stream());

This approach creates the same snapshot behavior without requiring a cast or suppression annotation.

desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java (1)

126-141: LGTM - Both suppressions are justified.

The @SuppressWarnings({"rawtypes", "unchecked"}) annotations are appropriate here:

  • rawtypes: The ViewLoader.load() API returns a raw View type
  • unchecked: The cast to Tuple2 on line 134 is necessarily unchecked since data is Object

Minor improvement: You could use a wildcard type for the instanceof check to be slightly more explicit about the raw type usage:

         if (view instanceof BsqSendView) {
             toggleGroup.selectToggle(send);
-            if (data instanceof Tuple2) {
-                ((BsqSendView) view).fillFromTradeData((Tuple2) data);
+            if (data instanceof Tuple2<?, ?>) {
+                ((BsqSendView) view).fillFromTradeData((Tuple2<?, ?>) data);
             }

This would still require unchecked but would eliminate the rawtypes warning.

desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java (1)

189-242: Local deprecation suppression for clonedOfferPayload is acceptable, but consider future migration

The suppression is tightly scoped to the deprecated OfferPayload constructor usage and keeps the rest of the method warning-free, which is fine for this Gradle upgrade pass. Longer term, consider refactoring to whatever non-deprecated factory/builder replaces this constructor to gradually reduce reliance on deprecated APIs.

core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java (1)

114-133: Targeted suppression keeps this deserializer quiet under Gradle 8.9

The new @SuppressWarnings("deprecation") on fromProto is appropriately scoped and preserves existing behavior constructing CashDepositAccountPayload.

If you touch this area again, you could consider tightening the return type to CashDepositAccountPayload for symmetry with other payloads, but that’s not required for this Gradle-focused change.

core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (1)

95-104: Method-level deprecation suppression is consistent and low-risk

fromProto is now the only place in this class with deprecation suppressed, which is appropriate given it directly reads deprecated proto fields.

Longer term, once the proto APIs are fully migrated, this annotation should be easy to remove since it’s confined to a single factory method.

core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (1)

102-121: Deprecation suppression on fromProto matches the project-wide pattern

This keeps the deprecation handling localized to the deserialization path and avoids affecting compile-time checks elsewhere in the class.

If more methods in this class start relying on deprecated proto APIs, you might later switch to a class-level suppression to avoid repetition, but the current method-level scope is preferable for now.

p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java (1)

47-51: Unchecked warning suppression in test fake is acceptable (minor nit)

Using @SuppressWarnings("unchecked") on the constructor is fine for this in-memory test helper. If you ever want to avoid method-level suppression, you could instead give PersistenceManager an explicit generic type in the mock to satisfy the compiler, but that’s purely cosmetic here.

p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (1)

32-35: Unchecked suppression is fine for this test fake; generics could be a later cleanup

Adding @SuppressWarnings("unchecked") on the constructor is a pragmatic way to silence the raw‑type/generics warning for addService(new MapStoreServiceFake()) in test code. If you want to tighten things later, you could parameterize AppendOnlyDataStoreServiceFake and MapStoreServiceFake with concrete type arguments instead of relying on a raw base type, but that’s not required for this PR.

core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java (1)

196-244: Reasonable deprecation suppression on legacy republish path

maybeRepublishTradeStatistics still needs to call deprecated TradeStatistics2/3.from(...) factories for backward compatibility, so suppressing deprecation warnings at the method level is a sensible, contained choice. No functional changes here; any follow‑up to remove the deprecated types can happen independently of this Gradle upgrade.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7598e4f and 4c90099.

⛔ Files ignored due to path filters (1)
  • gradle/wrapper/gradle-wrapper.jar is excluded by !**/*.jar
📒 Files selected for processing (77)
  • apitest/src/test/java/bisq/apitest/scenario/TradeTest.java (2 hunks)
  • core/src/main/java/bisq/core/offer/OpenOfferManager.java (1 hunks)
  • core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2 hunks)
  • core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (2 hunks)
  • core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (2 hunks)
  • core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java (1 hunks)
  • core/src/main/java/bisq/core/proto/CoreProtoResolver.java (2 hunks)
  • core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java (3 hunks)
  • core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java (1 hunks)
  • core/src/main/java/bisq/core/user/PreferencesPayload.java (0 hunks)
  • desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/components/MenuItem.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java (1 hunks)
  • desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java (1 hunks)
  • desktop/src/test/java/bisq/desktop/ComponentsDemo.java (1 hunks)
  • gradle/verification-metadata.xml (38 hunks)
  • gradle/wrapper/gradle-wrapper.properties (1 hunks)
  • p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java (1 hunks)
  • p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (1 hunks)
  • p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java (1 hunks)
  • proto/src/main/proto/pb.proto (1 hunks)
💤 Files with no reviewable changes (1)
  • core/src/main/java/bisq/core/user/PreferencesPayload.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Test Java 11, windows-latest
  • GitHub Check: Test Java 11, macOS-latest
  • GitHub Check: Test Java 11, ubuntu-latest
🔇 Additional comments (60)
gradle/wrapper/gradle-wrapper.properties (1)

3-3: Verify gradle-wrapper.jar was also updated.

Gradle 8.9 is a stable release that improves error and warning reporting for variant issues, exposes structural details of Java compilation errors for IDE integrators, and includes additional minor improvements. The wrapper URL update is correctly formatted.

However, the Gradle wrapper JAR file (gradle/wrapper/gradle-wrapper.jar) typically needs to be updated alongside the wrapper properties file. Verify that this file was regenerated or updated to match the 8.9 distribution. You can regenerate it by running:

./gradlew wrapper --gradle-version 8.9

Additionally, given the significant scope mentioned in the PR summary (60+ files with changes across proto, annotations, and resource management), confirm that:

  • All build plugins and tools are compatible with Gradle 8.9
  • CI/CD pipelines are configured to support Gradle 8.9
  • Gradle 8.9 provisioning uses a different marker file (.ready), so all existing toolchains will be re-provisioned on first use
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java (2)

81-82: LGTM - appropriate deprecation suppression for test instantiation.

The variable-level @SuppressWarnings("deprecation") annotation correctly suppresses warnings for the test object instantiation. Note that this test is currently disabled.


104-105: LGTM - consistent with other test suppressions.

The annotation pattern matches the previous test method and correctly suppresses deprecation warnings. This test is also currently disabled.

core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (1)

73-80: LGTM - consistent deprecation suppression.

The annotation follows the same pattern applied across payment payload classes for proto conversion methods.

core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java (1)

80-92: LGTM - deprecation suppression aligned with PR strategy.

The annotation is correctly applied to the proto conversion method, consistent with the broader Gradle 8.9 upgrade changes.

core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (1)

74-81: LGTM - standard proto deprecation suppression.

The annotation correctly suppresses deprecation warnings for the proto conversion logic.

core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1)

70-77: LGTM - consistent annotation pattern.

The deprecation suppression is appropriately applied to the proto conversion method.

core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java (1)

67-74: LGTM - deprecation handling complete.

The annotation follows the established pattern for proto conversion methods across all payment payload classes.

p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java (1)

67-85: LGTM - test-level deprecation suppression with noted TODO.

The annotation appropriately suppresses deprecation warnings for this protobuf conversion test. Note the commented TODO on line 79 regarding the use of proto resolvers, which may be related to the deprecation concerns.

core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (1)

80-92: LGTM - Appropriately scoped deprecation suppression.

The @SuppressWarnings("deprecation") annotation is correctly placed on the fromProto method to suppress warnings from the protobuf-generated API. The annotation is narrowly scoped to only this method rather than the entire class.

desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java (1)

242-244: Acceptable, though the scope is broad.

The @SuppressWarnings("deprecation") annotation is placed on the addContent() method which spans ~250 lines with many payment method form invocations. While this is a large scope for suppression, applying it at method level is still preferable to class level. The deprecation warnings likely stem from the various *Form.addFormForBuyer() calls interacting with deprecated payment payload APIs.

core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java (1)

74-84: LGTM - Consistent with the PR pattern.

The @SuppressWarnings("deprecation") annotation is correctly and narrowly scoped to the fromProto method, consistent with the same pattern applied across other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java (1)

82-90: LGTM - Consistent with the PR pattern.

The @SuppressWarnings("deprecation") annotation is correctly and narrowly scoped to the fromProto method, matching the pattern applied to other payment payload classes in this PR.

desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java (1)

612-725: Scoped deprecation suppression on payment-method form factory looks good

Suppressing deprecation warnings at this private factory method keeps the scope tight and avoids noise elsewhere, with no behavioral impact.

core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (1)

81-90: fromProto deprecation suppression is consistent and low-risk

Adding @SuppressWarnings("deprecation") on this static fromProto aligns with other payloads, keeps warnings centralized at the deserialization boundary, and doesn’t alter behavior.

core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (1)

74-84: StrikeAccountPayload.fromProto suppression matches the shared pattern

The deprecation suppression is correctly placed on the static fromProto factory and keeps the method signature and logic intact.

desktop/src/main/java/bisq/desktop/components/MenuItem.java (1)

141-149: Unchecked suppression on generic array creation is reasonable here

The @SuppressWarnings("unchecked") is scoped to the local array variable produced from a well-typed List<Class<? extends View>>, so it safely silences the unavoidable generic-array warning without widening the suppression scope.

core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (1)

82-94: TransferwiseUsdAccountPayload.fromProto deprecation suppression looks fine

This keeps deprecation warnings localized at the proto deserialization boundary and is consistent with other payload classes in the PR.

desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java (1)

60-100: Expanding suppression on load to include deprecation is acceptable

The load method is an integration point that necessarily touches deprecated APIs, so adding "deprecation" to the existing "unchecked" suppression is a pragmatic choice and doesn’t affect runtime behavior.

core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1)

67-74: CelPayAccountPayload.fromProto deprecation suppression aligns with other payloads

The annotation cleanly suppresses deprecation at the serialization boundary without altering logic or signatures.

core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (1)

72-79: Deprecation suppression on fromProto looks correct

Scoping @SuppressWarnings("deprecation") to the fromProto factory is appropriate here; it keeps existing proto mapping intact and avoids leaking suppression more broadly.

core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (1)

75-85: Consistent deprecation suppression on fromProto

Applying @SuppressWarnings("deprecation") at the fromProto method level matches the pattern across payloads and avoids noise from proto deprecations without altering behavior.

core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (1)

77-85: Method-level suppression is appropriate

The @SuppressWarnings("deprecation") on fromProto cleanly isolates proto-related deprecation warnings while keeping the deserialization logic as-is.

core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (1)

105-115: Minimal-scope suppression on proto builder is sensible

Using @SuppressWarnings("deprecation") on the local protobuf.PaymentAccountPayload.Builder keeps the deprecation handling tightly scoped to the deprecated constructor call and avoids hiding other potential warnings in this class.

core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java (1)

72-79: fromProto deprecation suppression matches project pattern

The added @SuppressWarnings("deprecation") on fromProto is consistent with other payment payloads and keeps proto deprecation noise localized.

core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (1)

89-106: Deprecation suppression on bank-account fromProto is fine

Adding @SuppressWarnings("deprecation") to fromProto cleanly handles deprecated proto usage without affecting the bank account field mapping.

core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (1)

74-84: Paytm fromProto suppression is correctly scoped

The new @SuppressWarnings("deprecation") on fromProto is consistent with similar payloads and keeps the Gradle/Java toolchain clean without changing the deserialization behavior.

core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (1)

72-80: Monese fromProto deprecation suppression looks good

The @SuppressWarnings("deprecation") annotation on fromProto is in line with the rest of the payload classes and introduces no behavioral change.

core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (1)

89-89: LGTM: Appropriate deprecation warning suppression.

The @SuppressWarnings("deprecation") annotation on fromProto methods across all payment payload files is appropriate for handling protobuf deprecation warnings. This is a standard pattern when working with deprecated protobuf APIs that cannot be immediately updated.

Note: This comment applies to the identical changes in NationalBankAccountPayload.java, ClearXchangeAccountPayload.java, PixAccountPayload.java, InteracETransferAccountPayload.java, USPostalMoneyOrderAccountPayload.java, SbpAccountPayload.java, and WesternUnionAccountPayload.java.

proto/src/main/proto/pb.proto (1)

1923-1923: Verify removal of deprecated field references.

Unable to automatically verify removal of buyer_security_deposit_as_long and buyerSecurityDepositAsLong references from the codebase due to repository access limitations. Please manually confirm:

  • No remaining references to this field exist in Java, proto, or other source files
  • All dependent code that previously used field 30 has been updated or removed
  • Backward compatibility for reading older persisted data is maintained (typically handled automatically by protobuf field reservation)
core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (1)

101-115: Method-level deprecation suppression here is appropriate

Annotation is narrowly scoped to the factory method that touches deprecated proto APIs and doesn’t affect behavior; matches the pattern used across other payloads.

core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (1)

67-74: Localized suppression keeps deprecation noise contained

The deprecation warning is suppressed only on the proto factory, which is the right granularity and leaves the rest of the class warning-clean.

core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (1)

72-79: Consistent deprecation suppression on proto factory

fromProto is the only place using deprecated proto accessors, so method-level suppression here is reasonable and keeps the rest of the class strict.

core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (1)

71-78: Deprecation suppression is scoped correctly

The warning is suppressed only where the deprecated proto fields are read, which is a sensible, minimal change.

core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java (1)

74-84: Suppression aligns with the rest of the proto factories

Adding @SuppressWarnings("deprecation") directly on fromProto cleanly isolates the deprecated proto usage without touching surrounding logic.

core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (1)

80-80: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (1)

74-74: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (1)

73-73: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (1)

89-89: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (1)

94-94: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (1)

94-94: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1)

74-74: LGTM - Consistent with PR-wide pattern.

The deprecation suppression follows the same approach as other payment payload classes in this PR.

core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java (1)

110-110: Verify whether the suppressed methods are standard protobuf APIs or project-specific generated/custom code.

The methods referenced—getExcludeFromJsonDataMap() and getMaxTradePeriod()—do not appear in standard Google Protocol Buffers documentation. Before requesting migration to alternative APIs, confirm whether these are generated code from Bisq-specific .proto files, custom helper methods, or internal wrappers. If they are project-generated, examine the corresponding .proto definitions and the protoc version used; if they are custom methods, consult the project's internal API documentation for deprecation status and recommended alternatives.

core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java (1)

73-80: Deprecation suppression on fromProto looks appropriate

Method-level @SuppressWarnings("deprecation") keeps the mapping logic intact and localized; the proto field mapping remains symmetrical with toProtoMessage(). No issues from my side.

core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (1)

142-165: Consistent deprecation suppression on fromProto

Adding @SuppressWarnings("deprecation") here is in line with other payloads and keeps the proto‑to‑POJO mapping unchanged. Looks good.

core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java (1)

78-89: Method-scoped deprecation suppression is fine here

@SuppressWarnings("deprecation") on fromProto is targeted, aligns with the rest of the PR, and doesn’t alter behavior. No further changes needed.

core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1)

75-82: fromProto deprecation warning suppression looks good

The added @SuppressWarnings("deprecation") is localized to fromProto, and the field mapping remains identical. LGTM.

core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (1)

97-113: Deprecation suppression on SEPA proto boundary methods is reasonable

Adding @SuppressWarnings("deprecation") to both toProtoMessage() and fromProto(...) keeps warnings confined to the proto boundary while preserving existing serialization/deserialization behavior. This matches the pattern elsewhere in the PR.

Also applies to: 115-129

gradle/verification-metadata.xml (1)

2-2: Verification metadata updates appear structurally consistent with Gradle 8.9 upgrade

The schema bump to dependency-verification-1.3.xsd and the added entries for develocity, updated Kotlin/Jacoco/ASM artifacts, etc., match the existing XML structure (components, artifacts, sha256). Assuming these were regenerated via Gradle’s dependency-verification tooling and ./gradlew verification passes, I don’t see issues.

If you haven’t already, please re-run Gradle’s dependency verification (e.g., ./gradlew help --verify-dependencies) to confirm all checksums and schema usage are accepted by Gradle 8.9.

Also applies to: 972-979, 2972-2979, 3034-3041, 3050-3057, 3066-3070, 3077-3083, 3092-3099, 3172-3176, 3185-3232, 3241-3248, 3257-3264, 3273-3280, 3289-3296, 3305-3312, 3313-3320, 3332-3342, 3351-3358, 3359-3366, 3375-3382, 3383-3390, 3399-3406, 3415-3422, 3431-3438, 3447-3454, 3455-3462, 3471-3478, 3487-3494, 3495-3502, 3511-3518, 3527-3534, 3543-3550, 3559-3566, 3575-3582, 3615-3625, 4132-4136, 4153-4160, 4177-4181, 4198-4205, 4222-4229, 3034-3091

core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (1)

75-84: Deprecation suppression on FasterPayments proto methods is fine

The new @SuppressWarnings("deprecation") annotations on toProtoMessage() and fromProto(...) are consistent with other payment payloads and don’t alter the serialization logic. Looks good.

Also applies to: 86-95

core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (1)

70-77: Method‑scoped deprecation suppression looks appropriate

The @SuppressWarnings("deprecation") is narrowly scoped to fromProto, which keeps the warning noise down without masking issues elsewhere. No behavioral change introduced here.

core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java (1)

57-112: Targeted deprecation suppressions around legacy trade stats are reasonable

Adding @SuppressWarnings("deprecation") on the constructor and both convertToTradeStatistics3 overloads keeps the compiler quiet while you still need to interop with TradeStatistics2 and related deprecated pieces. Scope is tight and does not affect behavior.

Also applies to: 119-157, 159-181

core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (1)

67-74: Consistent deprecation suppression on proto factory

The @SuppressWarnings("deprecation") on fromProto matches the pattern used in other payment payloads for legacy proto fields. Mapping code remains unchanged.

core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (1)

80-89: Localized deprecation suppression on fromProto

Suppressing deprecation warnings directly on fromProto keeps the proto interop path intact while avoiding noise from deprecated fields. No functional impact.

core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (1)

82-91: Deprecation suppression on Amazon gift card fromProto is consistent

The @SuppressWarnings("deprecation") on fromProto follows the same pattern as other payment payloads dealing with legacy proto fields. Implementation remains unchanged.

core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (1)

75-83: Swish fromProto deprecation suppression is aligned with the rest

Adding @SuppressWarnings("deprecation") directly to fromProto is a minimal, well‑scoped way to keep compatibility with deprecated proto elements while compiling cleanly under Gradle 8.9.

core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2)

231-282: Scoped deprecation suppression on toProtoMessage is appropriate

Limiting @SuppressWarnings("deprecation") to the proto serialization method keeps the unavoidable use of deprecated proto fields contained while keeping the rest of the class honest with respect to deprecation warnings. No behavioral change, and consistent with the PR’s warning‑cleanup goal.


284-337: fromProto deprecation suppression is well‑targeted

Applying @SuppressWarnings("deprecation") only on the static fromProto(protobuf.OfferPayload) factory cleanly isolates deprecation noise to the deserialization boundary, which is where backward‑compatibility with older proto schemas is expected. The construction logic remains unchanged.

core/src/main/java/bisq/core/proto/CoreProtoResolver.java (2)

101-259: Deprecation suppression on PaymentAccountPayload resolver is justified

This resolver must continue to handle older, deprecated payment account variants for existing data, so placing @SuppressWarnings("deprecation") on the method is a sensible, narrowly scoped way to keep the Gradle 8.9 build clean without changing behavior.


261-285: PersistableNetworkPayload resolver suppression matches compatibility needs

Similarly, adding @SuppressWarnings("deprecation") to the fromProto(protobuf.PersistableNetworkPayload) override cleanly acknowledges that it works with deprecated proto payloads while preserving full backward compatibility. No logic or error‑handling changes introduced.

Comment on lines +127 to 140
public void copyInputStreamToFileNew(final InputStream source,
final File destination,
int fileSize) throws IOException {
try (final FileOutputStream output = FileUtils.openOutputStream(destination)) {
final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
long count = 0;
int n;
while (EOF != (n = source.read(buffer))) {
output.write(buffer, 0, n);
count += n;
log.trace("Progress: {}/{}", count, fileSize);
updateProgress(count, fileSize);
}
} finally {
IOUtils.closeQuietly(source);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

InputStream is never closed, introducing a resource leak

download(...) calls urlConnection.getInputStream() and passes it here, but this method only wraps the FileOutputStream in try‑with‑resources. The source InputStream is never closed on any path, which can leak HTTP/file resources and also silently changes the previous contract where this method closed the stream.

You can restore safe behavior and keep the new style by including the InputStream in the try‑with‑resources:

-    public void copyInputStreamToFileNew(final InputStream source,
-                                         final File destination,
-                                         int fileSize) throws IOException {
-        try (final FileOutputStream output = FileUtils.openOutputStream(destination)) {
-            final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
-            long count = 0;
-            int n;
-            while (EOF != (n = source.read(buffer))) {
-                output.write(buffer, 0, n);
-                count += n;
-                log.trace("Progress: {}/{}", count, fileSize);
-                updateProgress(count, fileSize);
-            }
-        }
-    }
+    public void copyInputStreamToFileNew(final InputStream source,
+                                         final File destination,
+                                         int fileSize) throws IOException {
+        try (InputStream input = source;
+             FileOutputStream output = FileUtils.openOutputStream(destination)) {
+            final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
+            long count = 0;
+            int n;
+            while (EOF != (n = input.read(buffer))) {
+                output.write(buffer, 0, n);
+                count += n;
+                log.trace("Progress: {}/{}", count, fileSize);
+                updateProgress(count, fileSize);
+            }
+        }
+    }

This keeps the new resource‑safety for the output file and also ensures the input stream is always closed, matching the prior behavior.

🤖 Prompt for AI Agents
In
desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java
around lines 127 to 140, the method copyInputStreamToFileNew wraps only the
FileOutputStream in try-with-resources so the passed-in InputStream "source" is
never closed; modify the method to include the source InputStream in the
try-with-resources (i.e., try (InputStream src = source; FileOutputStream output
= FileUtils.openOutputStream(destination)) { ... }) so the input stream is
always closed on all paths, preserving the previous behavior and avoiding
resource leaks while keeping the output stream handling unchanged.

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.

1 participant