Skip to content

Request arbitration when mediation result is rejected#4657

Open
KimStrand wants to merge 1 commit intobisq-network:mainfrom
KimStrand:musig-connect-request-arbitration
Open

Request arbitration when mediation result is rejected#4657
KimStrand wants to merge 1 commit intobisq-network:mainfrom
KimStrand:musig-connect-request-arbitration

Conversation

@KimStrand
Copy link
Copy Markdown
Contributor

@KimStrand KimStrand commented Apr 14, 2026

Summary by CodeRabbit

  • New Features

    • Arbitration escalation: request arbitration from trades after mediation rejection, with confirmation and feedback.
    • Arbitration status shown in the trade UI alongside mediation, including delivery/progress indicators.
    • Background support for arbitrator processing and arbitration request handling.
  • Localization

    • Added arbitration-related UI/messages and log text.
  • Tests

    • New test suite for the dispute/arbitration flow; removed legacy trade-service tests.

@KimStrand KimStrand changed the title Request arbitration when mediation is rejected Request arbitration when mediation result is rejected Apr 14, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Warning

Rate limit exceeded

@KimStrand has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 26 minutes and 27 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 27 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d1626039-960f-4a7d-b1bf-264982d1702e

📥 Commits

Reviewing files that changed from the base of the PR and between 3070607 and fabaac9.

📒 Files selected for processing (21)
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateController.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateModel.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateView.java
  • i18n/src/main/resources/authorized_role.properties
  • i18n/src/main/resources/mu_sig.properties
  • mu-sig/src/main/java/bisq/mu_sig/MuSigService.java
  • support/src/main/java/bisq/support/SupportService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitrationResultService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitratorService.java
  • support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java
  • support/src/test/java/bisq/support/mediation/mu_sig/MuSigMediationResultServiceTest.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTrade.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDispute.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeService.java
  • trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java
  • trade/src/main/java/bisq/trade/mu_sig/mediation/MuSigTraderMediationService.java
  • trade/src/main/proto/trade.proto
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeDisputeServiceTest.java
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeServiceTest.java

Walkthrough

Adds arbitration support and refactors dispute handling: introduces a unified MuSigTradeDispute model, dispute orchestration service, arbitrator services and result signing/verification, updates trader mediation/arbitration services and desktop UI for arbitration state, and adds tests for the dispute flow.

Changes

Cohort / File(s) Summary
Desktop UI Arbitration Support
apps/desktop/.../MuSigPendingTTradesUtils.java, apps/desktop/.../MuSigTradeStateController.java, apps/desktop/.../MuSigTradeStateModel.java, apps/desktop/.../MuSigTradeStateView.java
Added helper to reject mediation and request arbitration, new arbitration UI state/properties, delivery-status tracking for arbitration requests, arbitration banner and label updates, and binding/unbinding changes.
Dispute Model & Protobuf
trade/.../MuSigTrade.java, trade/.../MuSigTradeDispute.java, trade/src/main/proto/trade.proto
Replaced separate dispute/mediation fields with new MuSigTradeDispute (state, optional mediation/arbitration results and signatures); updated protobuf and serialization, added getters/setters for trade dispute.
Dispute Orchestration
trade/.../MuSigTradeDisputeService.java, trade/.../MuSigTradeService.java
New package-private dispute service handling mediation/arbitration state machine, message routing, signature validation, pending-message queuing; MuSigTradeService now delegates dispute messages and adds requestArbitration(...).
Arbitration Services & Support
support/.../MuSigArbitratorService.java, support/.../MuSigArbitrationResultService.java, support/.../SupportService.java
Added arbitrator listener/service, arbitration-result signing and verification utilities, and integrated arbitrator service lifecycle into SupportService.
Trader Mediation/Arbitration APIs
trade/.../MuSigTraderMediationService.java, trade/.../MuSigTraderArbitrationService.java
Refactored mediation APIs to accept explicit parameters instead of MuSigTrade; converted arbitration trader service into non-lifecycle class and added requestArbitration and applyArbitrationStateToChannel.
I18n & Tests
i18n/.../authorized_role.properties, i18n/.../mu_sig.properties, trade/.../MuSigTradeDisputeServiceTest.java, trade/.../MuSigTradeServiceTest.java
Added arbitrator role and arbitration i18n keys; added comprehensive dispute-service tests and removed legacy MuSigTradeServiceTest.
Minor formatting
mu-sig/.../MuSigService.java
Whitespace/indentation tweak only.

Sequence Diagram(s)

sequenceDiagram
    participant Desktop as Desktop UI
    participant TradeSvc as MuSigTradeService
    participant DisputeSvc as MuSigTradeDisputeService
    participant ArbitrationTrader as MuSigTraderArbitrationService
    participant Network as NetworkService

    Desktop->>TradeSvc: onRejectMediation(trade)
    TradeSvc->>DisputeSvc: rejectMediationResult(trade)
    DisputeSvc->>DisputeSvc: set dispute state -> MEDIATION_CLOSED
    DisputeSvc->>ArbitrationTrader: (if conditions) requestArbitration(...)
    ArbitrationTrader->>Network: confidentialSend(MuSigArbitrationRequest)
    Network-->>ArbitrationTrader: send result
    ArbitrationTrader->>DisputeSvc: applyArbitrationStateToChannel(...)
    DisputeSvc->>Desktop: update UI / delivery status
Loading
sequenceDiagram
    participant Confidential as ConfidentialMessageService
    participant ArbitratorSvc as MuSigArbitratorService
    participant Auth as Banned/Role Authorization
    participant Persist as Trade Persistence

    Confidential->>ArbitratorSvc: onMessage(MuSigArbitrationRequest)
    ArbitratorSvc->>Auth: validate requester not banned & roles match
    Auth-->>ArbitratorSvc: authorized / rejected
    alt authorized & local identity found
        ArbitratorSvc->>Persist: log receipt & process request
    else rejected
        ArbitratorSvc->>ArbitratorSvc: drop/reject request
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • HenrikJannsen

Poem

🐰 I twitch my nose at disputes grown wide,
I hop from mediation to arbitration’s side.
With signatures neat and messages sent,
I guard each trade with carrot intent.
Hooray — justice hops in, problem solved with pride!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main feature: enabling arbitration requests as an escalation path when mediation results are rejected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
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: 3

Caution

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

⚠️ Outside diff range comments (1)
mu-sig/src/main/java/bisq/mu_sig/MuSigService.java (1)

284-299: ⚠️ Potential issue | 🟠 Major

Pass Optional.empty() for the arbitrator parameter instead of mediator.

The arbitrator selection code is deferred (lines 284–290 are commented out), but line 299 currently passes mediator for both the mediator and arbitrator parameters. This is semantically incorrect—these should be independent values. While the arbitrator field is not yet used in the current codebase, storing the mediator's UserProfile in the arbitrator field is misleading.

🔧 Proposed fix
         return takerCreatesProtocol(takerIdentity,
                 muSigOffer,
                 takersBaseSideAmount,
                 takersQuoteSideAmount,
                 takersPaymentMethodSpec,
                 takersAccount.getAccountPayload(),
                 mediator,
-                mediator);
+                Optional.empty());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mu-sig/src/main/java/bisq/mu_sig/MuSigService.java` around lines 284 - 299,
The call to takerCreatesProtocol currently passes the same Optional<UserProfile>
mediator for both mediator and arbitrator, which is incorrect since arbitrator
selection is disabled; update the call in MuSigService to pass mediator for the
mediator parameter and Optional.empty() for the arbitrator parameter (i.e.,
replace the second mediator argument with Optional.empty()) so the arbitrator
slot is explicitly empty while keeping mediator unchanged.
🧹 Nitpick comments (4)
trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java (1)

100-116: Consider using descriptive exceptions for orElseThrow() calls.

The bare orElseThrow() calls at lines 100, 111, and 114 will throw NoSuchElementException without context, making debugging harder if these fail in production.

♻️ Proposed improvement
-        MuSigOpenTradeChannel channel = findMuSigOpenTradeChannel(tradeId).orElseThrow();
+        MuSigOpenTradeChannel channel = findMuSigOpenTradeChannel(tradeId)
+                .orElseThrow(() -> new IllegalStateException("No open trade channel found for trade: " + tradeId));
         String encoded = Res.encode("muSig.arbitration.requester.tradeLogMessage", channel.getMyUserIdentity().getUserName());
         muSigOpenTradeChannelService.sendTradeLogMessage(encoded, channel);
         muSigOpenTradeChannelService.setDisputeAgentType(channel, MuSigDisputeAgentType.ARBITRATOR);

         NetworkId arbitratorNetworkId = arbitrator.getNetworkId();

         MuSigArbitrationRequest muSigArbitrationRequest = new MuSigArbitrationRequest(tradeId,
                 contract,
                 mediationResult,
                 mediationResultSignature,
-                userProfileService.findUserProfile(myIdentity.getId()).orElseThrow(),
+                userProfileService.findUserProfile(myIdentity.getId())
+                        .orElseThrow(() -> new IllegalStateException("User profile not found for identity: " + myIdentity.getId())),
                 userProfileService
                         .findUserProfile(peer.getNetworkId().getId())
-                        .orElseThrow(),
+                        .orElseThrow(() -> new IllegalStateException("Peer user profile not found: " + peer.getNetworkId().getId())),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java`
around lines 100 - 116, Replace the bare orElseThrow() calls with descriptive
exceptions to provide context when lookups fail: wrap the channel lookup
(findMuSigOpenTradeChannel(tradeId)), the my user profile
(userProfileService.findUserProfile(myIdentity.getId())), and the peer profile
(userProfileService.findUserProfile(peer.getNetworkId().getId())) in
orElseThrow(() -> new IllegalStateException("...")) or a domain-specific
NotFoundException that includes identifying info (tradeId, myIdentity.getId(),
peer.getNetworkId()) so the MuSigArbitrationRequest construction and subsequent
calls (muSigOpenTradeChannelService.sendTradeLogMessage, setDisputeAgentType)
will fail with clear messages rather than NoSuchElementException.
apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java (1)

87-106: Consider error handling between sequential service calls.

Lines 94-95 call rejectMediationResult followed by requestArbitration without error handling. If rejectMediationResult throws an exception, the feedback popup won't show, which may be acceptable. However, if rejectMediationResult succeeds but requestArbitration fails, the user might be left in an inconsistent state (mediation rejected but no arbitration request sent).

This may be acceptable given the existing pattern in requestMediation, but worth considering whether atomic behavior or retry logic is needed for this escalation flow.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java`
around lines 87 - 106, The sequential calls in
rejectMediationResultAndRequestArbitration (tradeService.rejectMediationResult
then tradeService.requestArbitration) need error handling so a failed
requestArbitration doesn't leave the trade half-processed; wrap the
requestArbitration call in a try/catch, log the exception and show a user-facing
error Popup (and offer retry), and if your service exposes a compensating method
(e.g. restoreMediationResult or similar) consider invoking it in the catch to
revert the prior rejectMediationResult; ensure
tradeService.rejectMediationResult, tradeService.requestArbitration and the
Popup feedback logic are updated accordingly.
trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java (2)

94-115: Same redundant pattern with arbitrator.

Same suggestion applies here: use arbitrator.get() instead of arbitrator.orElseThrow() after the isPresent() check.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java` around
lines 94 - 115, The code in requestArbitration uses an Optional "arbitrator"
that is already checked with arbitrator.isPresent() but then calls
arbitrator.orElseThrow() later; replace those orElseThrow() calls with
arbitrator.get() to avoid redundant checks and make intent clearer. Update the
method requestArbitration in MuSigTradeDisputeService to use arbitrator.get()
when passing the arbitrator into
muSigTraderArbitrationService.requestArbitration (and ensure any similar uses in
that method, e.g., of arbitrator variable) instead of arbitrator.orElseThrow().

66-84: Minor: Redundant orElseThrow() after isPresent() check.

After verifying mediator.isPresent(), you can safely use mediator.get() instead of mediator.orElseThrow(). The current code works correctly but the pattern is slightly misleading.

