fix: add proof size check when the VRF proof is empty (#765)

* fix: handle the case when the proof is empty Signed-off-by: 170210 <j170210@icloud.com> * chore: add test cases for ValidateProof Signed-off-by: 170210 <j170210@icloud.com> * fix: fix for all failed tests Signed-off-by: 170210 <j170210@icloud.com> * fixup: fix for comment Signed-off-by: 170210 <j170210@icloud.com> * fixup: fix for comment Signed-off-by: 170210 <j170210@icloud.com> * docs: update document Signed-off-by: 170210 <j170210@icloud.com> --------- Signed-off-by: 170210 <j170210@icloud.com>
Finschia · Mar 15, 2024 · 1279868 · 1279868
1 parent ae67c39
commit 1279868
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 17 deletions.
diff --git a/blockchain/msgs_test.go b/blockchain/msgs_test.go
@@ -3,13 +3,15 @@ package blockchain
 import (
 	"encoding/hex"
 	"fmt"
-	"github.com/gogo/protobuf/proto"
+	"math"
+	"testing"
+
 	"github.com/Finschia/ostracon/crypto"
+	"github.com/gogo/protobuf/proto"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"math"
-	"testing"
 
+	voivrf "github.com/oasisprotocol/curve25519-voi/primitives/ed25519/extra/ecvrf"
 	bcproto "github.com/tendermint/tendermint/proto/tendermint/blockchain"
 
 	ocbcproto "github.com/Finschia/ostracon/proto/ostracon/blockchain"
@@ -146,6 +148,9 @@ func TestEncodeDecodeValidateMsg(t *testing.T) {
 		nil,
 		sm.InitStateVersion.Consensus)
 	block.ProposerAddress = make([]byte, crypto.AddressSize)
+	round := int32(0)
+	proof := make([]byte, voivrf.ProofSize)
+	block.Entropy.Populate(round, proof)
 	bpb, err := block.ToProto()
 	require.NoError(t, err)
 
@@ -167,7 +172,7 @@ func TestEncodeDecodeValidateMsg(t *testing.T) {
 		{
 			name:    "ocbcproto.BlockResponse", // Ostracon
 			args:    args{pb: &ocbcproto.BlockResponse{Block: bpb}},
-			want:    []byte{0x1a, 0xf0, 0x1, 0xa, 0xed, 0x1, 0xa, 0x93, 0x1, 0xa, 0x2, 0x8, 0xb, 0x18, 0x3, 0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x32, 0x20, 0x1e, 0xba, 0x40, 0x13, 0xa, 0xf2, 0x5e, 0xd1, 0x9, 0x5f, 0x67, 0x86, 0xe5, 0x8d, 0xb9, 0x4d, 0xeb, 0xf4, 0x6a, 0x0, 0x7f, 0xc6, 0x8c, 0x20, 0x32, 0x39, 0x2f, 0xde, 0xdd, 0x32, 0x26, 0x7e, 0x3a, 0x20, 0xc4, 0xda, 0x88, 0xe8, 0x76, 0x6, 0x2a, 0xa1, 0x54, 0x34, 0x0, 0xd5, 0xd, 0xe, 0xaa, 0xd, 0xac, 0x88, 0x9, 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, 0x7f, 0x3a, 0x48, 0xc0, 0x4b, 0xf9, 0x6a, 0x20, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55, 0x72, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x12, 0xd, 0xa, 0xb, 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x1a, 0x0, 0x22, 0x41, 0x1a, 0x2, 0x12, 0x0, 0x22, 0x3b, 0x8, 0x2, 0x12, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x22, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc2, 0x3e, 0x0},
+			want:    []byte{0x1a, 0xc2, 0x2, 0xa, 0xbf, 0x2, 0xa, 0x93, 0x1, 0xa, 0x2, 0x8, 0xb, 0x18, 0x3, 0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x2a, 0x2, 0x12, 0x0, 0x32, 0x20, 0x1e, 0xba, 0x40, 0x13, 0xa, 0xf2, 0x5e, 0xd1, 0x9, 0x5f, 0x67, 0x86, 0xe5, 0x8d, 0xb9, 0x4d, 0xeb, 0xf4, 0x6a, 0x0, 0x7f, 0xc6, 0x8c, 0x20, 0x32, 0x39, 0x2f, 0xde, 0xdd, 0x32, 0x26, 0x7e, 0x3a, 0x20, 0xc4, 0xda, 0x88, 0xe8, 0x76, 0x6, 0x2a, 0xa1, 0x54, 0x34, 0x0, 0xd5, 0xd, 0xe, 0xaa, 0xd, 0xac, 0x88, 0x9, 0x60, 0x57, 0x94, 0x9c, 0xfb, 0x7b, 0xca, 0x7f, 0x3a, 0x48, 0xc0, 0x4b, 0xf9, 0x6a, 0x20, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55, 0x72, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x12, 0xd, 0xa, 0xb, 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x1a, 0x0, 0x22, 0x41, 0x1a, 0x2, 0x12, 0x0, 0x22, 0x3b, 0x8, 0x2, 0x12, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, 0x22, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc2, 0x3e, 0x52, 0x12, 0x50, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
 			wantErr: assert.NoError,
 		},
 		{

diff --git a/crypto/ed25519/migration.go b/crypto/ed25519/migration.go
@@ -41,14 +41,12 @@ func ProofToHash(proof []byte) ([]byte, error) {
 // ValidateProof returns an error if the proof is not empty, but its
 // size != vrf.ProofSize.
 func ValidateProof(h []byte) error {
-	if len(h) > 0 {
-		proofSize := len(h)
-		if proofSize != voivrf.ProofSize && proofSize != r2vrf.ProofSize {
-			return fmt.Errorf("expected size to be %d bytes, got %d bytes",
-				voivrf.ProofSize,
-				len(h),
-			)
-		}
+	proofSize := len(h)
+	if proofSize != voivrf.ProofSize && proofSize != r2vrf.ProofSize {
+		return fmt.Errorf("expected size to be %d bytes, got %d bytes",
+			voivrf.ProofSize,
+			proofSize,
+		)
 	}
 	return nil
 }

diff --git a/crypto/ed25519/migration_test.go b/crypto/ed25519/migration_test.go
@@ -72,6 +72,41 @@ func TestProofToHash(t *testing.T) {
 	}
 }
 
+func TestValidateProof(t *testing.T) {
+	cases := map[string]struct {
+		h     []byte
+		valid bool
+	}{
+		"empty proof": {
+			h:     []byte{},
+			valid: false,
+		},
+		"voi invalid proof": {
+			h:     make([]byte, voivrf.ProofSize),
+			valid: true,
+		},
+		"r2ishiguro proof": {
+			h:     make([]byte, r2vrf.ProofSize),
+			valid: true,
+		},
+		"invalid proof": {
+			h:     make([]byte, 1),
+			valid: false,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			err := ed25519.ValidateProof(tc.h)
+			if !tc.valid {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}
+
 func TestVersionControl(t *testing.T) {
 	vrf := ed25519.NewVersionedVrfNoProve()
 

diff --git a/rpc/core/blocks_test.go b/rpc/core/blocks_test.go
@@ -16,6 +16,7 @@ import (
 
 	cfg "github.com/Finschia/ostracon/config"
 	"github.com/Finschia/ostracon/crypto"
+	"github.com/Finschia/ostracon/crypto/ed25519"
 	tmrand "github.com/Finschia/ostracon/libs/rand"
 	ctypes "github.com/Finschia/ostracon/rpc/core/types"
 	rpctypes "github.com/Finschia/ostracon/rpc/jsonrpc/types"
@@ -306,7 +307,11 @@ func storeTestBlocks(startHeight, numToMakeBlocks, numToMakeTxs int64, state sm.
 			env.TxIndexer.Index(&abci.TxResult{Height: height, Index: uint32(txIndex), Tx: tx}) // nolint:errcheck
 		}
 		lastCommit := types.NewCommit(lastHeight, round, blockID, commitSigs)
-		block, _ := state.MakeBlock(height, txs, lastCommit, nil, proposer.Address, round, nil)
+		message := state.MakeHashMessage(round)
+		pk := ed25519.GenPrivKeyFromSecret([]byte("test private validator"))
+		privVal := types.NewMockPVWithParams(pk, false, false)
+		proof, _ := privVal.GenerateVRFProof(message)
+		block, _ := state.MakeBlock(height, txs, lastCommit, nil, proposer.Address, round, proof)
 		blockPart := block.MakePartSet(partSize)
 		// Indexing
 		env.BlockIndexer.Index(types.EventDataNewBlockHeader{Header: block.Header}) // nolint:errcheck

diff --git a/spec/core/data_structures.md b/spec/core/data_structures.md
@@ -53,4 +53,4 @@ Ostracon introduces Entropy as a new data structure. This represents height-spec
 | Name | Type | Description | Validation                                             |
 |------|------|-------------|--------------------------------------------------------|
 | Round | int32                     | Round in which proposer generate a vrf proof             | Must be >= 0                                           |
-| Proof | slice of bytes (`[]byte`) | Proof is a vrf proof | Length of proof must be == 0 or == 80 (curve25519-voi) |
+| Proof | slice of bytes (`[]byte`) | Proof is a vrf proof | Length of proof must be == 80 (curve25519-voi) |
diff --git a/store/store_test.go b/store/store_test.go
@@ -19,6 +19,7 @@ import (
 
 	cfg "github.com/Finschia/ostracon/config"
 	"github.com/Finschia/ostracon/crypto"
+	"github.com/Finschia/ostracon/crypto/ed25519"
 	"github.com/Finschia/ostracon/libs/log"
 	tmrand "github.com/Finschia/ostracon/libs/rand"
 	sm "github.com/Finschia/ostracon/state"
@@ -51,8 +52,13 @@ func makeTxs(height int64) (txs []types.Tx) {
 }
 
 func makeBlock(height int64, state sm.State, lastCommit *types.Commit) *types.Block {
+	round := int32(0)
+	message := state.MakeHashMessage(round)
+	pk := ed25519.GenPrivKeyFromSecret([]byte("test private validator"))
+	privVal := types.NewMockPVWithParams(pk, false, false)
+	proof, _ := privVal.GenerateVRFProof(message)
 	block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, nil,
-		state.Validators.SelectProposer(state.LastProofHash, height, 0).Address, 0, nil)
+		state.Validators.SelectProposer(state.LastProofHash, height, round).Address, round, proof)
 	return block
 }
 

diff --git a/types/block_test.go b/types/block_test.go
@@ -137,7 +137,10 @@ func TestBlockValidateBasic(t *testing.T) {
 		i := i
 		t.Run(tc.testName, func(t *testing.T) {
 			block := MakeBlock(h, txs, commit, evList, TestConsensusVersion)
-			block.ProposerAddress = valSet.SelectProposer([]byte{}, block.Height, 0).Address
+			round := int32(0)
+			block.ProposerAddress = valSet.SelectProposer([]byte{}, block.Height, round).Address
+			proof := make([]byte, vrf.ProofSize)
+			block.Entropy.Populate(round, proof)
 			tc.malleateBlock(block)
 			err = block.ValidateBasic()
 			assert.Equal(t, tc.expErr, err != nil, "#%d: %v", i, err)
@@ -812,19 +815,24 @@ func TestBlockIDValidateBasic(t *testing.T) {
 
 func TestBlockProtoBuf(t *testing.T) {
 	h := tmrand.Int63()
+	round := int32(0)
+	proof := make([]byte, vrf.ProofSize)
 	c1 := randCommit(time.Now())
 	b1 := MakeBlock(h, []Tx{Tx([]byte{1})}, &Commit{Signatures: []CommitSig{}}, []Evidence{}, TestConsensusVersion)
 	b1.ProposerAddress = tmrand.Bytes(crypto.AddressSize)
+	b1.Entropy.Populate(round, proof)
 
 	b2 := MakeBlock(h, []Tx{Tx([]byte{1})}, c1, []Evidence{}, TestConsensusVersion)
 	b2.ProposerAddress = tmrand.Bytes(crypto.AddressSize)
 	evidenceTime := time.Date(2019, 1, 1, 0, 0, 0, 0, time.UTC)
 	evi := NewMockDuplicateVoteEvidence(h, evidenceTime, "block-test-chain")
 	b2.Evidence = EvidenceData{Evidence: EvidenceList{evi}}
 	b2.EvidenceHash = b2.Evidence.Hash()
+	b2.Entropy.Populate(round, proof)
 
 	b3 := MakeBlock(h, []Tx{}, c1, []Evidence{}, TestConsensusVersion)
 	b3.ProposerAddress = tmrand.Bytes(crypto.AddressSize)
+	b3.Entropy.Populate(round, proof)
 	testCases := []struct {
 		msg      string
 		b1       *Block
@@ -1163,7 +1171,7 @@ func TestEntropyProto(t *testing.T) {
 		expPass bool
 	}{
 		{"success", &vp1, true},
-		{"success empty Entropy", &Entropy{}, true},
+		{"failed empty Entropy", &Entropy{}, false},
 	}
 
 	for _, tt := range tc {