feat: use xud default ports like 8886, 18886, 28886 #395
Conversation
| sed -i "s/<onion_address>/$XUD_ONION_ADDRESS/g" ~/.xud/xud.conf | ||
| } | ||
| echo "xud.conf not found - creating a new one..." | ||
| cp /app/sample-xud.conf $XUD_CONF |
There was a problem hiding this comment.
Why??? You shouldn't create any xud.conf file because this would overwrite defaults in future when we change them on xud side! We always want to use xud defaults so you can't hardcode them.
You can pull the current xud defaults from the sample config files here: https://github.com/ExchangeUnion/xud-docker/tree/master/images/utils/launcher/config, I am maintaining them there.
There was a problem hiding this comment.
We create xud.conf to config xud for a long history. And this config file is updated when xud image updated. And I do modify sample-xud.conf from xud source. I'm not sure about your concerning.
There was a problem hiding this comment.
Just don't create it, it will cause troubles overwriting things. We had that in multiple places before. xud has default values without any config and we assume these.
There was a problem hiding this comment.
We used to use command line options to customize xud configuration at the very beginning like other nodes. But later we changed to xud.conf file. So we are going to change it back? I remember we use this xud.conf becuase it's easy to change xud configuration by just editing host data/xud/xud.conf file and restarting xud container. And in case of xud container upgrading with old xud.conf file, we do override this file everytime. But if you specify --xud.preserve-config you could remain the old config file.
There was a problem hiding this comment.
Here is a full list of what we changed from sample-xud.conf:
networkhttp.hosthttp.portlnd.BTC.hostlnd.BTC.certpathlnd.LTC.hostlnd.LTC.portlnd.LTC.certpathp2p.addressesp2p.portp2p.torp2p.torportraiden.hostraiden.keystorepathrpc.hostrpc.port
I think we can leave http.port, p2p.port and rpc.port out becuase there is no change to their default value now.
There was a problem hiding this comment.
Since these 3 options are in sample-xud.conf. And their values are for simnet. If we want to use default values, we need to delete these 3 options from xud.conf. So I'll leave it as it was.
|
Ok all clarified, I'll test once travis tests passed. |
|
Currently the I think it is related to the lnd error |
Resolved. It's becuase I forgot to change |
|
This depends on ExchangeUnion/xud#1462. Can you please run it with this xud branch and see if it works? @reliveyy @peartobear |
|
It is picking up port
|
Yeah that is correct.. Mainnet - 8886, Testnet 18886, Simnet - 28886. In order for it to work, changes on xud side were required: ExchangeUnion/xud#1462. Since testing without this being merged is complicated, let's wait for @sangaman to get this in first. Once it's in and travis build new images you can test again. |
|
Are we good to merge ExchangeUnion/xud#1462? It shouldn't cause any issues if we are setting the ports explicitly via xucli, setting I do wonder if it makes sense to also use network-specific ports for the grpc web proxy instead of only 8080, but since we're not using that for the time being we can revisit it later. |
|
Needs @erkarl to approve to unblock the merge button. |
images/utils/launcher/node/xud.py
Outdated
| @@ -40,7 +40,14 @@ class Xud(Node): | |||
| def __init__(self, name, ctx): | |||
| super().__init__(name, ctx) | |||
|
|
|||
| self._cli = "xucli" | |||
| self._cli = None | |||
There was a problem hiding this comment.
This is not needed anymore since ExchangeUnion/xud#1462. Please remove @reliveyy
d31be79 to
81519bd
Compare
Co-Authored-By: Kilian Rausch ⚡️ <kilian.rausch@gmail.com>
This PR let xud image use ports 8885, 8886, 8887, 8080 for mainnet; 18885, 18886, 18887, 8080 for testnet; and 28885, 28886, 28887, 8080 for simnet.
Closes #380
Reopen previous PR #383