♻️ Suggested simplification
     public void requestMediation(MuSigTrade trade) {
         checkArgument(!bannedUserService.isUserProfileBanned(trade.getMyIdentity().getNetworkId()));
         MuSigContract contract = trade.getContract();
         Optional<UserProfile> mediator = contract.getMediator();
         if (mediator.isPresent()) {
             MuSigTradeDispute tradeDispute = trade.getTradeDispute();
             MuSigDisputeState current = tradeDispute.getDisputeState();
             if (current != MuSigDisputeState.NO_DISPUTE) {
                 log.warn("Cannot request mediation for trade {} because not in the right state.",
                         trade.getId());
                 return;
             }

             tradeDispute.setDisputeState(MuSigDisputeState.MEDIATION_REQUESTED);
             persist.run();
             muSigTraderMediationService.requestMediation(trade.getId(), trade.getMyIdentity(),
-                    trade.getPeer(), mediator.orElseThrow(), contract);
+                    trade.getPeer(), mediator.get(), contract);
         }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java` around
lines 66 - 84, The code in requestMediation uses mediator.orElseThrow()
immediately after checking mediator.isPresent(); replace the redundant
orElseThrow() with mediator.get() in the call to
muSigTraderMediationService.requestMediation so it clearly reflects the prior
presence check (update the call in MuSigTradeDisputeService.requestMediation
that passes mediator to muSigTraderMediationService.requestMediation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateController.java`:
- Around line 266-270: The branch in MuSigTradeStateController currently only
treats MuSigDisputeState.MEDIATION_CLOSED as escalation to arbitration; update
the conditional in the method that contains the shown snippet to treat
MuSigDisputeState.MEDIATION_RE_OPENED the same way by checking for both states
(MEDIATION_CLOSED || MEDIATION_RE_OPENED) and calling
MuSigPendingTTradesUtils.rejectMediationResultAndRequestArbitration(trade,
tradeService) for those cases, otherwise keep calling
tradeService.rejectMediationResult(trade).

In
`@support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitrationResultService.java`:
- Around line 40-41: Rename the local variable ArbitrationResultHash to
camelCase arbitrationResultHash in MuSigArbitrationResultService within the code
that assigns getArbitrationResultHash(arbitrationResult) and passes it to
SignatureUtil.sign; update both the declaration and its use so the variable name
matches Java conventions (e.g., byte[] arbitrationResultHash =
getArbitrationResultHash(arbitrationResult); return
SignatureUtil.sign(arbitrationResultHash, keyPair.getPrivate());).

In `@trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDispute.java`:
- Around line 142-161: The equals() and hashCode() currently compare the
Observable wrapper (disputeState) instead of the contained MuSigDisputeState
value; update both methods to use the Observable's contained value when
comparing and hashing (e.g., replace disputeState.equals(that.disputeState) with
Objects.equals(disputeState.getValue() / disputeState.get() as appropriate,
that.disputeState.getValue()/get()), and use
Objects.hashCode(disputeState.getValue()/get()) in hashCode) while keeping the
existing comparisons for muSigMediationResult, mediationResultSignature,
muSigArbitrationResult and arbitrationResultSignature unchanged.

---

Outside diff comments:
In `@mu-sig/src/main/java/bisq/mu_sig/MuSigService.java`:
- Around line 284-299: The call to takerCreatesProtocol currently passes the
same Optional<UserProfile> mediator for both mediator and arbitrator, which is
incorrect since arbitrator selection is disabled; update the call in
MuSigService to pass mediator for the mediator parameter and Optional.empty()
for the arbitrator parameter (i.e., replace the second mediator argument with
Optional.empty()) so the arbitrator slot is explicitly empty while keeping
mediator unchanged.

---

Nitpick comments:
In
`@apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java`:
- Around line 87-106: The sequential calls in
rejectMediationResultAndRequestArbitration (tradeService.rejectMediationResult
then tradeService.requestArbitration) need error handling so a failed
requestArbitration doesn't leave the trade half-processed; wrap the
requestArbitration call in a try/catch, log the exception and show a user-facing
error Popup (and offer retry), and if your service exposes a compensating method
(e.g. restoreMediationResult or similar) consider invoking it in the catch to
revert the prior rejectMediationResult; ensure
tradeService.rejectMediationResult, tradeService.requestArbitration and the
Popup feedback logic are updated accordingly.

In
`@trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java`:
- Around line 100-116: Replace the bare orElseThrow() calls with descriptive
exceptions to provide context when lookups fail: wrap the channel lookup
(findMuSigOpenTradeChannel(tradeId)), the my user profile
(userProfileService.findUserProfile(myIdentity.getId())), and the peer profile
(userProfileService.findUserProfile(peer.getNetworkId().getId())) in
orElseThrow(() -> new IllegalStateException("...")) or a domain-specific
NotFoundException that includes identifying info (tradeId, myIdentity.getId(),
peer.getNetworkId()) so the MuSigArbitrationRequest construction and subsequent
calls (muSigOpenTradeChannelService.sendTradeLogMessage, setDisputeAgentType)
will fail with clear messages rather than NoSuchElementException.

In `@trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java`:
- Around line 94-115: The code in requestArbitration uses an Optional
"arbitrator" that is already checked with arbitrator.isPresent() but then calls
arbitrator.orElseThrow() later; replace those orElseThrow() calls with
arbitrator.get() to avoid redundant checks and make intent clearer. Update the
method requestArbitration in MuSigTradeDisputeService to use arbitrator.get()
when passing the arbitrator into
muSigTraderArbitrationService.requestArbitration (and ensure any similar uses in
that method, e.g., of arbitrator variable) instead of arbitrator.orElseThrow().
- Around line 66-84: The code in requestMediation uses mediator.orElseThrow()
immediately after checking mediator.isPresent(); replace the redundant
orElseThrow() with mediator.get() in the call to
muSigTraderMediationService.requestMediation so it clearly reflects the prior
presence check (update the call in MuSigTradeDisputeService.requestMediation
that passes mediator to muSigTraderMediationService.requestMediation).
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ed5b8392-daca-41fd-9003-2eea0193c2d8

📥 Commits

Reviewing files that changed from the base of the PR and between 7b29814 and 36f00ed.

📒 Files selected for processing (20)
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateController.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateModel.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateView.java
  • i18n/src/main/resources/authorized_role.properties
  • i18n/src/main/resources/mu_sig.properties
  • mu-sig/src/main/java/bisq/mu_sig/MuSigService.java
  • support/src/main/java/bisq/support/SupportService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitrationResultService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitratorService.java
  • support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTrade.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDispute.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeService.java
  • trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java
  • trade/src/main/java/bisq/trade/mu_sig/mediation/MuSigTraderMediationService.java
  • trade/src/main/proto/trade.proto
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeDisputeServiceTest.java
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeServiceTest.java
💤 Files with no reviewable changes (1)
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeServiceTest.java

@KimStrand KimStrand force-pushed the musig-connect-request-arbitration branch 2 times, most recently from 42aba1c to 4b9e1c9 Compare April 14, 2026 12:12
@KimStrand
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@KimStrand KimStrand force-pushed the musig-connect-request-arbitration branch from 4b9e1c9 to 3070607 Compare April 14, 2026 13:09
@KimStrand
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

🧹 Nitpick comments (2)
support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java (1)

44-50: Prefer returning false instead of throwing for hash mismatch in verifier.

This path handles untrusted message data; treating mismatch as a normal verification failure keeps semantics consistent with other invalid-signature cases.

♻️ Proposed change
-        checkArgument(Arrays.equals(mediationResult.getContractHash(), ContractService.getContractHash(contract)),
-                "Contract hash from MuSigMediationResult does not match the given contract");
-        return verifyMediationResult(mediationResult, mediationResultSignature, publicKey);
+        if (!Arrays.equals(mediationResult.getContractHash(), ContractService.getContractHash(contract))) {
+            return false;
+        }
+        return verifyMediationResult(mediationResult, mediationResultSignature, publicKey);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java`
around lines 44 - 50, The verifier currently uses checkArgument to assert the
contract hash match in verifyMediationResult which throws on mismatch; instead
treat a hash mismatch as a normal verification failure: replace the
checkArgument(Arrays.equals(mediationResult.getContractHash(),
ContractService.getContractHash(contract)), ...) with an if check that returns
false when the hashes differ so verifyMediationResult (the overload that takes
publicKey) is only called for matching hashes and no exception is thrown for
untrusted input.
trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java (1)

139-145: Unused private method.

findMuSigOpenTradeChannel is defined but never called within this class. Consider removing it to avoid dead code.

♻️ Suggested removal
-    /* --------------------------------------------------------------------- */
-    // Private
-    /* --------------------------------------------------------------------- */
-
-    private Optional<MuSigOpenTradeChannel> findMuSigOpenTradeChannel(String tradeId) {
-        return muSigOpenTradeChannelService.findChannelByTradeId(tradeId);
-    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java`
around lines 139 - 145, The private helper method findMuSigOpenTradeChannel in
MuSigTraderArbitrationService is unused; remove the dead code by deleting the
entire method declaration (the method name findMuSigOpenTradeChannel and its
body that returns muSigOpenTradeChannelService.findChannelByTradeId(tradeId));
after removal, run a project-wide search to ensure no remaining references to
findMuSigOpenTradeChannel exist and update any callers if they were intended to
use muSigOpenTradeChannelService directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java`:
- Around line 44-50: The verifier currently uses checkArgument to assert the
contract hash match in verifyMediationResult which throws on mismatch; instead
treat a hash mismatch as a normal verification failure: replace the
checkArgument(Arrays.equals(mediationResult.getContractHash(),
ContractService.getContractHash(contract)), ...) with an if check that returns
false when the hashes differ so verifyMediationResult (the overload that takes
publicKey) is only called for matching hashes and no exception is thrown for
untrusted input.

In
`@trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java`:
- Around line 139-145: The private helper method findMuSigOpenTradeChannel in
MuSigTraderArbitrationService is unused; remove the dead code by deleting the
entire method declaration (the method name findMuSigOpenTradeChannel and its
body that returns muSigOpenTradeChannelService.findChannelByTradeId(tradeId));
after removal, run a project-wide search to ensure no remaining references to
findMuSigOpenTradeChannel exist and update any callers if they were intended to
use muSigOpenTradeChannelService directly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4f727cdf-f957-4741-b73b-803ef06e526d

📥 Commits

Reviewing files that changed from the base of the PR and between 36f00ed and 3070607.

📒 Files selected for processing (20)
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateController.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateModel.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateView.java
  • i18n/src/main/resources/authorized_role.properties
  • i18n/src/main/resources/mu_sig.properties
  • mu-sig/src/main/java/bisq/mu_sig/MuSigService.java
  • support/src/main/java/bisq/support/SupportService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitrationResultService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitratorService.java
  • support/src/main/java/bisq/support/mediation/mu_sig/MuSigMediationResultService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTrade.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDispute.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeService.java
  • trade/src/main/java/bisq/trade/mu_sig/arbitration/MuSigTraderArbitrationService.java
  • trade/src/main/java/bisq/trade/mu_sig/mediation/MuSigTraderMediationService.java
  • trade/src/main/proto/trade.proto
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeDisputeServiceTest.java
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeServiceTest.java
💤 Files with no reviewable changes (1)
  • trade/src/test/java/bisq/trade/mu_sig/MuSigTradeServiceTest.java
✅ Files skipped from review due to trivial changes (1)
  • mu-sig/src/main/java/bisq/mu_sig/MuSigService.java
🚧 Files skipped from review as they are similar to previous changes (10)
  • support/src/main/java/bisq/support/SupportService.java
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigTradeStateModel.java
  • i18n/src/main/resources/mu_sig.properties
  • apps/desktop/desktop/src/main/java/bisq/desktop/main/content/mu_sig/trade/pending/trade_state/MuSigPendingTTradesUtils.java
  • i18n/src/main/resources/authorized_role.properties
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitrationResultService.java
  • support/src/main/java/bisq/support/arbitration/mu_sig/MuSigArbitratorService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDispute.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeDisputeService.java
  • trade/src/main/java/bisq/trade/mu_sig/MuSigTradeService.java

@KimStrand KimStrand force-pushed the musig-connect-request-arbitration branch from 3070607 to fabaac9 Compare April 14, 2026 14:21
@KimStrand KimStrand requested a review from HenrikJannsen April 14, 2026 14:33
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