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

feat: Orderbook #422

Merged
merged 11 commits into from
Aug 2, 2022
Merged

feat: Orderbook #422

merged 11 commits into from
Aug 2, 2022

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Jul 22, 2022

Closes #14. Addresses #412.

This PR introduces basic functionality for displaying the orderbook inside Jam.

Hidden behind a feature flag for now.

Attention: Also needs adaptions in the Docker image. See joinmarket-webui/jam-docker#49.

Also, important: This feature introduces functionality that will not be available for users running jm "natively". The reverse proxy is a mandatory prerequisite for this to work as expected. If jam is not served from the reverse proxy, a user will see a generic message Error while loading the orderbook. Your current local setup might not support fetching the orderbook. Reason: <potential reason>.

The functionality can be further improved by eventually providing following features:

These features are explicitely not included is this PR, as setting up the orderbook and making it work is already tough to review. However, expect more to come in follow-up PRs!

image

Local (slightly outdated):
image

More Data:

This can be tested in the regtest environment without further changes, as the primary JM container exposes the orderbook on port 62601. In order to test the "production" changes to the docker image, please follow the steps described in joinmarket-webui/jam-docker#49.

@theborakompanioni theborakompanioni added enhancement New feature or request concept Wild idea, or too many details unknown yet labels Jul 22, 2022
@theborakompanioni theborakompanioni self-assigned this Jul 22, 2022
@theborakompanioni theborakompanioni marked this pull request as ready for review July 25, 2022 13:20
@theborakompanioni theborakompanioni requested review from a user and dergigi July 25, 2022 13:20
@ghost
Copy link

ghost commented Jul 26, 2022

I regularly have issue where the orderbook doesn't seem to be served on port 62601. Lately, it feels like it's not working more often than it is working. I just get an empty reply from the server on port 62601. Any ideas what that could be? Can't find anything in the logs either.

@theborakompanioni
Copy link
Collaborator Author

I regularly have issue where the orderbook doesn't seem to be served on port 62601. Lately, it feels like it's not working more often than it is working. I just get an empty reply from the server on port 62601. Any ideas what that could be? Can't find anything in the logs either.

What exactly do you mean by "empty" response? Like.. no content at all? Or "just" no orders, ie. an html page including

<h1>JoinMarket Orderbook</h1>
<h2>0 orders found by 0 counterparties</h2>

?

Locally, I experience major delays sometimes before the ob-watcher starts listening on 62601. It can take up to several minutes (here ~5m):

> docker exec -it jm_regtest_joinmarket tail -f /root/.joinmarket/logs/obwatch_stdout.log -n 200
2022-07-26 03:17:41,153 [DEBUG]  rpc: getblockchaininfo []
[...]
2022-07-26 09:18:54,506 [DEBUG]  >>pubmsg !orderbook

started http server, visit http://0.0.0.0:62601/

2022-07-26 03:23:05,268 [DEBUG]  J57e6BFPSgkZfiq2 has dusty minsize, capping at 27300

(The "started http server" message has no timestamp, but is logged at the same time as the next message at 03:23:05)

If you got an empty orderbook while the secondary container is already running the maker service, it might be that JM fails to publish the offer (and subsequently fails to republish e.g. if it reconnects to irc servers). This happens in my local setup from time to time and I cannot really pinpoint to the actual problem. Anyway.. the ob-watcher should at least serve the html page.

Let me know if it is one or the other and we'll try to fix it together.

@ghost
Copy link

ghost commented Jul 26, 2022

What exactly do you mean by "empty" response? Like.. no content at all? Or "just" no orders,

Yep no content at all and then it resets the connection:

$ curl -v http://localhost:62601
*   Trying 127.0.0.1:62601...
* Connected to localhost (127.0.0.1) port 62601 (#0)
> GET / HTTP/1.1
> Host: localhost:62601
> User-Agent: curl/7.79.1
> Accept: */*
>
* Empty reply from server
* Closing connection 0
curl: (52) Empty reply from server

@theborakompanioni
Copy link
Collaborator Author

theborakompanioni commented Aug 1, 2022

Yep no content at all and then it resets the connection:

curl: (52) Empty reply from server

Could not reproduce by now.. hmm.. 🤔

Follow-up PR with sorting and filtering (still WIP): #434

@@ -99,6 +101,7 @@ export default function Earn() {
const [isWaitingMakerStart, setIsWaitingMakerStart] = useState(false)
const [isWaitingMakerStop, setIsWaitingMakerStop] = useState(false)
const [isShowReport, setIsShowReport] = useState(false)
const [isShowOrderbook, setIsShowOrderbook] = useState(false)
Copy link

Choose a reason for hiding this comment

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

That naming hurts a little but is consistent with what's already there, so 👍 😅

@ghost
Copy link

ghost commented Aug 2, 2022

Not sure what's wrong with my setup but if it works, the orderbook parsing works fine. 👌

@ghost
Copy link

ghost commented Aug 2, 2022

One improvement suggestion: Put the whole table into a container like the UTXO list to not have it stretch from edge to edge which looks a bit weird and is inconsistent with the rest of the app. But this can also be done in one of the upcoming PRs e.g. the one that introduces sorting etc.

@theborakompanioni
Copy link
Collaborator Author

Not sure what's wrong with my setup but if it works, the orderbook parsing works fine. ok_hand

I am reluctant to merge it when it cannot be verified by other devs..
Ping @joinmarket-webui/implementation-contributors or @joinmarket-webui/concept-and-design-contributors : Is anybody willing to try it and verify that everything works on your end?

One improvement suggestion: Put the whole table into a container [...]. But this can also be done in one of the upcoming PRs e.g. the one that introduces sorting etc.

Yep. Good point. Will do that in #434!

@dnlggr Would you also take a look at the docker image changes in joinmarket-webui/jam-docker#49? Highly appreciated.

@ghost
Copy link

ghost commented Aug 2, 2022

It does work sometimes so I was able to fully test it. And when it doesn't work, it's definitely not related to your changes since it's not working at the API level -- not on the UI level.

@theborakompanioni theborakompanioni merged commit 2406c04 into master Aug 2, 2022
@theborakompanioni theborakompanioni deleted the orderbook branch August 2, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concept Wild idea, or too many details unknown yet enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orderbook parsing
1 participant