Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major errors happening when user tries to accept trade. #2910

Closed
spottedmarley opened this issue Jun 21, 2019 · 2 comments · Fixed by #2928
Closed

Major errors happening when user tries to accept trade. #2910

spottedmarley opened this issue Jun 21, 2019 · 2 comments · Fixed by #2928

Comments

@spottedmarley
Copy link

spottedmarley commented Jun 21, 2019

This error has happened several times since version 1.0 but this is the first time I have had the opportunity to get a screen capture of the actual error dialog and the log file to go with. I have traded alot using bisq and this never used to happen before 1.0.

What is common to each time I've seen this issue is that I have several open "money order" trades that are in the "Payment started" faze and waiting for me to receive and confirm payment. Then someone tries to accept one of my open trade offers and I get flooded with errors. The initial error states that someone attempted to accept my trade offer and an error occured.. this dialog only has a "close" button.. when I click "close" then I get the dialog that you see in the screen shot: Please see attached screen shot of the error dialog and log file.

You can even see in the screen shot that Bisq is telling me that "Your offer has been accepted ..." that is what sets this error off. The offer is not actually accepted though.

Here is the error that is copied to my clipboard: PeerAddress must not be null (sendEncryptedMailboxMessage)

When this error occures it tends to also corrupt my entire portfolio so that ALL of my trades are no longer listed. All my open trades and open offers are gone.

I have incremental backups of my data folder so I can recover from this but why does this keep happening? Like I said, it never happened until v1.0 and the DAO was turned on in the client.

bisq-error-2
bisq.log

You may also notice from the screen shot that, yes, I am currently watching the first season of Miami Vice.

@spottedmarley
Copy link
Author

I'm going back to v0.9.8 until the platform is a little more stable. Works like a charm :)

@a123b
Copy link
Contributor

a123b commented Jun 25, 2019

@spottedmarley The error message popped up ~3 minutes before the "Your offer has been accepted" message, right? It would be interesting to know whether the portfolio gets wiped after the error message or only after the trade notification.

Quick breakdown of the log file:

  • We are a maker selling BTC
  • Someone tries to take the offer
  • We disagree about the market price with a difference slightly above the max. allowed 1%
  • Sending the AckMessage for the taker's PayDepositRequest with the error message about the price disagreement fails because the PeerAddress is null
  • ~3 minutes later, the taker tries taking the offer again, this time, the price difference is within tolerance and everything works as expected (deposit tx gets published)

Looking at the code that's sending the AckMessage, it gets the PeerAddress from the Trade object (TradeProtocol.java#L201).
The problem is that the tradingPeerNodeAddress property of the Trade object is only set in MakerProcessPayDepositRequest.java#L95, which is after sending the AckMessage with the error is initiated in MakerProcessPayDepositRequest.java#L87.

What I don't understand is why the portfolio is wiped after that happens... maybe it has something to do with cleanup code getting skipped because of the NPE?

a123b added a commit to a123b/bisq that referenced this issue Jun 28, 2019
a123b added a commit to a123b/bisq that referenced this issue Jun 28, 2019
…er set yet

This should fix the errors on the maker side when there's a disagreement
about market price.

Fixes bisq-network#2910
Fixes bisq-network#2924
fflo pushed a commit to fflo/bisq that referenced this issue Jul 13, 2019
…er set yet

This should fix the errors on the maker side when there's a disagreement
about market price.

Fixes bisq-network#2910
Fixes bisq-network#2924
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 a pull request may close this issue.

2 participants