Configurable max clock drift and minor updates

Author: alpe

modules/network/keeper/msg_server.go

@@ -47,15 +47,15 @@ func (k msgServer) Attest(goCtx context.Context, msg *types.MsgAttest) (*types.M
 		return nil, errors.Wrapf(sdkerrors.ErrNotFound, "validator index not found for %s", msg.Validator)
 	}
 
-	if err := k.updateAttestationBitmap(ctx, msg, valIndexPos); err != nil {
-		return nil, errors.Wrap(err, "update attestation bitmap")
-	}
-
 	vote, err := k.verifyVote(ctx, msg)
 	if err != nil {
 		return nil, err
 	}
 
+	if err := k.updateAttestationBitmap(ctx, msg, valIndexPos); err != nil {
+		return nil, errors.Wrap(err, "update attestation bitmap")
+	}
+
 	if err := k.SetSignature(ctx, msg.Height, msg.Validator, vote.Signature); err != nil {
 		return nil, errors.Wrap(err, "store signature")
 	}
@@ -156,6 +156,14 @@ func (k msgServer) verifyVote(ctx sdk.Context, msg *types.MsgAttest) (*cmtproto.
 	if msg.Height != vote.Height {
 		return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, "vote height does not match attestation height")
 	}
+	if senderAddr, err := sdk.ValAddressFromBech32(msg.Validator); err != nil {
+		return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid validator address: %s", err)
+	} else if !bytes.Equal(senderAddr, vote.ValidatorAddress) {
+		return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "vote validator address does not match attestation validator address")
+	}
+	if len(vote.Signature) == 0 {
+		return nil, sdkerrors.ErrInvalidRequest.Wrap("empty signature")
+	}
 	header, _, err := k.blockSource.GetBlockData(ctx, uint64(vote.Height))
 	if err != nil {
 		return nil, errors.Wrapf(err, "block data for height %d", vote.Height)
@@ -168,8 +176,8 @@ func (k msgServer) verifyVote(ctx sdk.Context, msg *types.MsgAttest) (*cmtproto.
 		return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, "vote block ID hash does not match app hash for height %d", vote.Height)
 	}
 
-	maxClockDrift := 5 * time.Second // todo (Alex): make this a parameter?
-	if drift := vote.Timestamp.Sub(header.Time()); drift < 0*time.Nanosecond || drift > maxClockDrift {
+	maxClockDrift := time.Duration(k.GetParams(ctx).MaxClockDrift)
+	if drift := vote.Timestamp.Sub(header.Time()); drift < 0 || drift > maxClockDrift {
 		return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, "vote timestamp drift exceeds limit: %s", drift)
 	}
 

modules/network/keeper/msg_server_test.go

