Embed the scope NAVs CSV. #2050
Conversation
… those are specific to this upgrade.
…f that file to after the log message stating what's being done. Tweak the names of some stuff to standardize on all-caps NAV (go's standard for acronyms).
…ore info. Enhance TestAddScopeNAVsWithHeight to hit more cases and check also have it check the logs.
WalkthroughA new upgrade handler has been incorporated to set scope net asset values and update block height for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant UpgradeFiles
participant Logger
User->>App: Trigger Upgrade Handler
App->>UpgradeFiles: Open CSV File
UpgradeFiles-->>App: CSV Content
App->>Logger: Log error if file fails to open
alt File Opened Successfully
App->>App: Read and parse ScopeNAVs
App->>Logger: Log parsing errors
App->>User: Return parsed ScopeNAVs
end
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: 1
Outside diff range and nitpick comments (7)
app/upgrades_test.go (1)
Line range hint
762-872: Review the testing of scope NAV additions.The test
TestAddScopeNAVsWithHeightis well-structured and covers various scenarios including invalid UUIDs, missing scopes, and negative NAV values. However, consider the following improvements:
- Error Handling: The test logs an error for a negative NAV value but does not assert that this condition is handled correctly in the code. Ensure that the application logic prevents negative values from being added.
- Magic Numbers: The test uses specific numbers (e.g.,
406,407) directly in the code. It might be beneficial to define these as constants at the beginning of the test function for clarity and ease of maintenance.- UUID Validation: The test checks for an invalid UUID format. This is good, but ensure that the corresponding production code adequately handles this validation before attempting any operations with the UUID.
- Log Assertions: The test asserts log outputs which is a good practice for ensuring that the logs accurately reflect the operations performed. Ensure that all expected and unexpected conditions are covered by log assertions.
Consider adding more detailed comments within the test to explain the purpose of each assertion, especially for complex scenarios.
CHANGELOG.md (6)
Line range hint
207-238: Use consistent list markers in Markdown.The changelog uses dashes for list items, which is inconsistent with the expected asterisk style. Consider changing all list markers to asterisks for consistency.
- - Feature description + * Feature descriptionAlso applies to: 308-339, 368-370, 447-470, 825-825, 833-833, 878-878, 886-886, 932-932, 1333-1333, 1350-1350, 1402-1402, 1506-1506
Tools
Markdownlint
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
237-237: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
238-238: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
Line range hint
1328-1328: Remove multiple spaces after hash in Markdown heading.Ensure that there is only one space after the hash in a heading for proper Markdown formatting.
- ## Heading + ## Heading
Line range hint
868-868: Avoid duplicate headings in Markdown.The document contains multiple headings with the same content, which can confuse readers and disrupt document navigation.
- ### Bug Fixes + ### Bug Fixes in Version x.y.zAlso applies to: 909-909, 1203-1203
Line range hint
343-343: Enclose bare URLs in angle brackets or convert to Markdown links.Bare URLs should be enclosed in angle brackets or converted to Markdown links to improve readability and prevent potential rendering issues.
- http://example.com + <http://example.com>Also applies to: 374-374, 473-473, 523-523, 535-535, 551-551, 622-622, 633-633, 641-641, 677-677, 702-702, 714-714, 760-764, 811-811, 852-852, 905-905, 1018-1018
Line range hint
1060-1060: Remove spaces inside emphasis markers.Spaces inside emphasis markers can disrupt the intended formatting. Remove any leading or trailing spaces within emphasis markers.
- * emphasized text * + *emphasized text*
Line range hint
456-456: Remove spaces inside code span elements.Spaces inside code span elements can disrupt the intended formatting. Remove any leading or trailing spaces within code spans.
- ` code ` + `code`
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
app/upgrade_files/umber/testnet_scope_navs.csvis excluded by!**/*.csv
Files selected for processing (6)
- CHANGELOG.md (3 hunks)
- app/embed.go (1 hunks)
- app/scope_navs_updater.go (4 hunks)
- app/scope_navs_updater_test.go (2 hunks)
- app/upgrades.go (2 hunks)
- app/upgrades_test.go (3 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
207-207: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
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
237-237: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
238-238: 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
338-338: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
339-339: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
368-368: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
369-369: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
370-370: 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
468-468: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
469-469: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
825-825: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
833-833: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
878-878: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
886-886: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
932-932: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1333-1333: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1350-1350: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1402-1402: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1506-1506: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1328-1328: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading
868-868: null (MD024, no-duplicate-heading)
Multiple headings with the same content
909-909: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1203-1203: null (MD024, no-duplicate-heading)
Multiple headings with the same content
343-343: null (MD034, no-bare-urls)
Bare URL used
374-374: null (MD034, no-bare-urls)
Bare URL used
473-473: null (MD034, no-bare-urls)
Bare URL used
523-523: null (MD034, no-bare-urls)
Bare URL used
535-535: null (MD034, no-bare-urls)
Bare URL used
551-551: null (MD034, no-bare-urls)
Bare URL used
622-622: null (MD034, no-bare-urls)
Bare URL used
633-633: null (MD034, no-bare-urls)
Bare URL used
641-641: null (MD034, no-bare-urls)
Bare URL used
677-677: null (MD034, no-bare-urls)
Bare URL used
702-702: null (MD034, no-bare-urls)
Bare URL used
714-714: 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
763-763: null (MD034, no-bare-urls)
Bare URL used
764-764: null (MD034, no-bare-urls)
Bare URL used
811-811: null (MD034, no-bare-urls)
Bare URL used
852-852: null (MD034, no-bare-urls)
Bare URL used
905-905: null (MD034, no-bare-urls)
Bare URL used
1018-1018: null (MD034, no-bare-urls)
Bare URL used
1060-1060: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
1060-1060: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
456-456: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (4)
app/embed.go (1)
6-6: Approved the embedding of upgrade files.The use of
embed.FSto manage static files is a robust approach for ensuring that all necessary files are included in the binary and are accessible at runtime.app/scope_navs_updater.go (1)
14-14: Approved the refactoring and enhancements in scope NAVs management.Renaming
NetAssetValueWithHeighttoScopeNAVand updating the CSV file handling to useembed.FSenhances clarity and aligns with modern best practices of handling file resources in Go applications.Also applies to: 29-70
app/scope_navs_updater_test.go (1)
14-61: Approved the updates in test cases for scope NAVs.The test updates, including the new
assertEqualEntryfunction, ensure that the tests remain aligned with the changes in the main codebase and enhance the robustness of the testing suite.app/upgrades.go (1)
98-98: Approved the enhancements in upgrade handling for scope NAVs.The introduction of
addScopeNAVsandaddScopeNAVsWithHeightfunctions improves the management of scope net asset values during application upgrades, ensuring more robust and error-resilient operations.Also applies to: 573-616
Description
This PR will embed the CVS used for updating testnet scope NAVs.
Related to:
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
upgrade_filesdirectory for easier access during upgrades.Refactor
NetAssetValueWithHeighttoScopeNAV.Tests
ScopeNAVentries.