Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TODOs] grace_period_end_offset_blocks validation & cleanup #667

Merged
merged 9 commits into from
Jul 15, 2024
2 changes: 1 addition & 1 deletion pkg/relayer/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 4 additions & 7 deletions pkg/relayer/proxy/relay_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}

Expand Down
23 changes: 20 additions & 3 deletions x/shared/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
if params.GracePeriodEndOffsetBlocks >= params.NumBlocksPerSession {
return ErrSharedParamInvalid.Wrapf(
"GracePeriodEndOffsetBlocks (%v) must be less than NumBlocksPerSession (%v)",
params.GracePeriodEndOffsetBlocks,
params.NumBlocksPerSession,
)
}
return nil
}
4 changes: 0 additions & 4 deletions x/tokenomics/keeper/msg_update_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading