Skip to content

Commit

Permalink
Clarify comments
Browse files Browse the repository at this point in the history
  • Loading branch information
samsondav committed Nov 26, 2024
1 parent acf23f9 commit 9c0557d
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 13 deletions.
7 changes: 4 additions & 3 deletions llo/plugin_codecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,12 @@ func (c protoObservationCodec) Decode(b types.Observation) (Observation, error)
if len(pbuf.StreamValues) > 0 {
streamValues = make(StreamValues, len(pbuf.StreamValues))
for id, enc := range pbuf.StreamValues {
// StreamValues shouldn't have explicit nils, but for safety we
// ought to handle it anyway
sv, err := UnmarshalProtoStreamValue(enc)
if err != nil {
return Observation{}, err
// Byzantine behavior makes this observation invalid; a
// well-behaved node should never encode invalid or nil values
// here
return Observation{}, fmt.Errorf("failed to decode observation; invalid stream value for stream ID: %d; %w", id, err)
}
streamValues[id] = sv
}
Expand Down
13 changes: 3 additions & 10 deletions llo/plugin_outcome.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,19 +291,12 @@ func (p *Plugin) decodeObservations(aos []types.AttributedObservation, outctx oc
updateChannelDefinitionsByHash[channelHash] = defWithID
}

var missingObservations []llotypes.StreamID
for id, sv := range observation.StreamValues {
if sv != nil { // FIXME: nil checks don't work here. Test this and figure out what to do (also, are there other cases?)
streamObservations[id] = append(streamObservations[id], sv)
} else {
missingObservations = append(missingObservations, id)
}
// sv can never be nil here; validation is handled in the decoding
// of the observation
streamObservations[id] = append(streamObservations[id], sv)
}
if p.Config.VerboseLogging {
if len(missingObservations) > 0 {
sort.Slice(missingObservations, func(i, j int) bool { return missingObservations[i] < missingObservations[j] })
p.Logger.Debugw("Peer was missing observations", "streamIDs", missingObservations, "oracleID", ao.Observer, "stage", "Outcome", "seqNr", outctx.SeqNr)
}
p.Logger.Debugw("Got observations from peer", "stage", "Outcome", "sv", streamObservations, "oracleID", ao.Observer, "seqNr", outctx.SeqNr)
}
}
Expand Down
9 changes: 9 additions & 0 deletions llo/plugin_outcome_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,4 +616,13 @@ func Test_Outcome_Methods(t *testing.T) {
require.Len(t, unreportable, 1)
assert.Equal(t, "ChannelID: 2; Reason: IsReportable=false; no validAfterSeconds entry yet, this must be a new channel", unreportable[0].Error())
})

t.Run("decodeObservations", func(t *testing.T) {
t.Run("with missing observations", func(t *testing.T) {
p := &Plugin{}
aos := []types.AttributedObservation{}
outctx := ocr3types.OutcomeContext{}
tsNS, prr, srv, rcv, ucv, so := p.decodeObservations(aos, outctx)

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

tsNS declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

prr declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

srv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

rcv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

ucv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

so declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-test

assignment mismatch: 6 variables but p.decodeObservations returns 7 values

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

tsNS declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

prr declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

srv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

rcv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

ucv declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

so declared and not used

Check failure on line 625 in llo/plugin_outcome_test.go

View workflow job for this annotation

GitHub Actions / ci-lint

assignment mismatch: 6 variables but p.decodeObservations returns 7 values (typecheck)
})
})
}
2 changes: 2 additions & 0 deletions llo/stream_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var (

func UnmarshalProtoStreamValue(enc *LLOStreamValue) (sv StreamValue, err error) {
if enc == nil {
// StreamValues shouldn't have explicit nils, but for safety we
// ought to handle it without panicking
return nil, ErrNilStreamValue
}
switch enc.Type {
Expand Down

0 comments on commit 9c0557d

Please sign in to comment.