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

BOLT 3 : remove localpubkey and remote_delayedpubkey references #638

Conversation

ariard
Copy link
Contributor

@ariard ariard commented Jul 10, 2019

Right now BOLT 3 is trying to describe the keys derivation from the view point of both nodes, when it should only describe from one side, i.e from a local node building and signing a remote commitment transaction.
In a RevokeAndACK, Bob send his per_commitment_point to Alice, which uses it to derive a set of keys for next commitment transaction:

  • local_delayedpubkey to encumber to_local output and HTLC-Success/HTLC-Timeout outputs
  • remote_pubkey to encumber to_remote output
  • local_htlcpubkey and remote_htlcpubkey to encumber offered/received HTLC outputs

At no moment, she need to derive localpubkey or remote_delayedpubkey ones.

Yes, when Bob is going to verify the sigs, he is going to do the same derivation and from his viewpoint, he may consider local_delayedpubkey is remote_delayedpubkey and remote_pubkey as localpubkey but that implementation-dependent and shouldn't be in spec.

I had a quick skim, but I think option_simplified_commitment (#513) doesn't address this isssue even it s change derivation scheme.

I think it would be really nice to remove this ambiguity :)

Antoine Riard added 3 commits July 10, 2019 17:53
Description in Appendix doesn't match with the one in HTLC-Success/
HTLC-Timeout transactions section
Due to commitment transactions asymmetry (but symetric in semantic)
there is no such thing as a localpubkey
Due to commitment transaction asymmetry (but symetric in semantic)
there is no such thing as a remote_delayedpubkey
@rustyrussell
Copy link
Collaborator

Right now BOLT 3 is trying to describe the keys derivation from the view point of both nodes, when it should only describe from one side, i.e from a local node building and signing a remote commitment transaction.
In a RevokeAndACK, Bob send his per_commitment_point to Alice, which uses it to derive a set of keys for next commitment transaction:

* `local_delayedpubkey` to encumber  to_local output and HTLC-Success/HTLC-Timeout outputs

* `remote_pubkey` to encumber to_remote output

* `local_htlcpubkey` and `remote_htlcpubkey` to encumber offered/received HTLC outputs

At no moment, she need to derive localpubkey or remote_delayedpubkey ones.

Yes, when Bob is going to verify the sigs, he is going to do the same derivation and from his viewpoint, he may consider local_delayedpubkey is remote_delayedpubkey and remote_pubkey as localpubkey but that implementation-dependent and shouldn't be in spec.

No, that is absolutely required by the spec. You need to be able to generate both local and remote commitment transactions.

Unfortunately, that does make it more complex! Perhaps the description can be simplified somehow?

@ariard
Copy link
Contributor Author

ariard commented Jul 11, 2019

Yes, you need to generate both local and remote commitment transactions, but even when you build a local commitment transaction for yourself, there is no such thing as a localpubkey or remote_delayedpubkey. Transactions description should be described from a unique viewpoint, the one for which the commitment transaction is intended, and so pubkey names should be same on both sides.

On the implement-side. at no moment you need to derive the set of 6 keys like the spec let it suppose, every commitment transaction is going to only use 4 of these keys.

Is there a better way to describe this ambiguity than my current diff ?

@ariard
Copy link
Contributor Author

ariard commented May 29, 2023

This PR does not matter substantially for the spec.

@ariard ariard closed this May 29, 2023
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