-
Notifications
You must be signed in to change notification settings - Fork 39
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
(feat) Updated core protos to the changes required by TS SDK #229
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe updates span various modules, including fee validation, market fee parameters, timestamp conversion, parameter validation, and protocol buffer enhancements. Key changes include simplified fee validation logic, adjusted listing fees, new admin validation functions, a timestamp conversion utility, and additional fields in protocol buffers for market information. Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (6)
chain/exchange/types/exchange.pb.go
is excluded by!**/*.pb.go
chain/exchange/types/tx.pb.go
is excluded by!**/*.pb.go
exchange/derivative_exchange_rpc/pb/goadesign_goagen_injective_derivative_exchange_rpc.pb.go
is excluded by!**/*.pb.go
exchange/spot_exchange_rpc/pb/goadesign_goagen_injective_spot_exchange_rpc.pb.go
is excluded by!**/*.pb.go
exchange/trading_rpc/pb/goadesign_goagen_injective_trading_rpc.pb.go
is excluded by!**/*.pb.go
exchange/trading_rpc/pb/goadesign_goagen_injective_trading_rpc_grpc.pb.go
is excluded by!**/*.pb.go
Files selected for processing (9)
- chain/exchange/types/fee_validation.go (1 hunks)
- chain/exchange/types/params.go (3 hunks)
- chain/oracle/types/msgs.go (2 hunks)
- chain/peggy/types/params.go (4 hunks)
- chain/peggy/types/params_legacy.go (2 hunks)
- chain/wasmx/types/params.go (6 hunks)
- chain/wasmx/types/util.go (1 hunks)
- exchange/derivative_exchange_rpc/pb/injective_derivative_exchange_rpc.proto (2 hunks)
- exchange/spot_exchange_rpc/pb/injective_spot_exchange_rpc.proto (1 hunks)
Additional comments not posted (15)
chain/wasmx/types/util.go (1)
9-17
: Simplified control flow inIsAllowed
function.The change from a switch statement to a simpler if statement enhances readability and reduces complexity. Returning
false
by default is a safe approach as it denies access unless explicitly allowed, which is a good security practice.chain/exchange/types/fee_validation.go (1)
19-22
: Updated fee validation logic inValidateMakerWithTakerFee
.The addition of a clear condition when
makerFeeRate
is negative simplifies understanding and potentially improves error reporting. The detailed error message helps in debugging and ensuring that users are aware of the specific validation failure.chain/wasmx/types/params.go (1)
34-34
: Enhancements to parameter management inParams
.The addition of
KeyRegisterContractAccess
and theregisterContractAccess
parameter, along with its validation function, are well-integrated into the existing parameter structure. The validation logic invalidateAccessConfig
appears robust, checking for valid addresses and preventing duplicates, which is crucial for maintaining system integrity.Also applies to: 48-55, 66-66, 77-77, 99-101, 151-171
chain/peggy/types/params_legacy.go (1)
70-71
: Addition ofParamStoreAdmins
to parameter store.The integration of
ParamStoreAdmins
into the parameter store and the corresponding validation logic invalidateAdmins
(not shown in the provided code but implied to exist) enhances the configurability and security of the admin management process.Also applies to: 106-106
chain/peggy/types/params.go (4)
13-13
: ConstantDefaultParamspace
updated correctly.The update to use
ModuleName
aligns with the module's context, ensuring that the parameter space is correctly scoped.
36-36
: New fieldAdmins
added toParams
struct.The addition of the
Admins
field is crucial for managing administrative privileges within the module. Ensure that this field is properly utilized whereverParams
is instantiated or modified.
102-104
: Validation logic forAdmins
field.The addition of validation for the
Admins
field in theValidateBasic
method is a good practice. It ensures that all entries in theAdmins
array are checked for validity, enhancing security.
301-322
: New functionvalidateAdmins
added.This function correctly checks for the validity of admin addresses and ensures there are no duplicates. It uses the
AccAddressFromBech32
method which is standard for address validation in Cosmos SDK.+ func validateAdmins(i interface{}) error { + v, ok := i.([]string) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + + admins := make(map[string]struct{}) + for _, admin := range v { + adminAddr, err := sdk.AccAddressFromBech32(admin) + if err != nil { + return fmt.Errorf("invalid admin address: %s", admin) + } + if _, found := admins[adminAddr.String()]; found { + return fmt.Errorf("duplicate admin: %s", admin) + } + admins[adminAddr.String()] = struct{}{} + } + return nil + }chain/oracle/types/msgs.go (2)
344-350
: Updates toMsgRelayStorkPrices
validation logic.The conversion of timestamps to nanoseconds before comparison is a critical update for ensuring the validity of time-related data. This change helps maintain the integrity of the timestamp data by standardizing its format.
+ timestamp := ConvertTimestampToNanoSecond(p.Timestamp) + if timestamp > newestTimestamp { + newestTimestamp = timestamp + } + if timestamp < oldestTimestamp { + oldestTimestamp = timestamp + }
382-401
: New functionConvertTimestampToNanoSecond
added.This function handles the conversion of timestamps to nanoseconds based on their magnitude, which is a robust way to handle different timestamp formats. The use of a switch statement simplifies the logic and makes it easy to understand and maintain.
+ func ConvertTimestampToNanoSecond(timestamp uint64) (nanoSeconds uint64) { + switch { + case timestamp > 1e18: + return timestamp + case timestamp > 1e15: + return timestamp * 1_000 + case timestamp > 1e12: + return timestamp * 1_000_000 + default: + return timestamp * 1_000_000_000 + } + }exchange/spot_exchange_rpc/pb/injective_spot_exchange_rpc.proto (1)
95-96
: New fieldmin_notional
added toSpotMarketInfo
.The addition of the
min_notional
field to theSpotMarketInfo
message is appropriate for defining minimum market sizes. This is a crucial feature for risk management and market health.+ string min_notional = 13;chain/exchange/types/params.go (2)
24-28
: Changes to listing fees.The reduction in listing fees for both spot and derivative markets is significant, potentially making it easier for new markets to be listed. This could increase market activity but should be monitored for any unintended consequences.
- SpotMarketInstantListingFee int64 = 1000 - DerivativeMarketInstantListingFee int64 = 1000 + SpotMarketInstantListingFee int64 = 20 + DerivativeMarketInstantListingFee int64 = 20
530-551
: New functionvalidateAdmins
added to validate ExchangeAdmins.This function enhances the security by ensuring that all admin addresses are valid and unique. It uses
sdk.AccAddressFromBech32
for address validation, which is standard and reliable.+ func validateAdmins(i interface{}) error { + v, ok := i.([]string) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + + admins := make(map[string]struct{}) + for _, admin := range v { + adminAddr, err := sdk.AccAddressFromBech32(admin) + if err != nil { + return fmt.Errorf("invalid admin address: %s", admin) + } + if _, found := admins[adminAddr.String()]; found { + return fmt.Errorf("duplicate admin: %s", admin) + } + admins[adminAddr.String()] = struct{}{} + } + return nil + }exchange/derivative_exchange_rpc/pb/injective_derivative_exchange_rpc.proto (2)
127-128
: Approved addition ofmin_notional
field toDerivativeMarketInfo
.The addition of the
min_notional
field is correctly implemented. However, it would be beneficial to include a comment describing its purpose and usage to ensure clarity for all developers and systems interacting with this API.
255-256
: Approved addition ofmin_notional
field toBinaryOptionsMarketInfo
.This field has been correctly added and follows the protobuf standards. As with the
DerivativeMarketInfo
, including a descriptive comment for this field would enhance understanding and maintainability.
@ThomasRalee what is this PR for? I see that the only chages are in the compiled protos that come from injective-core and injective-indexer. But I update those compiled protos regularly in the parent branch. I don't get the need for this. |
Yes, you're right. let's close this PR |
Summary by CodeRabbit
New Features
min_notional
field to enhance market information in Derivative and Spot Market protocols.Improvements
Bug Fixes
Updates