pyln.client gossmap support #4582

rustyrussell · 2021-06-04T04:52:39Z

This is a minimal implementation; it probably wants some more helpers to make it less painful to use.

m-schmoock · 2021-06-04T10:02:23Z

Thanks, will have a look..

cdecker · 2021-06-21T09:13:40Z

Rebased on top of master to see if the CI failure was caused by the merge-base.

cdecker · 2021-06-21T10:37:48Z

I have absolutely no idea how changes in the pyln libs can cause peer_exp_wiregen.h to be regenerated... Tried it a couple of times locally and could reproduce.

m-schmoock · 2021-06-25T16:15:57Z

@cdecker
Strange. Found that in the logs too:
(it trieds to get Gossmap which pulls in pyln.spec.bolt7 import (channel_announcement, channel_update, ..)
Still I dont get the wiregen update...

+ pip3 install --user -U -r requirements.txt
Processing ./contrib/pyln-client
  DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
   pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
    ERROR: Command errored out with exit status 1:
     command: /opt/hostedtoolcache/Python/3.6.13/x64/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-53ma3np9/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-53ma3np9/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-usrh6h8g
         cwd: /tmp/pip-req-build-53ma3np9/
    Complete output (9 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-53ma3np9/setup.py", line 2, in <module>
        from pyln import client
      File "/tmp/pip-req-build-53ma3np9/pyln/client/__init__.py", line 3, in <module>
        from .gossmap import Gossmap
      File "/tmp/pip-req-build-53ma3np9/pyln/client/gossmap.py", line 3, in <module>
        from pyln.spec.bolt7 import (channel_announcement, channel_update,
    ModuleNotFoundError: No module named 'pyln.spec'
    ----------------------------------------
WARNING: Discarding file:///home/runner/work/lightning/lightning/contrib/pyln-client. Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

rustyrussell · 2021-06-28T05:24:42Z

Actually, experimental builds always regen (we don't ship the exp_ files).

It's the pip install failure that causes it to abort, not installing mako, and get upset.

fiatjaf · 2021-07-07T10:58:51Z

Does this mean we can add new channels to the gossip map while gossipd is running and they will be picked up automatically?

m-schmoock

A thing I noted...

contrib/pyln-client/setup.py

m-schmoock · 2021-08-18T09:51:29Z

@rustyrussell
Sorry for not being able to run this yet. This is what I did:
I manged to install bolt specs manually by going toi their directories, i.e.

cd contrib/pyln-spec/bolt1
python3 setup.py install --user

This was only doable after upgrading to newest pip sudo pip install --upgrade pip. Still the command errors in the end, but the bolt package installs anyway:

....
Copying pyln_bolt1-1.0.1.137-py3.9.egg to /home/will/.local/lib/python3.9/site-packages
pyln-bolt1 1.0.1.137 is already the active version in easy-install.pth

Installed /home/will/.local/lib/python3.9/site-packages/pyln_bolt1-1.0.1.137-py3.9.egg
Processing dependencies for pyln-bolt1==1.0.1.137
Searching for pyln.proto>=0.9.3
Reading https://pypi.org/simple/pyln.proto/
No local packages or working download links found for pyln.proto>=0.9.3
error: Could not find suitable distribution for Requirement.parse('pyln.proto>=0.9.3')

This can be ignored as I am able to install pyln-proto also manually by source. Not nice though. What I find strange that under the quoted URL https://pypi.org/simple/pyln.proto/ there is a version 0.9.3 °_° ?

When I then run the gossmap testcase it still complains about pyln.spec module missing maybe because I only have the bolts and not the pyln.spec 'parent module' ?:

../../cln/contrib/pyln-client/pyln/client/__init__.py:3: in <module>
    from .gossmap import Gossmap, GossmapNode, GossmapChannel, GossmapNodeId
../../cln/contrib/pyln-client/pyln/client/gossmap.py:3: in <module>
    from pyln.spec.bolt7 import (channel_announcement, channel_update,
E   ModuleNotFoundError: No module named 'pyln.spec'

m-schmoock · 2021-08-21T13:01:33Z

@rustyrussell
Ok, I got pyln-spec 'working' by manually copying it into my python environment. The setup.py does not work for me or I don't know how...

m-schmoock · 2021-08-22T15:50:32Z

@rustyrussell
I Added some additions by PR rustyrussell#7 into your repo

m-schmoock · 2021-08-26T08:45:39Z

contrib/pyln-client/pyln/client/gossmap.py

+
+class GossmapHalfchannel(object):
+    """One direction of a GossmapChannel."""
+    def __init__(self, channel: 'GossmapChannel', direction: int,


Didn't know that forward declaration was possible. Had to look it up myself to know why you put that in quotes :D

m-schmoock · 2021-08-26T09:32:12Z

Again I added more stuff in coop/wip/pr rustyrussell#8

Also:

Does this mean we can add new cannels to the gossip map while gossipd is running and they will be picked up automatically?

@rustyrussell you thumb-upped this question. Are we really going to push back changes to gossmap back to the gossipd daemon?

m-schmoock · 2021-08-26T13:28:28Z

Question:
Do we want the structure like currently implemented: Gossmap -> list[node] -> list[channel] -> [half_channel_1, half_channel_2]
Or do we want it more like flat structure of lightning-cli listchannels list[half_channel] or do we want both by keeping current and adding iterator functions?

rustyrussell · 2021-09-02T05:10:05Z

Again I added more stuff in coop/wip/pr rustyrussell#8

Also:

Does this mean we can add new cannels to the gossip map while gossipd is running and they will be picked up automatically?

@rustyrussell you thumb-upped this question. Are we really going to push back changes to gossmap back to the gossipd daemon?

No, I misread! You can use addgossip to send gossip messages to gossipd, and it will add it to the store if it accepts them though.

rustyrussell · 2021-09-02T05:14:34Z

Question:
Do we want the structure like currently implemented: Gossmap -> list[node] -> list[channel] -> [half_channel_1, half_channel_2]
Or do we want it more like flat structure of lightning-cli listchannels list[half_channel] or do we want both by keeping current and adding iterator functions?

The current structure is more useful: the JSON style of listchannels makes for a lot of redundancy.

Signed-off-by: Rusty Russell <[email protected]>

They're generally useful. Signed-off-by: Rusty Russell <[email protected]>

…ilable. Signed-off-by: Rusty Russell <[email protected]>

This is more efficient than converting them all to Pubkeys: about 3.8 seconds vs 5.4 seconds. Usually treating them as raw bytes is what we want anyway. [ Fixup by Michael Schmoock <[email protected]> ] Signed-off-by: Rusty Russell <[email protected]>

rustyrussell · 2021-09-07T04:10:28Z

Updated on top of #4763

vincenzopalazzo

only a small nit, by the way, excited to have the gossip map in here.

Fancy stuff can happens.

vincenzopalazzo · 2021-09-07T08:10:40Z

contrib/pyln-client/requirements.txt

@@ -1 +1,2 @@
 recommonmark>=0.7.*
+pyln-bolt7


It is better to fix the version here with >= latest one? Maybe pip can take the decision to use the old one if in the system there is an older version?

So we are putting here some dependencies that can cause errors in production, I will preferer to have a pyln-bolt7>=1.0.2.186

Hm, theres something about the pyln-spec dependency.

I put the gossip_store_channel_amount into the bolt7 spec package, not sure if I was supposed to, see 32cb69e. but the one that will be installed with pip is still the old one which results in:

ImportError: cannot import name 'gossip_store_channel_amount' from 'pyln.spec.bolt7'

So we need to put bolt7 >= 1.0.2.187 (last digit incremented) since the 186 does not have the gossip_store_channel_amount in its csv

187 is not released yet, but it will be the one with the changes we made in this branch... not sure how we handle something like this

I did the pypi upload, even though we hadn't merged, because I needed it to fix lnprototest...

Oh, I missed that change, it's completely wrong!

I will fix that a different way tomorrow...

m-schmoock · 2021-09-07T09:34:23Z

Added another testcase for the half channel stuff... no bugs detected.

This reads the `gossip_store_channel_amount` that always follows the `channel_announcement` messages. Therefore it uses an internal variable _last_scid to know what channel has been added last time.

Do not mix bytes and GossmapNodeId when accessing Gossmap.nodes dicts. Therefore the definion got GossmapNodeId also needed to be pulled to the beginning of the file.

Mainly fixing type annotations, but some real fixes: 1. GossmapHalfchannel.from_str() should be a classmethod. 2. update_channel had weird, unusable default values (fields can't be NULL, since we use it below). [ There was one more occurence where isinstance should be used above type() == xyz comparison. -- MS ] Signed-off-by: Rusty Russell <[email protected]>

…deletions. Suggested-by: @mschmook Signed-off-by: Rusty Russell <[email protected]>

This is likely easier for programmers and does not use more mem as we already load all this.

… __eq__

also improves test coverage Changelog-Added: pyln-client: routines for direct access to the gossip store as Gossmap

This only happens when a deletion is added by a running gossipd, so we put a deletion at the end of the store to test it. mypy noticed that this code was nonsensical, so clearly untested. The testing noticed that making a nodeid from a string was also buggy. Signed-off-by: Rusty Russell <[email protected]>

rustyrussell · 2021-09-07T20:59:04Z

OK, I removed the change to the spec package, re-running CI now.

m-schmoock · 2021-09-07T22:01:11Z

just re-added the commit for the half channel testcase that got missed by rebasing....

m-schmoock · 2021-09-07T22:04:41Z

ACK 56cac76

rustyrussell added pyln gossmap labels Jun 4, 2021

m-schmoock mentioned this pull request Jun 15, 2021

rebalance: nits lightningd/plugins#250

Merged

cdecker force-pushed the guilt/pyln-gossmap branch from 6462df4 to 15a4bf1 Compare June 21, 2021 09:13

rustyrussell force-pushed the guilt/pyln-gossmap branch 3 times, most recently from 7c4665f to eaf5911 Compare August 16, 2021 01:16

m-schmoock reviewed Aug 16, 2021

View reviewed changes

contrib/pyln-client/setup.py Outdated Show resolved Hide resolved

rustyrussell force-pushed the guilt/pyln-gossmap branch from eaf5911 to 9955a32 Compare August 16, 2021 20:48

rustyrussell force-pushed the guilt/pyln-gossmap branch from 9955a32 to 0d16253 Compare August 26, 2021 00:41

rustyrussell marked this pull request as ready for review August 26, 2021 00:44

rustyrussell requested a review from cdecker as a code owner August 26, 2021 00:44

rustyrussell force-pushed the guilt/pyln-gossmap branch 2 times, most recently from 626a6d3 to ef38730 Compare August 26, 2021 01:10

m-schmoock reviewed Aug 26, 2021

View reviewed changes

rustyrussell force-pushed the guilt/pyln-gossmap branch from 4c761c4 to 4b6938e Compare September 2, 2021 05:08

rustyrussell force-pushed the guilt/pyln-gossmap branch from 4b6938e to 6f7b580 Compare September 2, 2021 05:13

rustyrussell and others added 6 commits September 7, 2021 13:39

pyln-client/gossmap: add a little documentation.

6bf59b4

Signed-off-by: Rusty Russell <[email protected]>

pyln-proto: expose ShortChannelId and PublicKey.

ee5d3ba

They're generally useful. Signed-off-by: Rusty Russell <[email protected]>

pyln-client/gossmap: use ShortChannelId class from pyln.proto, if ava…

5fc88a5

…ilable. Signed-off-by: Rusty Russell <[email protected]>

pyln-client/gossmap: adds GossmapHalfchannel objects

a871262

pyln-client/gossmap: adds helpers to get channels and nodes

f19c43f

rustyrussell force-pushed the guilt/pyln-gossmap branch from c5949b9 to d3b0fd4 Compare September 7, 2021 04:09

vincenzopalazzo approved these changes Sep 7, 2021

View reviewed changes

rustyrussell marked this pull request as draft September 7, 2021 10:03

m-schmoock and others added 12 commits September 8, 2021 06:24

pyln-client/gossmap: adds channel satoshi capacity

fb56404

This reads the `gossip_store_channel_amount` that always follows the `channel_announcement` messages. Therefore it uses an internal variable _last_scid to know what channel has been added last time.

pyln-client/gossmap: extend testcase

d0d101e

pyln-client/gossmap: adds __repr__ functions

4547158

pyln-client/gossmap: Don't mix bytes and GossmapNodeId

f562e51

Do not mix bytes and GossmapNodeId when accessing Gossmap.nodes dicts. Therefore the definion got GossmapNodeId also needed to be pulled to the beginning of the file.

pyln-client/gossmap: make test gossip_store include channel_updates, …

e715302

…deletions. Suggested-by: @mschmook Signed-off-by: Rusty Russell <[email protected]>

pyln-client/gossmap: have channels link to their nodes instead of id

6cbdc5a

This is likely easier for programmers and does not use more mem as we already load all this.

pyln-client/gossmap: start internal function names with underscore _

56babd2

pyln-client/gossmap: let half channel have source and destination

67e62fc

pyln-client/gossmap: make node and node_id comparable with __lt__ and…

54a9796

… __eq__

pyln-client/gossmap: init GossmapNode and Id also with hexstring

5187a1e

also improves test coverage Changelog-Added: pyln-client: routines for direct access to the gossip store as Gossmap

rustyrussell force-pushed the guilt/pyln-gossmap branch from 3290c92 to b4a8cbf Compare September 7, 2021 20:58

rustyrussell marked this pull request as ready for review September 7, 2021 20:58

pyln-client/gossmap: adds testcase for half channels

56cac76

rustyrussell merged commit e9b801d into ElementsProject:master Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pyln.client gossmap support #4582

pyln.client gossmap support #4582

rustyrussell commented Jun 4, 2021

m-schmoock commented Jun 4, 2021

cdecker commented Jun 21, 2021

cdecker commented Jun 21, 2021

m-schmoock commented Jun 25, 2021

rustyrussell commented Jun 28, 2021

fiatjaf commented Jul 7, 2021

m-schmoock left a comment

m-schmoock commented Aug 18, 2021

m-schmoock commented Aug 21, 2021 •

edited

Loading

m-schmoock commented Aug 22, 2021

m-schmoock Aug 26, 2021

m-schmoock commented Aug 26, 2021

m-schmoock commented Aug 26, 2021 •

edited

Loading

rustyrussell commented Sep 2, 2021

rustyrussell commented Sep 2, 2021

rustyrussell commented Sep 7, 2021

vincenzopalazzo left a comment

vincenzopalazzo Sep 7, 2021

m-schmoock Sep 7, 2021

m-schmoock Sep 7, 2021

rustyrussell Sep 7, 2021

rustyrussell Sep 7, 2021

m-schmoock commented Sep 7, 2021

rustyrussell commented Sep 7, 2021

m-schmoock commented Sep 7, 2021

m-schmoock commented Sep 7, 2021

pyln.client gossmap support #4582

pyln.client gossmap support #4582

Conversation

rustyrussell commented Jun 4, 2021

m-schmoock commented Jun 4, 2021

cdecker commented Jun 21, 2021

cdecker commented Jun 21, 2021

m-schmoock commented Jun 25, 2021

rustyrussell commented Jun 28, 2021

fiatjaf commented Jul 7, 2021

m-schmoock left a comment

Choose a reason for hiding this comment

m-schmoock commented Aug 18, 2021

m-schmoock commented Aug 21, 2021 • edited Loading

m-schmoock commented Aug 22, 2021

m-schmoock Aug 26, 2021

Choose a reason for hiding this comment

m-schmoock commented Aug 26, 2021

m-schmoock commented Aug 26, 2021 • edited Loading

rustyrussell commented Sep 2, 2021

rustyrussell commented Sep 2, 2021

rustyrussell commented Sep 7, 2021

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo Sep 7, 2021

Choose a reason for hiding this comment

m-schmoock Sep 7, 2021

Choose a reason for hiding this comment

m-schmoock Sep 7, 2021

Choose a reason for hiding this comment

rustyrussell Sep 7, 2021

Choose a reason for hiding this comment

rustyrussell Sep 7, 2021

Choose a reason for hiding this comment

m-schmoock commented Sep 7, 2021

rustyrussell commented Sep 7, 2021

m-schmoock commented Sep 7, 2021

m-schmoock commented Sep 7, 2021

m-schmoock commented Aug 21, 2021 •

edited

Loading

m-schmoock commented Aug 26, 2021 •

edited

Loading