abci: Change validators to last_commit_info in RequestBeginBlock (#2074)

* change validators to last_commit_info in RequestBeginBlock
* do not send pubkeys with RequestBeginBlock

Refs #1856

CHANGELOG_PENDING.md

@@ -7,6 +7,7 @@ BREAKING CHANGES:
 - [tools] Removed `make ensure_deps` in favor of `make get_vendor_deps`
 - [p2p] Remove salsa and ripemd primitives, in favor of using chacha as a stream cipher, and hkdf
 - [abci] Changed time format from int64 to google.protobuf.Timestamp
+- [abci] Changed Validators to LastCommitInfo in RequestBeginBlock
 
 FEATURES:
 - [tools] Added `make check_dep`

abci/types/types.proto

@@ -71,10 +71,11 @@ message RequestQuery {
   bool prove = 4;
 }
 
+// NOTE: validators here have empty pubkeys.
 message RequestBeginBlock {
   bytes hash = 1;
   Header header = 2 [(gogoproto.nullable)=false];
-  repeated SigningValidator validators = 3  [(gogoproto.nullable)=false];
+  LastCommitInfo last_commit_info = 3 [(gogoproto.nullable)=false];
   repeated Evidence byzantine_validators = 4 [(gogoproto.nullable)=false];
 }
 
@@ -203,14 +204,14 @@ message ConsensusParams {
   BlockGossip block_gossip = 3;
 }
 
-// BlockSize contain limits on the block size.
+// BlockSize contains limits on the block size.
 message BlockSize {
   int32 max_bytes = 1;
   int32 max_txs = 2;
   int64 max_gas = 3;
 }
 
-// TxSize contain limits on the tx size.
+// TxSize contains limits on the tx size.
 message TxSize {
   int32 max_bytes = 1;
   int64 max_gas = 2;
@@ -223,6 +224,11 @@ message BlockGossip {
   int32 block_part_size_bytes = 1;
 }
 
+message LastCommitInfo {
+  int32 commit_round = 1;
+  repeated SigningValidator validators = 2 [(gogoproto.nullable)=false];
+}
+
 //----------------------------------------
 // Blockchain Types
 

abci/types/typespb_test.go

@@ -1646,6 +1646,62 @@ func TestBlockGossipMarshalTo(t *testing.T) {
 	}
 }
 
+func TestLastCommitInfoProto(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, false)
+	dAtA, err := github_com_gogo_protobuf_proto.Marshal(p)
+	if err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	msg := &LastCommitInfo{}
+	if err := github_com_gogo_protobuf_proto.Unmarshal(dAtA, msg); err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	littlefuzz := make([]byte, len(dAtA))
+	copy(littlefuzz, dAtA)
+	for i := range dAtA {
+		dAtA[i] = byte(popr.Intn(256))
+	}
+	if !p.Equal(msg) {
+		t.Fatalf("seed = %d, %#v !Proto %#v", seed, msg, p)
+	}
+	if len(littlefuzz) > 0 {
+		fuzzamount := 100
+		for i := 0; i < fuzzamount; i++ {
+			littlefuzz[popr.Intn(len(littlefuzz))] = byte(popr.Intn(256))
+			littlefuzz = append(littlefuzz, byte(popr.Intn(256)))
+		}
+		// shouldn't panic
+		_ = github_com_gogo_protobuf_proto.Unmarshal(littlefuzz, msg)
+	}
+}
+
+func TestLastCommitInfoMarshalTo(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, false)
+	size := p.Size()
+	dAtA := make([]byte, size)
+	for i := range dAtA {
+		dAtA[i] = byte(popr.Intn(256))
+	}
+	_, err := p.MarshalTo(dAtA)
+	if err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	msg := &LastCommitInfo{}
+	if err := github_com_gogo_protobuf_proto.Unmarshal(dAtA, msg); err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	for i := range dAtA {
+		dAtA[i] = byte(popr.Intn(256))
+	}
+	if !p.Equal(msg) {
+		t.Fatalf("seed = %d, %#v !Proto %#v", seed, msg, p)
+	}
+}
+
 func TestHeaderProto(t *testing.T) {
 	seed := time.Now().UnixNano()
 	popr := math_rand.New(math_rand.NewSource(seed))
@@ -2448,6 +2504,24 @@ func TestBlockGossipJSON(t *testing.T) {
 		t.Fatalf("seed = %d, %#v !Json Equal %#v", seed, msg, p)
 	}
 }
