feat: add oracle pair to market object

[matthiasmatt]

Description

• fix bug(perp): funding rates aren't working: market needs to specify which index price it uses in x/oracle explicitly #1701

Purpose

Why is this PR important?

Summary by CodeRabbit

• New Features

  • Introduced an Oracle Pair field to markets for better index price determination.

• Bug Fixes

  • Implemented validation to ensure market pairs are not empty.

• Tests

  • Added new test cases for market creation with oracle pairs and error handling.

• Documentation

  • Updated CHANGELOG to reflect the addition of oracle pairs to market objects.

• Refactor

  • Adjusted market creation logic to include oracle pairs.
  • Modified existing functions to utilize the new oracle pair field.

[coderabbitai]

Walkthrough

The codebase has been updated to integrate a new oracle_pair field across various components, enhancing the perpetual market module. This field specifies the oracle pair used for index pricing, affecting market creation, validation, and data retrieval processes. The changes ensure that the oracle pair is now a fundamental part of market-related structures and logic, with updates spanning from state definitions to client commands and testing utilities.

Changes

File Path	Change Summary
proto/nibiru/perp/v2/state.proto wasmbinding/bindings/msg.go wasmbinding/exec_perp.go wasmbinding/exec_perp_test.go wasmbinding/test/cli_test.go x/common/asset/pair.go x/common/testutil/genesis/perp_genesis.go x/common/testutil/mock/perp_market.go x/perp/v2/client/cli/gen_market.go x/perp/v2/client/cli/gen_market_test.go x/perp/v2/keeper/admin_test.go x/perp/v2/keeper/hooks.go x/perp/v2/module/abci.go x/perp/v2/types/genesis.go x/perp/v2/types/market.go x/perp/v2/types/market_test.go CHANGELOG.md	Added or modified fields to include oracle_pair, affecting market creation, validation, and data retrieval.
x/common/asset/pair.go	Modified Validate method to include a check for empty pair length.
x/perp/v2/types/genesis.go	Imported denoms package and used it to initialize OraclePair in DefaultMarket.

Assessment against linked issues

Objective	Addressed	Explanation
Funding rates calculation requires explicit index price specification for perp markets	✅	The addition of the oracle_pair field across various components ensures explicit mapping from perp market ID to oracle price ID, meeting the requirement for funding rate calculation.

🐇✨
In the code where markets thrive,
A new oracle pair comes alive.
With strings that bind, and checks that guard,
The rabbit's code hops forward, unmarred.
🌟🚀

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 X ?

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• You can reply to a review comment made by CodeRabbit.
• You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
• You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 8d4ee29 and c806ddf.

Files ignored due to filter (2)

• x/perp/v2/types/state.pb.go
• x/sudo/types/state.pb.go

Files selected for processing (15)

• proto/nibiru/perp/v2/state.proto (1 hunks)
• wasmbinding/bindings/msg.go (1 hunks)
• wasmbinding/exec_perp.go (1 hunks)
• x/common/asset/pair.go (1 hunks)
• x/common/testutil/genesis/perp_genesis.go (6 hunks)
• x/common/testutil/mock/perp_market.go (1 hunks)
• x/perp/v2/client/cli/gen_market.go (5 hunks)
• x/perp/v2/client/cli/gen_market_test.go (10 hunks)
• x/perp/v2/keeper/admin_test.go (1 hunks)
• x/perp/v2/keeper/hooks.go (1 hunks)
• x/perp/v2/keeper/hooks_test.go (9 hunks)
• x/perp/v2/module/abci.go (1 hunks)
• x/perp/v2/types/genesis.go (2 hunks)
• x/perp/v2/types/market.go (3 hunks)
• x/perp/v2/types/market_test.go (5 hunks)

Additional comments: 41

proto/nibiru/perp/v2/state.proto (1)

• 116-126: The addition of the oracle_pair field to the Market message with the field number 15 is correctly implemented. It uses a custom type and is marked as non-nullable, which is consistent with the PR's objective and the usage of other fields in the message. Ensure there are no field number conflicts within the Market message.

wasmbinding/bindings/msg.go (2)

• 90-93: The addition of the OraclePair field to the MarketParams struct is consistent with the PR's objective to enhance the Market object by specifying the oracle pair used for index price determination. Ensure that all relevant functions and methods that initialize or interact with MarketParams are updated to handle this new field. Additionally, check if there are any serialization or deserialization points in the code that need to be updated to accommodate this change.

• 93-93: The OraclePair field is added without the omitempty JSON tag, which implies that it is a required field in the JSON schema. Confirm that this is the intended behavior and that appropriate validation logic is in place to enforce the presence of this field. If a default value is expected when the field is not provided, consider adding the omitempty tag or ensure that the defaulting logic is implemented elsewhere in the code.

x/common/asset/pair.go (1)

• 84-92: The addition of a check for an empty Pair string in the Validate method is a good practice to ensure that the Pair is not instantiated with an invalid value. However, it's important to ensure that all parts of the codebase that create a Pair object are aware of this new validation rule to prevent runtime errors.

