Redesign Bisq Easy Protocol

[axpoems]

Resolves #1491

This PR corresponds to the change in the protocol and implements the proposal made in the issue.

A mention on system messages.
I have tested different options for that, and what I present here is what I think is a good solution. However, I would like to discuss this subject further.

I will make follow-up PRs to improve the UI alongside the already discussed changes in different issues.

[HenrikJannsen]

Looks great. Feels much better now!

BisqEasyBuyerAsMakerProtocol/BisqEasyBuyerAsTakerProtocol are missing the 2nd option of the order of btc address/fiat payment account data exchange.
It still works as it uses the event queue in the FSM (where the previously received message gets picked up), but I think its better to explicitly layout the valid variants of the protocol flow. Not sure if that would work in the case when users go offline in between as we do not persist the event queue... The event queue is more for race conditions with out of order events/messages (I think we do not have that in Bisq Easy, but might become a issue in more complex protocols).

Do you have any feedback or improvement suggestion to the FSM protocol definition? Was it easy for you to change the protocol?
We have duplicated code in the 2 pairs of protocols. I tried to move code to base classes/interfaces to avoid those duplications but it felt less transparent and harder to reason about the protocol flow, so I considered the cod duplication the lesser evil. Do you have any idea for improving that?

What about keeping the UI classes with the trade step numbers synced? SellerState3-> SellerState3a and SellerState4->SellerState3b?

The icon for completed looks a bit like a V. Maybe use a more common checkmark icon or differentiate even more from the 1-2-3 steps?

[HenrikJannsen]

ACK

[axpoems]

BisqEasyBuyerAsMakerProtocol/BisqEasyBuyerAsTakerProtocol are missing the 2nd option of the order of btc address/fiat payment account data exchange.

Thanks for spotting this. I have added it in #1569.

Do you have any feedback or improvement suggestion to the FSM protocol definition? Was it easy for you to change the protocol?

Yes, it was very easy to understand and straightforward to amend.
I did noticed the code duplication as you mention. It made me think if it made sense to split up seller/buyer roles into maker/taker at that level, since the core part of the protocol itself is exactly the same, the only thing that changes is the engagement part of it. But I does look clean to layout all the transitions in each file, so in that sense I agree with you that it is better to have it repeated.

What about keeping the UI classes with the trade step numbers synced? SellerState3-> SellerState3a and SellerState4->SellerState3b?

That's what I tried, maybe I missed something I don't see? I noticed as well that in phase3b and 4 the translations pairs needed to be updated, were you referring to this? I have changed this in #1569.
I have used this as the reference to update the UI and give the naming to the phases: #1491 (comment)

The icon for completed looks a bit like a V. Maybe use a more common checkmark icon or differentiate even more from the 1-2-3 steps?

Yes, I agree. I don't like it either, but I didn't want to spend more time on it since I want to change it. I also had in mind the second option that you mention, to leave that step out of the 1-2-3. Will follow-up with another PR.

[alvasw]

utACK