Register plugin hook explicit deserialize #3647
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have several of these, and they're not always called obvious things like "delete" or "free". `STEALS` provides a strong hint here. I only added it to a couple I knew about off the top of my head. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
register-plugin-hook-explicit-deserialize
branch
from
7a3afba to
7aa0403
Compare
…s explicit. They callback must take ownership of the payload (almost all do, but now it's explicit). And since the payload and cb_arg arguments to plugin_hook_call_() are always identical, make them a single parameter. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
One is called on every plugin return, and tells us whether to continue; the other is only called if every plugin says ok. This works for things like payload replacement, where we need to process the results from each plugin, not just the final one! We should probably turn everything into a chained callback next release. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
register-plugin-hook-explicit-deserialize
branch
from
7aa0403 to
c1ee2ab
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Noticed flakiness in #3611 where the payload replacement sometimes didn't work. The underlying reason is that we had the keysend plugin also on the
htlc_acceptedhook, and we only processed the results from the final hook.To be fair, @cdecker's original hook design explicitly separated the demarshalling from the callback step; I've returned to a similar mode, but only for chained hooks. We could probably convert the rest after release.
Unfortunately, this means #3611 would need rebasing on this PR :(