certusone
/
wormhole
mirror of https://github.com/certusone/wormhole.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

node: processor: Fix incorrect fallthrough in `handleCleanup` 

When processing pending observations, the `handleCleanup` function
checks if we already have a stored quorum VAA and deletes the in-memory
observation if one is found.  If a stored quorum VAA is not found, then
we're supposed to continue evaluating the other conditions and take
the appropriate action.  This was implemented using a `fallthrough`
statement.

Unfortunately, this is not how `fallthrough` works.  `fallthrough`
simply tells the compiler to execute the body of the next branch in
the switch block, *without evaluating the condition*.  It also doesn't
evaluate the conditions for any of the other branches in the switch.

In practice what this meant is that for local observations that didn't
have quorum we would always take the first branch, fall through to the
second branch, and then exit the switch.  Only once we had a quorum
(`s.submitted == true`) would we actually consider any of the other
branches in the switch.  It also meant that there was no case where we
would take the branch for re-observing messages that hadn't reached
quorum.

Fix this by moving the stored quorum VAA check into an if statement and
then falling through to the switch statement if one is not found.
This commit is contained in:
Chirantan Ekbote 2022-09-16 17:59:00 +09:00 committed by Evan Gray
parent 898cf160df
commit cbce1e7c1f
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
node/pkg/processor/cleanup.go
View File

@ -66,8 +66,7 @@ func (p *Processor) handleCleanup(ctx context.Context) {
for hash, s := range p.state.signatures {
delta := time.Since(s.firstObserved)
switch {
case !s.submitted && s.ourObservation != nil && delta > settlementTime:
if !s.submitted && s.ourObservation != nil && delta > settlementTime {
// Expire pending VAAs post settlement time if we have a stored quorum VAA.
//
// This occurs when we observed a message after the cluster has already reached
@ -81,7 +80,7 @@ func (p *Processor) handleCleanup(ctx context.Context) {
p.logger.Info("Expiring late VAA", zap.String("digest", hash), zap.Duration("delta", delta))
aggregationStateLate.Inc()
delete(p.state.signatures, hash)
break
continue
} else if err != db.ErrVAANotFound {
p.logger.Error("failed to look up VAA in database",
zap.String("digest", hash),
@ -89,8 +88,9 @@ func (p *Processor) handleCleanup(ctx context.Context) {
)
}
}
fallthrough
}
switch {
case !s.settled && delta > settlementTime:
// After 30 seconds, the observation is considered settled - it's unlikely that more observations will
// arrive, barring special circumstances. This is a better time to count misses than submission,