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

openchannel: make openchannel hook chainable #3960

Merged

Conversation

m-schmoock
Copy link
Collaborator

@m-schmoock m-schmoock commented Aug 20, 2020

This change is required to make the decomission feature possible: #3550

Status: Done. The code is functional and does not seem to have bugs. There is a strange workaround in tests/test_plugin.py test_openchannel_hook_chaining where it needs to check for actual hook execution order in order to have the test stable. this is caused by the way hooks are currently registered, see #4005 for details about this.

Question: Currently this change will just log UNUSUAL if a second hook also tries to set a close_to address and continues normal operation. Question: Is this sufficient or should rather want to fatal such plugin setups.

@rustyrussell
Copy link
Contributor

Definitely want this!

@m-schmoock m-schmoock force-pushed the openchannel-hook-chainable branch 11 times, most recently from d411ff3 to 1666f35 Compare August 28, 2020 11:10
@m-schmoock m-schmoock force-pushed the openchannel-hook-chainable branch 8 times, most recently from 2612dbb to b98e80d Compare September 3, 2020 11:32
@m-schmoock m-schmoock marked this pull request as ready for review September 3, 2020 12:29
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Just some minor nits and cleanups. Otherwise good work 👍

lightningd/opening_control.c Outdated Show resolved Hide resolved
tests/plugins/openchannel_hook_accept.py Outdated Show resolved Hide resolved
tests/plugins/openchannel_hook_reject.py Outdated Show resolved Hide resolved
tests/test_plugin.py Show resolved Hide resolved
tests/test_plugin.py Outdated Show resolved Hide resolved
@cdecker
Copy link
Member

cdecker commented Sep 6, 2020

ACK dbbad65

This will make the `openchannel_hook` chainable. Logic is as follows:
 - The first plugin that rejects terminates the chain
 - If more than one plugin uses the `close_to` parameter, take the first
   value and log_unusual for the others.

Changelog-Added: openchannel_hook is now chainable
@cdecker
Copy link
Member

cdecker commented Sep 7, 2020

Thanks for the fixup @m-schmoock, excellent work 👍

ACK e006d19

@cdecker cdecker added this to the v0.9.1 milestone Sep 7, 2020
@rustyrussell rustyrussell merged commit 2816c08 into ElementsProject:master Sep 7, 2020
@m-schmoock m-schmoock deleted the openchannel-hook-chainable branch September 7, 2020 20:49
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.

4 participants