Add scope navs for testnet to v1.19.0-rc1 upgrade handler #2046
Conversation
WalkthroughThe changes introduce functionality to update net asset values (NAVs) specific to the Provenance project for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CSVFile
participant ReadFunction as ReadNetAssetValues
participant App
participant Keeper as MetadataKeeper
User->>App: Initiate upgrade process
App->>CSVFile: Access CSV file for NAVs
CSVFile->>ReadFunction: Provide NAVs data
ReadFunction->>ReadFunction: Process CSV data
ReadFunction->>App: Return structured NAVs data
App->>Keeper: Set NAVs with heights
Keeper->>App: NAVs set confirmation
App->>User: Upgrade process complete
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (7)
CHANGELOG.md (7)
Line range hint
208-234: Use consistent list marker styleThe list items in the changelog use both asterisks and dashes interchangeably. For consistency, it's recommended to use a single style. Consider using asterisks for all list items.
- - Feature description + * Feature descriptionAlso applies to: 306-337, 366-368, 445-467, 823-823, 831-831, 876-876, 884-884
Line range hint
187-187: Remove excessive blank linesMultiple consecutive blank lines found. It's good practice to use a single blank line to separate sections for better readability.
-Also applies to: 930-930, 1331-1331, 1348-1348, 1504-1504
Line range hint
1326-1326: Remove multiple spaces after hash in headingThere are multiple spaces after the hash in the ATX style heading. This could lead to formatting issues. It's best to use a single space.
- ## Some Heading + ## Some Heading
Line range hint
866-866: Avoid duplicate headingsMultiple headings with the same content can confuse the reader and disrupt the document structure. Consider revising the headings to make them unique or merging sections if they cover the same content.
Also applies to: 907-907, 1201-1201
Line range hint
341-341: Avoid using bare URLsMarkdown links should use descriptive text rather than bare URLs. This improves the readability and accessibility of the document.
- https://example.com + [Example Site](https://example.com)Also applies to: 372-372, 471-471, 521-521, 533-533, 549-549, 620-620, 631-631, 639-639, 675-675, 700-700, 712-712, 758-762, 809-809, 850-850, 903-903, 1016-1016
Line range hint
1058-1058: Remove spaces inside emphasis markersSpaces found inside emphasis markers can lead to unintended formatting. Ensure there are no leading or trailing spaces inside emphasis markers.
- * some emphasized text * + *some emphasized text*
Line range hint
454-454: Remove spaces inside code spansSpaces found inside code span elements can lead to unintended formatting. Ensure there are no leading or trailing spaces inside code spans.
- ` some code ` + `some code`
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
app/upgrade_files/testnet_scope_navs.csvis excluded by!**/*.csv
Files selected for processing (6)
- CHANGELOG.md (1 hunks)
- app/scope_navs_updater.go (1 hunks)
- app/scope_navs_updater_test.go (1 hunks)
- app/upgrades.go (3 hunks)
- app/upgrades_test.go (4 hunks)
- x/metadata/keeper/scope.go (1 hunks)
Additional context used
Learnings (1)
app/upgrades_test.go (1)
User: SpicyLemon PR: provenance-io/provenance#2027 File: app/upgrades_test.go:656-704 Timestamp: 2024-06-12T17:07:16.238Z Learning: Due to SDK limitations, it's not possible to mock certain components needed to trigger negative test cases in the context of `TestSetNewGovParamsTestnet`.
Markdownlint
CHANGELOG.md
208-208: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
209-209: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
210-210: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
211-211: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
212-212: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
213-213: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
214-214: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
215-215: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
216-216: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
217-217: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
218-218: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
219-219: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
220-220: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
221-221: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
222-222: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
223-223: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
224-224: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
225-225: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
226-226: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
227-227: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
228-228: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
229-229: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
230-230: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
231-231: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
232-232: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
233-233: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
234-234: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
235-235: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
236-236: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
306-306: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
307-307: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
308-308: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
309-309: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
310-310: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
311-311: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
312-312: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
313-313: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
314-314: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
315-315: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
316-316: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
317-317: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
318-318: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
319-319: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
320-320: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
321-321: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
322-322: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
323-323: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
324-324: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
325-325: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
326-326: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
327-327: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
328-328: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
329-329: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
330-330: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
331-331: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
332-332: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
333-333: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
334-334: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
335-335: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
336-336: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
337-337: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
366-366: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
367-367: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
368-368: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
445-445: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
446-446: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
447-447: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
448-448: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
449-449: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
450-450: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
451-451: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
452-452: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
453-453: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
454-454: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
455-455: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
456-456: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
457-457: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
458-458: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
459-459: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
460-460: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
461-461: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
462-462: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
463-463: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
464-464: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
465-465: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
466-466: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
467-467: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
823-823: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
831-831: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
876-876: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
884-884: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
187-187: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
930-930: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1331-1331: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1348-1348: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1400-1400: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1504-1504: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1326-1326: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading
866-866: null (MD024, no-duplicate-heading)
Multiple headings with the same content
907-907: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1201-1201: null (MD024, no-duplicate-heading)
Multiple headings with the same content
341-341: null (MD034, no-bare-urls)
Bare URL used
372-372: null (MD034, no-bare-urls)
Bare URL used
471-471: null (MD034, no-bare-urls)
Bare URL used
521-521: null (MD034, no-bare-urls)
Bare URL used
533-533: null (MD034, no-bare-urls)
Bare URL used
549-549: null (MD034, no-bare-urls)
Bare URL used
620-620: null (MD034, no-bare-urls)
Bare URL used
631-631: null (MD034, no-bare-urls)
Bare URL used
639-639: null (MD034, no-bare-urls)
Bare URL used
675-675: null (MD034, no-bare-urls)
Bare URL used
700-700: null (MD034, no-bare-urls)
Bare URL used
712-712: null (MD034, no-bare-urls)
Bare URL used
758-758: null (MD034, no-bare-urls)
Bare URL used
759-759: null (MD034, no-bare-urls)
Bare URL used
760-760: null (MD034, no-bare-urls)
Bare URL used
761-761: null (MD034, no-bare-urls)
Bare URL used
762-762: null (MD034, no-bare-urls)
Bare URL used
809-809: null (MD034, no-bare-urls)
Bare URL used
850-850: null (MD034, no-bare-urls)
Bare URL used
903-903: null (MD034, no-bare-urls)
Bare URL used
1016-1016: null (MD034, no-bare-urls)
Bare URL used
1058-1058: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
1058-1058: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
454-454: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (6)
app/scope_navs_updater.go (1)
13-17: Well-defined new type for NAVs with heightThe struct
NetAssetValueWithHeightis well-defined and appropriately encapsulates the necessary fields for handling net asset values with associated block heights. The use ofmetadatatypes.NetAssetValueensures type consistency across the application.app/upgrades.go (1)
98-103: Integration of NAVs in Upgrade HandlerThe integration of the
ReadNetAssetValuesfunction within the upgrade handler (lines 98-102) is done correctly. The error handling and subsequent call toaddScopeNavsWithHeightensure that NAVs are processed and added appropriately during the upgrade.x/metadata/keeper/scope.go (1)
753-774: Review of the new functionSetNetAssetValueWithBlockHeightThis function is well-structured and follows the typical pattern of setting a net asset value with a specific block height. Here are a few observations and suggestions:
Error Handling: The function correctly handles potential errors from
netAssetValue.Validate()andctx.EventManager().EmitTypedEvent(). This is crucial for ensuring data integrity and proper event handling.Key Generation: The key for storing the net asset value is appropriately generated using
types.NetAssetValueKey(). This ensures that the data is stored in a predictable and organized manner.Data Storage: The use of
store.Set()for storing the serialized net asset value is standard practice in Cosmos SDK modules. It's good to see consistent use of the Cosmos SDK's store types.Event Emission: Emitting an event after setting the net asset value is a good practice as it aids in tracking changes and can be useful for clients or external systems monitoring these changes.
Performance Consideration: There doesn't appear to be any unnecessary computations or data manipulations, which is good for the performance of this function.
Overall, the function is implemented correctly and aligns with the Cosmos SDK's standards. It would be beneficial to ensure that there are corresponding unit tests that cover various scenarios, including error handling.
app/upgrades_test.go (3)
12-12: Addition of new imports.The import of
uuidis necessary for handling UUID operations within the new test cases. This is a good practice as it keeps UUID operations consistent and reliable.
27-27: Addition of new imports.The import of
metadatatypesfrom the Provenance metadata module is crucial for handling metadata-specific types within the tests. This aligns well with the new functionalities being tested related to net asset values.
[APROVED]
393-394: Log entries for new functionality.The log entries added for the new functionality of adding scope net asset values with heights are clear and informative. This is important for debugging and operational monitoring. Ensure that the log levels and messages are consistent with the rest of the application.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/scope_navs_updater.go (1 hunks)
- app/scope_navs_updater_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/scope_navs_updater.go
- app/scope_navs_updater_test.go
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passesSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores