[TODOs] grace_period_end_offset_blocks validation & cleanup

pkg/relayer/proxy/proxy_test.go

@@ -439,7 +439,7 @@ func TestRelayerProxy_Relays(t *testing.T) {
 			inputScenario:        sendRequestWithDifferentSession,
 
 			expectedErrCode: testproxy.JSONRPCInternalErrorCode,
-			expectedErrMsg:  "session mismatch",
+			expectedErrMsg:  "session ID mismatch",
 		},
 		{
 			desc: "Invalid relay supplier",

pkg/relayer/proxy/relay_verifier.go

@@ -65,15 +65,12 @@ func (rp *relayerProxy) VerifyRelayRequest(
 	// Since the retrieved sessionId was in terms of:
 	// - the current block height and sessionGracePeriod (which are not provided by the relayRequest)
 	// - serviceId (which is not provided by the relayRequest)
-	// - applicationAddress (which is used to to verify the relayRequest signature)
-	//
-	// TODO_BLOCKER(@Olshansk): Revisit the assumptions above and updated this if
-	// structure if necessary.
+	// - applicationAddress (which is used to verify the relayRequest signature)
 	if session.SessionId != sessionHeader.GetSessionId() {
 		return ErrRelayerProxyInvalidSession.Wrapf(
-			"session mismatch, expecting: %+v, got: %+v",
-			session.Header,
-			relayRequest.Meta.SessionHeader,
+			"session ID mismatch, expecting: %+v, got: %+v",
+			session.GetSessionId(),
+			relayRequest.Meta.GetSessionHeader().GetSessionId(),
 		)
 	}
 

x/session/keeper/session_hydrator.go

@@ -163,11 +163,6 @@ func (k Keeper) hydrateSessionApplication(ctx context.Context, sh *sessionHydrat
 func (k Keeper) hydrateSessionSuppliers(ctx context.Context, sh *sessionHydrator) error {
 	logger := k.Logger().With("method", "hydrateSessionSuppliers")
 
-	// TODO_BLOCKER(@Olshansk): Retrieve the suppliers at SessionStartBlockHeight,
-	// NOT THE CURRENT ONE which is what's provided by the context. For now, for
-	// simplicity, only retrieving the suppliers at the current block height which
-	// could create a discrepancy if new suppliers were staked mid session.
-	// TODO(@bryanchriswhite): Investigate if `BlockClient` + `ReplayObservable` where `N = SessionLength` could be used here.`
 	suppliers := k.supplierKeeper.GetAllSuppliers(ctx)
 
 	candidateSuppliers := make([]*sharedtypes.Supplier, 0)

x/shared/types/params.go

@@ -89,9 +89,6 @@ func (params *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 
 // Validate validates the set of params
 func (params *Params) ValidateBasic() error {
-	// TODO_BLOCKER(@bryanchriswhite): Add `SessionGracePeriodBlocks` as a shared param,
-	// introduce validation, and ensure it is strictly less than num blocks per session.
-
 	if err := ValidateNumBlocksPerSession(params.NumBlocksPerSession); err != nil {
 		return err
 	}
@@ -116,6 +113,10 @@ func (params *Params) ValidateBasic() error {
 		return err
 	}
 
+	if err := validateGracePeriodOffsetBlocksIsLessThanNumBlocksPerSession(params); err != nil {
+		return err
+	}
+
 	if err := validateClaimWindowOpenOffsetIsAtLeastGracePeriodEndOffset(params); err != nil {
 		return err
 	}
@@ -178,6 +179,10 @@ func validateIsUint64(value any) error {
 	return nil
 }
 
+// validateClaimWindowOpenOffsetIsAtLeastGracePeriodEndOffset validates that the ClaimWindowOpenOffsetBlocks
+// is at least as big as GracePeriodEndOffsetBlocks. The claim window cannot open until the grace period ends
+// to ensure that the seed for the earliest supplier claim commit height is only observed after the last relay
+// for a given session could be serviced.
 func validateClaimWindowOpenOffsetIsAtLeastGracePeriodEndOffset(params *Params) error {
 	if params.ClaimWindowOpenOffsetBlocks < params.GracePeriodEndOffsetBlocks {
 		return ErrSharedParamInvalid.Wrapf(
@@ -186,6 +191,18 @@ func validateClaimWindowOpenOffsetIsAtLeastGracePeriodEndOffset(params *Params)
 			params.GracePeriodEndOffsetBlocks,
 		)
 	}
+	return nil
+}
 
+// validateGracePeriodOffsetBlocksIsLessThanNumBlocksPerSession validates that the
+// GracePeriodEndOffsetBlocks is less than NumBlocksPerSession; i.e., one session.
+func validateGracePeriodOffsetBlocksIsLessThanNumBlocksPerSession(params *Params) error {
+	if params.GracePeriodEndOffsetBlocks >= params.NumBlocksPerSession {
+		return ErrSharedParamInvalid.Wrapf(
+			"GracePeriodEndOffsetBlocks (%v) must be less than NumBlocksPerSession (%v)",
+			params.GracePeriodEndOffsetBlocks,
+			params.NumBlocksPerSession,
+		)
+	}
 	return nil
 }

x/tokenomics/keeper/msg_update_params.go

@@ -14,10 +14,6 @@ func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams)
 		return nil, err
 	}
 
-	// TODO_BLOCKER(@Olshansk): How do we validate this is the same address that signed the request?
-	// Do we have to use `msg.GetSigners()` explicitly during the check/validation or
-	// does the `cosmos.msg.v1.signer` tag in the protobuf definition enforce
-	// this somewhere in the Cosmos SDK?
 	if msg.Authority != k.GetAuthority() {
 		return nil, types.ErrTokenomicsInvalidSigner.Wrapf(
 			"invalid authority; expected %s, got %s",