XU networks support #827

moshababo · 2019-03-12T15:42:44Z

Closes #781.

This required less changes then I expected, since we're not directly communicating with the underlying blockchains clients, but only with the 2nd layer clients. It might require more work on the docker config though (@reliveyy).

each node is now associated with a specific XU network
mainnet & testnet seed nodes are empty for now
added mapping between XU network to LN network config
XU network can be selected using n arg (-n=testnet for example). @kilrau let me know if you want me to change that to be used as a boolean signal, e.g. --testnet

BREAKING CHANGE: nodes database table scheme.

kilrau · 2019-03-12T17:18:35Z

Great!! Indeed I would say --testnet-style command line args are more common and therefore preferable.

# Conflicts: # lib/Config.ts # test/p2p/sanity.spec.ts

moshababo · 2019-03-14T20:55:49Z

@kilrau done.

sangaman

Sorry for not reviewing or discussing this change sooner but I finally went through this PR and the associated issue closely. I'll probably need to review again with a fresh set of eyes since this touches a lot of things. A couple of points in addition to the comments below:

I think it may make sense to use separate databases for each network, rather than just differentiating the nodes by network in the nodes table. The reason being that each network is entirely distinct from one another, and data that applies to one network is unrelated to data for another network. As an example, we wouldn't want testnet data contaminating/bloating the mainnet database.
I don't fully understand the need for the lnNetwork in addition to the xu network. Aren't these always going to be the same?
I sort of liked the way the commands arg for network was before with -n, although I get the appeal of being able to write out --testnet and such so I'm ok with leaving it if that's what others prefer. However it differs from the way you'd specify the network in the config file, which is still like network = "simnet" (and where having to write out testnet = true is a little clunkier). A neater solution might be to convert the boolean arguments to a string network argument within the xud bin file before passing it to the xud server code. You can also use yargsbuilt-inconflicts` API so that yargs will throw an error if a user tries to pass in more than one network value rather than having to check for that yourself. It would look like this:

.conflicts('mainnet', ['testnet', 'simnet', 'regtest'])
.conflicts('testnet', ['mainnet', 'simnet', 'regtest'])
// same for last two

bin/xud

lib/Config.ts

lib/constants/enums.ts

lib/Config.ts

lib/p2p/Peer.ts

kilrau · 2019-03-24T18:58:35Z

I think it may make sense to use separate databases for each network
data that applies to one network is unrelated to data for another network. As an example, we wouldn't want testnet data contaminating/bloating the mainnet database.

Agree!

moshababo · 2019-03-26T07:05:27Z

I think it may make sense to use separate databases for each network, rather than just differentiating the nodes by network in the nodes table. The reason being that each network is entirely distinct from one another, and data that applies to one network is unrelated to data for another network. As an example, we wouldn't want testnet data contaminating/bloating the mainnet database.

Sounds good, will do.

I don't fully understand the need for the lnNetwork in addition to the xu network. Aren't these always going to be the same?

See my comment here.

I sort of liked the way the commands arg for network was before with -n, although I get the appeal of being able to write out --testnet and such so I'm ok with leaving it if that's what others prefer. However it differs from the way you'd specify the network in the config file, which is still like network = "simnet" (and where having to write out testnet = true is a little clunkier). A neater solution might be to convert the boolean arguments to a string network argument within the xudbin file before passing it to the xud server code. You can also use yargsbuilt-inconflicts` API so that yargs will throw an error if a user tries to pass in more than one network value rather than having to check for that yourself. It would look like this:

Thanks, i’ll check it out.

ghost · 2019-03-26T15:19:28Z

I think it may make sense to use separate databases for each network

You could also consider using a separate schema per network if having separate DBs is too much work.

# Conflicts: # lib/p2p/Peer.ts # lib/p2p/Pool.ts # lib/p2p/errors.ts # test/p2p/sanity.spec.ts

sangaman

Looks great, I'd like to test this manually a bit before merging since it's a big PR but the code looks good. Thanks for making those changes.

sangaman · 2019-04-03T08:28:18Z

