-
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
Allow NAVs to have a greater volume than the marker's supply. #1883
Conversation
… being recorded, which takes more gas.
WalkthroughThe recent updates involve enhancing command-line interface testing by introducing a Changes
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 (
|
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.
Review Status
Actionable comments generated: 13
Configuration used: CodeRabbit UI
Files selected for processing (4)
- x/exchange/client/cli/cli_test.go (2 hunks)
- x/exchange/client/cli/tx_test.go (1 hunks)
- x/marker/keeper/keeper.go (2 hunks)
- x/marker/keeper/keeper_test.go (4 hunks)
Additional comments: 4
x/marker/keeper/keeper.go (1)
- 315-315: The change in the
SetNetAssetValue
function to allow NAVs to be recorded with volumes exceeding the marker's supply is aligned with the PR's objectives. However, it's crucial to ensure comprehensive testing around this logic change to prevent potential issues, such as the setting of invalid NAVs or unintended consequences on financial reporting and analysis.x/exchange/client/cli/cli_test.go (2)
- 536-537: The addition of the
gas
field to thetxCmdTestCase
struct is a good enhancement, allowing for more precise control over gas amounts in test cases.- 568-572: The handling of the
gas
field, including its default value and usage in constructing command arguments, is correctly implemented.x/exchange/client/cli/tx_test.go (1)
- 450-450: The addition of a
gas
field with a value of300_000
to the test case inTestCmdTxMarketSettle
is a good practice for specifying the gas amount for the transaction being tested. This ensures that the test accurately reflects the conditions under which the transaction will be executed in a live environment.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 1
CHANGELOG.md (1)
- 52-52: The addition of allowing NAV volume to exceed a marker's supply is a significant change that enhances the system's flexibility in handling various financial reporting scenarios. This change aligns with the PR objectives and addresses the limitations previously faced with markers having zero supply or cumulative NAVs exceeding supply.
* Remove the NAV restriction that volume is at most the supply. * Fix an exchange unit test that started to fail because the NAV is now being recorded, which takes more gas. * Add changelog entry.
Description
This PR makes it so that NAVs can be recorded with a volume that's greater than the marker's supply.
There are some markers that list the supply as zero (e.g.
fixed_supply
=false
). That limitation was preventing any NAV info for being recorded for them.There's also a legitimate case where the NAV is the sum of info over a period of time in which it's possible that the volume is being reported as greater than the supply.
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/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
gas
field for specifying gas amounts in CLI transactions.tourmaline-rc3
.cometbft-db v0.7.0
for enhanced stability.Keeper
struct to allow NAV volume to exceed a marker's supply.