refactor: sudo and admin name consistency + docs + remove struct

embedding

CHANGELOG.md

@@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1705](https://github.com/NibiruChain/nibiru/pull/1705) - feat(perp): Add oracle pair to market object
 * [#1718](https://github.com/NibiruChain/nibiru/pull/1718) - fix: fees does not require additional funds
 * [#1734](https://github.com/NibiruChain/nibiru/pull/1734) - feat(perp): MsgDonateToPerpFund
+* [#1752](https://github.com/NibiruChain/nibiru/pull/1752) - feat(oracle): MsgEditOracleParams sudo tx msg as part of #1642
 
 ### Non-breaking/Compatible Improvements
 

proto/nibiru/oracle/v1/tx.proto

@@ -3,6 +3,7 @@ package nibiru.oracle.v1;
 
 import "gogoproto/gogo.proto";
 import "google/api/annotations.proto";
+import "nibiru/oracle/v1/oracle.proto";
 
 option go_package = "github.com/NibiruChain/nibiru/x/oracle/types";
 
@@ -91,7 +92,7 @@ message MsgDelegateFeedConsent {
 message MsgDelegateFeedConsentResponse {}
 
 // MsgEditOracleParams: gRPC tx message for updating the x/oracle module params
-// [Admin] Only callable by sudoers.
+// [[SUDO][SUDO][SUDO][SUDO][SUDO]] O[SUDO]ly callable by su[SUDO]oers.
 message MsgEditOracleParams {
   string sender = 1;
 
@@ -150,4 +151,4 @@ message MsgEditOracleParams {
 
 // MsgEditOracleParamsResponse defines the Msg/EditOracleParams response
 // type.
-message MsgEditOracleParamsResponse {}
+message MsgEditOracleParamsResponse { nibiru.oracle.v1.Params new_params = 1; }

proto/nibiru/perp/v2/tx.proto

@@ -30,7 +30,7 @@ service Msg {
       returns (MsgDonateToEcosystemFundResponse) {}
 
   // ChangeCollateralDenom: Updates the collateral denom. A denom is valid if it
-  // is possible to make an sdk.Coin using it. [Admin] Only callable by sudoers.
+  // is possible to make an sdk.Coin using it. [SUDO] Only callable by sudoers.
   rpc ChangeCollateralDenom(MsgChangeCollateralDenom)
       returns (MsgChangeCollateralDenomResponse) {}
 
@@ -41,17 +41,17 @@ service Msg {
       returns (MsgWithdrawEpochRebatesResponse) {}
 
   // ShiftPegMultiplier: gRPC tx msg for changing a market's peg multiplier.
-  // [Admin] Only callable by sudoers.
+  // [SUDO] Only callable by sudoers.
   rpc ShiftPegMultiplier(MsgShiftPegMultiplier)
       returns (MsgShiftPegMultiplierResponse) {}
 
   // ShiftSwapInvariant: gRPC tx msg for changing a market's swap invariant.
-  // [Admin] Only callable by sudoers.
+  // [SUDO] Only callable by sudoers.
   rpc ShiftSwapInvariant(MsgShiftSwapInvariant)
       returns (MsgShiftSwapInvariantResponse) {}
 
   // WithdrawFromPerpFund: gRPC tx msg to withdraw from the perp fund module
-  // account. [Admin] Only callable by sudoers.
+  // account. [SUDO] Only callable by sudoers.
   rpc WithdrawFromPerpFund(MsgWithdrawFromPerpFund)
       returns (MsgWithdrawFromPerpFundResponse) {}
 }
@@ -358,7 +358,7 @@ message MsgDonateToEcosystemFundResponse {}
 // -----------------------  MsgChangeCollateralDenom -----------------------
 
 // MsgChangeCollateralDenom: Changes the collateral denom for the module.
-// [Admin] Only callable by sudoers.
+// [SUDO] Only callable by sudoers.
 message MsgChangeCollateralDenom {
   string sender = 1;
   string new_denom = 2;
@@ -400,7 +400,7 @@ message MsgWithdrawEpochRebatesResponse {
 // -------------------------- ShiftPegMultiplier --------------------------
 
 // ShiftPegMultiplier: gRPC tx msg for changing the peg multiplier.
-// Admin-only.
+// [SUDO] Only callable by sudoers.
 message MsgShiftPegMultiplier {
   string sender = 1;
   string pair = 2 [
@@ -419,7 +419,7 @@ message MsgShiftPegMultiplierResponse {}
 // -------------------------- ShiftSwapInvariant --------------------------
 
 // ShiftSwapInvariant: gRPC tx msg for changing the swap invariant.
-// Admin-only.
+// [SUDO] Only callable by sudoers.
 message MsgShiftSwapInvariant {
   string sender = 1;
   string pair = 2 [
@@ -438,7 +438,7 @@ message MsgShiftSwapInvariantResponse {}
 // -------------------------- WithdrawFromPerpFund --------------------------
 
 // WithdrawFromPerpFund: gRPC tx msg for changing the swap invariant.
-// Admin-only.
+// [SUDO] Only callable by sudoers.
 message MsgWithdrawFromPerpFund {
   string sender = 1;
   string amount = 2 [

wasmbinding/exec_perp.go

@@ -33,7 +33,7 @@ func (exec *ExecutorPerp) SetMarketEnabled(
 		return err
 	}
 
-	return exec.PerpV2.Admin.CloseMarket(ctx, pair)
+	return exec.PerpV2.Sudo().CloseMarket(ctx, pair)
 }
 
 func (exec *ExecutorPerp) CreateMarket(
@@ -71,7 +71,7 @@ func (exec *ExecutorPerp) CreateMarket(
 		}
 	}
 
-	return exec.PerpV2.Admin.CreateMarket(ctx, perpv2keeper.ArgsCreateMarket{
+	return exec.PerpV2.Sudo().CreateMarket(ctx, perpv2keeper.ArgsCreateMarket{
 		Pair:            pair,
 		PriceMultiplier: cwMsg.PegMult,
 		SqrtDepth:       cwMsg.SqrtDepth,

x/oracle/keeper/keeper.go

@@ -31,9 +31,7 @@ type Keeper struct {
 
 	distrModuleName string
 
-	// Extends the Keeper with sudo functions. See sudo.go.
-	Admin admin
-
+	// Module parameters
 	Params            collections.Item[types.Params]
 	ExchangeRates     collections.Map[asset.Pair, types.DatedPrice]
 	FeederDelegations collections.Map[sdk.ValAddress, sdk.AccAddress]
@@ -42,7 +40,9 @@ type Keeper struct {
 	Votes             collections.Map[sdk.ValAddress, types.AggregateExchangeRateVote]
 
 	// PriceSnapshots maps types.PriceSnapshot to the asset.Pair of the snapshot and the creation timestamp as keys.Uint64Key.
-	PriceSnapshots   collections.Map[collections.Pair[asset.Pair, time.Time], types.PriceSnapshot]
+	PriceSnapshots collections.Map[
+		collections.Pair[asset.Pair, time.Time],
+		types.PriceSnapshot]
 	WhitelistedPairs collections.KeySet[asset.Pair]
 	Rewards          collections.Map[uint64, types.Rewards]
 	RewardsID        collections.Sequence
@@ -88,7 +88,6 @@ func NewKeeper(
 			collections.Uint64KeyEncoder, collections.ProtoValueEncoder[types.Rewards](cdc)),
 		RewardsID: collections.NewSequence(storeKey, 9),
 	}
-	k.Admin = admin{&k}
 	return k
 }
 

x/oracle/keeper/msg_server.go

@@ -113,7 +113,7 @@ func (ms msgServer) AggregateExchangeRateVote(
 	hash := types.GetAggregateVoteHash(msg.Salt, msg.ExchangeRates, valAddr)
 	if aggregatePrevote.Hash != hash.String() {
 		return nil, sdkerrors.Wrapf(
-			types.ErrVerificationFailed, "must be given %s not %s", aggregatePrevote.Hash, hash,
+			types.ErrHashVerificationFailed, "must be given %s not %s", aggregatePrevote.Hash, hash,
 		)
 	}
 
@@ -169,13 +169,19 @@ func (ms msgServer) DelegateFeedConsent(
 }
 
 // EditOracleParams: gRPC tx msg for editing the oracle module params.
-// [Admin] Only callable by sudoers.
+// [[SUDO][SUDO][SUDO][SUDO][SUDO]] O[SUDO]ly callable by su[SUDO]oers.
 func (ms msgServer) EditOracleParams(
 	goCtx context.Context, msg *types.MsgEditOracleParams,
 ) (resp *types.MsgEditOracleParamsResponse, err error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
+	// Stateless field validation is already performed in msg.ValidateBasic()
+	// before the current scope is reached.
 	sender, _ := sdk.AccAddressFromBech32(msg.Sender)
-	return resp, ms.Admin.EditOracleParams(
-		ctx, PartialOracleParams{PbMsg: *msg}, sender,
+	newParams, err := ms.Sudo().EditOracleParams(
+		ctx, *msg, sender,
 	)
+	resp = &types.MsgEditOracleParamsResponse{
+		NewParams: &newParams,
+	}
+	return resp, err
 }

x/oracle/keeper/sudo.go

@@ -10,88 +10,90 @@ import (
 	oracletypes "github.com/NibiruChain/nibiru/x/oracle/types"
 )
 
-// Extends the Keeper with admin functions. Admin is syntactic sugar to separate
-// admin calls off from the other Keeper methods.
+// Sudo extends the Keeper with sudo functions. See sudo.go. Sudo is syntactic
+// sugar to separate admin calls off from the other Keeper methods.
 //
-// These Admin functions should:
+// These Sudo functions should:
 // 1. Not be called in other methods in the x/perp module.
-// 2. Only be callable from the x/sudo root or sudo contracts.
+// 2. Only be callable by the x/sudo root or sudo contracts.
 //
-// The intention behind "admin" is to make it more obvious to the developer that
-// an unsafe function is being used when it's called from "OracleKeeper.Admin"
-type admin struct{ *Keeper }
+// The intention behind "Keeper.Sudo()" is to make it more obvious to the
+// developer that an unsafe function is being used when it's called.
+func (k Keeper) Sudo() sudoExtension { return sudoExtension{k} }
 
-type PartialOracleParams struct {
-	PbMsg oracletypes.MsgEditOracleParams
-}
+type sudoExtension struct{ Keeper }
+
+// ------------------------------------------------------------------
+// Admin.EditOracleParams
 
-func (k admin) EditOracleParams(
-	ctx sdk.Context, newParams PartialOracleParams, sender sdk.AccAddress,
-) error {
+func (k sudoExtension) EditOracleParams(
+	ctx sdk.Context, newParams oracletypes.MsgEditOracleParams,
+	sender sdk.AccAddress,
+) (paramsAfter oracletypes.Params, err error) {
 	if err := k.SudoKeeper.CheckPermissions(sender, ctx); err != nil {
-		return err
+		return paramsAfter, err
 	}
 
 	params, err := k.Params.Get(ctx)
 	if err != nil {
-		// TODO: use typed error
-		return fmt.Errorf("get oracle params error: %s", err.Error())
+		return paramsAfter, fmt.Errorf("%w: failed to read oracle params", err)
 	}
 
-	mergedParams := newParams.MergeOracleParams(params)
-	k.UpdateParams(ctx, mergedParams)
-	return nil
+	paramsAfter = MergeOracleParams(newParams, params)
+	k.UpdateParams(ctx, paramsAfter)
+	return paramsAfter, nil
 }
 
 // MergeOracleParams: Takes the given oracle params and merges them into the
 // existing partial params, keeping any existing values that are not set in the
 // partial.
-func (partial PartialOracleParams) MergeOracleParams(
+func MergeOracleParams(
+	partial oracletypes.MsgEditOracleParams,
 	oracleParams oracletypes.Params,
 ) oracletypes.Params {
-	if partial.PbMsg.VotePeriod != nil {
-		oracleParams.VotePeriod = partial.PbMsg.VotePeriod.Uint64()
+	if partial.VotePeriod != nil {
+		oracleParams.VotePeriod = partial.VotePeriod.Uint64()
 	}
 
-	if partial.PbMsg.VoteThreshold != nil {
-		oracleParams.VoteThreshold = *partial.PbMsg.VoteThreshold
+	if partial.VoteThreshold != nil {
+		oracleParams.VoteThreshold = *partial.VoteThreshold
 	}
 
-	if partial.PbMsg.RewardBand != nil {
-		oracleParams.RewardBand = *partial.PbMsg.RewardBand
+	if partial.RewardBand != nil {
+		oracleParams.RewardBand = *partial.RewardBand
 	}
 
-	if partial.PbMsg.Whitelist != nil {
-		whitelist := make([]asset.Pair, len(partial.PbMsg.Whitelist))
-		for i, pair := range partial.PbMsg.Whitelist {
+	if partial.Whitelist != nil {
+		whitelist := make([]asset.Pair, len(partial.Whitelist))
+		for i, pair := range partial.Whitelist {
 			whitelist[i] = asset.MustNewPair(pair)
 		}
 
 		oracleParams.Whitelist = whitelist
 	}
 
-	if partial.PbMsg.SlashFraction != nil {
-		oracleParams.SlashFraction = *partial.PbMsg.SlashFraction
+	if partial.SlashFraction != nil {
+		oracleParams.SlashFraction = *partial.SlashFraction
 	}
 
-	if partial.PbMsg.SlashWindow != nil {
-		oracleParams.SlashWindow = partial.PbMsg.SlashWindow.Uint64()
+	if partial.SlashWindow != nil {
+		oracleParams.SlashWindow = partial.SlashWindow.Uint64()
 	}
 
-	if partial.PbMsg.MinValidPerWindow != nil {
-		oracleParams.MinValidPerWindow = *partial.PbMsg.MinValidPerWindow
+	if partial.MinValidPerWindow != nil {
+		oracleParams.MinValidPerWindow = *partial.MinValidPerWindow
 	}
 
-	if partial.PbMsg.TwapLookbackWindow != nil {
-		oracleParams.TwapLookbackWindow = time.Duration(partial.PbMsg.TwapLookbackWindow.Int64())
+	if partial.TwapLookbackWindow != nil {
+		oracleParams.TwapLookbackWindow = time.Duration(partial.TwapLookbackWindow.Int64())
 	}
 
-	if partial.PbMsg.MinVoters != nil {
-		oracleParams.MinVoters = partial.PbMsg.MinVoters.Uint64()
+	if partial.MinVoters != nil {
+		oracleParams.MinVoters = partial.MinVoters.Uint64()
 	}
 
-	if partial.PbMsg.ValidatorFeeRatio != nil {
-		oracleParams.ValidatorFeeRatio = *partial.PbMsg.ValidatorFeeRatio
+	if partial.ValidatorFeeRatio != nil {
+		oracleParams.ValidatorFeeRatio = *partial.ValidatorFeeRatio
 	}
 
 	return oracleParams

x/oracle/keeper/sudo_test.go

@@ -48,14 +48,16 @@ func (s *SuiteOracleSudo) TestEditOracleParams() {
 		ValidatorFeeRatio:  &validatorFeeRatio,
 	}
 
-	// Verify that params after are not equal
-	partialParams := oraclekeeper.PartialOracleParams{PbMsg: msgEditParams}
+	s.T().Log("Params before MUST NOT be equal to default")
 	defaultParams := oracletypes.DefaultParams()
 	currParams, err := nibiru.OracleKeeper.Params.Get(ctx)
 	s.NoError(err)
-	s.Equal(currParams, defaultParams)
-	fullParams := partialParams.MergeOracleParams(defaultParams)
-	s.NotEqual(defaultParams, fullParams)
+	s.Equal(currParams, defaultParams,
+		"Current params should be eqaul to defaults")
+	partialParams := msgEditParams
+	fullParams := oraclekeeper.MergeOracleParams(partialParams, defaultParams)
+	s.NotEqual(defaultParams, fullParams,
+		"new params after merge should not be defaults")
 
 	invalidSender := testutil.AccAddress()
 	oracleMsgServer := oraclekeeper.NewMsgServerImpl(nibiru.OracleKeeper)
@@ -66,10 +68,12 @@ func (s *SuiteOracleSudo) TestEditOracleParams() {
 	)
 	s.Error(err)
 
+	s.T().Log("Params after MUST be equal to new ones with partialParams")
 	okSender := testapp.DefaultSudoRoot()
 	msgEditParams.Sender = okSender.String()
-	_, err = oracleMsgServer.EditOracleParams(
+	resp, err := oracleMsgServer.EditOracleParams(
 		goCtx, &msgEditParams,
 	)
 	s.NoError(err)
+	s.EqualValues(resp.NewParams.String(), fullParams.String())
 }

x/oracle/types/errors.go

@@ -5,22 +5,35 @@ import (
 
 	"github.com/cometbft/cometbft/crypto/tmhash"
 
+	"sync/atomic"
+
 	sdkerrors "cosmossdk.io/errors"
 )
 
+var moduleErrorCodeIdx uint32 = 1
+
+// registerError: Cleaner way of using 'sdkerrors.Register' without as much time
+// manually writing integers.
+func registerError(msg string) *sdkerrors.Error {
+	// Atomic for thread safety on concurrent calls
+	atomic.AddUint32(&moduleErrorCodeIdx, 1)
+	return sdkerrors.Register(ModuleName, moduleErrorCodeIdx, msg)
+}
+
 // Oracle Errors
 var (
-	ErrInvalidExchangeRate   = sdkerrors.Register(ModuleName, 2, "invalid exchange rate")
-	ErrNoPrevote             = sdkerrors.Register(ModuleName, 3, "no prevote")
-	ErrNoVote                = sdkerrors.Register(ModuleName, 4, "no vote")
-	ErrNoVotingPermission    = sdkerrors.Register(ModuleName, 5, "unauthorized voter")
-	ErrInvalidHash           = sdkerrors.Register(ModuleName, 6, "invalid hash")
-	ErrInvalidHashLength     = sdkerrors.Register(ModuleName, 7, fmt.Sprintf("invalid hash length; should equal %d", tmhash.TruncatedSize))
-	ErrVerificationFailed    = sdkerrors.Register(ModuleName, 8, "hash verification failed")
-	ErrRevealPeriodMissMatch = sdkerrors.Register(ModuleName, 9, "reveal period of submitted vote do not match with registered prevote")
-	ErrInvalidSaltLength     = sdkerrors.Register(ModuleName, 10, "invalid salt length; should be 1~4")
-	ErrNoAggregatePrevote    = sdkerrors.Register(ModuleName, 11, "no aggregate prevote")
-	ErrNoAggregateVote       = sdkerrors.Register(ModuleName, 12, "no aggregate vote")
-	ErrUnknownPair           = sdkerrors.Register(ModuleName, 13, "unknown pair")
-	ErrNoValidTWAP           = sdkerrors.Register(ModuleName, 14, "TWA price not found")
+	ErrInvalidExchangeRate = registerError("invalid exchange rate")
+	ErrNoPrevote           = registerError("no prevote")
+	ErrNoVote              = registerError("no vote")
+	ErrNoVotingPermission  = registerError("unauthorized voter")
+	ErrInvalidHash         = registerError("invalid hash")
+	ErrInvalidHashLength   = registerError(
+		fmt.Sprintf("invalid hash length; should equal %d", tmhash.TruncatedSize))
+	ErrHashVerificationFailed = registerError("hash verification failed")
+	ErrRevealPeriodMissMatch  = registerError("reveal period of submitted vote do not match with registered prevote")
+	ErrInvalidSaltLength      = registerError("invalid salt length; should be 1~4")
+	ErrNoAggregatePrevote     = registerError("no aggregate prevote")
+	ErrNoAggregateVote        = registerError("no aggregate vote")
+	ErrUnknownPair            = registerError("unknown pair")
+	ErrNoValidTWAP            = registerError("TWA price not found")
 )