Skip to content

Begin dry-up ChannelMonitor key access #555

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

Merged

Conversation

ariard
Copy link

@ariard ariard commented Mar 20, 2020

Pull some commits out of #540 to ease review process.

This start to separate key access and tx generation material from pure detection even if redeem_script lay between each category.

Watchtower will be supported through external signer interface
where a watchtower implementation may differ from a local one
by the scope of key access and pre-signed datas.
@ariard ariard force-pushed the 2020-03-begin-dryup-chanmon-keys branch from 1a44317 to 09fae43 Compare March 20, 2020 22:07
@TheBlueMatt
Copy link
Collaborator

I was kinda OK with it as a dirty hack in #540, but especially if its gonna be its own PR, please dont copy Storage into OnchainTxHandler - everything in it is available in ChannelKeys, co using Storage is just a full copy of it.

@ariard ariard force-pushed the 2020-03-begin-dryup-chanmon-keys branch from 09fae43 to bd4a7cb Compare March 21, 2020 18:34
Antoine Riard added 3 commits March 21, 2020 17:03
Rename ChannelMonitor::Storage to OnchainDetection,
holder of channel state (base_key+per_commitment_point)
to detect onchain transactions accordingly.

Going further between splitting detection and transaction
generation, we endow OnchainTxHandler with keys access.
That way, in latter commits, we may remove secret keys entirely
from ChannelMonitor.
Key access is provided through ChanSigner.
check_spend_local_transaction is tasked with detection of
onchain local commitment transaction and generate HTLC transaction.
Signing an already onchain tx isn't necessary.
@ariard ariard force-pushed the 2020-03-begin-dryup-chanmon-keys branch from bd4a7cb to 502197d Compare March 21, 2020 21:04
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Much better than it was so gonna merge. Will follow up with a PR to fix the two comments but it can wait.


let onchain_detection = OnchainDetection {
keys: keys.clone(),
funding_info: Some(funding_info.clone()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: unnecessary funding_info clone.

revocation_base_key: PublicKey,
htlc_base_key: PublicKey,
}
struct OnchainDetection<ChanSigner: ChannelKeys> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think "OnchainDetection" is any better of a name. This should just be flattened and all the fields become a field directly in ChannelMonitor.

@TheBlueMatt TheBlueMatt merged commit e5bedc4 into lightningdevkit:master Mar 21, 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
Development

Successfully merging this pull request may close these issues.

2 participants