Skip to content

For Cycle 56 #1489

@alvasw

Description

@alvasw

Summary

Specify the total amount of BSQ you are requesting, along with the USD total and BSQ/USD rate (don't include the brackets!):

  • BSQ requested: 8720.93
  • USD requested: 15000
  • BSQ rate: 1.72 USD per BSQ
  • Previous compensation request (if applicable): For Cycle 55 #1464

Contributions delivered

Add contributions you have delivered and roles you have performed here as new rows in the table below. Role line-items should include an asterisk (*) in the team column.

Title Team USD Link Notes
bisq: PR Review bisq-network/bisq#6986 dev bisq-network/bisq#6986

Nit

Hi yonson2023,
I tested this PR again and it's doesn't include the code review changes, you applied to your previous MoneyBeam PR. Please apply those here too.

---------

ACK

Perfect! Thank you so much!

Test Setup

  • Created Pix and Wise accounts on v1.9.14 on Alice's and Bob's Bisq instance
  • Created Pix and Wise accounts on this PR on Alice's and Bob's Bisq instance

Trades (For Pix and Wise)

  • Between old accounts
    • Checked trade details screen
    • Tested copy button
  • Between new accounts
    • Checked trade details screen
    • Tested copy button
  • Between old and new accounts
    • Checked trade details screen
    • Tested copy button
bisq: PR Review bisq-network/bisq#7007 dev bisq-network/bisq#7007

ACK

bisq: PR Review bisq-network/bisq#7025 dev bisq-network/bisq#7025

ACK

bisq: PR Review bisq-network/bisq#7005 dev bisq-network/bisq#7005

Nit

The QR Code works! I received video recordings of users testing four SEPA banking apps.

Feedback

  • We can show the QR Code by default because most SEPA banking apps support QR Codes.
  • After scanning the QR Code users can't double check the data because it's hidden. I think we should show the BIC and IBAN while showing the QR Code (instead of hiding it).
  • Two of the tested banking apps sent the image to the banking servers to read the QR Code. In both images the text "[...] trade ID or other text like 'bitcoin', 'BTC', or 'Bisq. [...]" was visible. Banks that do OCR processing could flag users' accounts. We should add a softbreak to the text, so that it's never near the QR Code.
bisq: PR Review bisq-network/bisq#7008 dev bisq-network/bisq#7008

Concept ACK

Good catch! The toString() comparison is indeed bad.

I saw that you're using a AtomicLongs to count the item. Does this imply that detectMultipleHolderNames() is called from multiple threads? If that's the case suspiciousDisputesByTraderMap shouldn't be a HashMap.

Otherwise the following change is simpler and faster:

diff --git a/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java b/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java index 1be96332ef..09c722eed5 100644 --- a/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java +++ b/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java @@ -146,7 +146,10 @@ public class MultipleHolderNameDetection {      ///////////////////////////////////////////////////////////////////////////////////////////        public void detectMultipleHolderNames() { -        String previous = suspiciousDisputesByTraderMap.toString(); +        int previous = suspiciousDisputesByTraderMap.values().stream() +                .mapToInt(List::size) +                .sum(); +          getAllDisputesByTraderMap().forEach((key, value) -> {              Set<String> userNames = value.stream()                      .map(dispute -> { @@ -161,8 +164,12 @@ public class MultipleHolderNameDetection {                  suspiciousDisputesByTraderMap.put(key, value);              }          }); -        String updated = suspiciousDisputesByTraderMap.toString(); -        if (!previous.equals(updated)) { + +        int updated = suspiciousDisputesByTraderMap.values().stream() +                .mapToInt(List::size) +                .sum(); + +        if (previous != updated) {              listeners.forEach(Listener::onSuspiciousDisputeDetected);          }      }
bisq: PR Review bisq-network/bisq#7012 dev bisq-network/bisq#7012

Nit

getdaostatus is missing in the /bisq-cli --helpoutput.

bisq: PR Review bisq-network/bisq#7023 dev bisq-network/bisq#7023

Nit

I know that someone else wrote the original code but we should try to clean up existing code when working on it. We can avoid the redudant copying of the trade list by using Collections.unmodifiableList(getObservableList()).

Apart from that I aggree with dutu. API consumers should be able to tell whether a trade failed.

bisq: Test Maker Transaction Vouts dev bisq-network/bisq#7013
bisq: Refactor and Add Maker Transaction Vouts Tests dev bisq-network/bisq#7014
bisq: Test All Maker BTC Cases dev bisq-network/bisq#7015
bisq: Test All Taker BTC Cases dev bisq-network/bisq#7019
  • Add takerInvalidFeeBtcAddressTest
  • Test all Taker BTC cases
    • takerCheckFeeAddressBtcInvalidFeeAddress
    • takerCheckFeeAddressBtcTooOldValidFee
    • takerExactFeeMatchBtcTest
    • takerHigherBtcFeeThanExpected
    • takerLowerButWithinToleranceBtcFee
    • takerPassFilterFeeCheck
    • takerFailFilterFeeCheck
    • takerNoFilterFeeMatchesDifferentDaoParameter
    • takerNoFilterFeeTooLow
bisq: Test all Maker BSQ Cases dev bisq-network/bisq#7020
  • unconfirmedTransaction
  • newBsqTx
  • makerExactFeeMatchBsqTest
  • makerHigherBsqFeeThanExpected
  • makerLowerButWithinToleranceBsqFee
  • makerPassFilterFeeCheck
  • makerFailFilterFeeCheck
  • makerNoFilterFeeMatchesDifferentDaoParameter
  • makerNoFilterFeeTooLow
bisq: Test all Taker BSQ cases dev bisq-network/bisq#7021
  • unconfirmedTransaction
  • newBsqTx
  • takerExactFeeMatchBsqTest
  • takerHigherBsqFeeThanExpected
  • takerLowerButWithinToleranceBsqFee
  • takerPassFilterFeeCheck
  • takerFailFilterFeeCheck
  • takerNoFilterFeeMatchesDifferentDaoParameter
  • takerNoFilterFeeTooLow
bisq: Implement AsyncFileWriter dev bisq-network/bisq#7027
bisq: Implement AtomicFileWriter dev bisq-network/bisq#7032

First, the AtomicFileWriter writes data to a rolling file and before swapping the rolling file with the active file. This makes all file writes atomic and let's Bisq recover from crashes.

Changes:

bisq2: Set Tor Log Level to 'notice' in Production dev bisq-network/bisq2#1645
bisq2: Add second tor seednode dev bisq-network/bisq2#1646
bisq2: Create Bisq Daemon dev bisq-network/bisq2#1654
bisq2: Test TorSignatureUtils dev bisq-network/bisq2#1665
  • signEmptyMessage
  • signMessage
  • verifyInvalidMessage
  • verifyInvalidSignature
bisq2: Fix Tor Address Validation Bugs dev bisq-network/bisq2#1667
dev 15000 Total for items above.

Metadata

Metadata

Assignees

Labels

parsed:validhttps://bisq.wiki/Compensation#Ensure_your_request_is_validteam:devhttps://bisq.wiki/Dev_Teamwas:acceptedIndicates that a compensation request was accepted by DAO voting

Type

No type

Projects

Status

Closed

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions