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 2&7: exchanging of announcement signatures #92

Closed
pm47 opened this issue Jan 26, 2017 · 6 comments
Closed

BOLT 2&7: exchanging of announcement signatures #92

pm47 opened this issue Jan 26, 2017 · 6 comments

Comments

@pm47
Copy link
Collaborator

pm47 commented Jan 26, 2017

I think there might be a chicken-and-egg issue in the way we exchange announcement signatures with funding_locked messages.

Each party can compute its own announcement-bitcoin-signature, but it needs the other party's announcement-bitcoin-signature in order to compute the announcement-node-signature the way it is currently specified. So we can't actually build valid funding_locked messages.

Maybe we can work around this issue by not including the other node's announcement-bitcoin-signature in our announcement-node-signature (channel id needs to be included)? It would still prove that each node owns their respective bitcoin address, which can be linked to an actual tx, and that both nodes agreed on the announcement.

@Roasbeef
Copy link
Collaborator

Excellent find! I'm not sure how we missed this the first time around...

I think your suggested solution is pretty straightforward. To eliminate the circular dependency nodes simply sign the channel auth digest (channelID+Bitcoin keys), leaving out the other parties signature from that digest.

Additionally, I think we need to enforce an additional constraint on the signatures within the announcement themselves. In order for us to possibly implement hash-based state reconciliation (rsync algo, hash time series, Patricia trie, etc), we need to ensure that only one encoding of the signature is accepted. Otherwise, our encoding won't be deterministic. In order words, we should enforce a low S (just like in Bitcoin) encoding for all announcement signatures propagated within the network.

@cdecker
Copy link
Collaborator

cdecker commented Jan 27, 2017

Actually re-reading the spec I think we are mixing a few concerns here, the funding_locked message, which is part of the channel lifecycle messages are being misused to also transfer the signatures. Maybe we should split the signatures out into their own messages? That way we would have explicit opt-in to channel announcements by both parties.

And is there a point to also signing the signatures at all? Maybe we should reorder fields so that we have the 4 signatures at the beginning and they simply sign everything below the signature fields (offset 256):

[64:node-signature-1]
[64:node-signature-2]
[64:bitcoin-signature-1]
[64:bitcoin-signature-2]
[8:channel-id]
[33:node-id-1]
[33:node-id-2]
[33:bitcoin-key-1]
[33:bitcoin-key-2]
[2:len]
[len:features]

@pm47
Copy link
Collaborator Author

pm47 commented Jan 28, 2017

is there a point to also signing the signatures at all?

I think you're right, signing the node ids & bitcoin keys is enough and it's easier to implement this way 👍

@rustyrussell
Copy link
Collaborator

rustyrussell commented Jan 30, 2017 via email

@rustyrussell
Copy link
Collaborator

rustyrussell commented Jan 30, 2017 via email

cdecker added a commit to cdecker/lightning-rfc that referenced this issue Jan 31, 2017
Reorders the `channel-id` and `bitcoin-signature-x` fields so that the
signed part of the message is contiguous. Simplifies the signing logic
not to just simple signatures of a contiguous region of the message,
no need to sign signatures, they all commit to the same payload. This
also removes the chicken and egg problem @pm47 reported in lightning#92.
Furthermore it specifies that the signed payload also includes any
future appended fields.
rustyrussell pushed a commit that referenced this issue Feb 1, 2017
Reorders the `channel-id` and `bitcoin-signature-x` fields so that the
signed part of the message is contiguous. Simplifies the signing logic
not to just simple signatures of a contiguous region of the message,
no need to sign signatures, they all commit to the same payload. This
also removes the chicken and egg problem @pm47 reported in #92.
Furthermore it specifies that the signed payload also includes any
future appended fields.
@cdecker
Copy link
Collaborator

cdecker commented Feb 3, 2017

The chicken-and-egg problem should be addressed by #96, while the discussion about splitting off the signature exchange from the channel lifecycle is being tracked in #97. Seems this is fixed, closing 😉

@cdecker cdecker closed this as completed Feb 3, 2017
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

No branches or pull requests

4 participants