x/common/testutil/genesis/perp_genesis.go (5)

• 18-24: The addition of the OraclePair field to the Market struct is consistent with the PR's objective to enhance the way index prices are determined for markets. The use of asset.NewPair to initialize the OraclePair field is appropriate, as it ensures that the oracle pair is created with the correct asset types.

• 47-53: The OraclePair field is correctly assigned using asset.NewPair with the appropriate currency codes, ensuring that the oracle pair matches the market pair's base currency with the standard USD reference.

• 76-82: The addition of the OraclePair field for the OSMO/NUSD market pair is consistent with the changes made for other market pairs, using asset.NewPair to correctly initialize the oracle pair.

• 137-143: The OraclePair field is being initialized with asset.Registry.Pair, which is expected to work correctly if the Registry contains the necessary pair definitions. This approach is consistent with the initialization of the Pair field.

• 199-205: The OraclePair field is added to the Market struct within the PerpV2Genesis function, using asset.Registry.Pair to initialize it. This is consistent with the changes in the AddPerpV2Genesis function and aligns with the PR's objectives.

x/common/testutil/mock/perp_market.go (1)

• 27-31: The addition of the OraclePair field to the TestMarket function is consistent with the PR's objective to enhance the Market object by specifying the oracle pair used. The initialization using asset.NewPair(denoms.BTC, denoms.USD) seems appropriate, assuming that denoms.BTC and denoms.USD are valid denominations within the system and that the NewPair function is designed to handle such inputs correctly.

x/perp/v2/client/cli/gen_market.go (4)

• 27-31: The addition of the FlagOraclePair constant is consistent with the PR's objective to add an oracle_pair field to the Market object.

• 40-46: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [33-44]

The addMarketGenesisFlags map has been updated to include the FlagOraclePair with a default value and usage documentation. This change is necessary for the new flag to be recognized and used within the command-line interface.

• 145-150: The newMarketFromFlags function has been updated to parse the FlagOraclePair from the flag set. It is important to ensure that all error checks are handled correctly and that the oraclePairStr is validated before being used to create a Market object.

• 200-206: The Market struct is updated to include the OraclePair field, and the market.Validate() function is called to ensure the Market object is valid. This is a critical step to ensure that the new oracle_pair field is correctly integrated and validated within the Market object.

x/perp/v2/client/cli/gen_market_test.go (4)

• 21-25: The addition of the oraclePair field to the test cases is consistent with the PR's objective to enhance the Market object by specifying the oracle pair used for index prices. This change ensures that the new field is included in the test setup and will be validated accordingly.

• 35-35: The oraclePair field is consistently set to "token0:token1" across test cases, which seems appropriate for testing purposes. This mock value is likely used to simulate a valid oracle pair in the context of the tests.

Also applies to: 46-46, 57-57, 68-68, 79-79, 90-90, 101-101, 123-123

• 112-112: The oraclePair field is set to "invalidPair" in one of the test cases, which is a good inclusion to test the validation logic for invalid oracle pairs. This ensures that the system can handle and report errors for invalid input as expected.

• 140-140: The addition of the oraclePair field to the command arguments in the test setup is correct and ensures that the command-line interface is tested for the new field's parsing and handling.

x/perp/v2/keeper/admin_test.go (1)

• 113-121: The addition of a test case for an invalid oracle pair is a good practice to ensure the new functionality is working as expected. It's important to verify that the error message matches the expected output when an invalid oracle pair is used.

x/perp/v2/keeper/hooks_test.go (9)

• 19-23: The introduction of pairBtcUsd is consistent with the PR's objective to enhance the handling of oracle pairs. Ensure that this variable is indeed related to the new oracle_pair field and that the test cases accurately reflect the intended use of this field.

• 26-31: The test case "index > mark" has been updated to use pairBtcUsd for inserting oracle price snapshots. Verify that this change aligns with the updated logic for oracle pairs and that the test case remains valid for the intended test scenario.

• 40-45: Similar to the previous comment, ensure that the use of pairBtcUsd in the "index < mark" test case is appropriate and that the test case accurately tests the new functionality related to the oracle_pair field.

• 54-59: For the "index > mark - max funding rate" test case, confirm that the use of pairBtcUsd is correct and that the test case is structured to validate the behavior of the market with the new oracle_pair field.

• 68-73: Again, for the "index < mark - max funding rate" test case, verify that the changes are consistent with the PR's objectives and that the test case effectively tests the market's behavior with the new oracle_pair field.

• 82-87: In the "index == mark" test case, ensure that the insertion of the oracle price snapshot using pairBtcUsd is intended and that the test case reflects the correct usage of the oracle_pair field.

• 110-116: The "0 price mark" test case has been updated to insert an oracle price snapshot with a zero price using pairBtcUsd. Confirm that this test case is designed to validate the behavior of the market with the new oracle_pair field when the oracle price is zero.

• 125-131: For the "market closed" test case, verify that the insertion of the oracle price snapshot with pairBtcUsd is correct and that the test case accurately tests the market's behavior when it is closed, in the context of the new oracle_pair field.

