search

ExchangeUnion / xud-docker-api

GNU Affero General Public License v3.0
0 stars 0 forks source link

feat: add console apis #18

Closed reliveyy closed 3 years ago

reliveyy commented 3 years ago

How to test?

# install mkcert
mkcert -install
mkcert localhost 127.0.0.1 ::1 -cert-file tls.crt -key-file tls.key

sudo mkdir -p ~/.xud-docker/simnet/data/proxy
sudo cp tls.* ~/.xud-docker/simnet/data/proxy

# start xud-docker (select simnet)
bash xud.sh

# start backend
scripts/run.sh simnet

# start frontend
export REACT_APP_API_URL=https://localhost:8080
export SSL_CRT_FILE=tls.crt
export SSL_KEY_FILE=tls.key
yarn start

image

krrprr commented 3 years ago

Looks good! :+1: Just a couple of questions:

ghost commented 3 years ago 
  ![console](https://user-images.githubusercontent.com/24500376/99641833-734cd780-2a53-11eb-8982-bb77964639b1.png)

If this only happens for formatted output (fancy tables etc.) I would consider addressing it in a follow-up PR. We can also get json output with -j.

* Creating and providing the tls certificate will later on be taken care of by xud-docker, right?

@reliveyy before merging this PR - can we make it testable from xud-docker? Something like bash xud.sh -b xud-ui-console?

reliveyy commented 3 years ago

can we make it testable from xud-docker? Something like bash xud.sh -b xud-ui-console?

@erkarl Yep. Will do.

reliveyy commented 3 years ago

@krrprr

Does the response have to look like this atm or is there something wrong on my side (Ubuntu + Firefox)?

Could it be the missing Unicode border characters in your browser?

Creating and providing the tls certificate will later on be taken care of by xud-docker, right?

I think I will make this TLS feature optional before I present it on xud-docker side.

krrprr commented 3 years ago

Could it be the missing Unicode border characters in your browser?

Not sure but it's the default settings, so we have to consider that the users may encounter this as well.

I think I will make this TLS feature optional

Does it mean there would be an option going back to non-encrypted HTTP between the UI and API? Or some specific feature about the TLS?

reliveyy commented 3 years ago

Does the response have to look like this atm or is there something wrong on my side (Ubuntu + Firefox)?

@krrprr Sorry it's not your browser problem. I used another socket.io event for output (console-id-output) and it seems fixed the problem mostly. But we could still see some random ��� in getinfo response. I remember this happens in Travis-CI output. I guess it is because some unexpected truncated data. So I enlarge the read buffer size from 1024 to 65536 but it doesn't solve this problem totally.

kilrau commented 3 years ago

bash xud.sh -b xud-ui-console do we have this by now?

reliveyy commented 3 years ago

bash xud.sh -b xud-ui-console do we have this by now?

Sorry. Not yet but soon. @kilrau

kilrau commented 3 years ago

Also missing closing statement for https://github.com/ExchangeUnion/xud-ui/issues/29 I think

krrprr commented 3 years ago

Beautiful! The output looks very nice (checked Firefox and Brave): console-nice

Looking forward to the yet unimplemented features.

kilrau commented 3 years ago
krrprr commented 3 years ago

Added the help command.

Title of the console view and resizing implemented in https://github.com/ExchangeUnion/xud-ui-dashboard/pull/12

What is left to be done in this PR:

As per discussed, the other unimplemented commands are accessible via the UI and do not have to be implemented in the console right away.

raladev commented 3 years ago
kilrau commented 3 years ago

help contains commands that does not work in xud ui console :(

Which? I believe all commands should work.

also it does not contain new xud commands, so, maybe lets bump prio of ExchangeUnion/xud-docker#680 to P1 ?

Hmm, the real problem is that this all-in-one help output is hardcoded. Let's talk it through with @reliveyy once the binary launcher is out and usable, but not before

raladev commented 3 years ago

Which? I believe all commands should work.

deposit/withdraw/status (they were mentioned in first message) + bitcoin-cli/litecoin-cli return command not found instead on container not found.

krrprr commented 3 years ago

ExchangeUnion/xud-docker#794 (comment) - second point still exists;

Sub-point of second point: current utils implementation prints help if user prints incorrect command, but in ui console it does not work, would be good to have same behavior. Screenshot from 2020-11-26 17-46-06

Let's move those to a follow-up issue.

I cant copy data from console, even using CTRL+C :*( (native app problem)

Yes, only appears in the native app. Works in browser. We can probably implement the ctrl/cmd + C functionality. But rather in a separate issue as it is not directly linked to the console.

console still contains broken symbols

Tried increasing the buffer array size (like it was done when previously fixing the issue) but it did not help. Think it's currently not worth the time it would take unless @reliveyy has some ideas.

boltzcli command works in different way in xud-ui. it looks like it uses btc as a hidden parameter, as a result i cant get info about ltc swaps

This is actually critical and needs to be solved before the release but just by looking at the code I honestly haven't figured out how to solve this. @reliveyy any hints?

reliveyy commented 3 years ago

boltzcli command works in different way in xud-ui. it looks like it uses btc as a hidden parameter, as a result i cant get info about ltc swaps

@raladev @krrprr I've looked into boltz.py source code in xud-docker and I found out boltzcli is actually a wrapper script in that image.