test: cover four-attester IBC quorum

Author: randygrok

modules/network/keeper/keeper.go

@@ -282,8 +282,8 @@ func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (boo
 		return false, fmt.Errorf("invalid quorum fraction: %w", err)
 	}
 
-	requiredPower := math.LegacyNewDec(int64(totalPower)).Mul(quorumFrac).TruncateInt().Uint64()
-	return votedPower >= requiredPower, nil
+	requiredPower := math.LegacyNewDec(int64(totalPower)).Mul(quorumFrac)
+	return math.LegacyNewDec(int64(votedPower)).GT(requiredPower), nil
 }
 
 // IsSoftConfirmed checks if a block at a given height is soft-confirmed

modules/network/keeper/msg_server_test.go

@@ -367,6 +367,11 @@ func TestAttestWithMultipleAttestersTracksQuorumAndSignatures(t *testing.T) {
 			consensusAddress: sdk.ValAddress("attester3").String(),
 			vote:             bytes.Repeat([]byte{0x03}, MinVoteLen),
 		},
+		{
+			authority:        sdk.AccAddress("operator4").String(),
+			consensusAddress: sdk.ValAddress("attester4").String(),
+			vote:             bytes.Repeat([]byte{0x04}, MinVoteLen),
+		},
 	}
 	for _, attester := range attesters {
 		_, err := server.JoinAttesterSet(ctx, &types.MsgJoinAttesterSet{
@@ -396,15 +401,15 @@ func TestAttestWithMultipleAttestersTracksQuorumAndSignatures(t *testing.T) {
 
 		totalPower, err := keeper.GetTotalPower(ctx)
 		require.NoError(t, err)
-		require.Equal(t, uint64(3), totalPower)
+		require.Equal(t, uint64(4), totalPower)
 
 		quorumReached, err := keeper.CheckQuorum(ctx, votedPower, totalPower)
 		require.NoError(t, err)
-		require.Equal(t, i >= 1, quorumReached)
+		require.Equal(t, i >= 2, quorumReached)
 
 		lastAttestedHeight, err := keeper.GetLastAttestedHeight(ctx)
 		require.NoError(t, err)
-		if i == 0 {
+		if i < 2 {
 			require.Zero(t, lastAttestedHeight)
 		} else {
 			require.Equal(t, height, lastAttestedHeight)

pkg/rpc/core/blocks_test.go

@@ -345,96 +345,19 @@ func TestCommit_AttesterModeUsesAttesterValidatorSetForHeader(t *testing.T) {
 func TestCommit_AttesterModeBuildsMultiAttesterCommitWithAbsentSignatures(t *testing.T) {
 	require := require.New(t)
 
-	sequencerPrivKey := ed25519.GenPrivKey()
-	sequencerPubKey := sequencerPrivKey.PubKey()
-	sequencerLibP2PPrivKey, err := crypto.UnmarshalEd25519PrivateKey(sequencerPrivKey.Bytes())
-	require.NoError(err)
-	sequencerLibP2PPubKey := sequencerLibP2PPrivKey.GetPublic()
-	sequencerAddress := sequencerPubKey.Address().Bytes()[:20]
-
-	mockSigner := &MockSigner{}
-	mockSigner.On("GetPublic").Return(sequencerLibP2PPubKey, nil)
-	mockSigner.On("GetAddress").Return(sequencerAddress, nil)
-
-	env = setupTestEnvironment(mockSigner)
-	env.AttesterMode = true
-	defer func() {
-		env.AttesterMode = false
-	}()
-
-	chainID := "test-chain"
-	blockHeight := uint64(1)
-	now := time.Now()
-
-	sequencerValidatorHash, err := adapter.ValidatorHasherProvider()(sequencerAddress, sequencerLibP2PPubKey)
-	require.NoError(err)
-	blockData, rollkitHeader := createTestBlock(blockHeight, chainID, now, sequencerAddress, sequencerValidatorHash, 1)
-
-	lastCommit, err := env.Adapter.GetLastCommit(context.Background(), blockHeight)
-	require.NoError(err)
-	abciHeader, err := adapter.ToABCIHeader(rollkitHeader, lastCommit)
-	require.NoError(err)
+	fixture := setupFourAttesterCommitTest(t, 3)
+	commitResult := fixture.commitResult
 
-	attesterPrivKeysByAddress := make(map[string]ed25519.PrivKey)
-	attesterValidators := make([]*cmttypes.Validator, 0, 4)
-	for range 4 {
-		privKey := ed25519.GenPrivKey()
-		pubKey := privKey.PubKey().(ed25519.PubKey)
-		attesterPrivKeysByAddress[string(pubKey.Address())] = privKey
-		attesterValidators = append(attesterValidators, cmttypes.NewValidator(pubKey, 1))
-	}
-	attesterValidatorSet := cmttypes.NewValidatorSet(attesterValidators)
-	abciHeader.ValidatorsHash = attesterValidatorSet.Hash()
-	abciHeader.NextValidatorsHash = attesterValidatorSet.Hash()
-	abciHeader.ProposerAddress = attesterValidatorSet.Proposer.Address
-
-	abciBlock, err := adapter.ToABCIBlock(abciHeader, lastCommit, blockData)
-	require.NoError(err)
-	blockParts, err := abciBlock.MakePartSet(cmttypes.BlockPartSizeBytes)
-	require.NoError(err)
-	blockID := &cmttypes.BlockID{
-		Hash:          abciHeader.Hash(),
-		PartSetHeader: blockParts.Header(),
-	}
-	require.NoError(env.Adapter.Store.SaveBlockID(context.Background(), blockHeight, blockID))
-
-	attesterQueries := make([]mockAttesterNetworkQuery, 0, len(attesterValidatorSet.Validators))
-	for validatorIndex, validator := range attesterValidatorSet.Validators {
-		privKey := attesterPrivKeysByAddress[string(validator.Address)]
-		pubKey := privKey.PubKey().(ed25519.PubKey)
-		query := mockAttesterNetworkQuery{
-			attesterValAddress: sdk.ValAddress(validator.Address).String(),
-			attesterPubKey:     pubKey,
-		}
-		if validatorIndex < 3 {
-			query.voteBytes = signAttesterVoteWithIndex(
-				t,
-				chainID,
-				blockHeight,
-				rollkitHeader.Time(),
-				blockID,
-				int32(validatorIndex),
-				privKey,
-			)
-		}
-		attesterQueries = append(attesterQueries, query)
-	}
-	mockAttesterNetworkQueriesForSet(t, env.Adapter.App.(*MockApp), attesterQueries)
-
-	mockBlock(blockHeight, rollkitHeader, blockData, make([]byte, 64), sequencerLibP2PPubKey, sequencerAddress)
-
-	commitResult := callCommitRPC(t, blockHeight)
-
-	require.Equal(attesterValidatorSet.Hash(), []byte(commitResult.ValidatorsHash))
-	require.Equal(attesterValidatorSet.Hash(), []byte(commitResult.NextValidatorsHash))
+	require.Equal(fixture.attesterValidatorSet.Hash(), []byte(commitResult.ValidatorsHash))
+	require.Equal(fixture.attesterValidatorSet.Hash(), []byte(commitResult.NextValidatorsHash))
 	require.Len(commitResult.Commit.Signatures, 4)
 
 	commitSignatures := 0
 	absentSignatures := 0
 	for i, signature := range commitResult.Commit.Signatures {
 		if i < 3 {
 			require.Equal(cmttypes.BlockIDFlagCommit, signature.BlockIDFlag)
-			require.Equal(attesterValidatorSet.Validators[i].Address.Bytes(), []byte(signature.ValidatorAddress))
+			require.Equal(fixture.attesterValidatorSet.Validators[i].Address.Bytes(), []byte(signature.ValidatorAddress))
 			commitSignatures++
 			continue
 		}
@@ -443,10 +366,33 @@ func TestCommit_AttesterModeBuildsMultiAttesterCommitWithAbsentSignatures(t *tes
 	}
 	require.Equal(3, commitSignatures)
 	require.Equal(1, absentSignatures)
-	require.NoError(attesterValidatorSet.VerifyCommitLight(
-		chainID,
+	require.NoError(fixture.attesterValidatorSet.VerifyCommitLight(
+		fixture.chainID,
 		commitResult.Commit.BlockID,
-		int64(blockHeight),
+		int64(fixture.blockHeight),
+		commitResult.Commit,
+	))
+}
+
+func TestCommit_AttesterModeWithTwoOfFourSignaturesIsNotIBCVerifiable(t *testing.T) {
+	require := require.New(t)
+
+	fixture := setupFourAttesterCommitTest(t, 2)
+	commitResult := fixture.commitResult
+
+	require.Len(commitResult.Commit.Signatures, 4)
+	for i, signature := range commitResult.Commit.Signatures {
+		if i < 2 {
+			require.Equal(cmttypes.BlockIDFlagCommit, signature.BlockIDFlag)
+			require.Equal(fixture.attesterValidatorSet.Validators[i].Address.Bytes(), []byte(signature.ValidatorAddress))
+			continue
+		}
+		require.Equal(cmttypes.BlockIDFlagAbsent, signature.BlockIDFlag)
+	}
+	require.Error(fixture.attesterValidatorSet.VerifyCommitLight(
+		fixture.chainID,
+		commitResult.Commit.BlockID,
+		int64(fixture.blockHeight),
 		commitResult.Commit,
 	))
 }
@@ -540,6 +486,103 @@ func createTestBlock(height uint64, chainID string, baseTime time.Time, validato
 	return blockData, rollkitHeader
 }
 
+type fourAttesterCommitFixture struct {
+	chainID              string
+	blockHeight          uint64
+	attesterValidatorSet *cmttypes.ValidatorSet
+	commitResult         *ctypes.ResultCommit
+}
+
+func setupFourAttesterCommitTest(t *testing.T, signedAttesters int) fourAttesterCommitFixture {
+	t.Helper()
+	require := require.New(t)
+
+	sequencerPrivKey := ed25519.GenPrivKey()
+	sequencerPubKey := sequencerPrivKey.PubKey()
+	sequencerLibP2PPrivKey, err := crypto.UnmarshalEd25519PrivateKey(sequencerPrivKey.Bytes())
+	require.NoError(err)
+	sequencerLibP2PPubKey := sequencerLibP2PPrivKey.GetPublic()
+	sequencerAddress := sequencerPubKey.Address().Bytes()[:20]
+
+	mockSigner := &MockSigner{}
+	mockSigner.On("GetPublic").Return(sequencerLibP2PPubKey, nil)
+	mockSigner.On("GetAddress").Return(sequencerAddress, nil)
+
+	env = setupTestEnvironment(mockSigner)
+	env.AttesterMode = true
+	t.Cleanup(func() {
+		env.AttesterMode = false
+	})
+
+	chainID := "test-chain"
+	blockHeight := uint64(1)
+	now := time.Now()
+
+	sequencerValidatorHash, err := adapter.ValidatorHasherProvider()(sequencerAddress, sequencerLibP2PPubKey)
+	require.NoError(err)
+	blockData, rollkitHeader := createTestBlock(blockHeight, chainID, now, sequencerAddress, sequencerValidatorHash, 1)
+
+	lastCommit, err := env.Adapter.GetLastCommit(context.Background(), blockHeight)
+	require.NoError(err)
+	abciHeader, err := adapter.ToABCIHeader(rollkitHeader, lastCommit)
+	require.NoError(err)
+
+	attesterPrivKeysByAddress := make(map[string]ed25519.PrivKey)
+	attesterValidators := make([]*cmttypes.Validator, 0, 4)
+	for range 4 {
+		privKey := ed25519.GenPrivKey()
+		pubKey := privKey.PubKey().(ed25519.PubKey)
+		attesterPrivKeysByAddress[string(pubKey.Address())] = privKey
+		attesterValidators = append(attesterValidators, cmttypes.NewValidator(pubKey, 1))
+	}
+	attesterValidatorSet := cmttypes.NewValidatorSet(attesterValidators)
+	abciHeader.ValidatorsHash = attesterValidatorSet.Hash()
+	abciHeader.NextValidatorsHash = attesterValidatorSet.Hash()
+	abciHeader.ProposerAddress = attesterValidatorSet.Proposer.Address
+
+	abciBlock, err := adapter.ToABCIBlock(abciHeader, lastCommit, blockData)
+	require.NoError(err)
+	blockParts, err := abciBlock.MakePartSet(cmttypes.BlockPartSizeBytes)
+	require.NoError(err)
+	blockID := &cmttypes.BlockID{
+		Hash:          abciHeader.Hash(),
+		PartSetHeader: blockParts.Header(),
+	}
+	require.NoError(env.Adapter.Store.SaveBlockID(context.Background(), blockHeight, blockID))
+
+	attesterQueries := make([]mockAttesterNetworkQuery, 0, len(attesterValidatorSet.Validators))
+	for validatorIndex, validator := range attesterValidatorSet.Validators {
+		privKey := attesterPrivKeysByAddress[string(validator.Address)]
+		pubKey := privKey.PubKey().(ed25519.PubKey)
+		query := mockAttesterNetworkQuery{
+			attesterValAddress: sdk.ValAddress(validator.Address).String(),
+			attesterPubKey:     pubKey,
+		}
+		if validatorIndex < signedAttesters {
+			query.voteBytes = signAttesterVoteWithIndex(
+				t,
+				chainID,
+				blockHeight,
+				rollkitHeader.Time(),
+				blockID,
+				int32(validatorIndex),
+				privKey,
+			)
+		}
+		attesterQueries = append(attesterQueries, query)
+	}
+	mockAttesterNetworkQueriesForSet(t, env.Adapter.App.(*MockApp), attesterQueries)
+
+	mockBlock(blockHeight, rollkitHeader, blockData, make([]byte, 64), sequencerLibP2PPubKey, sequencerAddress)
+
+	return fourAttesterCommitFixture{
+		chainID:              chainID,
+		blockHeight:          blockHeight,
+		attesterValidatorSet: attesterValidatorSet,
+		commitResult:         callCommitRPC(t, blockHeight),
+	}
+}
+
 func signAttesterVote(
 	t *testing.T,
 	chainID string,

pkg/rpc/core/status.go

@@ -1,6 +1,7 @@
 package core
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"time"
@@ -172,6 +173,10 @@ func isAttesterNode() bool {
 
 // getLastAttestedHeight returns the highest block height attested by this validator
 func getLastAttestedHeight(ctx *rpctypes.Context) (uint64, error) {
+	return getLastAttestedHeightFromContext(ctx.Context())
+}
+
+func getLastAttestedHeightFromContext(ctx context.Context) (uint64, error) {
 	// Create protobuf request for LastAttestedHeight query (O(1) operation)
 	req := &networktypes.QueryLastAttestedHeightRequest{}
 
@@ -182,7 +187,7 @@ func getLastAttestedHeight(ctx *rpctypes.Context) (uint64, error) {
 	}
 
 	// Use ABCI Query with the new endpoint
-	res, err := env.Adapter.App.Query(ctx.Context(), &abci.RequestQuery{
+	res, err := env.Adapter.App.Query(ctx, &abci.RequestQuery{
 		Path: "/evabci.network.v1.Query/LastAttestedHeight",
 		Data: reqBytes,
 	})
@@ -199,6 +204,9 @@ func getLastAttestedHeight(ctx *rpctypes.Context) (uint64, error) {
 		if err := response.Unmarshal(res.Value); err != nil {
 			return 0, fmt.Errorf("failed to unmarshal response: %w", err)
 		}
+		if response.Height < 0 {
+			return 0, fmt.Errorf("last attested height cannot be negative: %d", response.Height)
+		}
 		return uint64(response.Height), nil
 	}
 

pkg/rpc/core/utils.go

@@ -24,7 +24,11 @@ func normalizeHeight(ctx context.Context, height *int64) (uint64, error) {
 	if height == nil || *height < 0 {
 		// Handle negative heights if they have special meaning
 		// (e.g., -1 for latest)
-		heightValue, err = env.Adapter.RollkitStore.Height(ctx)
+		if env != nil && env.AttesterMode {
+			heightValue, err = getLastAttestedHeightFromContext(ctx)
+		} else {
+			heightValue, err = env.Adapter.RollkitStore.Height(ctx)
+		}
 		if err != nil {
 			return 0, fmt.Errorf("failed to get current height: %w", err)
 		}

pkg/rpc/core/utils_test.go

@@ -1,14 +1,24 @@
 package core
 
 import (
+	"context"
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
 	"strings"
 	"testing"
 
+	abci "github.com/cometbft/cometbft/abci/types"
+	cmtlog "github.com/cometbft/cometbft/libs/log"
+	"github.com/cosmos/gogoproto/proto"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+
+	rollkitmocks "github.com/evstack/ev-node/test/mocks"
+
+	networktypes "github.com/evstack/ev-abci/modules/network/types"
+	"github.com/evstack/ev-abci/pkg/adapter"
 )
 
 func TestTruncateNodeID(t *testing.T) {
@@ -58,3 +68,35 @@ func TestTruncateNodeID(t *testing.T) {
 		assert.Equal(t, exactLengthNodeIDStr, truncatedNodeID, "Truncated Node ID should be the same as input for exact length")
 	})
 }
+
+func TestNormalizeHeightAttesterModeUsesLastAttestedHeight(t *testing.T) {
+	originalEnv := env
+	t.Cleanup(func() { env = originalEnv })
+
+	mockStore := new(rollkitmocks.MockStore)
+	mockApp := new(MockApp)
+	env = &Environment{
+		Adapter: &adapter.Adapter{
+			RollkitStore: mockStore,
+			App:          mockApp,
+		},
+		AttesterMode: true,
+		Logger:       cmtlog.NewNopLogger(),
+	}
+
+	queryResponse, err := proto.Marshal(&networktypes.QueryLastAttestedHeightResponse{
+		Height: 14,
+	})
+	require.NoError(t, err)
+
+	mockApp.On("Query", mock.Anything, mock.MatchedBy(func(req *abci.RequestQuery) bool {
+		return req.Path == "/evabci.network.v1.Query/LastAttestedHeight"
+	})).Return(&abci.ResponseQuery{Code: 0, Value: queryResponse}, nil).Once()
+
+	height, err := normalizeHeight(context.Background(), nil)
+
+	require.NoError(t, err)
+	require.Equal(t, uint64(14), height)
+	mockStore.AssertNotCalled(t, "Height", mock.Anything)
+	mockApp.AssertExpectations(t)
+}