• 140-146: Lastly, in the "not correct epoch id" test case, ensure that the use of pairBtcUsd for the oracle price snapshot is appropriate and that the test case is structured to validate the market's behavior with the new oracle_pair field when the epoch ID is not correct.

x/perp/v2/module/abci.go (3)

• 45-56: The changes in the EndBlocker function correctly replace references to amm.Pair with market.OraclePair for fetching the exchange rate TWAP, and the error logging has been updated to include both market.Pair and market.OraclePair. This aligns with the PR's objective to enhance the way index prices are determined for markets by specifying the oracle pair used for this purpose. Ensure that the OracleKeeper.GetExchangeRateTwap method is designed to handle the market.OraclePair as expected and that the rest of the system is compatible with these changes.

• 47-49: The error handling for failing to fetch the TWAP index price logs the error and sets indexTwap to a negative value. This approach may have downstream effects depending on how indexTwap is used elsewhere in the system. It's important to ensure that other parts of the code are prepared to handle negative values for indexTwap appropriately.

• 52-53: The error handling for a nil or zero indexTwap is consistent with the handling of markTwap, logging an error and continuing the loop. This ensures that one failure does not halt the processing of all markets in the EndBlocker function.

x/perp/v2/types/genesis.go (3)

• 10-10: The addition of the denoms package is consistent with the new OraclePair field initialization in the Market struct.

• 68-68: The OraclePair field is initialized correctly using the pair.BaseDenom() and denoms.USD. Ensure that the denoms.USD constant is correctly defined within the denoms package and that it represents the expected denomination.

• 65-69: The DefaultMarket function now includes the OraclePair field in the Market struct. This change should be reflected in all places where Market objects are created or expected. It's important to verify that the rest of the codebase is updated to handle this new field.

x/perp/v2/types/market.go (3)

• 49-52: The validation for OraclePair is correctly added to the Validate method, ensuring that the new field adheres to the expected format and rules.

• 101-104: The WithOraclePair method is added following the builder pattern used in the rest of the file, allowing for fluent setting of the OraclePair field on the Market struct.

• 190-192: The addition of the OraclePair comparison in the MarketsAreEqual function is correct, ensuring that the new field is taken into account when comparing two Market instances.

x/perp/v2/types/market_test.go (4)

• 10-11:
  The addition of imports for github.com/NibiruChain/nibiru/x/common/asset and github.com/NibiruChain/nibiru/x/common/denoms is consistent with the usage of asset.Registry.Pair and denoms constants in the test cases.

• 31-32:
  The WithOraclePair method is correctly used to set the oracle_pair field in the Market struct within the test setup, ensuring that the new field is included in the validation process.

• 68-70:
  The new test case added to TestValidate function correctly checks for an error when an invalid oracle pair is provided, which aligns with the PR's objective to enhance validation of the Market object.

• 164-166:
  The TestMarketEqual function is updated with a new test case to check the equality of the oracle_pair field, ensuring that the Market struct comparison logic is correctly handling the new field.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between c806ddf and 7eb1cc3.

Files selected for processing (1)

• CHANGELOG.md (1 hunks)

Additional comments: 1

CHANGELOG.md (1)

• 63-63: The addition to the CHANGELOG.md correctly documents the new feature of adding an oracle pair to the market object as part of the perpetual (perp) module updates.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 7eb1cc3 and 519867c.

Files selected for processing (8)

• wasmbinding/exec_perp.go (1 hunks)
• wasmbinding/exec_perp_test.go (1 hunks)
• wasmbinding/test/cli_test.go (1 hunks)
• x/common/asset/pair.go (1 hunks)
• x/perp/v2/client/cli/gen_market.go (5 hunks)
• x/perp/v2/client/cli/gen_market_test.go (10 hunks)
• x/perp/v2/keeper/admin_test.go (1 hunks)
• x/perp/v2/keeper/hooks.go (1 hunks)

Files skipped from review due to trivial changes (1)

• x/perp/v2/keeper/admin_test.go

Files skipped from review as they are similar to previous changes (5)

• wasmbinding/exec_perp.go
• x/common/asset/pair.go
• x/perp/v2/client/cli/gen_market.go
• x/perp/v2/client/cli/gen_market_test.go
• x/perp/v2/keeper/hooks.go

Additional comments: 2

wasmbinding/exec_perp_test.go (1)

• 193-193: The addition of the OraclePair field to the MarketParams struct within the CreateMarket message is consistent with the PR's objective to enhance the market object with oracle pair information. Ensure that the pair.String() method provides the correct format for the OraclePair and that any necessary validations for the OraclePair are implemented and called where needed.

wasmbinding/test/cli_test.go (1)

• 62-65: The addition of the OraclePair field to the Market object in the test setup is consistent with the PR's objective and appears to be correctly implemented. Ensure that the OraclePair is being used appropriately in all relevant parts of the codebase, especially where market parameters are handled or oracle prices are fetched.

[Unique-Divine]

Running some actions on this PR to fix the codecov step in the test action