From f18f1cb3dbd03f7b4738f5b64e2bc2f3bcad47b4 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Sun, 10 Apr 2022 05:04:34 +0200 Subject: [PATCH] BlockStore holds extended commit Cherry-pick 8d504d4b50ec6afbdffe2df7ababbef30e15053d and fix conflicts. Signed-off-by: Thane Thomson --- internal/blocksync/pool.go | 32 ++- internal/blocksync/reactor.go | 42 +++- internal/consensus/reactor.go | 6 +- internal/consensus/state.go | 22 +- internal/state/execution.go | 48 ++--- internal/state/services.go | 3 +- internal/store/store.go | 47 +++- internal/test/factory/commit.go | 4 +- test/e2e/runner/evidence.go | 2 +- types/block.go | 369 +++++++++++++++++++++++++++++--- types/test_util.go | 2 +- types/vote.go | 28 +++ types/vote_set.go | 20 +- 13 files changed, 515 insertions(+), 110 deletions(-) diff --git a/internal/blocksync/pool.go b/internal/blocksync/pool.go index 7f133a7a1..69bdd1a18 100644 --- a/internal/blocksync/pool.go +++ b/internal/blocksync/pool.go @@ -208,12 +208,13 @@ func (pool *BlockPool) IsCaughtUp() bool { // We need to see the second block's Commit to validate the first block. // So we peek two blocks at a time. // The caller will verify the commit. -func (pool *BlockPool) PeekTwoBlocks() (first *types.Block, second *types.Block) { +func (pool *BlockPool) PeekTwoBlocks() (first *types.Block, second *types.Block, firstExtCommit *types.ExtendedCommit) { pool.mtx.RLock() defer pool.mtx.RUnlock() if r := pool.requesters[pool.height]; r != nil { first = r.getBlock() + firstExtCommit = r.getExtendedCommit() } if r := pool.requesters[pool.height+1]; r != nil { second = r.getBlock() @@ -222,7 +223,7 @@ func (pool *BlockPool) PeekTwoBlocks() (first *types.Block, second *types.Block) } // PopRequest pops the first block at pool.height. -// It must have been validated by 'second'.Commit from PeekTwoBlocks(). +// It must have been validated by 'second'.Commit from PeekTwoBlocks(), TODO: (?) and its corresponding ExtendedCommit. func (pool *BlockPool) PopRequest() { pool.mtx.Lock() defer pool.mtx.Unlock() @@ -268,10 +269,15 @@ func (pool *BlockPool) RedoRequest(height int64) types.NodeID { // AddBlock validates that the block comes from the peer it was expected from and calls the requester to store it. // TODO: ensure that blocks come in order for each peer. -func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, blockSize int) { +func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extCommit *types.ExtendedCommit, blockSize int) { pool.mtx.Lock() defer pool.mtx.Unlock() + if block.Height != extCommit.Height { + pool.logger.Error("heights don't match, not adding block", "block_height", block.Height, "commit_height", extCommit.Height) + return + } + requester := pool.requesters[block.Height] if requester == nil { pool.logger.Error("peer sent us a block we didn't expect", @@ -286,7 +292,7 @@ func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, blockSi return } - if requester.setBlock(block, peerID) { + if requester.setBlock(block, extCommit, peerID) { atomic.AddInt32(&pool.numPending, -1) peer := pool.peers[peerID] if peer != nil { @@ -460,6 +466,7 @@ func (pool *BlockPool) debug() string { } else { str += fmt.Sprintf("H(%v):", h) str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil) + str += fmt.Sprintf("C?(%v) ", pool.requesters[h].extCommit != nil) } } return str @@ -548,9 +555,10 @@ type bpRequester struct { gotBlockCh chan struct{} redoCh chan types.NodeID // redo may send multitime, add peerId to identify repeat - mtx sync.Mutex - peerID types.NodeID - block *types.Block + mtx sync.Mutex + peerID types.NodeID + block *types.Block + extCommit *types.ExtendedCommit } func newBPRequester(logger log.Logger, pool *BlockPool, height int64) *bpRequester { @@ -576,13 +584,14 @@ func (bpr *bpRequester) OnStart(ctx context.Context) error { func (*bpRequester) OnStop() {} // Returns true if the peer matches and block doesn't already exist. -func (bpr *bpRequester) setBlock(block *types.Block, peerID types.NodeID) bool { +func (bpr *bpRequester) setBlock(block *types.Block, extCommit *types.ExtendedCommit, peerID types.NodeID) bool { bpr.mtx.Lock() if bpr.block != nil || bpr.peerID != peerID { bpr.mtx.Unlock() return false } bpr.block = block + bpr.extCommit = extCommit bpr.mtx.Unlock() select { @@ -598,6 +607,12 @@ func (bpr *bpRequester) getBlock() *types.Block { return bpr.block } +func (bpr *bpRequester) getExtendedCommit() *types.ExtendedCommit { + bpr.mtx.Lock() + defer bpr.mtx.Unlock() + return bpr.extCommit +} + func (bpr *bpRequester) getPeerID() types.NodeID { bpr.mtx.Lock() defer bpr.mtx.Unlock() @@ -615,6 +630,7 @@ func (bpr *bpRequester) reset() { bpr.peerID = "" bpr.block = nil + bpr.extCommit = nil } // Tells bpRequester to pick another peer and try again. diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index fd9bf4d7a..587f9f1fa 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -76,7 +76,7 @@ type Reactor struct { stateStore sm.Store blockExec *sm.BlockExecutor - store *store.BlockStore + store sm.BlockStore pool *BlockPool consReactor consensusReactor blockSync *atomicBool @@ -186,15 +186,24 @@ func (r *Reactor) OnStop() { func (r *Reactor) respondToPeer(ctx context.Context, msg *bcproto.BlockRequest, peerID types.NodeID, blockSyncCh *p2p.Channel) error { block := r.store.LoadBlock(msg.Height) if block != nil { + extCommit := r.store.LoadBlockExtCommit(msg.Height) + if extCommit == nil { + r.logger.Error("peer requesting a block; we have the block but not its extended commit (%v)", block) + return fmt.Errorf("blockstore has block but not extended commit %v", block) + } blockProto, err := block.ToProto() if err != nil { - r.logger.Error("failed to convert msg to protobuf", "err", err) + r.logger.Error("failed to convert block to protobuf", "err", err) return err } + extCommitProto := extCommit.ToProto() return blockSyncCh.Send(ctx, p2p.Envelope{ - To: peerID, - Message: &bcproto.BlockResponse{Block: blockProto}, + To: peerID, + Message: &bcproto.BlockResponse{ + Block: blockProto, + ExtCommit: extCommitProto, + }, }) } @@ -236,8 +245,15 @@ func (r *Reactor) handleMessage(ctx context.Context, chID p2p.ChannelID, envelop "err", err) return err } + extCommit, err := types.ExtendedCommitFromProto(msg.ExtCommit) + if err != nil { + r.logger.Error("failed to convert extended commit from proto", + "peer", envelope.From, + "err", err) + return err + } - r.pool.AddBlock(envelope.From, block, block.Size()) + r.pool.AddBlock(envelope.From, block, extCommit, block.Size()) case *bcproto.StatusRequest: return blockSyncCh.Send(ctx, p2p.Envelope{ @@ -448,6 +464,10 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh ) switch { + case r.pool.startHeight > state.InitialHeight && blocksSynced == 0: + //If we have state-synced, we need to blocksync at least one block + continue + case r.pool.IsCaughtUp(): r.logger.Info("switching to consensus reactor", "height", height) @@ -490,9 +510,12 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh // TODO: Uncouple from request routine. // see if there are any blocks to sync - first, second := r.pool.PeekTwoBlocks() - if first == nil || second == nil { - // we need both to sync the first block + first, second, extCommit := r.pool.PeekTwoBlocks() + if first == nil || second == nil || extCommit == nil { + if first != nil && extCommit == nil { + r.logger.Error("peeked a block without extended commit", "height", first.Height) + } + // we need all to sync the first block continue } else { // try again quickly next loop @@ -517,6 +540,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh // NOTE: We can probably make this more efficient, but note that calling // first.Hash() doesn't verify the tx contents, so MakePartSet() is // currently necessary. + // TODO Should we also validate against the extended commit? if err = state.Validators.VerifyCommitLight(chainID, firstID, first.Height, second.LastCommit); err != nil { err = fmt.Errorf("invalid last commit: %w", err) r.logger.Error( @@ -549,7 +573,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh r.pool.PopRequest() // TODO: batch saves so we do not persist to disk every block - r.store.SaveBlock(first, firstParts, second.LastCommit) + r.store.SaveBlock(first, firstParts, extCommit) var err error diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go index 53e0b540d..defeffbc4 100644 --- a/internal/consensus/reactor.go +++ b/internal/consensus/reactor.go @@ -794,10 +794,10 @@ func (r *Reactor) gossipVotesRoutine(ctx context.Context, ps *PeerState, voteCh // catchup logic -- if peer is lagging by more than 1, send Commit blockStoreBase := r.state.blockStore.Base() if blockStoreBase > 0 && prs.Height != 0 && rs.Height >= prs.Height+2 && prs.Height >= blockStoreBase { - // Load the block commit for prs.Height, which contains precommit + // Load the block's extended commit for prs.Height, which contains precommit // signatures for prs.Height. - if commit := r.state.blockStore.LoadBlockCommit(prs.Height); commit != nil { - if ok, err := r.pickSendVote(ctx, ps, commit, voteCh); err != nil { + if extCommit := r.state.blockStore.LoadBlockExtCommit(prs.Height); extCommit != nil { + if ok, err := r.pickSendVote(ctx, ps, extCommit, voteCh); err != nil { return } else if ok { logger.Debug("picked Catchup commit to send", "height", prs.Height) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 490801ad2..8ed7a4943 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -695,19 +695,15 @@ func (cs *State) sendInternalMessage(ctx context.Context, mi msgInfo) { // Reconstruct LastCommit from SeenCommit, which we saved along with the block, // (which happens even before saving the state) func (cs *State) reconstructLastCommit(state sm.State) { - commit := cs.blockStore.LoadSeenCommit() - if commit == nil || commit.Height != state.LastBlockHeight { - commit = cs.blockStore.LoadBlockCommit(state.LastBlockHeight) - } - - if commit == nil { + extCommit := cs.blockStore.LoadBlockExtCommit(state.LastBlockHeight) + if extCommit == nil { panic(fmt.Sprintf( "failed to reconstruct last commit; commit for height %v not found", state.LastBlockHeight, )) } - lastPrecommits := types.CommitToVoteSet(state.ChainID, commit, state.LastValidators) + lastPrecommits := extCommit.ToVoteSet(state.ChainID, state.LastValidators) if !lastPrecommits.HasTwoThirdsMajority() { panic("failed to reconstruct last commit; does not have +2/3 maj") } @@ -1400,16 +1396,17 @@ func (cs *State) createProposalBlock(ctx context.Context) (*types.Block, error) return nil, errors.New("entered createProposalBlock with privValidator being nil") } - var commit *types.Commit + //TODO: wouldn't it be easier if CreateProposalBlock accepted cs.LastCommit directly? + var extCommit *types.ExtendedCommit switch { case cs.Height == cs.state.InitialHeight: // We're creating a proposal for the first block. // The commit is empty, but not nil. - commit = types.NewCommit(0, 0, types.BlockID{}, nil) + extCommit = types.NewExtendedCommit(0, 0, types.BlockID{}, nil) case cs.LastCommit.HasTwoThirdsMajority(): // Make the commit from LastCommit - commit = cs.LastCommit.MakeCommit() + extCommit = cs.LastCommit.MakeExtendedCommit() default: // This shouldn't happen. cs.logger.Error("propose step; cannot propose anything without commit for the previous block") @@ -1425,7 +1422,7 @@ func (cs *State) createProposalBlock(ctx context.Context) (*types.Block, error) proposerAddr := cs.privValidatorPubKey.Address() - ret, err := cs.blockExec.CreateProposalBlock(ctx, cs.Height, cs.state, commit, proposerAddr, cs.LastCommit.GetVotes()) + ret, err := cs.blockExec.CreateProposalBlock(ctx, cs.Height, cs.state, extCommit, proposerAddr) if err != nil { panic(err) } @@ -1923,8 +1920,7 @@ func (cs *State) finalizeCommit(ctx context.Context, height int64) { // NOTE: the seenCommit is local justification to commit this block, // but may differ from the LastCommit included in the next block precommits := cs.Votes.Precommits(cs.CommitRound) - seenCommit := precommits.MakeCommit() - cs.blockStore.SaveBlock(block, blockParts, seenCommit) + cs.blockStore.SaveBlock(block, blockParts, precommits.MakeExtendedCommit()) } else { // Happens during replay if we already saved the block but didn't commit logger.Debug("calling finalizeCommit on already stored block", "height", block.Height) diff --git a/internal/state/execution.go b/internal/state/execution.go index 06dfc0b5c..70e285445 100644 --- a/internal/state/execution.go +++ b/internal/state/execution.go @@ -7,7 +7,6 @@ import ( abciclient "github.com/tendermint/tendermint/abci/client" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/encoding" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/internal/eventbus" @@ -87,9 +86,8 @@ func (blockExec *BlockExecutor) CreateProposalBlock( ctx context.Context, height int64, state State, - commit *types.Commit, + extCommit *types.ExtendedCommit, proposerAddr []byte, - votes []*types.Vote, ) (*types.Block, error) { maxBytes := state.ConsensusParams.Block.MaxBytes @@ -101,15 +99,15 @@ func (blockExec *BlockExecutor) CreateProposalBlock( maxDataBytes := types.MaxDataBytes(maxBytes, evSize, state.Validators.Size()) txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas) + commit := extCommit.StripExtensions() block := state.MakeBlock(height, txs, commit, evidence, proposerAddr) - localLastCommit := buildLastCommitInfo(block, blockExec.store, state.InitialHeight) rpp, err := blockExec.appClient.PrepareProposal( ctx, &abci.RequestPrepareProposal{ MaxTxBytes: maxDataBytes, Txs: block.Txs.ToSliceOfBytes(), - LocalLastCommit: extendedCommitInfo(localLastCommit, votes), + LocalLastCommit: extendedCommitInfo(*extCommit), ByzantineValidators: block.Evidence.ToABCI(), Height: block.Height, Time: block.Time, @@ -416,43 +414,29 @@ func buildLastCommitInfo(block *types.Block, store Store, initialHeight int64) a } } +//TODO reword // extendedCommitInfo expects a CommitInfo struct along with all of the // original votes relating to that commit, including their vote extensions. The // order of votes does not matter. -func extendedCommitInfo(c abci.CommitInfo, votes []*types.Vote) abci.ExtendedCommitInfo { - if len(c.Votes) != len(votes) { - panic(fmt.Sprintf("extendedCommitInfo: number of votes from commit differ from the number of votes supplied (%d != %d)", len(c.Votes), len(votes))) - } - votesByVal := make(map[string]*types.Vote) - for _, vote := range votes { - if vote != nil { - valAddr := vote.ValidatorAddress.String() - if _, ok := votesByVal[valAddr]; ok { - panic(fmt.Sprintf("extendedCommitInfo: found duplicate vote for validator with address %s", valAddr)) - } - votesByVal[valAddr] = vote - } - } - vs := make([]abci.ExtendedVoteInfo, len(c.Votes)) - for i := range vs { +func extendedCommitInfo(extCommit types.ExtendedCommit) abci.ExtendedCommitInfo { + vs := make([]abci.ExtendedVoteInfo, len(extCommit.ExtendedSignatures)) + for i, ecs := range extCommit.ExtendedSignatures { var ext []byte - // votes[i] will be nil if c.Votes[i].SignedLastBlock is false - if c.Votes[i].SignedLastBlock { - valAddr := crypto.Address(c.Votes[i].Validator.Address).String() - vote, ok := votesByVal[valAddr] - if !ok || vote == nil { - panic(fmt.Sprintf("extendedCommitInfo: validator with address %s signed last block, but could not find vote for it", valAddr)) - } - ext = vote.Extension + if ecs.ForBlock() { + ext = ecs.VoteExtension } vs[i] = abci.ExtendedVoteInfo{ - Validator: c.Votes[i].Validator, - SignedLastBlock: c.Votes[i].SignedLastBlock, + Validator: abci.Validator{ + Address: ecs.ValidatorAddress, + //TODO: Important: why do we need power here? + Power: 0, + }, + SignedLastBlock: ecs.ForBlock(), VoteExtension: ext, } } return abci.ExtendedCommitInfo{ - Round: c.Round, + Round: extCommit.Round, Votes: vs, } } diff --git a/internal/state/services.go b/internal/state/services.go index 5d04d2c82..05160e2a2 100644 --- a/internal/state/services.go +++ b/internal/state/services.go @@ -26,7 +26,7 @@ type BlockStore interface { LoadBlockMeta(height int64) *types.BlockMeta LoadBlock(height int64) *types.Block - SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) + SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) PruneBlocks(height int64) (uint64, error) @@ -36,6 +36,7 @@ type BlockStore interface { LoadBlockCommit(height int64) *types.Commit LoadSeenCommit() *types.Commit + LoadBlockExtCommit(height int64) *types.ExtendedCommit } //----------------------------------------------------------------------------- diff --git a/internal/store/store.go b/internal/store/store.go index eb03e5fe6..e188eab7c 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -273,11 +273,31 @@ func (bs *BlockStore) LoadBlockCommit(height int64) *types.Commit { } commit, err := types.CommitFromProto(pbc) if err != nil { - panic(fmt.Errorf("error reading block commit: %w", err)) + panic(fmt.Errorf("error from proto block commit: %w", err)) } return commit } +func (bs *BlockStore) LoadBlockExtCommit(height int64) *types.ExtendedCommit { + var pbec = new(tmproto.ExtendedCommit) + bz, err := bs.db.Get(extCommitKey(height)) + if err != nil { + panic(err) + } + if len(bz) == 0 { + return nil + } + err = proto.Unmarshal(bz, pbec) + if err != nil { + panic(fmt.Errorf("error reading block extended commit: %w", err)) + } + extCommit, err := types.ExtendedCommitFromProto(pbec) + if err != nil { + panic(fmt.Errorf("error from proto block extended commit: %w", err)) + } + return extCommit +} + // LoadSeenCommit returns the last locally seen Commit before being // cannonicalized. This is useful when we've seen a commit, but there // has not yet been a new block at `height + 1` that includes this @@ -298,7 +318,7 @@ func (bs *BlockStore) LoadSeenCommit() *types.Commit { commit, err := types.CommitFromProto(pbc) if err != nil { - panic(fmt.Errorf("error from proto commit: %w", err)) + panic(fmt.Errorf("error from proto seen commit: %w", err)) } return commit } @@ -446,7 +466,7 @@ func (bs *BlockStore) batchDelete( // If all the nodes restart after committing a block, // we need this to reload the precommits to catch-up nodes to the // most recent height. Otherwise they'd stall at H-1. -func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) { +func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) { if block == nil { panic("BlockStore can only save a non-nil block") } @@ -462,6 +482,10 @@ func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, s if !blockParts.IsComplete() { panic("BlockStore can only save complete block part sets") } + if height != seenCommit.Height { + panic(fmt.Sprintf("BlockStore cannot save seen commit of a different height (block:%d, commit%d)", + height, seenCommit.Height)) + } // Save block parts. This must be done before the block meta, since callers // typically load the block meta first as an indication that the block exists @@ -494,12 +518,18 @@ func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, s } // Save seen commit (seen +2/3 precommits for block) - pbsc := seenCommit.ToProto() + pbsc := seenCommit.StripExtensions().ToProto() seenCommitBytes := mustEncode(pbsc) if err := batch.Set(seenCommitKey(), seenCommitBytes); err != nil { panic(err) } + pbec := seenCommit.ToProto() + extCommitBytes := mustEncode(pbec) + if err := batch.Set(extCommitKey(height), extCommitBytes); err != nil { + panic(err) + } + if err := batch.WriteSync(); err != nil { panic(err) } @@ -586,6 +616,7 @@ const ( prefixBlockCommit = int64(2) prefixSeenCommit = int64(3) prefixBlockHash = int64(4) + prefixExtCommit = int64(5) ) func blockMetaKey(height int64) []byte { @@ -635,6 +666,14 @@ func seenCommitKey() []byte { return key } +func extCommitKey(height int64) []byte { + key, err := orderedcode.Append(nil, prefixExtCommit, height) + if err != nil { + panic(err) + } + return key +} + func blockHashKey(hash []byte) []byte { key, err := orderedcode.Append(nil, prefixBlockHash, string(hash)) if err != nil { diff --git a/internal/test/factory/commit.go b/internal/test/factory/commit.go index bc4022499..d2be469f3 100644 --- a/internal/test/factory/commit.go +++ b/internal/test/factory/commit.go @@ -8,7 +8,7 @@ import ( "github.com/tendermint/tendermint/types" ) -func MakeCommit(ctx context.Context, blockID types.BlockID, height int64, round int32, voteSet *types.VoteSet, validators []types.PrivValidator, now time.Time) (*types.Commit, error) { +func MakeCommit(ctx context.Context, blockID types.BlockID, height int64, round int32, voteSet *types.VoteSet, validators []types.PrivValidator, now time.Time) (*types.ExtendedCommit, error) { // all sign for i := 0; i < len(validators); i++ { pubKey, err := validators[i].GetPubKey(ctx) @@ -37,5 +37,5 @@ func MakeCommit(ctx context.Context, blockID types.BlockID, height int64, round } } - return voteSet.MakeCommit(), nil + return voteSet.MakeExtendedCommit(), nil } diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go index 10db5d58b..4d949eb7d 100644 --- a/test/e2e/runner/evidence.go +++ b/test/e2e/runner/evidence.go @@ -177,7 +177,7 @@ func generateLightClientAttackEvidence( ConflictingBlock: &types.LightBlock{ SignedHeader: &types.SignedHeader{ Header: header, - Commit: commit, + Commit: commit.StripExtensions(), }, ValidatorSet: conflictingVals, }, diff --git a/types/block.go b/types/block.go index cb1b43ea5..9035e6f91 100644 --- a/types/block.go +++ b/types/block.go @@ -609,6 +609,15 @@ type CommitSig struct { Signature []byte `json:"signature"` } +type ExtendedCommitSig struct { + BlockIDFlag BlockIDFlag `json:"block_id_flag"` + ValidatorAddress Address `json:"validator_address"` + Timestamp time.Time `json:"timestamp"` + Signature []byte `json:"signature"` + VoteExtension []byte `json:"extension"` + ExtensionSignature []byte `json:"extension_signature"` +} + // NewCommitSigForBlock returns new CommitSig with BlockIDFlagCommit. func NewCommitSigForBlock(signature []byte, valAddr Address, ts time.Time) CommitSig { return CommitSig{ @@ -633,16 +642,32 @@ func NewCommitSigAbsent() CommitSig { } } +//TODO: When all UTs passing, try removing the Commit and CommitSig version of new functions +func NewExtendedCommitSigAbsent() ExtendedCommitSig { + return ExtendedCommitSig{ + BlockIDFlag: BlockIDFlagAbsent, + } +} + // ForBlock returns true if CommitSig is for the block. func (cs CommitSig) ForBlock() bool { return cs.BlockIDFlag == BlockIDFlagCommit } +func (ecs ExtendedCommitSig) ForBlock() bool { + return ecs.BlockIDFlag == BlockIDFlagCommit +} + // Absent returns true if CommitSig is absent. func (cs CommitSig) Absent() bool { return cs.BlockIDFlag == BlockIDFlagAbsent } +func (ecs ExtendedCommitSig) Absent() bool { + //TODO What about BlockIDFlagNil? + return ecs.BlockIDFlag == BlockIDFlagAbsent +} + // CommitSig returns a string representation of CommitSig. // // 1. first 6 bytes of signature @@ -674,6 +699,21 @@ func (cs CommitSig) BlockID(commitBlockID BlockID) BlockID { return blockID } +func (ecs ExtendedCommitSig) BlockID(commitBlockID BlockID) BlockID { + var blockID BlockID + switch ecs.BlockIDFlag { + case BlockIDFlagAbsent: + blockID = BlockID{} + case BlockIDFlagCommit: + blockID = commitBlockID + case BlockIDFlagNil: + blockID = BlockID{} + default: + panic(fmt.Sprintf("Unknown BlockIDFlag: %v", ecs.BlockIDFlag)) + } + return blockID +} + // ValidateBasic performs basic validation. func (cs CommitSig) ValidateBasic() error { switch cs.BlockIDFlag { @@ -714,6 +754,62 @@ func (cs CommitSig) ValidateBasic() error { return nil } +func (ecs ExtendedCommitSig) ValidateBasic() error { + switch ecs.BlockIDFlag { + case BlockIDFlagAbsent: + case BlockIDFlagCommit: + case BlockIDFlagNil: + default: + return fmt.Errorf("unknown BlockIDFlag: %v", ecs.BlockIDFlag) + } + + switch ecs.BlockIDFlag { + case BlockIDFlagAbsent: + if len(ecs.ValidatorAddress) != 0 { + return errors.New("validator address is present") + } + if !ecs.Timestamp.IsZero() { + return errors.New("time is present") + } + if len(ecs.Signature) != 0 { + return errors.New("signature is present") + } + if len(ecs.VoteExtension) != 0 { + return errors.New("extension is present") + } + if len(ecs.ExtensionSignature) != 0 { + return errors.New("extension signature is present") + } + default: + if len(ecs.ValidatorAddress) != crypto.AddressSize { + return fmt.Errorf("expected ValidatorAddress size to be %d bytes, got %d bytes", + crypto.AddressSize, + len(ecs.ValidatorAddress), + ) + } + // NOTE: Timestamp validation is subtle and handled elsewhere. + if len(ecs.Signature) == 0 { + return errors.New("signature is missing") + } + if len(ecs.Signature) > MaxSignatureSize { + return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize) + } + //TODO move this to a better place + const MaxExtensionSize = 100 + if len(ecs.VoteExtension) > 100 { + return fmt.Errorf("extension is too big (max: %d)", MaxExtensionSize) + } + if len(ecs.ExtensionSignature) == 0 { + return errors.New("extension signature is missing") + } + if len(ecs.ExtensionSignature) > MaxSignatureSize { + return fmt.Errorf("extension signature is too big (max: %d)", MaxSignatureSize) + } + } + + return nil +} + // ToProto converts CommitSig to protobuf func (cs *CommitSig) ToProto() *tmproto.CommitSig { if cs == nil { @@ -728,10 +824,24 @@ func (cs *CommitSig) ToProto() *tmproto.CommitSig { } } +func (ecs *ExtendedCommitSig) ToProto() *tmproto.ExtendedCommitSig { + if ecs == nil { + return nil + } + + return &tmproto.ExtendedCommitSig{ + BlockIdFlag: tmproto.BlockIDFlag(ecs.BlockIDFlag), + ValidatorAddress: ecs.ValidatorAddress, + Timestamp: ecs.Timestamp, + Signature: ecs.Signature, + VoteExtension: ecs.VoteExtension, + ExtensionSignature: ecs.ExtensionSignature, + } +} + // FromProto sets a protobuf CommitSig to the given pointer. // It returns an error if the CommitSig is invalid. func (cs *CommitSig) FromProto(csp tmproto.CommitSig) error { - cs.BlockIDFlag = BlockIDFlag(csp.BlockIdFlag) cs.ValidatorAddress = csp.ValidatorAddress cs.Timestamp = csp.Timestamp @@ -740,6 +850,17 @@ func (cs *CommitSig) FromProto(csp tmproto.CommitSig) error { return cs.ValidateBasic() } +func (ecs *ExtendedCommitSig) FromProto(ecsp tmproto.ExtendedCommitSig) error { + ecs.BlockIDFlag = BlockIDFlag(ecsp.BlockIdFlag) + ecs.ValidatorAddress = ecsp.ValidatorAddress + ecs.Timestamp = ecsp.Timestamp + ecs.Signature = ecsp.Signature + ecs.VoteExtension = ecsp.VoteExtension + ecs.ExtensionSignature = ecsp.ExtensionSignature + + return ecs.ValidateBasic() +} + //------------------------------------- // Commit contains the evidence that a block was committed by a set of validators. @@ -757,7 +878,18 @@ type Commit struct { // Memoized in first call to corresponding method. // NOTE: can't memoize in constructor because constructor isn't used for // unmarshaling. - hash tmbytes.HexBytes + hash tmbytes.HexBytes + //bitArray *bits.BitArray +} + +// ExtendedCommit is similar to Commit, except that its signatures also retain +// their corresponding vote extensions and vote extension signatures. +type ExtendedCommit struct { + Height int64 `json:"height,string"` + Round int32 `json:"round"` + BlockID BlockID `json:"block_id"` + ExtendedSignatures []ExtendedCommitSig `json:"signatures"` + bitArray *bits.BitArray } @@ -771,28 +903,82 @@ func NewCommit(height int64, round int32, blockID BlockID, commitSigs []CommitSi } } -// CommitToVoteSet constructs a VoteSet from the Commit and validator set. +func NewExtendedCommit(height int64, round int32, blockID BlockID, extCommitSigs []ExtendedCommitSig) *ExtendedCommit { + return &ExtendedCommit{ + Height: height, + Round: round, + BlockID: blockID, + ExtendedSignatures: extCommitSigs, + } +} + +// func (commit Commit) ToExtCommitNoExt() *ExtendedCommit { +// // For every validator, get the precommit with extensions +// extCommitSigs := make([]ExtendedCommitSig, len(commit.Signatures)) +// for i, cs := range commit.Signatures { +// extCommitSigs[i] = ExtendedCommitSig{ +// BlockIDFlag: cs.BlockIDFlag, +// ValidatorAddress: cs.ValidatorAddress, +// Timestamp: cs.Timestamp, +// Signature: cs.Signature, +// } +// } +// return NewExtendedCommit(commit.Height, commit.Round, commit.BlockID, extCommitSigs) +// } + +// ToVoteSet constructs a VoteSet from the Commit and validator set. // Panics if signatures from the commit can't be added to the voteset. // Inverse of VoteSet.MakeCommit(). -func CommitToVoteSet(chainID string, commit *Commit, vals *ValidatorSet) *VoteSet { - voteSet := NewVoteSet(chainID, commit.Height, commit.Round, tmproto.PrecommitType, vals) - for idx, commitSig := range commit.Signatures { - if commitSig.Absent() { +func (extCommit *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet { + voteSet := NewVoteSet(chainID, extCommit.Height, extCommit.Round, tmproto.PrecommitType, vals) + for idx, extCommitSig := range extCommit.ExtendedSignatures { + if extCommitSig.Absent() { continue // OK, some precommits can be missing. } - vote := commit.GetVote(int32(idx)) - if err := vote.ValidateBasic(); err != nil { + vote := extCommit.GetExtendedVote(int32(idx)) + if err := vote.ValidateWithExtension(); err != nil { panic(fmt.Errorf("failed to validate vote reconstructed from LastCommit: %w", err)) } added, err := voteSet.AddVote(vote) if !added || err != nil { - panic(fmt.Errorf("failed to reconstruct LastCommit: %w", err)) + panic(fmt.Errorf("failed to reconstruct vote set from extended commit: %w", err)) } } return voteSet } -// GetVote converts the CommitSig for the given valIdx to a Vote. +func (extCommit *ExtendedCommit) StripExtensions() *Commit { + commitSigs := make([]CommitSig, len(extCommit.ExtendedSignatures)) + for idx, extCommitSig := range extCommit.ExtendedSignatures { + commitSigs[idx] = CommitSig{ + BlockIDFlag: extCommitSig.BlockIDFlag, + ValidatorAddress: extCommitSig.ValidatorAddress, + Timestamp: extCommitSig.Timestamp, + Signature: extCommitSig.Signature, + } + } + return NewCommit(extCommit.Height, extCommit.Round, extCommit.BlockID, commitSigs) +} + +// func (extCommit *ExtendedCommit) ExtensionsStripped() bool { +// strippedIdx := -1 +// for idx, extCommitSig := range extCommit.Signatures { +// if !extCommitSig.ForBlock() { +// continue +// } +// if len(extCommitSig.ExtensionSignature) == 0 { +// strippedIdx = idx +// } else if strippedIdx != -1 { +// panic(fmt.Sprintf("vote extension signature is missing at index %v but present at index %v in extended commit %v", +// strippedIdx, idx, extCommit)) +// } +// } +// return strippedIdx != -1 +// } + +// GetVote converts the CommitSig for the given valIdx to a Vote. Commits do +// not contain vote extensions, so the vote extension and vote extension +// signature will not be present in the returned vote. // Returns nil if the precommit at valIdx is nil. // Panics if valIdx >= commit.Size(). func (commit *Commit) GetVote(valIdx int32) *Vote { @@ -809,6 +995,25 @@ func (commit *Commit) GetVote(valIdx int32) *Vote { } } +// GetExtendedVote converts the ExtendedCommitSig for the given valIdx to a +// Vote with vote extensions. +// Panics if valIdx >= extCommit.Size(). +func (extCommit *ExtendedCommit) GetExtendedVote(valIdx int32) *Vote { + commitSig := extCommit.ExtendedSignatures[valIdx] + return &Vote{ + Type: tmproto.PrecommitType, + Height: extCommit.Height, + Round: extCommit.Round, + BlockID: commitSig.BlockID(extCommit.BlockID), + Timestamp: commitSig.Timestamp, + ValidatorAddress: commitSig.ValidatorAddress, + ValidatorIndex: valIdx, + Signature: commitSig.Signature, + Extension: commitSig.VoteExtension, + ExtensionSignature: commitSig.ExtensionSignature, + } +} + // VoteSignBytes returns the bytes of the Vote corresponding to valIdx for // signing. // @@ -825,20 +1030,32 @@ func (commit *Commit) VoteSignBytes(chainID string, valIdx int32) []byte { // Type returns the vote type of the commit, which is always VoteTypePrecommit // Implements VoteSetReader. -func (commit *Commit) Type() byte { +// func (commit *Commit) Type() byte { +// return byte(tmproto.PrecommitType) +// } + +func (extCommit *ExtendedCommit) Type() byte { return byte(tmproto.PrecommitType) } // GetHeight returns height of the commit. // Implements VoteSetReader. -func (commit *Commit) GetHeight() int64 { - return commit.Height +// func (commit *Commit) GetHeight() int64 { +// return commit.Height +// } + +func (extCommit *ExtendedCommit) GetHeight() int64 { + return extCommit.Height } // GetRound returns height of the commit. // Implements VoteSetReader. -func (commit *Commit) GetRound() int32 { - return commit.Round +// func (commit *Commit) GetRound() int32 { +// return commit.Round +// } + +func (extCommit *ExtendedCommit) GetRound() int32 { + return extCommit.Round } // Size returns the number of signatures in the commit. @@ -850,31 +1067,58 @@ func (commit *Commit) Size() int { return len(commit.Signatures) } +func (extCommit *ExtendedCommit) Size() int { + if extCommit == nil { + return 0 + } + return len(extCommit.ExtendedSignatures) +} + // BitArray returns a BitArray of which validators voted for BlockID or nil in this commit. // Implements VoteSetReader. -func (commit *Commit) BitArray() *bits.BitArray { - if commit.bitArray == nil { - commit.bitArray = bits.NewBitArray(len(commit.Signatures)) - for i, commitSig := range commit.Signatures { +// func (commit *Commit) BitArray() *bits.BitArray { +// if commit.bitArray == nil { +// commit.bitArray = bits.NewBitArray(len(commit.Signatures)) +// for i, commitSig := range commit.Signatures { +// // TODO: need to check the BlockID otherwise we could be counting conflicts, +// // not just the one with +2/3 ! +// commit.bitArray.SetIndex(i, !commitSig.Absent()) +// } +// } +// return commit.bitArray +// } + +func (extCommit *ExtendedCommit) BitArray() *bits.BitArray { + if extCommit.bitArray == nil { + extCommit.bitArray = bits.NewBitArray(len(extCommit.ExtendedSignatures)) + for i, extCommitSig := range extCommit.ExtendedSignatures { // TODO: need to check the BlockID otherwise we could be counting conflicts, // not just the one with +2/3 ! - commit.bitArray.SetIndex(i, !commitSig.Absent()) + extCommit.bitArray.SetIndex(i, !extCommitSig.Absent()) } } - return commit.bitArray + return extCommit.bitArray } // GetByIndex returns the vote corresponding to a given validator index. // Panics if `index >= commit.Size()`. // Implements VoteSetReader. -func (commit *Commit) GetByIndex(valIdx int32) *Vote { - return commit.GetVote(valIdx) +// func (commit *Commit) GetByIndex(valIdx int32) *Vote { +// return commit.GetVote(valIdx) +// } + +func (extCommit *ExtendedCommit) GetByIndex(valIdx int32) *Vote { + return extCommit.GetExtendedVote(valIdx) } // IsCommit returns true if there is at least one signature. // Implements VoteSetReader. -func (commit *Commit) IsCommit() bool { - return len(commit.Signatures) != 0 +// func (commit *Commit) IsCommit() bool { +// return len(commit.Signatures) != 0 +// } + +func (extCommit *ExtendedCommit) IsCommit() bool { + return len(extCommit.ExtendedSignatures) != 0 } // ValidateBasic performs basic validation that doesn't involve state data. @@ -904,6 +1148,31 @@ func (commit *Commit) ValidateBasic() error { return nil } +func (extCommit *ExtendedCommit) ValidateBasic() error { + if extCommit.Height < 0 { + return errors.New("negative Height") + } + if extCommit.Round < 0 { + return errors.New("negative Round") + } + + if extCommit.Height >= 1 { + if extCommit.BlockID.IsNil() { + return errors.New("commit cannot be for nil block") + } + + if len(extCommit.ExtendedSignatures) == 0 { + return errors.New("no signatures in commit") + } + for i, extCommitSig := range extCommit.ExtendedSignatures { + if err := extCommitSig.ValidateBasic(); err != nil { + return fmt.Errorf("wrong ExtendedCommitSig #%d: %v", i, err) + } + } + } + return nil +} + // Hash returns the hash of the commit func (commit *Commit) Hash() tmbytes.HexBytes { if commit == nil { @@ -969,6 +1238,25 @@ func (commit *Commit) ToProto() *tmproto.Commit { return c } +func (extCommit *ExtendedCommit) ToProto() *tmproto.ExtendedCommit { + if extCommit == nil { + return nil + } + + c := new(tmproto.ExtendedCommit) + sigs := make([]tmproto.ExtendedCommitSig, len(extCommit.ExtendedSignatures)) + for i := range extCommit.ExtendedSignatures { + sigs[i] = *extCommit.ExtendedSignatures[i].ToProto() + } + c.ExtendedSignatures = sigs + + c.Height = extCommit.Height + c.Round = extCommit.Round + c.BlockID = extCommit.BlockID.ToProto() + + return c +} + // FromProto sets a protobuf Commit to the given pointer. // It returns an error if the commit is invalid. func CommitFromProto(cp *tmproto.Commit) (*Commit, error) { @@ -1000,6 +1288,35 @@ func CommitFromProto(cp *tmproto.Commit) (*Commit, error) { return commit, commit.ValidateBasic() } +func ExtendedCommitFromProto(ecp *tmproto.ExtendedCommit) (*ExtendedCommit, error) { + if ecp == nil { + return nil, errors.New("nil ExtendedCommit") + } + + var ( + extCommit = new(ExtendedCommit) + ) + + bi, err := BlockIDFromProto(&ecp.BlockID) + if err != nil { + return nil, err + } + + sigs := make([]ExtendedCommitSig, len(ecp.ExtendedSignatures)) + for i := range ecp.ExtendedSignatures { + if err := sigs[i].FromProto(ecp.ExtendedSignatures[i]); err != nil { + return nil, err + } + } + extCommit.ExtendedSignatures = sigs + + extCommit.Height = ecp.Height + extCommit.Round = ecp.Round + extCommit.BlockID = *bi + + return extCommit, extCommit.ValidateBasic() +} + //----------------------------------------------------------------------------- // Data contains the set of transactions included in the block diff --git a/types/test_util.go b/types/test_util.go index 8aea2f02c..916a158e2 100644 --- a/types/test_util.go +++ b/types/test_util.go @@ -33,7 +33,7 @@ func makeCommit(ctx context.Context, blockID BlockID, height int64, round int32, } } - return voteSet.MakeCommit(), nil + return voteSet.MakeExtendedCommit().StripExtensions(), nil } func signAddVote(ctx context.Context, privVal PrivValidator, vote *Vote, voteSet *VoteSet) (signed bool, err error) { diff --git a/types/vote.go b/types/vote.go index f20ee491e..9593b9aae 100644 --- a/types/vote.go +++ b/types/vote.go @@ -109,6 +109,34 @@ func (vote *Vote) CommitSig() CommitSig { } } +func (vote *Vote) ExtendedCommitSig() ExtendedCommitSig { + if vote == nil { + return NewExtendedCommitSigAbsent() + } + + var blockIDFlag BlockIDFlag + switch { + case vote.BlockID.IsComplete(): + blockIDFlag = BlockIDFlagCommit + if vote.ExtensionSignature == nil { + panic(fmt.Sprintf("Invalid vote %v - a BlockID is complete but missing vote extension signature", vote)) + } + case vote.BlockID.IsNil(): + blockIDFlag = BlockIDFlagNil + default: + panic(fmt.Sprintf("Invalid vote %v - expected BlockID to be either empty or complete", vote)) + } + + return ExtendedCommitSig{ + BlockIDFlag: blockIDFlag, + ValidatorAddress: vote.ValidatorAddress, + Timestamp: vote.Timestamp, + Signature: vote.Signature, + VoteExtension: vote.Extension, + ExtensionSignature: vote.ExtensionSignature, + } +} + // VoteSignBytes returns the proto-encoding of the canonicalized Vote, for // signing. Panics if the marshaling fails. // diff --git a/types/vote_set.go b/types/vote_set.go index b4d149576..0c783d5d1 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -611,31 +611,31 @@ func (voteSet *VoteSet) sumTotalFrac() (int64, int64, float64) { // // Panics if the vote type is not PrecommitType or if there's no +2/3 votes for // a single block. -func (voteSet *VoteSet) MakeCommit() *Commit { +func (voteSet *VoteSet) MakeExtendedCommit() *ExtendedCommit { if voteSet.signedMsgType != tmproto.PrecommitType { - panic("Cannot MakeCommit() unless VoteSet.Type is PrecommitType") + panic("Cannot MakeExtendCommit() unless VoteSet.Type is PrecommitType") } voteSet.mtx.Lock() defer voteSet.mtx.Unlock() // Make sure we have a 2/3 majority if voteSet.maj23 == nil { - panic("Cannot MakeCommit() unless a blockhash has +2/3") + panic("Cannot MakeExtendCommit() unless a blockhash has +2/3") } - // For every validator, get the precommit - commitSigs := make([]CommitSig, len(voteSet.votes)) + // For every validator, get the precommit with extensions + extCommitSigs := make([]ExtendedCommitSig, len(voteSet.votes)) for i, v := range voteSet.votes { - commitSig := v.CommitSig() + extCommitSig := v.ExtendedCommitSig() // if block ID exists but doesn't match, exclude sig - if commitSig.ForBlock() && !v.BlockID.Equals(*voteSet.maj23) { - commitSig = NewCommitSigAbsent() + if extCommitSig.ForBlock() && !v.BlockID.Equals(*voteSet.maj23) { + extCommitSig = NewExtendedCommitSigAbsent() } - commitSigs[i] = commitSig + extCommitSigs[i] = extCommitSig } - return NewCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, commitSigs) + return NewExtendedCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, extCommitSigs) } //--------------------------------------------------------------------------------