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.

Sign up for GitHub

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

Jump to bottom

Bisq Easy protocol improvements #1569

Merged
merged 6 commits into from
Jan 18, 2024
Merged

Bisq Easy protocol improvements #1569

merged 6 commits into from
Jan 18, 2024

Conversation

axpoems
Copy link
Contributor

@axpoems axpoems commented Jan 2, 2024

Based on #1566

@axpoems axpoems mentioned this pull request Jan 2, 2024
Merged
@axpoems
Copy link
Contributor Author

axpoems commented Jan 2, 2024

Also changing the action button's text in my last commit:
image

HenrikJannsen
HenrikJannsen approved these changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

HenrikJannsen
HenrikJannsen requested changes Jan 6, 2024
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

NACK

The use case where the seller sends the account data first triggers for the buyer the change to step 2 thus he cannot send the btc address.

@axpoems axpoems force-pushed the redesign-BisqEasy-protocol-review branch from c72703d to fbb8afd Compare January 12, 2024 11:40
@axpoems axpoems requested a review from HenrikJannsen January 12, 2024 11:40
axpoems and others added 6 commits January 12, 2024 12:41
@axpoems
Add missing transitions in Buyer's BisqEasy protocol
b687a7d
@axpoems
Refactor: rename translation keys to match trade step
144b377
@axpoems
Change action button's text in BisqEasy closeTrade popup
0a5f7ee
@axpoems
Improve seller state management in Bisq Easy protocol
17dcbef 
* Remove logic to decide next step
* Add all the combinations to cover explicitly all possible states a
  seller can encounter
@axpoems
Improve buyer state management in Bisq Easy protocol
d711d25 
* Add all the combinations to cover explicitly all possible states a
  buyer can encounter
@HenrikJannsen @axpoems
Add more logging to FSM
0b83d6f
@axpoems axpoems force-pushed the redesign-BisqEasy-protocol-review branch from fbb8afd to 0b83d6f Compare January 12, 2024 11:43
HenrikJannsen
HenrikJannsen approved these changes Jan 13, 2024
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

I do not have time atm for an in depth testing/review but the PR looks good to me.
When testing all scenarios its important to also test the cases that peers are offline at any state.

alvasw
alvasw approved these changes Jan 18, 2024
View reviewed changes
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

utACK

@alvasw alvasw merged commit f45bd25 into bisq-network:main Jan 18, 2024
15 checks passed
@axpoems axpoems deleted the redesign-BisqEasy-protocol-review branch January 18, 2024 08:48
@axpoems axpoems mentioned this pull request Jan 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HenrikJannsen HenrikJannsen HenrikJannsen approved these changes

@alvasw alvasw alvasw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@axpoems @alvasw @HenrikJannsen