lib/Config.ts

          break;
      }
    }
  }

  private getDefaultDbPath = () => {
-    return path.join(this.xudir, 'xud.db');
+    return path.join(this.xudir, `xud-${this.network}.db`);


Just a thought but we can keep the xud.db filename for mainnet and only specify a network if it's not mainnet, so this would be something like:

this.network === XuNetwork.MainNet ? 'xud.db' : `xud-${this.network}.db`

I wouldn't mind it being more explicit for each network.

Yes, i'd rather have it be more explicit.

ghost · 2019-04-06T08:31:53Z

lib/Config.ts

          break;
      }
    }
  }

  private getDefaultDbPath = () => {
-    return path.join(this.xudir, 'xud.db');
+    return path.join(this.xudir, `xud-${this.network}.db`);


I wouldn't mind it being more explicit for each network.

ghost · 2019-04-06T08:40:10Z

lib/p2p/seeds/simnet.ts

@@ -0,0 +1,16 @@
+import * as db from '../../db/types';
+
+export default [


Really nice to have these seeds on a network level :).

ghost · 2019-04-06T08:43:02Z

Also, needs a rebase.

# Conflicts: # lib/db/DB.ts # lib/p2p/Peer.ts # lib/p2p/Pool.ts # test/unit/DB.spec.ts

moshababo · 2019-04-06T18:25:14Z

@sangaman I wasn't sure if you still want to manually test this, so i'll leave it open for you to merge.

sangaman · 2019-04-09T15:43:45Z

Thanks. I'm merging this now and will do #869 shortly after since they both involve breaking db changes.

XU networks support

6606209

moshababo requested review from a user and sangaman March 12, 2019 15:42

kilrau requested a review from reliveyy March 12, 2019 17:10

reliveyy approved these changes Mar 13, 2019

View reviewed changes

Specify non-simnet network via boolean param

d337f88

ghost assigned moshababo Mar 14, 2019

ghost added the in progress label Mar 14, 2019

moshababo added 2 commits March 14, 2019 22:49

Merge branch 'master' into xunetworks

a527ecd

# Conflicts: # lib/Config.ts # test/p2p/sanity.spec.ts

Merge fix

5579ced

sangaman reviewed Mar 23, 2019

View reviewed changes

moshababo added 9 commits March 30, 2019 19:41

Merge branch 'master' into xunetworks

91aa948

# Conflicts: # lib/p2p/Peer.ts # lib/p2p/Pool.ts # lib/p2p/errors.ts # test/p2p/sanity.spec.ts

Use separate database per network

5ca50f2

Config

a2f9966

Add simnet to xud cmd args, detect conflicts via yargs

6bb2734

XUNetwork -> XuNetwork

4e2b3b1

Refactor magicValsXuNetwork

659b3af

readonly properties

3ce2d4f

lint

ab86316

Remove lnNetwork

84ae104

moshababo requested a review from sangaman March 30, 2019 19:15

sangaman approved these changes Apr 3, 2019

View reviewed changes

sangaman added the breaking A breaking or non-backwards compatible change label Apr 3, 2019

kilrau mentioned this pull request Apr 4, 2019

feat(database): enable WETH trading pair and currency by default #856

Closed

ghost approved these changes Apr 6, 2019

View reviewed changes

moshababo added 2 commits April 6, 2019 21:08

Merge branch 'master' into xunetworks

de43f09

# Conflicts: # lib/db/DB.ts # lib/p2p/Peer.ts # lib/p2p/Pool.ts # test/unit/DB.spec.ts

fix test

daba9ee

sangaman merged commit a5a06e1 into master Apr 9, 2019

sangaman deleted the xunetworks branch April 9, 2019 15:49

ghost removed the in progress label Apr 9, 2019

ghost mentioned this pull request May 24, 2019

Add default currencies + pairs to xud ExchangeUnion/xud-docker#9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XU networks support #827

XU networks support #827

moshababo commented Mar 12, 2019 •

edited

Loading

kilrau commented Mar 12, 2019 •

edited

Loading

moshababo commented Mar 14, 2019

sangaman left a comment

kilrau commented Mar 24, 2019

moshababo commented Mar 26, 2019

ghost commented Mar 26, 2019

sangaman left a comment

sangaman Apr 3, 2019

ghost Apr 6, 2019

moshababo Apr 6, 2019

ghost Apr 6, 2019

ghost Apr 6, 2019

ghost commented Apr 6, 2019

moshababo commented Apr 6, 2019

sangaman commented Apr 9, 2019

		@@ -0,0 +1,16 @@
		import * as db from '../../db/types';

		export default [

XU networks support #827

XU networks support #827

Conversation

moshababo commented Mar 12, 2019 • edited Loading

kilrau commented Mar 12, 2019 • edited Loading

moshababo commented Mar 14, 2019

sangaman left a comment

Choose a reason for hiding this comment

kilrau commented Mar 24, 2019

moshababo commented Mar 26, 2019

ghost commented Mar 26, 2019

sangaman left a comment

Choose a reason for hiding this comment

sangaman Apr 3, 2019

Choose a reason for hiding this comment

ghost Apr 6, 2019

Choose a reason for hiding this comment

moshababo Apr 6, 2019

Choose a reason for hiding this comment

ghost Apr 6, 2019

Choose a reason for hiding this comment

ghost Apr 6, 2019

Choose a reason for hiding this comment

ghost commented Apr 6, 2019

moshababo commented Apr 6, 2019

sangaman commented Apr 9, 2019

moshababo commented Mar 12, 2019 •

edited

Loading

kilrau commented Mar 12, 2019 •

edited

Loading