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

Make some trade simulation functions re-usable #5128

Merged
merged 3 commits into from
Feb 3, 2021

Conversation

ghubstan
Copy link
Contributor

Code duplication needs to be reduced as new scripts are added.

  • Refactor the createpaymentacct functions.
    The steps required to create a payment account are still displayed, but only for Alice, not Bob.

  • Treat the trade protocol simulation as an atomic function.
    This will reduce 'main' simulation script size as new ones are added.

  • Move createoffer command generation to its own function.

This PRs branch was created off 01-fix-bash-syntax, and #5116 needs to be reviewed and merged before this one.

Many unnecesary braces around ${VARIABLE} references were removed.
Code duplication needs to be reduced as new scripts are added.

- Refactor the createpaymentacct functions.
  The steps required to create a payment account are still
  displayed, but only for Alice, not Bob.

- Treat the trade protocol simulation as an atomic function.
  This will reduce 'main' simulation script size as new
  ones are added.
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

A bit odd indeed with the atomic function to perform the full trade, but it does what it should in this case.

@sqrrm sqrrm merged commit 5278973 into bisq-network:master Feb 3, 2021
@ghubstan
Copy link
Contributor Author

ghubstan commented Feb 3, 2021

utACK

A bit odd indeed with the atomic function to perform the full trade, but it does what it should in this case.

I wanted the calling scripts to be simpler.
I broke various protocol steps into their own functions -- waitfortradedepositpublished, waitfortradedepositconfirmed, waitfortradepaymentsent, etc. -- in the next PR. That made executetrade a little smaller, but still atomic.

@ghubstan ghubstan deleted the 02-refactor-simulation-scripts branch February 3, 2021 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants