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

XUD doesn't update LND's pubkey #1013

Closed
offerm opened this issue Jun 9, 2019 · 9 comments
Closed

XUD doesn't update LND's pubkey #1013

offerm opened this issue Jun 9, 2019 · 9 comments
Labels
bug Something isn't working enhancement New feature or request p2p Peer to peer networking P3 low priority

Comments

@offerm
Copy link
Contributor

offerm commented Jun 9, 2019

Looks like XUD keeps in its database the identity pubkeys of LND.
As such, replacing LND creates a problem since peers are still using the old identities.

for example:

on test3 I now have this:

xud@xud-test-3:~/scripts$ lndbtc-lncli getinfo
{
	"version": "0.6.1-beta commit=v0.6.1-beta-rc2-dirty",
	"identity_pubkey": "02043a2b3414e0a4ba8b1426e5a33b7368cd6bdfb2bce97f00d78fb5e18af6003f",
	"alias": "LND for BTC on 10002/10012",
	"num_pending_channels": 0,
	"num_active_channels": 0,
	"num_inactive_channels": 0,
	"num_peers": 0,
	"block_height": 1981,
	"block_hash": "05910bfd7c95bd052ac6d68e0876b80190d3ff7ffde33e04d083540ad46f6588",
	"best_header_timestamp": 1560093417,
	"synced_to_chain": true,
	"testnet": false,
	"chains": [
		{
			"chain": "bitcoin",
			"network": "simnet"
		}
	],
	"uris": null
}

but on test2 I see this

xud@xud-test-2:~$ xucli listpeers
{
  "peersList": [
    {
      "address": "xud1.test.exchangeunion.com:8885",
      "nodePubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
      "lndPubKeysMap": [
        [
          "BTC",
          "038c0ea7e0dd31103fb16888a1afb5f046785a0e3b2f5f51f082bd961838212eb3"
        ],
        [
          "LTC",
          "027cd7ac85c2196a2002420f1316f0e3b9b5a73920cb21874fd63bb28fe778101b"
        ]
      ],
      "inbound": false,
      "pairsList": [
        "LTC/BTC",
        "WETH/BTC"
      ],
      "xudVersion": "1.0.0-alpha.11",
      "secondsConnected": 191,
      "raidenAddress": "0x7ed0299Fa1ADA71D10536B866231D447cDFa48b9"
    },
    {
      "address": "xud3.test.exchangeunion.com:8885",
      "nodePubKey": "03fd337659e99e628d0487e4f87acf93e353db06f754dccc402f2de1b857a319d0",
      "lndPubKeysMap": [
        [
          "BTC",
          "030c2ffd29a92e2dd2fb6fb046b0d9157e0eda8b11caa0e439d0dd6a46a444381c"
        ],
        [
          "LTC",
          "03301fd51f87728426918f284174d7d8d324585106f190276f4546f85ed6de123e"
        ]
      ],
      "inbound": false,
      "pairsList": [
        "LTC/BTC",
        "WETH/BTC"
      ],
      "xudVersion": "1.0.0-alpha.11",
      "secondsConnected": 191,
      "raidenAddress": "0x283EAD0338f8C202b996146d77a542fDfB60d437"
    },
    {
      "address": "92.191.159.161:56294",
      "nodePubKey": "025598976b93fd07c30ff5e98ae3ff593098068cb9c921a799956914f4a593bfb2",
      "lndPubKeysMap": [],
      "inbound": true,
      "pairsList": [
        "LTC/BTC",
        "BTC/LTC"
      ],
      "xudVersion": "1.0.0-alpha.11",
      "secondsConnected": 189,
      "raidenAddress": ""
    }
  ]
}
@offerm
Copy link
Contributor Author

offerm commented Jun 9, 2019

Might be that an old XUD process was still running.

@kilrau
Copy link
Contributor

kilrau commented Jun 10, 2019

Very good point! I believe "caching" swap client addresses (lnd URIs and raiden addresses) locally is legit, but we definitely need a (pull) update mechanism:

These are the situations I can think of where we should pull new swap client addresses from a peer:

  • on every connect <node_uri>
  • on openchannel (Channel management #909)
  • on swaps failing because swap client can't be reached, but peers are connected on xud level

Which ones are missing? Which ones are we doing already? @sangaman @erkarl

I'd categorize this as post-1.0.0 hardening though.

@kilrau kilrau added this to the post-1.0.0 milestone Jun 10, 2019
@kilrau kilrau added enhancement New feature or request p2p Peer to peer networking labels Jun 10, 2019
@sangaman
Copy link
Collaborator

sangaman commented Jun 10, 2019

We don't actually store or cache lnd pub keys between sessions. We request them anew each time we connect (or reconnect). I'm thinking this is probably indeed a case of an old xud process or something along those lines.

I think we can close this issues unless there are any lingering concerns.

@kilrau kilrau removed this from the post-1.0.0 milestone Jun 11, 2019
@offerm
Copy link
Contributor Author

offerm commented Jun 13, 2019

I had this problem again.

@sangaman please have another look:

  • are we keeping LND pubkey in database?
  • do we refresh them on every LND connection?

@sangaman
Copy link
Collaborator

We definitely don't keep lnd pubkeys anywhere in the database. Can you describe exactly what is happening? When did the lnd pubkey change? While xud was running?

Every time xud verifies a connection to lnd it calls GetInfo and saves the lnd pub key to share with peers it connects to. It should refresh (and notify its peers of the new pub key) if lnd is shuts down and restarts with a different pub key, but this is not something I've personally tested in a while and there is no simulation test for it currently.

@kilrau
Copy link
Contributor

kilrau commented Jun 17, 2019

if lnd is shuts down and restarts with a different pub key, but this is not something I've personally tested in a while and there is no simulation test for it currently.

added issue to add this to the simulation tests: #1040

@kilrau kilrau changed the title XUD stores LND's identity_pubkey XUD should update LND's pubkey Aug 29, 2019
@kilrau kilrau added the bug Something isn't working label Oct 9, 2019
@kilrau
Copy link
Contributor

kilrau commented Oct 9, 2019

I'd say we have to invest some time trying to reproduce this, if not close.

@kilrau kilrau closed this as completed Oct 9, 2019
@kilrau kilrau reopened this Oct 9, 2019
@kilrau kilrau added P3 low priority P2 mid priority and removed P3 low priority labels Oct 9, 2019
@kilrau
Copy link
Contributor

kilrau commented Oct 10, 2019

Check for TODO in code to broadcast node state update packet if nodepubkey is detected

@kilrau kilrau added P3 low priority and removed P2 mid priority labels Oct 10, 2019
@kilrau kilrau changed the title XUD should update LND's pubkey XUD doesn't update LND's pubkey Jan 15, 2020
@kilrau
Copy link
Contributor

kilrau commented Apr 4, 2020

Will be handled by #1040 (comment) in future.

@kilrau kilrau closed this as completed Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request p2p Peer to peer networking P3 low priority
Projects
None yet
Development

No branches or pull requests

3 participants