feat(x/warden): Analyzers smart contracts #279
Conversation
WalkthroughWalkthroughThis update enhances the Changes
Possibly related issues
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 Configration File (
|
This comment has been minimized.
This comment has been minimized.
|
to fully replace "SignMethods" with analyzers, one missing piece would be the ability to substitute the "data for signing" (e.g. we want an analyzer that takes an unsigned eth transaction, and somehow returns the hash of the transaction that we'll need to sign) I'll do that in a subsequent PR |
There was a problem hiding this comment.
Actionable comments posted: 8
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (27)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/app/app.go (2 hunks)
- warden/testutil/keeper/warden.go (1 hunks)
- warden/x/intent/keeper/actions.go (4 hunks)
- warden/x/intent/keeper/actions_keeper.go (2 hunks)
- warden/x/intent/keeper/intents.go (1 hunks)
- warden/x/intent/keeper/keeper.go (2 hunks)
- warden/x/intent/types/types.go (1 hunks)
- warden/x/warden/keeper/keeper.go (4 hunks)
- warden/x/warden/keeper/msg_server_add_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_key_request.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (3 hunks)
- warden/x/warden/keeper/msg_server_remove_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_key.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_space.go (1 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
- warden/x/warden/keeper/spaces.go (1 hunks)
- warden/x/warden/keeper/wasm.go (1 hunks)
- warden/x/warden/module/module.go (3 hunks)
- warden/x/warden/types/expected_keepers.go (1 hunks)
Files skipped from review due to trivial changes (3)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/lib.rs
Additional Context Used
Path-based Instructions (19)
warden/x/intent/types/types.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/types/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/intent/keeper/intents.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_add_space_owner.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/testutil/keeper/warden.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_remove_space_owner.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_update_key.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_update_space.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/intent/keeper/actions_keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/spaces.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_new_key_request.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/intent/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/shield.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/wasm.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_new_signature_request.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/intent/keeper/actions.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/app/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (24)
contracts/contracts/basic-analyzer/src/bin/schema.rs (1)
1-10: The schema generation setup usingwrite_api!macro is correctly implemented.contracts/contracts/basic-analyzer/src/error.rs (1)
1-11: The error handling setup usingthiserrorcrate is correctly implemented and follows Rust best practices.contracts/contracts/basic-analyzer/src/msg.rs (1)
1-16: The message handling setup usingcosmwasm_schemaandcw_serdeis correctly implemented and follows best practices for CosmWasm contracts.warden/x/intent/types/types.go (1)
4-12: TheActionHandlertype definition is correctly implemented and follows Go best practices for defining handler functions.warden/x/warden/types/expected_keepers.go (1)
Line range hint
1-28: The keeper interfaces are correctly defined and follow Go best practices for modular and reusable components in blockchain applications.warden/x/intent/keeper/intents.go (1)
13-13: TheGetIntentmethod is correctly implemented and follows Go best practices for database access in a blockchain context.warden/x/warden/keeper/msg_server_add_space_owner.go (1)
40-40: TheAddOwnerActionHandlermethod is correctly implemented and follows Go best practices for handling actions in a blockchain context.warden/testutil/keeper/warden.go (1)
45-45: The test setup inWardenKeeperis correctly implemented and follows Go best practices for setting up a blockchain keeper in a test environment.warden/x/warden/keeper/shield.go (3)
20-30: The identifier expansion logic is well-implemented and handles different cases appropriately.
32-56: The space owners expansion logic is correctly implemented with efficient error handling and data structure usage.
58-76: The analyzer expansion logic is correctly implemented, efficiently handling the mapping of values.warden/x/warden/keeper/wasm.go (3)
14-30: The contract execution and result parsing logic inExecuteAnalyzerare correctly implemented with clear error handling.
48-63: The contract execution logic inexecuteContractis robust, with clear error handling and interaction with the Wasm keeper.
65-82: The JSON parsing and AST construction logic inparseAnalyzerValuesare correctly implemented with comprehensive type handling.warden/x/warden/keeper/msg_server_new_signature_request.go (2)
Line range hint
10-47: The logic for handling new signature requests inNewSignatureRequestis correctly implemented, with robust error handling and analyzer execution.
55-67: The execution and aggregation of analyzer results inexecuteAnalyzersare correctly implemented with efficient error handling.warden/x/warden/keeper/keeper.go (1)
Line range hint
9-107: The updates to theKeeperstructure and its constructor are correctly implemented, ensuring proper initialization of dependencies and validation of the authority address.warden/x/intent/keeper/actions.go (3)
47-47: The logic to check if an action is ready inCheckActionReadyis correctly implemented, with comprehensive intent evaluation and status updating.
68-68: The action execution logic inExecuteActionis robust, correctly handling different action types and their results.
93-93: The logic to add and immediately try executing a new action inAddActionis correctly implemented, with efficient handling of intents and immediate execution checks.proto/warden/warden/v1beta2/tx.proto (1)
215-215: The addition of theanalyzersfield to theMsgNewSignatureRequestmessage is correctly implemented and aligns with the PR's objectives.warden/x/warden/module/module.go (1)
Line range hint
200-223: The addition of theGetWasmKeeperfunction as an optional input in the module setup is correctly implemented, enhancing flexibility and integration with the Wasm keeper.warden/app/app.go (2)
211-212: The addition ofapp.GetWasmKeeperto supply the Wasm keeper in the application configuration is consistent with the handling of other module keepers like the IBC keeper. This ensures that the Wasm keeper can be accessed correctly throughout the application.
492-495: The implementation of theGetWasmKeepermethod is correct and follows the established pattern for getter methods in the application. This method will facilitate the access to the Wasm keeper across different modules of the application.
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (28)
- CHANGELOG.md (1 hunks)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/app/app.go (2 hunks)
- warden/testutil/keeper/warden.go (1 hunks)
- warden/x/intent/keeper/actions.go (4 hunks)
- warden/x/intent/keeper/actions_keeper.go (2 hunks)
- warden/x/intent/keeper/intents.go (1 hunks)
- warden/x/intent/keeper/keeper.go (2 hunks)
- warden/x/intent/types/types.go (1 hunks)
- warden/x/warden/keeper/keeper.go (4 hunks)
- warden/x/warden/keeper/msg_server_add_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_key_request.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (3 hunks)
- warden/x/warden/keeper/msg_server_remove_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_key.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_space.go (1 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
- warden/x/warden/keeper/spaces.go (1 hunks)
- warden/x/warden/keeper/wasm.go (1 hunks)
- warden/x/warden/module/module.go (3 hunks)
- warden/x/warden/types/expected_keepers.go (1 hunks)
Files skipped from review as they are similar to previous changes (25)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/bin/schema.rs
- contracts/contracts/basic-analyzer/src/contract.rs
- contracts/contracts/basic-analyzer/src/error.rs
- contracts/contracts/basic-analyzer/src/lib.rs
- contracts/contracts/basic-analyzer/src/msg.rs
- proto/warden/warden/v1beta2/tx.proto
- warden/app/app.go
- warden/testutil/keeper/warden.go
- warden/x/intent/keeper/actions.go
- warden/x/intent/keeper/actions_keeper.go
- warden/x/intent/keeper/intents.go
- warden/x/intent/keeper/keeper.go
- warden/x/intent/types/types.go
- warden/x/warden/keeper/keeper.go
- warden/x/warden/keeper/msg_server_add_space_owner.go
- warden/x/warden/keeper/msg_server_new_key_request.go
- warden/x/warden/keeper/msg_server_new_signature_request.go
- warden/x/warden/keeper/msg_server_update_key.go
- warden/x/warden/keeper/msg_server_update_space.go
- warden/x/warden/keeper/spaces.go
- warden/x/warden/keeper/wasm.go
- warden/x/warden/module/module.go
- warden/x/warden/types/expected_keepers.go
Additional Context Used
LanguageTool (4)
CHANGELOG.md (4)
Near line 59: Unpaired symbol: ‘[’ seems to be missing
Context: ...replaceignite chain serve## [v0.3.0](https://github.com/warden-protocol/ward...
Near line 93: Unpaired symbol: ‘[’ seems to be missing
Context: ...sm integration related docs. ## [v0.2.0](https://github.com/warden-protocol/ward...
Near line 111: Unpaired symbol: ‘[’ seems to be missing
Context: ... the SignIntent of the Space * [#110](https://github.com/warden-protocol/ward...
Near line 123: Unpaired symbol: ‘[’ seems to be missing
Context: ...o derive a layer 1 address) ## [v0.1.0](https://github.com/warden-protocol/ward...
Path-based Instructions (3)
warden/x/warden/keeper/msg_server_remove_space_owner.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/shield.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (4)
warden/x/warden/keeper/msg_server_remove_space_owner.go (1)
41-41: Ensure proper error handling and context propagation.Verification successful
The output from the executed script indicates that there are multiple instances of error handling using the pattern
if err != nilwithin theRemoveOwnerActionHandlerfunction. This suggests that the function does include error handling mechanisms, which is a positive sign for robustness and reliability.However, the script output does not provide specific details about the context in which these error checks are performed, nor does it show how errors are handled or propagated. To fully verify the adequacy of error handling and context propagation, it would be beneficial to see the surrounding code for each error check to understand the actions taken when an error is encountered.
To proceed, I will generate a script to extract more detailed information around each occurrence of error handling in the function.
The detailed context provided around the error handling in the
RemoveOwnerActionHandlerfunction shows that errors are consistently checked and handled appropriately. Each instance where an error could occur (such as fetching a space or an action message) is followed by a checkif err != nil, and in each case, the function returnsnilalong with the error, effectively propagating it upwards.This pattern of error handling is robust as it ensures that errors do not go unnoticed and are handled as soon as they occur, preventing any further execution that might depend on the erroneous state. Additionally, the use of
return nil, erris a common and recommended practice in Go for functions that can return an error, ensuring that the error is not silently ignored and can be handled by the caller.Given this information, it appears that the function
RemoveOwnerActionHandleradheres to good practices in terms of error handling and context propagation. Therefore, the initial review comment concerning the need to ensure proper error handling and context propagation is verified as addressed in the implementation.Proper error handling and context propagation confirmed in `RemoveOwnerActionHandler`. - The function includes multiple checks for errors immediately after operations that can fail. - Each error is handled by returning `nil` and the error itself, which is a standard practice in Go for propagating errors.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling and context propagation in `RemoveOwnerActionHandler`. # Test: Search for error handling patterns in the function. rg --type go 'if err != nil' warden/x/warden/keeper/msg_server_remove_space_owner.goLength of output: 171
Script:
#!/bin/bash # Description: Extract detailed context around error handling in `RemoveOwnerActionHandler`. # Extract lines around each 'if err != nil' to see how errors are handled. rg --type go --context 5 'if err != nil' warden/x/warden/keeper/msg_server_remove_space_owner.goLength of output: 1043
warden/x/warden/keeper/shield.go (2)
20-30: Ensure identifier handling covers all necessary cases.Verification successful
The verification process has confirmed that the
Expandmethod inshield.goappropriately handles all necessary identifier cases. The method processes identifiers with the prefix "analyzer.", the specific identifier "space.owners", and includes a default case for any unknown identifiers. This comprehensive handling ensures that the method behaves as expected for known inputs and gracefully handles unexpected ones.
- Handled Cases:
- Identifiers starting with "analyzer." are expanded using
expandAnalyzers.- The identifier "space.owners" is expanded using
expandSpaceOwners.- Any other identifiers result in an error, clearly stating that the identifier is unknown.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all necessary identifier cases are handled in `Expand`. # Test: Search for identifier handling patterns in the method. rg --type go 'if ident.Value ==' warden/x/warden/keeper/shield.goLength of output: 103
Script:
#!/bin/bash # Description: Search for all references or comparisons involving `ident.Value` in the entire codebase to ensure all identifier cases are handled. # Test: Use ripgrep to find all occurrences of `ident.Value` in Go files. rg --type go 'ident.Value' -A 2 -B 2 warden/x/warden/keeper/Length of output: 1153
58-76: Ensure robust handling of analyzer values and keys.Verification successful
Robust handling of analyzer values and keys is confirmed in the
expandAnalyzersmethod.
- The method includes checks for the existence of both contracts and keys, ensuring that missing entries are handled gracefully.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify robust handling of analyzer values and keys in `expandAnalyzers`. # Test: Search for analyzer value handling patterns in the method. rg --type go 'if !found {' warden/x/warden/keeper/shield.goLength of output: 147
Script:
#!/bin/bash # Description: Verify robust handling of analyzer values and keys in `expandAnalyzers`. # Test: Search for analyzer value handling patterns in the method. rg --type go 'if !found \{' warden/x/warden/keeper/shield.goLength of output: 88
CHANGELOG.md (1)
51-51: Ensure the changelog entry is clear and concise.
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (3)
CHANGELOG.md (3)
Line range hint
93-93: Fix missing bracket in link.- ## [v0.2.0](https://github.com/warden-protocol/wardenprotocol/releases/tag/v0.2.0) - 2024-03-26 + ## [v0.2.0](https://github.com/warden-protocol/wardenprotocol/releases/tag/v0.2.0) - 2024-03-26Ensure all links are correctly formatted.
Line range hint
100-100: Fix grammatical number of noun.- Refactor of `x/intent`'s and `x/warden`'s store design to improve queries performance: + Refactor of `x/intent`'s and `x/warden`'s store design to improve query performance:Ensure correct grammatical number for nouns.
Line range hint
123-123: Fix missing bracket in link.- ## [v0.1.0](https://github.com/warden-protocol/wardenprotocol/releases/tag/v0.1.0) - 2024-03-06 + ## [v0.1.0](https://github.com/warden-protocol/wardenprotocol/releases/tag/v0.1.0) - 2024-03-06Ensure all links are correctly formatted.
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (12)
- CHANGELOG.md (1 hunks)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (4 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
- warden/x/warden/keeper/wasm.go (1 hunks)
Files skipped from review as they are similar to previous changes (9)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/bin/schema.rs
- contracts/contracts/basic-analyzer/src/contract.rs
- contracts/contracts/basic-analyzer/src/error.rs
- contracts/contracts/basic-analyzer/src/lib.rs
- contracts/contracts/basic-analyzer/src/msg.rs
- proto/warden/warden/v1beta2/tx.proto
- warden/x/warden/keeper/msg_server_new_signature_request.go
Additional Context Used
LanguageTool (6)
CHANGELOG.md (6)
Near line 59: Unpaired symbol: ‘[’ seems to be missing
Context: ...replaceignite chain serve## [v0.3.0](https://github.com/warden-protocol/ward...
Near line 93: Unpaired symbol: ‘[’ seems to be missing
Context: ...sm integration related docs. ## [v0.2.0](https://github.com/warden-protocol/ward...
Near line 100: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ndx/warden's store design to improve queries performance: * (x/intent) [#117](ht...
Near line 103: You might be missing the article “the” here.
Context: ...otocol/wardenprotocol/pull/122) ImproveSpacesByOwnerquery by adding a new "ow...
Near line 111: Unpaired symbol: ‘[’ seems to be missing
Context: ... the SignIntent of the Space * [#110](https://github.com/warden-protocol/ward...
Near line 123: Unpaired symbol: ‘[’ seems to be missing
Context: ...o derive a layer 1 address) ## [v0.1.0](https://github.com/warden-protocol/ward...
Path-based Instructions (3)
warden/x/warden/keeper/shield.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/wasm.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (9)
warden/x/warden/keeper/shield.go (4)
20-22: Ensure proper handling of analyzer identifiers.The
Expandfunction now includes a call toexpandAnalyzerswhen the identifier starts with "analyzer.". Verify that this change correctly integrates with the rest of the system and that all necessary tests are updated to cover this new functionality.
58-76: Ensure proper handling of analyzer values.The
expandAnalyzersfunction processes analyzer values and returns the corresponding AST expression. Verify that this function correctly handles all edge cases and that the analyzer values are correctly parsed and utilized.
80-82: Ensure proper context handling for analyzer values.The
WithAnalyzerValuesfunction sets analyzer values in the context. Verify that this function is correctly used throughout the codebase and that the context is properly managed.
84-90: Ensure proper retrieval of analyzer values from context.The
analyzerValuesfunction retrieves analyzer values from the context. Verify that this function correctly handles all edge cases and that the retrieved values are correctly utilized.warden/x/warden/keeper/wasm.go (5)
21-33: Ensure proper execution and parsing of analyzer contracts.The
ExecuteAnalyzerfunction executes an analyzer contract and parses its results. Verify that this function correctly handles all edge cases, including error handling and parsing of the contract's response.
43-49: Ensure proper construction of analyzer messages.The
newAnalyzerMsgfunction constructs a new analyzer message. Verify that this function correctly handles all edge cases and that the constructed message is correctly formatted.
51-66: Ensure proper execution of contracts using wasm keeper.The
executeContractfunction executes a contract using the wasm keeper. Verify that this function correctly handles all edge cases, including error handling and proper interaction with the wasm keeper.
73-90: Ensure proper parsing of analyzer contract results.The
parseAnalyzerValuesfunction parses the results of an analyzer contract. Verify that this function correctly handles all edge cases, including error handling and proper parsing of the contract's response.
92-121: Ensure proper parsing of shield expressions from JSON.The
parseShieldExpressionfunction parses a shield expression from JSON. Verify that this function correctly handles all edge cases, including error handling and proper parsing of different data types.
I actually updated this PR to include this. I slightly changed the return type from the analyzer contract to include an option If an analyzer returns a |
This will let x/warden access x/wasm features (i.e. invoke a smart contract).
Users will be able to specify the list of contracts to be invoked on a per-request basis. The invoked contracts will provide their values to the intent definition at the "Preprocess" stage.
Execute all the requested "analyzers" and put their result values in the Go's ctx of the request.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (3)
CHANGELOG.md (3)
Line range hint
100-100: Consider changing "queries performance" to "query performance" to correct the grammatical number.- to improve queries performance: + to improve query performance:
Line range hint
103-103: Consider adding "the" before "SpacesByOwner" to improve readability.- Improve `SpacesByOwner` query by adding a new "owner -> space id" index. + Improve the `SpacesByOwner` query by adding a new "owner -> space id" index.
Line range hint
108-108: Consider using a full URL for the Cosmos SDK pull request to maintain consistency with other entries.- The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 + The same command was submitted to Cosmos SDK: [https://github.com/cosmos/cosmos-sdk/pull/19870](https://github.com/cosmos/cosmos-sdk/pull/19870)
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (28)
- CHANGELOG.md (1 hunks)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/app/app.go (2 hunks)
- warden/testutil/keeper/warden.go (1 hunks)
- warden/x/intent/keeper/actions.go (4 hunks)
- warden/x/intent/keeper/actions_keeper.go (2 hunks)
- warden/x/intent/keeper/intents.go (1 hunks)
- warden/x/intent/keeper/keeper.go (2 hunks)
- warden/x/intent/types/types.go (1 hunks)
- warden/x/warden/keeper/keeper.go (4 hunks)
- warden/x/warden/keeper/msg_server_add_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_key_request.go (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (4 hunks)
- warden/x/warden/keeper/msg_server_remove_space_owner.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_key.go (1 hunks)
- warden/x/warden/keeper/msg_server_update_space.go (1 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
- warden/x/warden/keeper/spaces.go (1 hunks)
- warden/x/warden/keeper/wasm.go (1 hunks)
- warden/x/warden/module/module.go (3 hunks)
- warden/x/warden/types/expected_keepers.go (1 hunks)
Files skipped from review as they are similar to previous changes (27)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/bin/schema.rs
- contracts/contracts/basic-analyzer/src/contract.rs
- contracts/contracts/basic-analyzer/src/error.rs
- contracts/contracts/basic-analyzer/src/lib.rs
- contracts/contracts/basic-analyzer/src/msg.rs
- proto/warden/warden/v1beta2/tx.proto
- warden/app/app.go
- warden/testutil/keeper/warden.go
- warden/x/intent/keeper/actions.go
- warden/x/intent/keeper/actions_keeper.go
- warden/x/intent/keeper/intents.go
- warden/x/intent/keeper/keeper.go
- warden/x/intent/types/types.go
- warden/x/warden/keeper/keeper.go
- warden/x/warden/keeper/msg_server_add_space_owner.go
- warden/x/warden/keeper/msg_server_new_key_request.go
- warden/x/warden/keeper/msg_server_new_signature_request.go
- warden/x/warden/keeper/msg_server_remove_space_owner.go
- warden/x/warden/keeper/msg_server_update_key.go
- warden/x/warden/keeper/msg_server_update_space.go
- warden/x/warden/keeper/shield.go
- warden/x/warden/keeper/spaces.go
- warden/x/warden/keeper/wasm.go
- warden/x/warden/module/module.go
- warden/x/warden/types/expected_keepers.go
Additional Context Used
LanguageTool (2)
CHANGELOG.md (2)
Near line 100: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ndx/warden's store design to improve queries performance: * (x/intent) [#117](ht...
Near line 103: You might be missing the article “the” here.
Context: ...otocol/wardenprotocol/pull/122) ImproveSpacesByOwnerquery by adding a new "ow...
Path-based Instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
CHANGELOG.md (1)
51-51: Ensure consistent indentation for unordered lists.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (3)
CHANGELOG.md (3)
Line range hint
64-64: Correct indentation for unordered lists.- * x/warden can now resolve `warden.space.owners` in Intent definitions into the list of owners of the space + * x/warden can now resolve `warden.space.owners` in Intent definitions into the list of owners of the space - * Derive Osmosis addresses from ECDSA_SECP256K1 keys - * Extract DataForSigning for Osmosis Amino JSON transactions + * Derive Osmosis addresses from ECDSA_SECP256K1 keys + * Extract DataForSigning for Osmosis Amino JSON transactions - * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 + * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 - * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it. This allows faster queries "by address" by storing the list of addresses directly inside the action, instead of having to re-evaluate the linked intent for each action. + * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it. This allows faster queries "by address" by storing the list of addresses directly inside the action, instead of having to re-evaluate the linked intent for each action. - * (x/warden) [#121](https://github.com/warden-protocol/wardenprotocol/pull/121) Separate Keys queries into `AllKeys`, `KeysBySpaceId`, and `KeyById`. This allowed some improvements, especially in the `KeysBySpaceId` that can benefit from a new "space id -> key id" index. + * (x/warden) [#121](https://github.com/warden-protocol/wardenprotocol/pull/121) Separate Keys queries into `AllKeys`, `KeysBySpaceId`, and `KeyById`. This allowed some improvements, especially in the `KeysBySpaceId` that can benefit from a new "space id -> key id" index.Also applies to: 76-77, 101-103, 108-108, 111-111
Line range hint
41-41: Reduce multiple consecutive blank lines to a single blank line.- + - + - +Also applies to: 122-122, 127-128
Line range hint
108-108: Format bare URL as a markdown link.- * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 + * The same command was submitted to Cosmos SDK: [Cosmos SDK PR #19870](https://github.com/cosmos/cosmos-sdk/pull/19870)
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (4 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
Files skipped from review as they are similar to previous changes (10)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/bin/schema.rs
- contracts/contracts/basic-analyzer/src/contract.rs
- contracts/contracts/basic-analyzer/src/error.rs
- contracts/contracts/basic-analyzer/src/lib.rs
- contracts/contracts/basic-analyzer/src/msg.rs
- proto/warden/warden/v1beta2/tx.proto
- warden/x/warden/keeper/msg_server_new_signature_request.go
- warden/x/warden/keeper/shield.go
Additional Context Used
Path-based Instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
CHANGELOG.md (1)
51-51: Ensure consistent indentation for unordered lists.- * (wardend) analyzers smart contract: a way for 3rd party builders to provide metadata to shield's intents during new signature requests + * (wardend) analyzers smart contract: a way for 3rd party builders to provide metadata to shield's intents during new signature requests
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (5)
CHANGELOG.md (5)
Line range hint
100-100: Consider changing "queries performance" to "query performance" to correct the grammatical number.- to improve queries performance: + to improve query performance:
Line range hint
103-103: Add the article "the" before "new 'owner -> space id' index" for grammatical correctness.- by adding a new "owner -> space id" index. + by adding the new "owner -> space id" index.
Line range hint
64-64: Fix indentation for unordered list items to maintain consistency.- * x/warden can now resolve `warden.space.owners` in Intent definitions into the list of owners of the space + * x/warden can now resolve `warden.space.owners` in Intent definitions into the list of owners of the space - * Derive Osmosis addresses from ECDSA_SECP256K1 keys + * Derive Osmosis addresses from ECDSA_SECP256K1 keys - * Extract DataForSigning for Osmosis Amino JSON transactions + * Extract DataForSigning for Osmosis Amino JSON transactions - * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it. + * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it. - * (x/warden) [#121](https://github.com/warden-protocol/wardenprotocol/pull/121) Separate Keys queries into `AllKeys`, `KeysBySpaceId`, and `KeyById`. + * (x/warden) [#121](https://github.com/warden-protocol/wardenprotocol/pull/121) Separate Keys queries into `AllKeys`, `KeysBySpaceId`, and `KeyById`. - * (x/warden) [#122](https://github.com/warden-protocol/wardenprotocol/pull/122) Improve `SpacesByOwner` query by adding a new "owner -> space id" index. + * (x/warden) [#122](https://github.com/warden-protocol/wardenprotocol/pull/122) Improve `SpacesByOwner` query by adding a new "owner -> space id" index. - * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 + * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 - * [#110](https://github.com/warden-protocol/wardenprotocol/pull/110) Add MsgUpdateKey for updating a key's intent + * [#110](https://github.com/warden-protocol/wardenprotocol/pull/110) Add MsgUpdateKey for updating a key's intentAlso applies to: 76-76, 77-77, 101-101, 102-102, 103-103, 108-108, 111-111
Line range hint
41-41: Remove extra blank lines to adhere to the formatting standards.-Also applies to: 122-122, 127-127, 128-128
Line range hint
108-108: Consider linking the URL to enhance readability and maintain consistency with other entries.- * The same command was submitted to Cosmos SDK: https://github.com/cosmos/cosmos-sdk/pull/19870 + * The same command was submitted to Cosmos SDK: [Cosmos SDK PR #19870](https://github.com/cosmos/cosmos-sdk/pull/19870)
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/warden/warden/v1beta2/tx.pulsar.gois excluded by!api/**contracts/Cargo.lockis excluded by!**/*.lockwarden/x/warden/types/v1beta2/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- contracts/contracts/basic-analyzer/.cargo/config (1 hunks)
- contracts/contracts/basic-analyzer/Cargo.toml (1 hunks)
- contracts/contracts/basic-analyzer/src/bin/schema.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/contract.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/error.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/lib.rs (1 hunks)
- contracts/contracts/basic-analyzer/src/msg.rs (1 hunks)
- proto/warden/warden/v1beta2/tx.proto (1 hunks)
- warden/x/warden/keeper/msg_server_new_signature_request.go (4 hunks)
- warden/x/warden/keeper/shield.go (2 hunks)
Files skipped from review as they are similar to previous changes (10)
- contracts/contracts/basic-analyzer/.cargo/config
- contracts/contracts/basic-analyzer/Cargo.toml
- contracts/contracts/basic-analyzer/src/bin/schema.rs
- contracts/contracts/basic-analyzer/src/contract.rs
- contracts/contracts/basic-analyzer/src/error.rs
- contracts/contracts/basic-analyzer/src/lib.rs
- contracts/contracts/basic-analyzer/src/msg.rs
- proto/warden/warden/v1beta2/tx.proto
- warden/x/warden/keeper/msg_server_new_signature_request.go
- warden/x/warden/keeper/shield.go
Additional Context Used
LanguageTool (2)
CHANGELOG.md (2)
Near line 100: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ndx/warden's store design to improve queries performance: * (x/intent) [#117](ht...
Near line 103: You might be missing the article “the” here.
Context: ...otocol/wardenprotocol/pull/122) ImproveSpacesByOwnerquery by adding a new "ow...
Path-based Instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
CHANGELOG.md (1)
51-51: Ensure consistent indentation for unordered lists.
see #262
In this PR I'm implementing the "analyzers" features. When requesting a new signature, users are now able to ask to execute a list of smart contracts of a special kind called "analyzers". An Analyzer takes the input binary for the signature request and provides some values (metadata) that will be part of the preprocessing step of the shield's AST.
In this PR I'm also implementing a very basic smart contract for an analyzer that simply returns the length in bytes of the input payload.
To summarize, users can write this intent definition:
Then, send a MsgNewSignatureRequest specifying
warden1nc5tatafv6eyq7llkr2gv50ff9e22mnf70qgjlv737ktmt4eswrq075d7kin theanalyzerslist.And the smart contract is expected to return an object containing a key
some_foo_variablesand a boolean value.The final intent's AST is frozen and stored in the Action, as usual.
Summary by CodeRabbit
New Features
wardendanalyzer.Bug Fixes
Documentation
CHANGELOG.mdwith a summary of new features and changes.Refactor
Chores
Cargo.tomlfile for thebasic-analyzerpackage.basic-analyzermodule.basic-analyzercontract.