+func TestLastCommitInfoJSON(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, true)
+	marshaler := github_com_gogo_protobuf_jsonpb.Marshaler{}
+	jsondata, err := marshaler.MarshalToString(p)
+	if err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	msg := &LastCommitInfo{}
+	err = github_com_gogo_protobuf_jsonpb.UnmarshalString(jsondata, msg)
+	if err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	if !p.Equal(msg) {
+		t.Fatalf("seed = %d, %#v !Json Equal %#v", seed, msg, p)
+	}
+}
 func TestHeaderJSON(t *testing.T) {
 	seed := time.Now().UnixNano()
 	popr := math_rand.New(math_rand.NewSource(seed))
@@ -3350,6 +3424,34 @@ func TestBlockGossipProtoCompactText(t *testing.T) {
 	}
 }
 
+func TestLastCommitInfoProtoText(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, true)
+	dAtA := github_com_gogo_protobuf_proto.MarshalTextString(p)
+	msg := &LastCommitInfo{}
+	if err := github_com_gogo_protobuf_proto.UnmarshalText(dAtA, msg); err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	if !p.Equal(msg) {
+		t.Fatalf("seed = %d, %#v !Proto %#v", seed, msg, p)
+	}
+}
+
+func TestLastCommitInfoProtoCompactText(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, true)
+	dAtA := github_com_gogo_protobuf_proto.CompactTextString(p)
+	msg := &LastCommitInfo{}
+	if err := github_com_gogo_protobuf_proto.UnmarshalText(dAtA, msg); err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	if !p.Equal(msg) {
+		t.Fatalf("seed = %d, %#v !Proto %#v", seed, msg, p)
+	}
+}
+
 func TestHeaderProtoText(t *testing.T) {
 	seed := time.Now().UnixNano()
 	popr := math_rand.New(math_rand.NewSource(seed))
@@ -4128,6 +4230,28 @@ func TestBlockGossipSize(t *testing.T) {
 	}
 }
 
