Cx receipt fixes

consensus/quorum/quorom_test.go

@@ -52,6 +52,38 @@ func TestPolicyStrings(t *testing.T) {
 	}
 }
 
+func TestUniformVerifierQuorumByMask(t *testing.T) {
+	verifier := &uniformVerifier{pubKeyCnt: 4}
+	publics := make([]bls.PublicKeyWrapper, verifier.pubKeyCnt)
+	for i := range publics {
+		publics[i].Object = bls.RandPrivateKey().GetPublicKey()
+		publics[i].Bytes.FromLibBLSPublicKey(publics[i].Object)
+	}
+
+	tests := []struct {
+		name   string
+		bitmap []byte
+		quorum bool
+	}{
+		{name: "no signers", bitmap: []byte{0x00}, quorum: false},
+		{name: "below threshold", bitmap: []byte{0x03}, quorum: false},
+		{name: "at threshold", bitmap: []byte{0x07}, quorum: true},
+		{name: "padding bits", bitmap: []byte{0xf0}, quorum: false},
+		{name: "nil mask", bitmap: nil, quorum: false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var mask *bls.Mask
+			if test.bitmap != nil {
+				mask = bls.NewMask(publics)
+				require.NoError(t, mask.SetMask(test.bitmap))
+			}
+			assert.Equal(t, test.quorum, verifier.IsQuorumAchievedByMask(mask))
+		})
+	}
+}
+
 func TestAddingQuoromParticipants(t *testing.T) {
 	decider := NewDecider(SuperMajorityVote, shard.BeaconChainShardID)
 

consensus/quorum/verifier.go

@@ -70,15 +70,18 @@ func newUniformVerifier(committee *shard.Committee) (*uniformVerifier, error) {
 	}, nil
 }
 
-// IsQuorumAchievedByMask returns whether the quorum is achieved with the provided mask,
-// which is whether more than (2/3+1) nodes is included in mask.
+// IsQuorumAchievedByMask returns whether the quorum is achieved with the provided mask.
 func (uv *uniformVerifier) IsQuorumAchievedByMask(mask *bls_cosi.Mask) bool {
-	got := int64(len(mask.Publics))
-	exp := uv.thresholdKeyCount()
-	// Theoretically speaking, greater or equal will do the work. But current logic is more strict
-	// without equal, thus conform to current logic implemented.
-	// (engineImpl.VerifySeal, uniformVoteWeight.IsQuorumAchievedByMask)
-	return got > exp
+	if mask == nil {
+		return false
+	}
+	var signerCount int64
+	for i := range mask.Publics {
+		if enabled, err := mask.IndexEnabled(i); err == nil && enabled {
+			signerCount++
+		}
+	}
+	return signerCount >= uv.thresholdKeyCount()
 }
 
 func (uv *uniformVerifier) thresholdKeyCount() int64 {

core/blockchain_impl.go

@@ -2708,7 +2708,14 @@ func (bc *BlockChainImpl) CXMerkleProof(toShardID uint32, block *block.Header) (
 
 func (bc *BlockChainImpl) WriteCXReceiptsProofSpent(db rawdb.DatabaseWriter, cxps []*types.CXReceiptsProof) error {
 	for _, cxp := range cxps {
-		if cxp.Header != nil && bc.Config().IsCXMerkleProofReplayFixEpoch(cxp.Header.Epoch()) {
+		// Key the spent-marker off the signed Header, not the unauthenticated
+		// MerkleProof.ShardID/BlockNum: those fields are only bound to the
+		// Header by ValidateCXReceiptsProof from IsCXMerkleProofReplayFixEpoch
+		// onward, so a proof claiming an earlier epoch can carry a mutated
+		// MerkleProof while keeping a genuine Header/signature. Deriving the
+		// key from MerkleProof would let such a mutated copy of an already
+		//-applied receipt look unspent and be replayed for a fresh credit.
+		if cxp.Header != nil {
 			if err := rawdb.WriteCXReceiptsProofSpentWithKey(
 				db, cxp.Header.ShardID(), cxp.Header.Number().Uint64(),
 			); err != nil {
@@ -2726,7 +2733,10 @@ func (bc *BlockChainImpl) WriteCXReceiptsProofSpent(db rawdb.DatabaseWriter, cxp
 func (bc *BlockChainImpl) IsSpent(cxp *types.CXReceiptsProof) bool {
 	shardID := cxp.MerkleProof.ShardID
 	blockNum := cxp.MerkleProof.BlockNum.Uint64()
-	if cxp.Header != nil && bc.Config().IsCXMerkleProofReplayFixEpoch(cxp.Header.Epoch()) {
+	// See WriteCXReceiptsProofSpent: always resolve the spent-marker key from
+	// the signed Header so the check can't be bypassed by mutating the
+	// unauthenticated MerkleProof fields on a genuine, previously-applied proof.
+	if cxp.Header != nil {
 		shardID = cxp.Header.ShardID()
 		blockNum = cxp.Header.Number().Uint64()
 	}

core/blockchain_impl_test.go

@@ -11,6 +11,51 @@ import (
 	staking "github.com/harmony-one/harmony/staking/types"
 )
 
+// TestIsSpentIgnoresMutatedMerkleProofIdentity guards against replaying a
+// genuine, already-applied CXReceiptsProof by mutating the unauthenticated
+// MerkleProof.ShardID/BlockNum while keeping the same signed Header: the
+// spent-marker must be keyed off the Header, which cannot be altered without
+// invalidating the commit signature, not off MerkleProof fields that
+// ValidateCXReceiptsProof only binds to the Header from
+// IsCXMerkleProofReplayFixEpoch onward.
+func TestIsSpentIgnoresMutatedMerkleProofIdentity(t *testing.T) {
+	key, _ := crypto.GenerateKey()
+	chain, _, header, database := getTestEnvironment(*key)
+
+	header = header.With().ShardID(1).Number(big.NewInt(42)).Header()
+
+	original := &types.CXReceiptsProof{
+		Header: header,
+		MerkleProof: &types.CXMerkleProof{
+			ShardID:  1,
+			BlockNum: big.NewInt(42),
+		},
+	}
+
+	batch := database.NewBatch()
+	if err := chain.WriteCXReceiptsProofSpent(batch, []*types.CXReceiptsProof{original}); err != nil {
+		t.Fatalf("WriteCXReceiptsProofSpent failed: %v", err)
+	}
+	if err := batch.Write(); err != nil {
+		t.Fatalf("batch.Write failed: %v", err)
+	}
+
+	if !chain.IsSpent(original) {
+		t.Fatal("expected original proof to be marked spent")
+	}
+
+	replay := &types.CXReceiptsProof{
+		Header: header, // same genuine, signed header
+		MerkleProof: &types.CXMerkleProof{
+			ShardID:  99,               // mutated, unauthenticated
+			BlockNum: big.NewInt(9999), // mutated, unauthenticated
+		},
+	}
+	if !chain.IsSpent(replay) {
+		t.Fatal("expected replay with mutated MerkleProof identity to be detected as already spent")
+	}
+}
+
 func TestPrepareStakingMetadata(t *testing.T) {
 	key, _ := crypto.GenerateKey()
 	chain, db, header, _ := getTestEnvironment(*key)

internal/chain/engine_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/harmony-one/harmony/block"
 	blockfactory "github.com/harmony-one/harmony/block/factory"
 	"github.com/harmony-one/harmony/consensus/engine"
+	"github.com/harmony-one/harmony/consensus/quorum"
 	consensus_sig "github.com/harmony-one/harmony/consensus/signature"
 	"github.com/harmony-one/harmony/crypto/bls"
 	bls_core "github.com/harmony-one/harmony/crypto/bls/core"
@@ -498,6 +499,42 @@ func TestVerifiedSigCacheKeyIncludesShardID(t *testing.T) {
 	}
 }
 
+func TestVerifySignatureRejectsEmptyPreStakingQuorum(t *testing.T) {
+	chain := makeFakeBlockChain()
+	eng := NewEngine()
+	state := makeDefaultCommittee()
+	committee, err := state.FindCommitteeByID(shard.BeaconChainShardID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pubKeys, err := committee.BLSPublicKeys()
+	if err != nil {
+		t.Fatal(err)
+	}
+	verifier, err := quorum.NewVerifier(committee, big.NewInt(0), false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	eng.epochCtxCache.Add(epochCtxKey{shardID: shard.BeaconChainShardID, epoch: 0}, epochCtx{
+		qrVerifier: verifier,
+		pubKeys:    pubKeys,
+	})
+
+	err = eng.verifySignature(chain, payloadArgs{
+		blockHash: common.Hash{1},
+		shardID:   shard.BeaconChainShardID,
+		epoch:     big.NewInt(0),
+		number:    2,
+		viewID:    1,
+	}, sigArgs{
+		sig:    bls.SerializedSignature{},
+		bitmap: make([]byte, (len(pubKeys)+7)/8),
+	})
+	if err == nil || err.Error() != "not enough signature collected" {
+		t.Fatalf("expected empty quorum rejection, got %v", err)
+	}
+}
+
 // setupTimestampValidationChain returns a chain wired with the timestamp
 // validation fork active, the parent header committed at parentTime, and a
 // helper to build child headers parented to it.