Skip to content

channeldb: write the CommitSig in CommitDiff in LN wire format #4099

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.

Sign up for GitHub

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

Closed
Roasbeef wants to merge 3 commits into from
Closed

channeldb: write the CommitSig in CommitDiff in LN wire format #4099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
060cda6
channeldb: mark ApplyMigration as a test helper
Roasbeef Mar 19, 2020
386ccaf
channeldb/migration19: create migration 19, copy over requires types+…
Roasbeef Mar 19, 2020
b5876b2
channeldb/migration19: add body and tests for migration 19
Roasbeef Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions channeldb/channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1866,12 +1866,12 @@ func deserializeLogUpdates(r io.Reader) ([]LogUpdate, error) {
return logUpdates, nil
}

func serializeCommitDiff(w io.Writer, diff *CommitDiff) error {
func serializeCommitDiff(w io.Writer, diff *CommitDiff) error { // nolint: dupl
Comment thread
Roasbeef marked this conversation as resolved.
Outdated
if err := serializeChanCommit(w, &diff.Commitment); err != nil {
return err
}

if err := diff.CommitSig.Encode(w, 0); err != nil {
if err := WriteElements(w, diff.CommitSig); err != nil {
Comment thread
Roasbeef marked this conversation as resolved.
Outdated
return err
}

Expand Down Expand Up @@ -1917,10 +1917,16 @@ func deserializeCommitDiff(r io.Reader) (*CommitDiff, error) {
return nil, err
}

d.CommitSig = &lnwire.CommitSig{}
if err := d.CommitSig.Decode(r, 0); err != nil {
var msg lnwire.Message
if err := ReadElements(r, &msg); err != nil {
return nil, err
}
commitSig, ok := msg.(*lnwire.CommitSig)
if !ok {
return nil, fmt.Errorf("expected lnwire.CommitSig, instead "+
"read: %T", msg)
}
d.CommitSig = commitSig

d.LogUpdates, err = deserializeLogUpdates(r)
if err != nil {
Expand Down
21 changes: 19 additions & 2 deletions channeldb/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package channeldb

import (
"bytes"
"encoding/binary"
"fmt"
"io"
Expand Down Expand Up @@ -178,7 +179,17 @@ func WriteElement(w io.Writer, element interface{}) error {
}

case lnwire.Message:
if _, err := lnwire.WriteMessage(w, e, 0); err != nil {
var msgBuf bytes.Buffer
if _, err := lnwire.WriteMessage(&msgBuf, e, 0); err != nil {
return err
}

msgLen := uint16(len(msgBuf.Bytes()))
if err := WriteElements(w, msgLen); err != nil {
return err
}

if _, err := w.Write(msgBuf.Bytes()); err != nil {
Comment thread
Roasbeef marked this conversation as resolved.
Outdated
return err
}

Expand Down Expand Up @@ -394,7 +405,13 @@ func ReadElement(r io.Reader, element interface{}) error {
*e = bytes

case *lnwire.Message:
msg, err := lnwire.ReadMessage(r, 0)
var msgLen uint16
if err := ReadElement(r, &msgLen); err != nil {
Copy link
Copy Markdown
Contributor

@cfromknecht cfromknecht Jun 15, 2020

Choose a reason for hiding this comment

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

do we know that CommitSig is the only lnwire.Message that isn't length-prefixed?

These are all the messages I found passing through this codec in channeldb unit tests

DECODING CommitSig
DECODING UpdateAddHTLC
DECODING UpdateFailHTLC
DECODING UpdateFulfillHTLC
ENCODING CommitSig
ENCODING UpdateAddHTLC
ENCODING UpdateFailHTLC
ENCODING UpdateFulfillHTLC

Copy link
Copy Markdown
Member Author

@Roasbeef Roasbeef Jun 16, 2020

Choose a reason for hiding this comment

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

I think one other message is the ChannelReestablish that we store within the channel close summary. In the pst I think I had portions of this in this PR, but it's been a bit since I've rebased the PR that adds the actual change in the lnwire logic.

I'm gonna rebase that on top of this, and run the tests there, which in the past have let me catch other issues in this base migration.

Copy link
Copy Markdown
Member Author

@Roasbeef Roasbeef Jun 16, 2020

Choose a reason for hiding this comment

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

Oh I think those are the LogUpdates in the greater commit diff.

Copy link
Copy Markdown
Member Author

@Roasbeef Roasbeef Jun 16, 2020

Choose a reason for hiding this comment

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

ChannelReestablish should be handled when we migrate the closed channel summaries using the same construct:

  • read out using the old non-length prefixed version
  • write out using the new length prefixed version

return err
}

msgReader := io.LimitReader(r, int64(msgLen))
msg, err := lnwire.ReadMessage(msgReader, 0)
if err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions channeldb/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ var (
number: 18,
migration: mig.CreateTLB(peersBucket),
},
{
// Migrate to length prefixed wire messages everywhere
// in the database.
number: 19,
migration: migration19.MigrateDatabaseWireMessages,
},
}

// Big endian is the preferred byte order, due to cursor scans over
Expand Down
Loading