channeldb/channel: fix HasChanStatus for ChanStatusDefault

This commit resovles a lingering issue w/in the codebase wrt how the
ChannelStatus flags are defined. Currently ChannelStatus is improperly
used to define a bit field and the individual flags themselves. As a
result, HasChanStatus accepts queries on particular status (combinations
of flags) and individual flags themselves.

This is an issue because the way HasChanStatus computes whether the
channel has a particular status assumes the provided inputs are all
flags (or at least combinations of flags).

However, ChanStatusDefault is simply the absence of any other flag.
Hence, HasChanStatus will always return true when querying for
ChanStatusDefault because status&0 == 0 is always true.

Longer term we should should consider splitting these definitions into
flags and particular states, and change the way construct or operate on
them, but for now I've just special-cased this one value. Fortunately,
we don't query HasChannelStatus w/ ChanStatusDefault anywhere in the
codebase so we dodge a bullet here, but it'd be nice to have some
greater assurances moving forward.
This commit is contained in:
Conner Fromknecht 2020-04-10 16:01:21 -07:00
parent 2c2b79d300
commit f71cc951fd
No known key found for this signature in database
GPG Key ID: E7D737B67FA592C7
2 changed files with 65 additions and 0 deletions

@ -711,6 +711,12 @@ func (c *OpenChannel) HasChanStatus(status ChannelStatus) bool {
}
func (c *OpenChannel) hasChanStatus(status ChannelStatus) bool {
// Special case ChanStatusDefualt since it isn't actually flag, but a
// particular combination (or lack-there-of) of flags.
if status == ChanStatusDefault {
return c.chanStatus == ChanStatusDefault
}
return c.chanStatus&status == status
}

@ -1581,3 +1581,62 @@ func TestBalanceAtHeight(t *testing.T) {
})
}
}
// TestHasChanStatus asserts the behavior of HasChanStatus by checking the
// behavior of various status flags in addition to the special case of
// ChanStatusDefault which is treated like a flag in the code base even though
// it isn't.
func TestHasChanStatus(t *testing.T) {
tests := []struct {
name string
status ChannelStatus
expHas map[ChannelStatus]bool
}{
{
name: "default",
status: ChanStatusDefault,
expHas: map[ChannelStatus]bool{
ChanStatusDefault: true,
ChanStatusBorked: false,
},
},
{
name: "single flag",
status: ChanStatusBorked,
expHas: map[ChannelStatus]bool{
ChanStatusDefault: false,
ChanStatusBorked: true,
},
},
{
name: "multiple flags",
status: ChanStatusBorked | ChanStatusLocalDataLoss,
expHas: map[ChannelStatus]bool{
ChanStatusDefault: false,
ChanStatusBorked: true,
ChanStatusLocalDataLoss: true,
},
},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
c := &OpenChannel{
chanStatus: test.status,
}
for status, expHas := range test.expHas {
has := c.HasChanStatus(status)
if has == expHas {
continue
}
t.Fatalf("expected chan status to "+
"have %s? %t, got: %t",
status, expHas, has)
}
})
}
}