@@ -275,10 +275,11 @@ func TestAttest(t *testing.T) {
 
 func TestVerifyVote(t *testing.T) {
 	var (
-		myHash           = sha256.Sum256([]byte("app_hash"))
-		myAppHash        = myHash[:]
-		validatorPrivKey = ed25519.GenPrivKey()
-		valAddrStr       = sdk.ValAddress(validatorPrivKey.PubKey().Address()).String()
+		myHash                = sha256.Sum256([]byte("app_hash"))
+		myAppHash             = myHash[:]
+		validatorPrivKey      = ed25519.GenPrivKey()
+		valAddrStr            = sdk.ValAddress(validatorPrivKey.PubKey().Address()).String()
+		otherValidatorPrivKey = ed25519.GenPrivKey()
 	)
 
 	// Setup test environment with block store
@@ -299,12 +300,14 @@ func TestVerifyVote(t *testing.T) {
 
 	testCases := map[string]struct {
 		voteFn func(t *testing.T) *cmtproto.Vote
+		sender string
 		expErr error
 	}{
 		"valid vote": {
 			voteFn: func(t *testing.T) *cmtproto.Vote {
 				return VoteFixture(myAppHash, validatorPrivKey)
 			},
+			sender: valAddrStr,
 		},
 		"timestamp drift": {
 			voteFn: func(t *testing.T) *cmtproto.Vote {
@@ -313,6 +316,7 @@ func TestVerifyVote(t *testing.T) {
 					vote.Signature = must(validatorPrivKey.Sign(cmttypes.VoteSignBytes("testing", vote)))
 				})
 			},
+			sender: valAddrStr,
 			expErr: sdkerrors.ErrInvalidRequest,
 		},
 		"block data not found": {
@@ -322,36 +326,51 @@ func TestVerifyVote(t *testing.T) {
 					vote.Signature = must(validatorPrivKey.Sign(cmttypes.VoteSignBytes("testing", vote)))
 				})
 			},
+			sender: valAddrStr,
 			expErr: sdkerrors.ErrInvalidRequest,
 		},
 		"vote and block hash mismatch": {
 			voteFn: func(t *testing.T) *cmtproto.Vote {
 				hash := sha256.Sum256([]byte("wrong_hash"))
 				return VoteFixture(hash[:], validatorPrivKey)
 			},
+			sender: valAddrStr,
 			expErr: sdkerrors.ErrInvalidRequest,
 		},
 		"validator not found": {
 			voteFn: func(t *testing.T) *cmtproto.Vote {
 				return VoteFixture(myAppHash, ed25519.GenPrivKey())
 			},
-			expErr: sdkerrors.ErrNotFound,
+			expErr: sdkerrors.ErrUnauthorized,
+			sender: sdk.ValAddress(otherValidatorPrivKey.PubKey().Address()).String(),
 		},
 		"invalid vote signature": {
 			voteFn: func(t *testing.T) *cmtproto.Vote {
 				return VoteFixture(myAppHash, validatorPrivKey, func(vote *cmtproto.Vote) {
 					vote.Signature = []byte("invalid signature")
 				})
 			},
+			sender: valAddrStr,
 			expErr: sdkerrors.ErrInvalidRequest,
 		},
+		"invalid sender": {
+			voteFn: func(t *testing.T) *cmtproto.Vote {
+				return VoteFixture(myAppHash, validatorPrivKey)
+			},
+			sender: sdk.ValAddress(otherValidatorPrivKey.PubKey().Address()).String(),
+			expErr: sdkerrors.ErrUnauthorized,
+		},
 	}
 	for name, spec := range testCases {
 		t.Run(name, func(t *testing.T) {
 			ctx, _ := parentCtx.CacheContext()
 
 			// when
-			vote, err := env.Server.verifyVote(ctx, &types.MsgAttest{Height: 10, Vote: must(proto.Marshal(spec.voteFn(t)))})
+			vote, err := env.Server.verifyVote(ctx, &types.MsgAttest{
+				Height:    10,
+				Validator: spec.sender,
+				Vote:      must(proto.Marshal(spec.voteFn(t))),
+			})
 
 			// then
 			if spec.expErr != nil {

modules/network/types/params.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"fmt"
+	"time"
 
 	"cosmossdk.io/math"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
@@ -32,13 +33,15 @@ func NewParams(
 	minParticipation math.LegacyDec,
 	pruneAfter uint64,
 	signMode SignMode,
+	maxClockDrift time.Duration,
 ) Params {
 	return Params{
 		EpochLength:      epochLength,
 		QuorumFraction:   quorumFraction.String(),
 		MinParticipation: minParticipation.String(),
 		PruneAfter:       pruneAfter,
 		SignMode:         signMode,
+		MaxClockDrift:    maxClockDrift,
 	}
 }
 
@@ -50,6 +53,7 @@ func DefaultParams() Params {
 		DefaultMinParticipation,
 		DefaultPruneAfter,
 		DefaultSignMode,
+		5*time.Second,
 	)
 }
 
@@ -81,6 +85,9 @@ func (p Params) Validate() error {
 	if err := validateSignMode(p.SignMode); err != nil {
 		return err
 	}
+	if p.MaxClockDrift < 0 {
+		return fmt.Errorf("max clock drift must be positive: %s", p.MaxClockDrift)
+	}
 	return nil
 }
 

modules/network/types/types.pb.go

@@ -7,6 +7,8 @@ option go_package = "github.com/rollkit/go-execution-abci/modules/network/types"
 import "gogoproto/gogo.proto";
 import "cosmos_proto/cosmos.proto";
 import "google/protobuf/duration.proto";
+import "amino/amino.proto";
+
 
 // Params defines the parameters for the network module.
 message Params {
@@ -24,6 +26,11 @@ message Params {
 
   // sign_mode determines when validators must sign
   SignMode sign_mode = 5;
+
+  // max_clock_drift maximum a timestamp for a vote can drift
+  google.protobuf.Duration max_clock_drift = 6
+  [(gogoproto.stdduration) = true, (gogoproto.nullable) = false, (amino.dont_omitempty) = true];
+
 }
 
 // SignMode defines when validators must sign