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

Watchtowers attempt 3: the full channelding #3659

Merged
merged 14 commits into from
May 7, 2020

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Apr 21, 2020

This is the third variant of the watchtower hook, the channelding. The main
difference with respect to #3645 is that the penalty transaction is now
created by channeld which already has a contextualized connection to hsmd,
so we can now drop the changes to the hsmd wire protocol, and just add the
penalty creation capability to the connection between hsmd and
channeld. It also removes the need to synchronously talk to the hsmd from
lightningd which was a bit ugly.

flow

Closes #3601
Closes #3645

Fixes #3422
Fixes #1353

@cdecker cdecker changed the title Watchtower3 Watchtowers attempt 3: the full channelding Apr 21, 2020
@cdecker cdecker added this to the Next Release milestone Apr 27, 2020
@cdecker cdecker force-pushed the watchtower3 branch 3 times, most recently from 0e31bcf to ea49d81 Compare April 28, 2020 15:00
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.

Some commit order issues (I don't think this will bisect?), and one minor possible cleanup with I can append separately.

I think I'll try to reorder the commits myself to save another round-trip!

msg = wire_sync_read(tmpctx, HSM_FD);
if (!msg || !fromwire_hsm_sign_tx_reply(msg, &sig)) {
status_broken("Reading sign_tx_reply: %s", tal_hex(tmpctx, msg));
abort();
Copy link
Contributor

Choose a reason for hiding this comment

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

status_failed(STATUS_FAIL_HSM_IO, ....

Comment on lines 1988 to 2007
static void
commitment_revocation_hook_cb(struct commitment_revocation_payload *p,
const char *buffer, const jsmntok_t *toks){
commitment_revocation_hook_cb(struct commitment_revocation_payload *p STEALS){
wallet_penalty_base_delete(p->wallet, p->channel_id, p->commitnum);
}

REGISTER_PLUGIN_HOOK(commitment_revocation, PLUGIN_HOOK_CHAIN,
static bool
commitment_revocation_hook_deserialize(struct commitment_revocation_payload *p,
const char *buffer,
const jsmntok_t *toks)
{
return true;
}


REGISTER_PLUGIN_HOOK(commitment_revocation,
commitment_revocation_hook_deserialize,
commitment_revocation_hook_cb,
struct commitment_revocation_payload *,
commitment_revocation_hook_serialize,
struct commitment_revocation_payload *);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a fixup for the previous commit, not part of pytest: Add a test for the commitment_revocation hook ?

Comment on lines 2124 to 2134
plugin_hook_call_commitment_revocation(ld, payload, payload);
plugin_hook_call_commitment_revocation(ld, payload);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This too...

cdecker added 14 commits May 7, 2020 10:11
We need the txs around, so don't throw them away after generating them.
`lightningd` passes in all the known penalty_bases when starting a new
`channeld` instance, which tracks them internally, eventually matching them
with revocations and passing them back to `lightningd` so it can create the
penalty transaction. From here it is just a small step to having `channeld`
also generate the penalty transaction if desired.
`channeld` will start creating the penalty transactions in one of the next
commits, so it should know the penalty feerate.
Changelog-Added: plugin: Added a new `commitment_revocation` hook that provides the plugin with penalty transactions for all revoked transactions.
@rustyrussell
Copy link
Contributor

OK, I rebased to avoid the (trivial) conflicts and reordered commits so it compiles and passes tests at all points.

@rustyrussell
Copy link
Contributor

Ack af88a72

@rustyrussell rustyrussell merged commit c7efe0b into ElementsProject:master May 7, 2020
fiatjaf pushed a commit to fiatjaf/mcldsp that referenced this pull request May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants