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

channeldb: modify updateChanBucket to no longer auto-create buckets #1272

Merged
merged 2 commits into from
May 23, 2018

Conversation

Roasbeef
Copy link
Member

In this commit, we modify the existing updateChanBucket function to no
longer auto-create buckets if they don't exist. We do this in order to
fix a class of bug that could arise wherein after a channel has actually
be closed (and the parent buckets removed) a method that mutates the
channel state is called, which then re-creates the relevant set of
buckets. As a result, subsequent calls to any RPCs which need to read
all the channels will fail as most of the fields won't actually be
populated.

After this commit, the fullSync method is the only one that's able to
create the full bucket hierarchy.

In this commit, we modify the existing updateChanBucket function to no
longer auto-create buckets if they don't exist. We do this in order to
fix a class of bug that could arise wherein after a channel has actually
be closed (and the parent buckets removed) a method that mutates the
channel state is called, which then re-creates the relevant set of
buckets. As a result, subsequent calls to any RPCs which need to read
all the channels will fail as most of the fields won't actually be
populated.

After this commit, the fullSync method is the only one that's able to
create the full bucket hierarchy.
Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

Nice fixes!

// level, creating the bucket (if it doesn't exist), for this channel
// itself.
var chanPointBuf bytes.Buffer
chanPointBuf.Grow(outPointSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't necessary since outPointSize is less than 64

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. Also remove other instances in this file.

This optimization isn't needed as a bytes.Buffer already comes
pre-allocated with 64-bytes as a fast path.
@Roasbeef Roasbeef merged commit aeee390 into lightningnetwork:master May 23, 2018
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.

None yet

2 participants