-
Notifications
You must be signed in to change notification settings - Fork 228
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: add params to disable script #254
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
WalkthroughWalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant MsgServer
participant Params
participant Keeper
participant User
User->>MsgServer: Request Script Execution
MsgServer->>Keeper: Check ScriptEnabled
Keeper-->>MsgServer: Return ScriptEnabled status
alt Script Enabled
MsgServer->>User: Execute Script
else Script Disabled
MsgServer->>User: Return ErrScriptDisabled
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
=======================================
Coverage 40.61% 40.62%
=======================================
Files 264 264
Lines 25082 25096 +14
=======================================
+ Hits 10187 10195 +8
- Misses 13326 13329 +3
- Partials 1569 1572 +3
|
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 UI
Review profile: CHILL
Files ignored due to path filters (2)
client/docs/swagger-ui/swagger.yaml
is excluded by!**/*.yaml
x/move/types/types.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (8)
- api/initia/gov/v1/gov.pulsar.go (25 hunks)
- api/initia/move/v1/types.pulsar.go (31 hunks)
- proto/initia/move/v1/types.proto (2 hunks)
- x/move/keeper/msg_server.go (1 hunks)
- x/move/keeper/msg_server_test.go (1 hunks)
- x/move/keeper/params.go (1 hunks)
- x/move/types/errors.go (1 hunks)
- x/move/types/params.go (4 hunks)
Additional comments not posted (25)
x/move/types/errors.go (1)
51-52
: New error constantErrScriptDisabled
added.The addition of
ErrScriptDisabled
is consistent with the existing error handling pattern and enhances error management by providing a specific error for disabled script execution.x/move/keeper/params.go (1)
51-59
: New methodScriptEnabled
added toKeeper
.The
ScriptEnabled
method is well-implemented, providing functionality to check if script execution is enabled. It includes proper error handling for parameter retrieval.x/move/types/params.go (4)
17-18
: New constantDefaultScriptEnabled
added.The addition of
DefaultScriptEnabled
with a value oftrue
enhances configuration options and is integrated seamlessly into the default parameters.
39-39
: Integration ofScriptEnabled
intoDefaultParams
.The
ScriptEnabled
field is correctly included in theDefaultParams
function, ensuring it is part of the default parameter set.
79-79
: Propagation ofScriptEnabled
inToRaw
method.The
ScriptEnabled
field is correctly propagated in theToRaw
method, maintaining consistency in parameter conversion.
90-90
: Propagation ofScriptEnabled
inToParams
method.The
ScriptEnabled
field is correctly propagated in theToParams
method, ensuring consistent handling in parameter conversion.proto/initia/move/v1/types.proto (2)
33-34
: Addition ofscript_enabled
field.The
script_enabled
field is added to bothParams
andRawParams
messages to control script execution. This change aligns with the PR's objective to introduce parameters for disabling scripts. Ensure that this field is correctly handled in all relevant parts of the codebase.Also applies to: 60-61
38-38
: Reordering of fields inParams
.The
allowed_publishers
field index has been updated from 4 to 5. Ensure that this change does not affect backward compatibility with existing serialized data.Run the following script to verify the usage of
Params
and ensure compatibility:x/move/keeper/msg_server_test.go (1)
69-82
: Addition ofTest_ScriptDisabled
.The test function
Test_ScriptDisabled
effectively verifies the behavior when script execution is disabled. This enhances test coverage and ensures robustness.x/move/keeper/msg_server.go (2)
140-145
: Script execution check inScript
method.The addition of a check to determine if script execution is enabled enhances the robustness of the method by ensuring that scripts are only executed when allowed.
140-140
: Context parameter name change.The change from
context
toctx
for the parameter name is a good practice to avoid shadowing the built-incontext
package and aligns with Go conventions.api/initia/gov/v1/gov.pulsar.go (9)
243-243
: Addition offd_Params_vesting
is appropriate.The declaration of the field descriptor for the new
Vesting
field in theParams
struct is necessary for its management.
268-268
: Initialization offd_Params_vesting
is correct.The field descriptor for the
Vesting
field is properly initialized in theinit()
function.
450-455
: Update toRange
method is appropriate.Including the
Vesting
field in theRange
method ensures consistent handling of all fields in theParams
struct.
509-510
: Update toHas
method is correct.The method now checks for the presence of the
Vesting
field, consistent with other fields in theParams
struct.
565-566
: Update toClear
method is appropriate.The method now clears the
Vesting
field, maintaining consistency with other fields in theParams
struct.
652-654
: Update toGet
method is correct.The method now retrieves the value of the
Vesting
field, consistent with other fields in theParams
struct.
721-722
: Update toSet
method is appropriate.The method now sets the value of the
Vesting
field, maintaining consistency with other fields in theParams
struct.
787-791
: Update toMutable
method is correct.The method now provides a mutable reference to the
Vesting
field, consistent with other fields in theParams
struct.
873-875
: Update toNewField
method is appropriate.The method now returns a new value for the
Vesting
field, maintaining consistency with other fields in theParams
struct.api/initia/move/v1/types.pulsar.go (5)
19-59
: LGTM: Version increment for_Params_5_list
.The changes from
_Params_4_list
to_Params_5_list
are straightforward and correctly handle list operations.
Line range hint
433-489
: LGTM: Serialization updates forScriptEnabled
field.The
ProtoMethods
functions correctly handle the serialization and deserialization of theScriptEnabled
field.Also applies to: 1082-1123
657-675
: LGTM: Unmarshalling updates forScriptEnabled
field.The
Unmarshal
functions correctly parse theScriptEnabled
field.Also applies to: 1290-1309
Line range hint
68-348
: LGTM: Addition ofScriptEnabled
field inParams
.The
ScriptEnabled
field is consistently handled across methods in theParams
struct.However, ensure that the usage of
ScriptEnabled
is correctly integrated across the codebase.Run the following script to verify the usage of
ScriptEnabled
:Verification successful
Verification Successful:
ScriptEnabled
is consistently integrated across the codebase.The
ScriptEnabled
field is well-represented in various parts of the codebase, including parameter management, message handling, and tests. This suggests that the integration is thorough and consistent.
- Files with
ScriptEnabled
references:
x/move/keeper/params.go
x/move/keeper/msg_server.go
x/move/keeper/msg_server_test.go
x/move/types/params.go
x/move/types/types.pb.go
api/initia/move/v1/types.pulsar.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `ScriptEnabled` in the codebase. # Test: Search for the usage of `ScriptEnabled`. Expect: Consistent usage across the codebase. rg --type go 'ScriptEnabled'Length of output: 4065
Line range hint
748-1000
: LGTM: Addition ofScriptEnabled
field inRawParams
.The
ScriptEnabled
field is consistently handled across methods in theRawParams
struct.However, ensure that the usage of
ScriptEnabled
is correctly integrated across the codebase.Run the following script to verify the usage of
ScriptEnabled
:Verification successful
Verified: Consistent usage of
ScriptEnabled
across the codebase.The
ScriptEnabled
field is consistently integrated and used across various files and contexts, including protobuf definitions, comparisons, and function implementations.
- Files and contexts:
x/move/types/types.pb.go
: Field definition and usage in comparisons.x/move/keeper/msg_server_test.go
: Test case assignments.x/move/keeper/msg_server.go
: Function calls.x/move/keeper/params.go
: Function definitions and returns.api/initia/move/v1/types.pulsar.go
: Multiple method implementations and accessors.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `ScriptEnabled` in the codebase. # Test: Search for the usage of `ScriptEnabled`. Expect: Consistent usage across the codebase. rg --type go 'ScriptEnabled'Length of output: 4065
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...