From 5031c82150106c0971fd32fc0eea9193340aec9b Mon Sep 17 00:00:00 2001 From: William Banfield Date: Thu, 19 May 2022 12:53:17 -0400 Subject: [PATCH] log on stripped extensions --- internal/blocksync/reactor.go | 4 +++- internal/consensus/state.go | 10 +++++----- types/block.go | 7 ++++++- types/vote.go | 7 +++++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index 210fde044..304d1b824 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -590,7 +590,9 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh // if vote extensions were required at this height, ensure they exist. err = extCommit.EnsureExtensions() } else if err == nil && !state.ConsensusParams.ABCI.VoteExtensionsEnabled(extCommit.Height) { - extCommit.StripExtensions() + if stripped := extCommit.StripExtensions(); stripped { + r.logger.Error("commit included extension data but vote extensions are not enabled") + } } // If either of the checks failed we log the error and request for a new block // at that height diff --git a/internal/consensus/state.go b/internal/consensus/state.go index b49226284..5dce21530 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -2407,16 +2407,15 @@ func (cs *State) addVote( } } } else { - if vote.Extension != nil || vote.ExtensionSignature != nil { - cs.logger.Error("vote included extension data but vote extensions are not enabled", "peer", peerID) - } // Vote extensions are not enabled on the network. // strip the extension data from the vote in case any is present. // // TODO punish a peer if it sent a vote with an extension when the feature // is disabled on the network. // https://github.com/tendermint/tendermint/issues/8565 - vote.StripExtensions() + if stripped := vote.StripExtension(); stripped { + cs.logger.Error("vote included extension data but vote extensions are not enabled", "peer", peerID) + } } height := cs.Height @@ -2620,7 +2619,8 @@ func (cs *State) signAddVote( return nil } if !cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(vote.Height) { - vote.StripExtensions() + // The signer will sign the extension, make sure to remove the data on the way out + vote.StripExtension() } cs.sendInternalMessage(ctx, msgInfo{&VoteMessage{vote}, "", tmtime.Now()}) cs.logger.Debug("signed and pushed vote", "height", cs.Height, "round", cs.Round, "vote", vote) diff --git a/types/block.go b/types/block.go index aedd90341..9267dcc98 100644 --- a/types/block.go +++ b/types/block.go @@ -1105,11 +1105,16 @@ func (ec *ExtendedCommit) EnsureExtensions() error { // StripExtensions removes all VoteExtension data from an ExtendedCommit. This // is useful when dealing with an ExendedCommit but vote extension data is // expected to be absent. -func (ec *ExtendedCommit) StripExtensions() { +func (ec *ExtendedCommit) StripExtensions() bool { + stripped := false for idx := range ec.ExtendedSignatures { + if len(ec.ExtendedSignatures[idx].Extension) > 0 || len(ec.ExtendedSignatures[idx].ExtensionSignature) > 0 { + stripped = true + } ec.ExtendedSignatures[idx].Extension = nil ec.ExtendedSignatures[idx].ExtensionSignature = nil } + return stripped } // ToCommit converts an ExtendedCommit to a Commit by removing all vote diff --git a/types/vote.go b/types/vote.go index 20f95df13..f7006b8cd 100644 --- a/types/vote.go +++ b/types/vote.go @@ -112,11 +112,14 @@ func (vote *Vote) CommitSig() CommitSig { } } -// StripExtensions removes any extension data from the vote. Useful if the +// StripExtension removes any extension data from the vote. Useful if the // chain has not enabled vote extensions. -func (vote *Vote) StripExtensions() { +// Returns true if extension data was present before stripping and false otherwise. +func (vote *Vote) StripExtension() bool { + stripped := len(vote.Extension) > 0 || len(vote.ExtensionSignature) > 0 vote.Extension = nil vote.ExtensionSignature = nil + return stripped } // ExtendedCommitSig attempts to construct an ExtendedCommitSig from this vote.