+func TestLastCommitInfoSize(t *testing.T) {
+	seed := time.Now().UnixNano()
+	popr := math_rand.New(math_rand.NewSource(seed))
+	p := NewPopulatedLastCommitInfo(popr, true)
+	size2 := github_com_gogo_protobuf_proto.Size(p)
+	dAtA, err := github_com_gogo_protobuf_proto.Marshal(p)
+	if err != nil {
+		t.Fatalf("seed = %d, err = %v", seed, err)
+	}
+	size := p.Size()
+	if len(dAtA) != size {
+		t.Errorf("seed = %d, size %v != marshalled size %v", seed, size, len(dAtA))
+	}
+	if size2 != size {
+		t.Errorf("seed = %d, size %v != before marshal proto.Size %v", seed, size, size2)
+	}
+	size3 := github_com_gogo_protobuf_proto.Size(p)
+	if size3 != size {
+		t.Errorf("seed = %d, size %v != after marshal proto.Size %v", seed, size, size3)
+	}
+}
+
 func TestHeaderSize(t *testing.T) {
 	seed := time.Now().UnixNano()
 	popr := math_rand.New(math_rand.NewSource(seed))

docs/app-dev/abci-spec.md

@@ -160,9 +160,8 @@ See below for more details on the message types and how they are used.
 - **Request**:
   - `Hash ([]byte)`: The block's hash. This can be derived from the
     block header.
-  - `Header (struct{})`: The block header
-  - `Validators ([]SigningValidator)`: List of validators in the current validator
-    set and whether or not they signed a vote in the LastCommit
+  - `Header (struct{})`: The block header.
+  - `LastCommitInfo (LastCommitInfo)`: Info about the last commit.
   - `ByzantineValidators ([]Evidence)`: List of evidence of
     validators that acted maliciously
 - **Response**:
@@ -171,8 +170,9 @@ See below for more details on the message types and how they are used.
   - Signals the beginning of a new block. Called prior to
     any DeliverTxs.
   - The header is expected to at least contain the Height.
-  - The `Validators` and `ByzantineValidators` can be used to
-    determine rewards and punishments for the validators.
+  - The `LastCommitInfo` and `ByzantineValidators` can be used to determine
+    rewards and punishments for the validators. NOTE validators here do not
+    include pubkeys.
 
 ### CheckTx
 
@@ -326,3 +326,10 @@ See below for more details on the message types and how they are used.
     It is the proposer's local time when block was created.
   - `TotalVotingPower (int64)`: Total voting power of the validator set at
     height `Height`
+
+### LastCommitInfo
+
+- **Fields**:
+  - `CommitRound (int32)`: Commit round.
+  - `Validators ([]SigningValidator)`: List of validators in the current
+    validator set and whether or not they signed a vote.

state/execution.go

@@ -188,9 +188,12 @@ func execBlockOnProxyApp(logger log.Logger, proxyAppConn proxy.AppConnConsensus,
 
 	// Begin block
 	_, err := proxyAppConn.BeginBlockSync(abci.RequestBeginBlock{
-		Hash:                block.Hash(),
-		Header:              types.TM2PB.Header(&block.Header),
-		Validators:          signVals,
+		Hash:   block.Hash(),
+		Header: types.TM2PB.Header(&block.Header),
+		LastCommitInfo: abci.LastCommitInfo{
+			CommitRound: int32(block.LastCommit.Round()),
+			Validators:  signVals,
+		},
 		ByzantineValidators: byzVals,
 	})
 	if err != nil {
@@ -245,7 +248,7 @@ func getBeginBlockValidatorInfo(block *types.Block, lastValSet *types.ValidatorS
 			vote = block.LastCommit.Precommits[i]
 		}
 		val := abci.SigningValidator{
-			Validator:       types.TM2PB.Validator(val),
+			Validator:       types.TM2PB.ValidatorWithoutPubKey(val),
 			SignedLastBlock: vote != nil,
 		}
 		signVals[i] = val

state/execution_test.go

@@ -293,7 +293,7 @@ func (app *testApp) Info(req abci.RequestInfo) (resInfo abci.ResponseInfo) {
 }
 
 func (app *testApp) BeginBlock(req abci.RequestBeginBlock) abci.ResponseBeginBlock {
-	app.Validators = req.Validators
+	app.Validators = req.LastCommitInfo.Validators
 	app.ByzantineValidators = req.ByzantineValidators
 	return abci.ResponseBeginBlock{}
 }

types/protobuf.go

@@ -50,6 +50,13 @@ func (tm2pb) Header(header *Header) abci.Header {
 	}
 }
 
+func (tm2pb) ValidatorWithoutPubKey(val *Validator) abci.Validator {
+	return abci.Validator{
+		Address: val.PubKey.Address(),
+		Power:   val.VotingPower,
+	}
+}
+
 // XXX: panics on unknown pubkey type
 func (tm2pb) Validator(val *Validator) abci.Validator {
 	return abci.Validator{
@@ -129,7 +136,7 @@ func (tm2pb) Evidence(ev Evidence, valSet *ValidatorSet, evTime time.Time) abci.
 
 	return abci.Evidence{
 		Type:             evType,
-		Validator:        TM2PB.Validator(val),
+		Validator:        TM2PB.ValidatorWithoutPubKey(val),
 		Height:           ev.Height(),
 		Time:             evTime,
 		TotalVotingPower: valSet.TotalVotingPower(),

types/protobuf_test.go

@@ -102,6 +102,9 @@ func TestABCIEvidence(t *testing.T) {
 	)
 
 	assert.Equal(t, "duplicate/vote", abciEv.Type)
+
+	// test we do not send pubkeys
+	assert.Empty(t, abciEv.Validator.PubKey)
 }
 
 type pubKeyEddie struct{}
@@ -120,3 +123,21 @@ func TestABCIValidatorFromPubKeyAndPower(t *testing.T) {
 	assert.Panics(t, func() { TM2PB.ValidatorFromPubKeyAndPower(nil, 10) })
 	assert.Panics(t, func() { TM2PB.ValidatorFromPubKeyAndPower(pubKeyEddie{}, 10) })
 }
+
+func TestABCIValidatorWithoutPubKey(t *testing.T) {
+	pkEd := ed25519.GenPrivKey().PubKey()
+
+	abciVal := TM2PB.ValidatorWithoutPubKey(&Validator{
+		Address:     pkEd.Address(),
+		PubKey:      pkEd,
+		VotingPower: 10,
+	})
+
+	// pubkey must be nil
+	tmValExpected := abci.Validator{
+		Address: pkEd.Address(),
+		Power:   10,
+	}
+
+	assert.Equal(t, tmValExpected, abciVal)
+}