Skip to content

listpeers: include private field in channels output #2230

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

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Jan 7, 2019

Reveal channel's 'privacy' in listpeers output

Suggested-By: @shesek

@niftynei niftynei requested a review from rustyrussell January 7, 2019 23:50
@niftynei niftynei requested a review from cdecker as a code owner January 7, 2019 23:50
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor testing nit only... Should also add a test somewhere for a non-private channel...

@@ -805,6 +805,12 @@ def test_private_channel(node_factory):
assert not l1.daemon.is_in_log('Received node_announcement for node {}'.format(l2.info['id']))
assert not l2.daemon.is_in_log('Received node_announcement for node {}'.format(l1.info['id']))

# test for 'private' flag in rpc output
assert len(l1.rpc.listpeers()) == 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does what you want? This passes because the output only has one field 4, "peers".

The next line checks there's only one entry in the peers array: only_one asserts that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this check isnt' really checking anything worthwhile, rm'd.

Reveal channel's 'privacy' in `listpeers` output

Suggested-By: @shesek
@niftynei niftynei force-pushed the nifty/add_private_flag_to_channel_output branch from 5065646 to 7154738 Compare January 8, 2019 01:25
@niftynei
Copy link
Collaborator Author

niftynei commented Jan 8, 2019

Cool. Test for non-private channel's flag is one line down :)

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack 7154738

@rustyrussell rustyrussell merged commit efa3887 into ElementsProject:master Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants