Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

Conversation

@rodkeys
Copy link
Contributor

@rodkeys rodkeys commented Mar 11, 2020

image

@drwasho drwasho requested a review from cpacia March 11, 2020 03:30
@drwasho drwasho added the ordering ordering issues label Mar 11, 2020
@drwasho drwasho requested a review from hoffmabc March 11, 2020 03:30
@drwasho
Copy link
Member

drwasho commented Mar 11, 2020

Rod and I tested this the following way:

  1. The buyer makes a purchase to an offline seller
  2. The buyer then immediately disputes the order
  3. The buyer then goes offline
  4. The moderator decides the outcome of the dispute
  5. The seller comes online and accepts the decision from the moderator, and then goes offline
  6. The buyer comes online

Now, for one reason or another, DISPUTE_CLOSE messages from the seller aren't being detected, so the buyer sees that they can accept the dispute. What was happening was that they would attempt to do this and it would fail with unknown script type as the response.

This response is still there and is expected because the funds have already moved out of the multisig escrow (if anything the error should be more descriptive here). THE PROBLEM this PR fixes is that the order state would update before it would try and spend the funds. If it did this, it would leave the order in a bad state where it didn't have all the data it needed and it even if it did receive the DISPUTE_CLOSE message, it couldn't progress to the DECIDED state in the correct way.

As a side note, we're looking into why offline messages are being so poorly retrieved from the push nodes.

@OpenBazaar OpenBazaar deleted a comment from OB1Bot Mar 11, 2020
Copy link
Member

@hoffmabc hoffmabc left a comment

Choose a reason for hiding this comment

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

LGTM. Need a double sanity check by @cpacia and it's good to go.

@cpacia cpacia merged commit ce555f2 into OpenBazaar:master Mar 11, 2020
Copy link
Member

@hoffmabc hoffmabc left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ordering ordering issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants