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

Shoehorn channel-opening flow into first-trade flow #1974

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

luckysori
Copy link
Contributor

@luckysori luckysori commented Feb 7, 2024

Fixes #1793.

channel-opening-flow-v0.mp4

TODOs:

  • Let user trade normally after channel-opening trade.
  • Fix failing UI test. Adapted it with the new flow.
  • Add another UI test for when the channel is already established.
  • Fix confirmation slider bug when confirming channel configuration.
  • Close all bottom sheets when first trade succeeds.
  • Add e2e test for submit_channel_opening_order API.
  • Use TradeValues to calculate coordinator margin.
  • Set channel-opening fee to 0. When we are ready to charge for opening channels we can read this information from the liquidity options through the backend.
  • Do not assume coordinator's proportional liquidity in UI and use leverage in TradeConstraints instead.
  • Cap coordinator's maximum liquidity per channel.
  • Fix webapp if affected.
  • Pop all bottom sheets once the channel-opening trade is confirmed.
  • Include funding transaction fees, and fee reserve, in channel configuration screen.

@luckysori luckysori requested review from bonomat and holzeis February 7, 2024 05:52
@luckysori luckysori self-assigned this Feb 7, 2024
@luckysori luckysori force-pushed the feat/choose-channel-size branch from 8bd7275 to eceed92 Compare February 7, 2024 05:56
@luckysori luckysori changed the title Shoehorn channel-opening flow into first trade flow Shoehorn channel-opening flow into first-trade flow Feb 7, 2024
@luckysori luckysori force-pushed the feat/choose-channel-size branch 5 times, most recently from 492ef32 to 5c78048 Compare February 7, 2024 07:08
@bonomat
Copy link
Contributor

bonomat commented Feb 7, 2024

The flow looks awesome

@holzeis
Copy link
Contributor

holzeis commented Feb 7, 2024

@luckysori Quick question without having looked at the PR yet. Will this change have an impact on the webapp?

@luckysori
Copy link
Contributor Author

luckysori commented Feb 7, 2024

@luckysori Quick question without having looked at the PR yet. Will this change have an impact on the webapp?

Great question. My intention was to not change the webapp at the moment, but I didn't do it properly. See: https://github.com/get10101/10101/pull/1974/files#diff-8923b602c94d37f595a2e3736668c11f089c092ba9f4774d4b2ce305619c0b35R221.

I set the optional ChannelOpeningParams to None, but that will always imply that the channel is already open. I will play around with it tomorrow to make sure that it works like it did before this PR. It shouldn't be much work.

@luckysori luckysori force-pushed the feat/choose-channel-size branch from aa44a29 to ef790c3 Compare February 9, 2024 05:05
@luckysori
Copy link
Contributor Author

@luckysori Quick question without having looked at the PR yet. Will this change have an impact on the webapp?

Great question. My intention was to not change the webapp at the moment, but I didn't do it properly. See: https://github.com/get10101/10101/pull/1974/files#diff-8923b602c94d37f595a2e3736668c11f089c092ba9f4774d4b2ce305619c0b35R221.

I set the optional ChannelOpeningParams to None, but that will always imply that the channel is already open. I will play around with it tomorrow to make sure that it works like it did before this PR. It shouldn't be much work.

I think I fixed it now, but I can't test it because I get an SSL error when running the webapp locally.

@luckysori luckysori force-pushed the feat/choose-channel-size branch 3 times, most recently from abf8941 to bc852ad Compare February 9, 2024 12:57
@holzeis holzeis force-pushed the feat/choose-channel-size branch 4 times, most recently from a828782 to 1f3e843 Compare February 10, 2024 19:55
@holzeis
Copy link
Contributor

holzeis commented Feb 10, 2024

Simulator.Screen.Recording.-.iPhone.13.Pro.-.2024-02-10.at.20.58.10.mp4

@holzeis
Copy link
Contributor

holzeis commented Feb 10, 2024

@bonomat @luckysori please have a look at #1974 (comment) and let me know what you think. I took it a bit far (which I probably shouldn't have 🙈), but I got too far to stop now 😅 .

@luckysori
Copy link
Contributor Author

luckysori commented Feb 11, 2024

@bonomat @luckysori please have a look at #1974 (comment) and let me know what you think. I took it a bit far (which I probably shouldn't have 🙈), but I got too far to stop now 😅 .

I think it looks good, although I would make sure the e2e tests pass before merging.

I can't seem to find the diff between my version and yours, which is a bit unfortunate.

@holzeis holzeis force-pushed the feat/choose-channel-size branch 4 times, most recently from a3e6913 to 07581c9 Compare February 13, 2024 14:49
In particular, how to run the `webapp` with and without TLS. And where
to find the website based on that.
@holzeis holzeis force-pushed the feat/choose-channel-size branch from 07581c9 to 782843e Compare February 13, 2024 14:56
@holzeis
Copy link
Contributor

holzeis commented Feb 13, 2024

run the e2e tests locally! everything passed. going ahead with the merge.

@holzeis holzeis enabled auto-merge February 13, 2024 15:02
@holzeis holzeis added this pull request to the merge queue Feb 13, 2024
Merged via the queue into main with commit 255eedc Feb 13, 2024
@holzeis holzeis deleted the feat/choose-channel-size branch February 13, 2024 15:12
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 this pull request may close these issues.

Use liquidity options after opening the first position
3 participants