[TRA-64] Use market specific insurance fund for cross or isolated markets #1132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
WalkthroughThe update enhances various aspects of the protocol, including refining insurance fund management with the addition of 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 (
|
shrenujb
commented
Mar 1, 2024
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (16)
- protocol/app/module_accounts.go (1 hunks)
- protocol/mocks/ClobKeeper.go (1 hunks)
- protocol/testutil/constants/perpetuals.go (1 hunks)
- protocol/testutil/constants/prices.go (5 hunks)
- protocol/testutil/daemons/pricefeed/exchange_config/market_id.go (1 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/keeper/deleveraging.go (3 hunks)
- protocol/x/clob/keeper/deleveraging_test.go (2 hunks)
- protocol/x/clob/keeper/liquidations.go (1 hunks)
- protocol/x/clob/keeper/process_single_match.go (1 hunks)
- protocol/x/clob/types/expected_keepers.go (2 hunks)
- protocol/x/clob/types/liquidations_keeper.go (1 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
- protocol/x/subaccounts/keeper/transfer.go (3 hunks)
- protocol/x/subaccounts/keeper/transfer_test.go (8 hunks)
- protocol/x/subaccounts/types/expected_keepers.go (2 hunks)
Additional comments: 34
protocol/x/subaccounts/types/expected_keepers.go (2)
- 75-75: The addition of the
GetInsuranceFundNamemethod to thePerpetualsKeeperinterface is a logical extension to support the new functionality of market-specific insurance funds. This method will likely be used to retrieve the name of the insurance fund associated with a given perpetual ID, which is crucial for dynamic insurance fund management.- 92-92: The addition of the
SendCoinsmethod to theBankKeeperinterface is significant. It introduces a more generic way of transferring coins between accounts, which could be utilized in various contexts beyond the scope of insurance fund payments. This change enhances the flexibility of theBankKeeperinterface but should be carefully evaluated to ensure it aligns with the overall design principles of the system and does not introduce security risks.protocol/x/clob/types/liquidations_keeper.go (1)
- 54-54: The update to the
GetInsuranceFundBalancemethod signature to include aperpetualId uint32parameter is a necessary change to support market-specific insurance funds. This allows querying the insurance fund balance for a specific market, aligning with the PR's objectives to enhance flexibility and functionality in managing insurance funds. Ensure that all calls to this method throughout the codebase have been updated to pass the newperpetualIdparameter.protocol/testutil/daemons/pricefeed/exchange_config/market_id.go (1)
- 79-79: The introduction of the
MARKET_ISO_USDconstant with the value999_999is a clear and straightforward way to represent an arbitrary isolated market within the system. This addition supports the PR's objectives by facilitating the testing and configuration of isolated markets. The choice of999_999as the value seems arbitrary but is acceptable as long as it does not conflict with other market IDs and is documented appropriately.protocol/app/module_accounts.go (1)
- 42-42: The update to the comment for the
insurance fund accountinmodule_accounts.goby removing the term "clob" reflects a broader applicability of the insurance fund beyond the central limit order book (CLOB) model. This change is consistent with the PR's objectives to support market-specific insurance funds and enhances clarity by removing potentially confusing terminology.protocol/x/clob/types/expected_keepers.go (2)
- 74-74: The update to the
TransferInsuranceFundPaymentsmethod in theSubaccountsKeeperinterface to include aperpetualId uint32parameter is essential for supporting market-specific insurance fund transfers. This change allows for more granular control over insurance fund payments, aligning with the PR's objectives. Ensure that all implementations of this interface have been updated accordingly.- 143-143: The addition of the
GetInsuranceFundModuleAddressmethod to thePerpetualsKeeperinterface is a logical step to support dynamic retrieval of insurance fund module addresses based on the perpetual ID. This method enhances the system's flexibility in managing insurance funds for different markets. It's important to ensure that this method is implemented efficiently and securely, considering the potential implications on the system's overall architecture.protocol/x/subaccounts/keeper/transfer.go (1)
- 277-299: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [261-296]
The update to the
TransferInsuranceFundPaymentsmethod to include aperpetualId uint32parameter and the switch to using theSendCoinsAPI instead ofSendCoinsFromModuleToModuleis a significant change. This update allows for dynamic determination of the sender and receiver modules based on the insurance fund delta sign and the specific perpetual ID, enhancing the flexibility and functionality of insurance fund management. However, it's crucial to ensure that the dynamic determination of module addresses (fromModuleandtoModule) is secure and does not introduce vulnerabilities, such as unauthorized fund transfers. Additionally, the use ofSendCoinsshould be carefully evaluated to ensure it aligns with the intended use cases and does not bypass important checks or permissions thatSendCoinsFromModuleToModulemight enforce.protocol/x/clob/abci.go (1)
- 122-122: The addition of the hardcoded value
0as a parameter toGetInsuranceFundBalancewithin themetrics.SetGaugecall raises questions about its purpose and implications for market-specific insurance fund management. Can you clarify if this value is intended to represent a specific market or if it's a placeholder for future dynamic assignment? Ensuring that this implementation aligns with the intended functionality for market-specific insurance funds is crucial.protocol/testutil/constants/prices.go (6)
- 29-29: The introduction of the
IsoUsdPairconstant with the value "ISO-USD" is a clear and straightforward addition, aligning with the PR's objective to support isolated markets. This change appears correctly implemented.- 38-38: Setting the exponent for
IsoUsdPairto-8is consistent with the protocol's handling of other currency pairs. It's important to ensure this value accurately reflects the desired precision for theISO-USDmarket.- 206-214: The addition of exchange configuration for the
ISO-USDpair underTestMarketExchangeConfigsis crucial for testing and simulation purposes. It's important to verify that the specified exchange and ticker are correct and that the configuration aligns with the protocol's requirements for market data sources.- 243-249: Adding market parameters for
IsoUsdPairwith corresponding exchange configuration is a significant change that supports the introduction of the isolated market. Ensure that the parameters, such asMinExchangesandMinPriceChangePpm, are set according to the protocol's standards for market management.- 269-272: The addition of a market price entry for
IsoUsdPairis essential for testing and ensures that the new market is correctly integrated into the protocol's pricing mechanisms. Confirm that the price and exponent values are appropriate for theISO-USDmarket.- 279-279: Updating
TestMarketIdsToExponentsto includeIsoUsdExponentfor the newISO-USDmarket is a necessary change for consistency and correctness in testing. This update appears correctly implemented.protocol/testutil/constants/perpetuals.go (1)
- 302-313: The addition of the
IsoUsd_IsolatedMarketconstant is well-defined and correctly implements theperptypes.Perpetualstruct with appropriate parameters for the ISO-USD market. This change aligns with the PR's objective to support market-specific insurance funds, specifically introducing an isolated market type for the ISO-USD pair. The parameters chosen, such asMarketId,AtomicResolution,DefaultFundingPpm, andLiquidityTier, are consistent with the existing pattern for defining perpetual markets in this file. TheMarketTypeis correctly set toPERPETUAL_MARKET_TYPE_ISOLATED, indicating this market's isolated nature.One thing to note is the
AtomicResolutionandDefaultFundingPpmvalues are set similarly to other market definitions in this file, which suggests consistency. However, it's important to ensure that these values are indeed appropriate for the ISO-USD market's specific requirements and characteristics. If these parameters were determined based on market analysis or specific business requirements, no further action is needed. Otherwise, it might be beneficial to verify these values to ensure they accurately reflect the intended market behavior and risk management strategies.Overall, the addition of the
IsoUsd_IsolatedMarketconstant is correctly implemented and contributes to the protocol's functionality by enabling support for an isolated market type. This enhancement is a positive step towards achieving the PR's objectives of improving risk management and operational efficiency for both cross and isolated markets.protocol/x/clob/keeper/process_single_match.go (2)
- 435-435: The addition of the
perpetualIdparameter to theTransferInsuranceFundPaymentsmethod call is a critical change that aligns with the PR's objective of enabling market-specific insurance fund management. This modification ensures that insurance fund payments can now be associated with specific perpetual markets, which is essential for the implementation of market-specific insurance funds. The change appears to be correctly implemented within the context of thepersistMatchedOrdersfunction. However, it's important to ensure that all calls toTransferInsuranceFundPaymentsthroughout the codebase have been updated to include the newperpetualIdparameter to maintain consistency and avoid potential runtime errors.- 432-438: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-500]
Given the significant changes introduced in this PR, including the addition of the
perpetualIdparameter to various functions to support market-specific insurance funds, it's crucial to ensure comprehensive testing and verification. This includes unit tests, integration tests, and end-to-end tests to cover the new functionality and its integration with existing features. Additionally, considering the potential impact on the system's architecture, it would be advisable to perform thorough performance testing to assess any implications on transaction processing times, resource utilization, and overall system stability.protocol/x/clob/keeper/deleveraging.go (2)
- 133-144: The function
GetInsuranceFundBalancenow correctly includes aperpetualId uint32parameter to fetch the balance of the insurance fund specific to a market. This change aligns with the PR's objective to support market-specific insurance funds. However, ensure that all calls to this function throughout the codebase have been updated to pass the newperpetualIdparameter.- 262-270: The modification to
IsValidInsuranceFundDeltato include aperpetualId uint32parameter is a necessary change to validate insurance fund deltas for specific markets. This update is consistent with the goal of enabling market-specific insurance fund management. Similar to the previous function, verify that all invocations ofIsValidInsuranceFundDeltahave been updated accordingly.Verification successful
The absence of output from the script indicates that there are no calls to
IsValidInsuranceFundDeltawith the incorrect number of parameters, suggesting that all invocations of this function have been updated to include theperpetualId uint32parameter as required. This supports the review comment's assertion that the modification to include aperpetualIdparameter is consistent with enabling market-specific insurance fund management and that the necessary updates have been made.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to IsValidInsuranceFundDelta without the perpetualId parameter. Expecting no results. ast-grep --lang go --pattern $'IsValidInsuranceFundDelta($_, $_)'Length of output: 66
protocol/x/subaccounts/keeper/transfer_test.go (4)
- 674-720: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [665-715]
The addition of the
perpetualfield in theTestTransferInsuranceFundPaymentstest cases is a significant change that aligns with the PR's objective to support market-specific insurance funds. This change allows the tests to specify which market (cross or isolated) the insurance fund transactions are related to, enhancing the protocol's flexibility and functionality. It's crucial to ensure that the perpetual IDs used in the tests correspond to valid market configurations and that the expected outcomes are accurately reflected in the test assertions.
- 753-754: Creating liquidity tiers in the setup phase for the
TestTransferInsuranceFundPaymentstest function is a good practice. It ensures that the testing environment accurately reflects the protocol's operational context, where different liquidity tiers might affect the behavior of insurance fund transfers. This addition enhances the test's reliability in simulating real-world scenarios.- 772-781: The setup for creating perpetual markets within the
TestTransferInsuranceFundPaymentstest cases is crucial for testing the new functionality of market-specific insurance funds. By specifying market parameters such asMarketId,AtomicResolution, andMarketType, the tests can more accurately simulate the protocol's behavior in managing insurance funds for different market configurations. It's important to ensure that the perpetual market parameters used in the tests are consistent with the protocol's expected configurations for cross and isolated markets.- 824-835: The use of
require.PanicsWithErrorandrequire.ErrorIsfor error handling in theTestTransferInsuranceFundPaymentstest cases is appropriate. These assertions ensure that the function behaves as expected under various conditions, including error scenarios and successful transfers. It's important to verify that the error messages and conditions tested are aligned with the protocol's error handling strategy and accurately reflect the possible outcomes of insurance fund transfers.protocol/mocks/ClobKeeper.go (1)
- 233-233: The addition of
perpetualId uint32as a parameter to theGetInsuranceFundBalancemethod in theClobKeepermock correctly aligns with the PR's objective to support market-specific insurance funds. This change enables the mock to simulate behavior that is consistent with the updated interface, which is crucial for testing components that depend onClobKeeper. Ensure that all tests and usages of this mock are updated accordingly to pass the newperpetualIdparameter.protocol/x/clob/keeper/liquidations.go (1)
- 1136-1136: The addition of
perpetualIdas a parameter to theIsValidInsuranceFundDeltafunction call is a critical change that aligns with the PR's objective to support market-specific insurance funds. This modification enables the validation of insurance fund deltas with respect to specific perpetual markets, enhancing the flexibility and functionality of the protocol's risk management system.protocol/x/perpetuals/keeper/perpetual.go (5)
- 34-46: The function
GetInsuranceFundNamecorrectly handles the retrieval of insurance fund names based on the perpetual market type. It uses a clear and straightforward logic to differentiate between isolated and cross market types, returning a specific name format for isolated markets and a generic name for cross markets. The use ofpanicfor invalid market types is a strong choice, ensuring that such errors are caught during development/testing phases.- 49-54: The function
GetInsuranceFundModuleAddresseffectively retrieves the module address for an insurance fund by first getting the insurance fund name usingGetInsuranceFundName. It properly handles errors and returns the module address usingauthtypes.NewModuleAddress. This function is well-implemented, with clear error handling and straightforward logic.- 56-56: Throughout the file, there's consistent and proper error handling, especially in functions interacting with external modules or performing critical operations. This approach enhances the robustness and reliability of the code.
- 56-56: The use of
panicin several places (e.g., lines 46, 1089, 1143, etc.) for handling unexpected conditions or invariant violations is noteworthy. While this can be effective during development/testing to catch serious issues early, it's crucial to ensure that these scenarios are extremely unlikely or impossible in production environments to avoid potential disruptions.- 56-56: The modular structure of the file, with clear separation of concerns among functions and well-organized sections for liquidity tiers, parameters, and premium processing, contributes to the maintainability and readability of the code. This organization facilitates easier navigation and understanding of the codebase.
protocol/x/clob/keeper/deleveraging_test.go (3)
- 112-112: The addition of the
0argument toGetInsuranceFundBalancein the testTestGetInsuranceFundBalancecorrectly reflects the updated function signature to support market-specific insurance funds. This change is consistent with the PR objectives.- 119-119: The addition of the
0argument toGetInsuranceFundBalancein the testTestGetInsuranceFundBalancecorrectly reflects the updated function signature to support market-specific insurance funds. This change is consistent with the PR objectives.- 206-206: The addition of the
0argument toIsValidInsuranceFundDeltain the testTestIsValidInsuranceFundDeltacorrectly reflects the updated function signature to support market-specific insurance funds. This change is consistent with the PR objectives.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (11)
- protocol/daemons/liquidation/client/grpc_helper_test.go (2 hunks)
- protocol/testing/e2e/gov/perpetuals_test.go (1 hunks)
- protocol/testutil/constants/perpetuals.go (1 hunks)
- protocol/testutil/constants/pricefeed.go (1 hunks)
- protocol/testutil/constants/prices.go (6 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/keeper/deleveraging.go (3 hunks)
- protocol/x/clob/keeper/deleveraging_test.go (8 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
- protocol/x/prices/keeper/msg_server_update_market_prices_test.go (8 hunks)
Files skipped from review as they are similar to previous changes (5)
- protocol/testutil/constants/perpetuals.go
- protocol/testutil/constants/prices.go
- protocol/x/clob/abci.go
- protocol/x/clob/keeper/deleveraging_test.go
- protocol/x/perpetuals/keeper/perpetual.go
Additional comments: 9
protocol/testutil/keeper/perpetuals.go (1)
- 170-184: The function
CreateTestPerpetualscorrectly implements the logic for creating test perpetuals using predefined parameters. It's well-structured for its intended use in testing scenarios. Usingrequire.NoErrorfor error handling is appropriate here, as it directly fails the test if an error occurs, which is a common pattern in test code. This function enhances the test suite's capabilities by allowing for the easy creation of test perpetuals, contributing to more comprehensive testing of the perpetuals functionality.protocol/x/prices/keeper/msg_server_update_market_prices_test.go (2)
- 7-8: The re-addition of the
errorsmodimport is appropriate as it's used for error handling within the test cases. This ensures that the tests can properly handle and assert error conditions, contributing to more robust and reliable tests.- 50-50: The updates to market prices for different scenarios, including the addition of
MarketId3, enhance the test coverage by including a new market ID. This is a positive change, ensuring that the tests are comprehensive and cover a wider range of scenarios. The use of constants for price values and market IDs is a good practice, improving the maintainability and readability of the test code.Also applies to: 60-60, 83-83, 106-106, 129-129, 152-152, 175-175
protocol/daemons/liquidation/client/grpc_helper_test.go (2)
- 8-8: The import of
cosmossdk.io/loghas been added. Ensure that this package is used within the file. If it's not directly used, this import might be unnecessary and could be removed to keep the import list clean and concise.- 413-413: Adding
constants.TestMarketPrices[3]to theTestGetAllMarketPricesfunction's expected results array. This change implies that the test now expects an additional market price in the response. Ensure that this addition aligns with the intended behavior of the system under test and that the corresponding setup in the mock (or the actual implementation if applicable) correctly returns this additional market price.protocol/testing/e2e/gov/perpetuals_test.go (1)
- 22-22: The change in the
Idfield value within thePerpetualParamsstruct from a non-zero value to 0 could have implications on the uniqueness and identification of perpetual parameters. Ensure that setting theIdto 0 does not conflict with the protocol's logic for identifying or distinguishing perpetual markets.protocol/testutil/constants/pricefeed.go (1)
- 16-16: The addition of
MarketId3aligns with the pattern of defining market IDs and supports the PR's objective of enhancing market-specific functionalities. However, it's essential to ensure that this new market ID is integrated and utilized correctly within the testing framework.Verification successful
The addition of
MarketId3has been successfully integrated and utilized within the testing framework, specifically in the context of price updates for markets. This confirms the correctness and relevance of the addition, aligning with the objectives of enhancing market-specific functionalities.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that MarketId3 is used in relevant test cases rg "MarketId3" --type goLength of output: 1045
protocol/x/clob/keeper/deleveraging.go (2)
- 135-152: The function
GetInsuranceFundBalancehas been updated to include aperpetualId uint32parameter, which allows fetching the balance of the insurance fund specific to a perpetual market. This change aligns with the PR's objective to support market-specific insurance funds. However, it's important to ensure that all calls to this function throughout the codebase have been updated to pass the newperpetualIdparameter to avoid runtime errors.Verification successful
The search results indicate that the function
GetInsuranceFundBalanceis indeed being called with theperpetualIdparameter in the codebase. The occurrences are found within the same file where the function is defined (deleveraging.go), in test files (deleveraging_test.go), and in a mock file (ClobKeeper.go). This confirms that the function signature change to include aperpetualId uint32parameter has been correctly propagated to its calls within the codebase, aligning with the PR's objective to support market-specific insurance funds.Based on this information, it can be concluded that the review comment regarding the need to ensure all calls to
GetInsuranceFundBalancehave been updated to pass the newperpetualIdparameter has been verified successfully.* 279-287: The `IsValidInsuranceFundDelta` function now includes a `perpetualId uint32` parameter, enabling validation of insurance fund deltas for specific markets. This change is consistent with the PR's objectives. However, it's essential to review the logic that calculates the current insurance fund balance and adds the delta to ensure it correctly handles negative deltas and potential underflows.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to GetInsuranceFundBalance without the perpetualId parameter. Expecting no results. ast-grep --lang go --pattern $'GetInsuranceFundBalance($_)'Length of output: 60
Script:
#!/bin/bash # Refine the search to find any calls to GetInsuranceFundBalance, to verify if the perpetualId parameter is being passed. rg "GetInsuranceFundBalance\(" --goLength of output: 466
Script:
#!/bin/bash # Correct the search command to find any calls to GetInsuranceFundBalance, ensuring the search is executed across all files. rg "GetInsuranceFundBalance\("Length of output: 852
Verification successful
The provided script output includes the full content of the
deleveraging.gofile, which contains theIsValidInsuranceFundDeltafunction. This function is crucial for verifying the handling of negative deltas and potential underflows as mentioned in the review comment. The relevant part of the function is as follows:// IsValidInsuranceFundDelta returns true if the insurance fund has enough funds to cover the insurance // fund delta. Specifically, this function returns true if either of the following are true: // - The `insuranceFundDelta` is non-negative. // - The insurance fund balance + `insuranceFundDelta` is greater-than-or-equal-to 0. func (k Keeper) IsValidInsuranceFundDelta(ctx sdk.Context, insuranceFundDelta *big.Int, perpetualId uint32) bool { // Non-negative insurance fund deltas are valid. if insuranceFundDelta.Sign() >= 0 { return true } // The insurance fund delta is valid if the insurance fund balance is non-negative after adding // the delta. currentInsuranceFundBalance := k.GetInsuranceFundBalance(ctx, perpetualId) return new(big.Int).Add(currentInsuranceFundBalance, insuranceFundDelta).Sign() >= 0 }This function checks if the insurance fund delta is non-negative, in which case it immediately returns true, indicating that the insurance fund delta is valid. If the insurance fund delta is negative, it then checks if the insurance fund balance remains non-negative after adding the delta. This is done by adding the current insurance fund balance to the insurance fund delta and checking if the result is non-negative.
The use of
big.Intfor representing the insurance fund balance and delta ensures that the arithmetic operations are safe from integer overflow and underflow issues, which are common pitfalls in financial calculations. TheSign()method is used to determine the sign of the result, ensuring that the function correctly handles negative deltas.Given this analysis, the function appears to be correctly handling negative deltas and potential underflows by leveraging the properties of
big.Intfor safe arithmetic operations. Therefore, the original review comment's concern about ensuring the logic correctly handles negative deltas and potential underflows is addressed by the implementation of theIsValidInsuranceFundDeltafunction.Based on this analysis, the conclusion is:
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Manual verification needed to ensure the logic correctly handles negative deltas and potential underflows. echo "Review the logic in IsValidInsuranceFundDelta for handling negative deltas and potential underflows."Length of output: 210
Script:
#!/bin/bash # Find the file containing the IsValidInsuranceFundDelta function and display its content to review the handling of negative deltas and potential underflows. fd "deleveraging.go" --exec cat {}Length of output: 28541
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- protocol/testutil/constants/pricefeed.go (2 hunks)
- protocol/x/clob/keeper/liquidations_test.go (1 hunks)
- protocol/x/prices/keeper/msg_server_update_market_prices_test.go (12 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/testutil/constants/pricefeed.go
- protocol/x/prices/keeper/msg_server_update_market_prices_test.go
Additional comments: 1
protocol/x/clob/keeper/liquidations_test.go (1)
- 245-248: The method call
SendCoinshas been changed to directly use module addresses instead of module names. This change aligns with the Cosmos SDK's approach to handling module accounts and ensures that the correct module addresses are used for transactions. This modification enhances the security and correctness of the module-to-module coin transfer process.
Comment on lines
+43
to
+47
| if perpetual.Params.MarketType == types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_ISOLATED { | ||
| return types.InsuranceFundName + ":" + lib.UintToString(perpetualId), nil | ||
| } else if perpetual.Params.MarketType == types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS { | ||
| return types.InsuranceFundName, nil | ||
| } |
There was a problem hiding this comment.
Question, is our cosmos-sdk version up to date enough for this code snippet to work?
There was a problem hiding this comment.
Are you referring to the address.Module API?
It should be considering we already use authtypes.NewModuleAddress which uses that underneath
There was a problem hiding this comment.
Is there a reason we use the : delimiter in a single string compared to calling the function with insurance_fund and the perpetual id as a key?
There was a problem hiding this comment.
I don't think the 2 solutions would be very different in terms of impact (having separate addresses for these accounts) and the latter would require a bigger refactor
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- protocol/x/clob/keeper/liquidations_test.go (5 hunks)
- protocol/x/prices/keeper/smoothed_price_test.go (3 hunks)
- protocol/x/prices/keeper/update_price_test.go (3 hunks)
- protocol/x/prices/keeper/validate_market_price_updates_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/clob/keeper/liquidations_test.go
Additional comments: 10
protocol/x/prices/keeper/smoothed_price_test.go (3)
- 61-61: The addition of
constants.MarketId3to theexpectedResultmap in the test case "Mixed updates and additions - mix of present and missing index prices, smoothed prices" correctly reflects the intention to test the handling of new market price updates. This change aligns with the PR objectives of supporting market-specific insurance funds.- 83-84: The error message concatenation in the test case "Interpolation errors - returns error" has been updated to include errors for all markets, including
MarketId3. This change ensures comprehensive testing of error handling during price interpolation, which is crucial for the reliability of the price updating mechanism.- 91-97: The test case "Single interpolation error - returns error, continues updating other markets" has been updated to include an error message for
MarketId3and to reflect the expected results accurately. This test case effectively demonstrates the system's resilience by continuing to update other markets even when an interpolation error occurs for specific markets. This behavior is essential for maintaining the integrity of market data in the face of partial failures.protocol/x/prices/keeper/update_price_test.go (3)
- 203-203: The addition of a new market price update for
MarketId3withPrice3in the test case "Multiple market price updates, some from smoothed price and some from index price" is a positive change. It expands the test coverage to include more market scenarios, ensuring that the system can handle updates for an additional market. This aligns with the PR's objective of enhancing market-specific functionality.- 234-234: Similarly, the inclusion of
MarketId3in the test case "Mix of valid and invalid index prices" ensures that the system's behavior is tested against a broader range of market conditions. This addition is crucial for verifying that the system can accurately process updates for new markets, which is essential for the protocol's adaptability and scalability.- 252-252: The update in the test case "Mix of valid, invalid, and invalid historical smoothed prices" to include
MarketId3further demonstrates the system's capability to handle a variety of market conditions. Testing the system's response to valid, invalid, and historically smoothed prices for an additional market ensures a comprehensive evaluation of the system's robustness and reliability.protocol/x/prices/keeper/validate_market_price_updates_test.go (4)
- 6-7: The addition of the import statement for
errorsmod "cosmossdk.io/errors"is appropriate for enhanced error handling in the test cases. This aligns with best practices for error handling in Go, especially when using Cosmos SDK.- 376-378: The addition of
constants.MarketId3toexpectedMarketIdsslices in test cases is appropriate and aligns with the PR objectives to support market-specific insurance funds. This ensures comprehensive testing of the new functionality.- 394-394: Including
constants.MarketId3in theexpectedMarketIdsslice for the test case "Non-empty proposed updates, Non-empty local updates, single missing market" is a good practice. It ensures that the test case accurately reflects the protocol's support for market-specific insurance funds.- 407-407: The update to
expectedMarketIdsto includeconstants.MarketId0, constants.MarketId2, constants.MarketId3in the test case "Non-empty proposed updates, Non-empty local updates, multiple missing markets, sorted" is well-justified. It ensures that the test coverage is comprehensive and aligns with the protocol's enhancements.
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- protocol/x/clob/keeper/deleveraging.go (3 hunks)
- protocol/x/subaccounts/keeper/transfer_test.go (8 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/subaccounts/keeper/transfer_test.go
Additional comments: 2
protocol/x/clob/keeper/deleveraging.go (2)
- 133-153: The modification to
GetInsuranceFundBalanceto include aperpetualId uint32parameter is a crucial update that aligns with the PR's objective of supporting market-specific insurance funds. This change allows for querying the balance of an insurance fund specific to a perpetual market, enhancing the flexibility and functionality of the protocol.
- Ensure that all calls to
GetInsuranceFundBalancethroughout the codebase have been updated to include theperpetualIdargument.- The error handling for
GetInsuranceFundModuleAddressnot finding an address (lines 141-144) correctly returnsnilif an error occurs, which is appropriate given the function's return type. However, consider logging the error or handling it more explicitly to aid in debugging and error tracing.
- 280-288: The update to
IsValidInsuranceFundDeltato include aperpetualId uint32parameter and the use ofGetInsuranceFundBalancewithin its logic are significant improvements that support the PR's objective of enabling market-specific insurance fund management.
- This change ensures that the validity of an insurance fund delta can be checked against the balance of a specific market's insurance fund, enhancing the protocol's flexibility and risk management capabilities.
- The logic correctly handles non-negative insurance fund deltas as valid and checks if the insurance fund balance remains non-negative after applying a negative delta, which is sound logic for insurance fund management.
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- protocol/testutil/constants/perpetuals.go (1 hunks)
- protocol/x/clob/keeper/deleveraging.go (3 hunks)
- protocol/x/clob/keeper/liquidations_test.go (6 hunks)
- protocol/x/clob/keeper/mev_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/testutil/constants/perpetuals.go
- protocol/x/clob/keeper/liquidations_test.go
Additional comments: 4
protocol/x/clob/keeper/deleveraging.go (2)
- 155-162: The addition of
GetCrossInsuranceFundBalanceis a significant update for fetching the balance of the cross-insurance fund. This function is straightforward and mirrors the structure ofGetInsuranceFundBalance. However, it directly usesperptypes.InsuranceFundModuleAddresswithout checking for errors or existence, which assumes that the address is always correctly set and available. Ensure that this assumption is valid and that there are no edge cases where the address might not be set or accessible, leading to potential issues.- 280-288: The update to
IsValidInsuranceFundDeltato include aperpetualIdparameter is crucial for validating insurance fund deltas in a market-specific context. This change aligns with the PR's objectives to handle insurance funds based on the market type. However, consider the following:
- The logic assumes that any non-negative insurance fund delta is valid. While this makes sense, ensure that there are no additional conditions or checks needed for specific market types or scenarios.
- The function uses
GetInsuranceFundBalanceto fetch the current insurance fund balance. Given the earlier comments onGetInsuranceFundBalance, ensure that any potential issues or improvements in that function are also considered here.protocol/x/clob/keeper/mev_test.go (2)
- 839-845: The
TestGetMidPricesfunction provides comprehensive coverage for calculating mid prices under various scenarios, effectively testing the functionality with different order book states.- 839-845: The
TestCumulativePnL_CalculateMevfunction effectively tests the MEV calculation under various scenarios, ensuring the functionality behaves as expected with different PnL states.
Comment on lines
+133
to
+153
| // GetInsuranceFundBalance returns the current balance of the specific insurance fund based on the | ||
| // perpetual (in quote quantums). | ||
| // This calls the Bank Keeper’s GetBalance() function for the Module Address of the insurance fund. | ||
| func (k Keeper) GetInsuranceFundBalance( | ||
| ctx sdk.Context, | ||
| ) ( | ||
| balance *big.Int, | ||
| ) { | ||
| func (k Keeper) GetInsuranceFundBalance(ctx sdk.Context, perpetualId uint32) (balance *big.Int) { | ||
| usdcAsset, exists := k.assetsKeeper.GetAsset(ctx, assettypes.AssetUsdc.Id) | ||
| if !exists { | ||
| panic("GetInsuranceFundBalance: Usdc asset not found in state") | ||
| } | ||
| insuranceFundAddr, err := k.perpetualsKeeper.GetInsuranceFundModuleAddress(ctx, perpetualId) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| insuranceFundBalance := k.bankKeeper.GetBalance( | ||
| ctx, | ||
| types2.InsuranceFundModuleAddress, | ||
| insuranceFundAddr, | ||
| usdcAsset.Denom, | ||
| ) | ||
|
|
||
| // Return as big.Int. | ||
| return insuranceFundBalance.Amount.BigInt() | ||
| } |
There was a problem hiding this comment.
The implementation of GetInsuranceFundBalance now includes a perpetualId parameter, allowing it to fetch the balance for a specific perpetual's insurance fund. This is a crucial update for supporting market-specific insurance funds. However, there are a few points to consider:
- The function now panics if the USDC asset is not found. Ensure that this behavior is consistent with the overall error handling strategy of the application. It might be more appropriate to return an error instead of panicking to allow the caller to handle the situation gracefully.
- The error handling for
GetInsuranceFundModuleAddressis to returnnilif an error occurs. This could lead to silent failures. Consider logging the error or handling it in a way that makes the failure more visible or actionable.
Comment on lines
+839
to
+845
| mockBankKeeper.On( | ||
| "SendCoins", | ||
| mock.Anything, | ||
| mock.Anything, | ||
| mock.Anything, | ||
| mock.Anything, | ||
| ).Return(nil) |
There was a problem hiding this comment.
The mock setup for "SendCoins" in TestRecordMevMetrics does not specify argument matchers for its parameters. This could lead to the mock accepting any arguments, potentially masking issues in the test where incorrect parameters are passed. It's recommended to use mock.AnythingOfType("string") or specific matchers to ensure the mock is called with the expected arguments.
- mockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
+ mockBankKeeper.On("SendCoins", mock.AnythingOfType("string"), mock.AnythingOfType("sdk.AccAddress"), mock.AnythingOfType("sdk.Coins")).Return(nil)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| mockBankKeeper.On( | |
| "SendCoins", | |
| mock.Anything, | |
| mock.Anything, | |
| mock.Anything, | |
| mock.Anything, | |
| ).Return(nil) | |
| mockBankKeeper.On( | |
| "SendCoins", | |
| mock.AnythingOfType("string"), | |
| mock.AnythingOfType("sdk.AccAddress"), | |
| mock.AnythingOfType("sdk.Coins"), | |
| ).Return(nil) |
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- protocol/x/clob/keeper/process_operations_liquidations_test.go (10 hunks)
- protocol/x/clob/keeper/process_operations_long_term_test.go (2 hunks)
- protocol/x/clob/keeper/process_operations_test.go (1 hunks)
Additional comments: 9
protocol/x/clob/keeper/process_operations_long_term_test.go (1)
- 581-584: The change from
SendCoinsFromModuleToModuletoSendCoinsand the replacement ofsatypes.ModuleNameandperptypes.InsuranceFundNamewithauthtypes.NewModuleAddress(satypes.ModuleName)andauthtypes.NewModuleAddress(perptypes.InsuranceFundName)respectively, aligns with the PR's objective to enhance insurance fund handling. This modification ensures that the correct insurance fund is dynamically selected based on the market context, improving the system's accuracy and flexibility.However, it's crucial to ensure that the
authtypes.NewModuleAddressfunction is correctly implemented and returns the expected module address for bothsatypes.ModuleNameandperptypes.InsuranceFundName. This is important for maintaining the integrity of transactions and ensuring that funds are correctly transferred between modules.protocol/x/clob/keeper/process_operations_liquidations_test.go (8)
- 203-206: The change from
SendCoinsFromModuleToModuletoSendCoinsand the use ofauthtypes.NewModuleAddressfor both sender and receiver addresses in theTestProcessProposerMatches_Liquidation_Successtest case is a significant modification. This change aligns with the PR's objective to use market-specific insurance funds. However, ensure that the new method and address generation logic are correctly implemented and that they accurately represent the intended behavior for handling insurance funds in liquidation scenarios. The correctness of these changes hinges on the proper functioning of theSendCoinsmethod and the generation of module addresses, which must be verified in the broader context of the system's architecture and the specific requirements of the insurance fund handling logic.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(10_000_000))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(250_000_000))).Return(nil).Once()
- 287-290: Similar to the previous comment, the change to use
SendCoinsandauthtypes.NewModuleAddressfor transferring funds between the insurance fund and the module account in a scenario where the insurance fund covers a loss is crucial. This adjustment is part of the PR's goal to enhance the handling of insurance funds. It's essential to ensure that the logic for determining the direction of the fund transfer (from the insurance fund to the module account in this case) and the amount (1_000_000in this scenario) is accurately implemented according to the new requirements. The correctness and impact of these changes should be assessed in the context of the overall system and the specific functionalities related to insurance fund management.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(10_100_000))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(perptypes.InsuranceFundName), authtypes.NewModuleAddress(satypes.ModuleName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(1_000_000))).Return(nil).Once()
- 367-370: The use of
SendCoinsfor transferring funds between module accounts in a scenario involving multiple partial fills during liquidation is noted. This change is part of the broader modifications to enhance insurance fund handling. It's important to verify that the logic for calculating the amount to be transferred (62_500_000in this case) accurately reflects the intended behavior, especially in scenarios involving partial fills. The correctness of these changes should be evaluated in the context of the system's requirements for handling partial liquidations and the management of insurance funds.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(2_500_000))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(62_500_000))).Return(nil).Twice()
- 471-474: This change, similar to the previous ones, involves using
SendCoinsfor fund transfers in a scenario with negative insurance fund delta. It's crucial to ensure that the logic for determining the direction and amount of the fund transfer is correctly implemented. The specific amount (250_000in this case) should accurately reflect the requirements for handling scenarios where the insurance fund covers losses. The correctness of these changes needs to be assessed in the broader context of insurance fund management and the system's overall architecture.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(2_525_000))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(perptypes.InsuranceFundName), authtypes.NewModuleAddress(satypes.ModuleName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(250_000))).Return(nil).Twice()
- 576-587: The logic for handling both positive and negative insurance fund deltas through
SendCoinstransactions is a critical part of the PR's objectives. It's essential to ensure that the logic for calculating the amounts and determining the direction of each transaction is accurately implemented. The specific amounts (750_000for payment to the insurance fund and250_000for coverage by the insurance fund) should reflect the intended behavior for scenarios involving both positive and negative deltas. The correctness and impact of these changes should be evaluated in the context of the system's requirements for insurance fund management.- bk.On("SendCoinsFromModuleToModule", mock.Anything, mock.Anything, authtypes.FeeCollectorName, mock.Anything).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(750_000))).Return(nil).Once() + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(perptypes.InsuranceFundName), authtypes.NewModuleAddress(satypes.ModuleName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(250_000))).Return(nil).Once()
- 680-692: The changes related to insurance fund payments in a scenario accounting for state changes from previous fills are significant. The use of
SendCoinsfor transactions and the specific amounts (378_735and121_265) need to accurately reflect the intended behavior, especially in scenarios where the maximum liquidation fee is capped. It's crucial to verify that these changes are correctly implemented and align with the system's requirements for managing insurance fund deltas and liquidation fees.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.Anything).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(378_735))).Return(nil).Once() + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(121_265))).Return(nil).Once()
- 787-790: The logic for transferring funds between module accounts in a scenario involving state setup for liquidation tests is noted. The use of
SendCoinsand the specific amount (125_000_000) for the insurance fund payment should be carefully reviewed to ensure they accurately represent the intended behavior for handling liquidations. It's important to assess the correctness of these changes in the context of the system's overall architecture and the specific requirements for managing insurance funds during liquidation processes.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(5_000_000))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(125_000_000))).Return(nil).Once()
- 900-903: The logic for handling small position sizes in liquidation scenarios through
SendCoinstransactions is critical. The specific amount (25) for the insurance fund payment needs to accurately reflect the intended behavior, especially in scenarios involving positions smaller than theStepBaseQuantums. It's essential to verify that these changes are correctly implemented and align with the system's requirements for managing insurance funds and liquidation processes in such specific scenarios.- bk.On("SendCoinsFromModuleToModule", mock.Anything, satypes.ModuleName, authtypes.FeeCollectorName, mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(1))).Return(nil) + bk.On("SendCoins", mock.Anything, authtypes.NewModuleAddress(satypes.ModuleName), authtypes.NewModuleAddress(perptypes.InsuranceFundName), mock.MatchedBy(testutil_bank.MatchUsdcOfAmount(25))).Return(nil)
vincentwschau
approved these changes
Mar 5, 2024
Eric-Warehime
pushed a commit
to skip-mev/v4-chain
that referenced
this pull request
Mar 12, 2024
…kets (dydxprotocol#1132) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com>
Eric-Warehime
added a commit
to skip-mev/v4-chain
that referenced
this pull request
Mar 12, 2024
commit d98f859 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 11 12:46:53 2024 -0700 Update sample pregenesis Signed-off-by: Eric <eric.warehime@gmail.com> commit 7f178fe Author: Mohammed Affan <affanmd@nyu.edu> Date: Mon Mar 11 13:46:08 2024 -0400 [OTE-209] Emit metrics gated through execution mode (dydxprotocol#1157) Signed-off-by: Eric <eric.warehime@gmail.com> commit 47e365d Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 11 13:43:16 2024 -0400 add aggregate comlink response code stats (dydxprotocol#1162) Signed-off-by: Eric <eric.warehime@gmail.com> commit 7774ad9 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Fri Mar 8 17:30:49 2024 -0500 [TRA-70] Add state migrations for isolated markets (dydxprotocol#1155) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit 89c7405 Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Thu Mar 7 17:28:06 2024 -0500 [CT-517] E2E tests batch cancel (dydxprotocol#1149) * more end to end test * extraprint * more e2e test Signed-off-by: Eric <eric.warehime@gmail.com> commit 41a3a41 Author: Teddy Ding <teddy@dydx.exchange> Date: Thu Mar 7 15:42:30 2024 -0500 [OTE-200] OIMF protos (dydxprotocol#1125) * OIMF protos * add default genesis value, modify methods interface * fix genesis file * fix integration test * lint Signed-off-by: Eric <eric.warehime@gmail.com> commit 2a062b1 Author: Teddy Ding <teddy@dydx.exchange> Date: Thu Mar 7 15:24:15 2024 -0500 Add `v5` upgrade handler and set up container upgrade test (dydxprotocol#1153) * wip * update preupgrade_genesis * fix preupgrade_genesis.json * nit * setupUpgradeStoreLoaders for v5.0.0 Signed-off-by: Eric <eric.warehime@gmail.com> commit b7942b3 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Thu Mar 7 13:43:48 2024 -0500 [CT-647] construct the initial orderbook snapshot (dydxprotocol#1147) * [CT-647] construct the initial orderbook snapshot * [CT-647] initialize new streams and send orderbook snapshot (dydxprotocol#1152) * [CT-647] initialize new streams and send orderbook snapshot * use sync once * comments Signed-off-by: Eric <eric.warehime@gmail.com> commit c67a3c6 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Thu Mar 7 12:40:37 2024 -0500 [TRA-84] Move SA module address transfers to use perpetual based SA accounts (dydxprotocol#1146) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange Signed-off-by: Eric <eric.warehime@gmail.com> commit dba23e0 Author: Mohammed Affan <affanmd@nyu.edu> Date: Thu Mar 7 10:34:11 2024 -0500 update readme link to point to the right page (dydxprotocol#1151) Signed-off-by: Eric <eric.warehime@gmail.com> commit b5870d5 Author: Tian <tian@dydx.exchange> Date: Wed Mar 6 16:43:04 2024 -0500 [TRA-86] scaffold x/vault (dydxprotocol#1148) * scaffold x/vault Signed-off-by: Eric <eric.warehime@gmail.com> commit 0eca041 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Wed Mar 6 10:48:42 2024 -0500 [CT-652] add command line flag for full node streaming (dydxprotocol#1145) Signed-off-by: Eric <eric.warehime@gmail.com> commit b319cb8 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Tue Mar 5 21:58:35 2024 -0500 [CT-646] stream offchain updates through stream manager (dydxprotocol#1138) * [CT-646] stream offchain updates through stream manager * comments * fix lint * get rid of finished * comments * comments Signed-off-by: Eric <eric.warehime@gmail.com> commit 1c54620 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Tue Mar 5 16:34:19 2024 -0500 [TRA-78] Add function to retrieve collateral pool addr for a subaccount (dydxprotocol#1142) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit b8c1d62 Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Tue Mar 5 15:03:28 2024 -0500 [OTE-141] implement post /compliance/geoblock (dydxprotocol#1129) Signed-off-by: Eric <eric.warehime@gmail.com> commit ab8c570 Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Tue Mar 5 11:19:53 2024 -0500 Fix mock-gen dydxprotocol#1140 Signed-off-by: Eric <eric.warehime@gmail.com> commit 12506a1 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Mon Mar 4 21:33:28 2024 -0500 [TRA-64] Use market specific insurance fund for cross or isolated markets (dydxprotocol#1132) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit 929f09e Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Mon Mar 4 13:48:04 2024 -0800 [CT-514] Clob `MsgBatchCancel` functionality (dydxprotocol#1110) * wip implementation * use new cometbft * Revert "use new cometbft" This reverts commit e5b8a03. * go mod tidy * basic e2e test * more msgBatchCancels in code * repeated fixed32 -> uint32 * remove debug prints * update cometbft replace go.mod sha * one more debug print * typo * regen indexer protos * update comment on proto * proto comment changes * extract stateful validation into own fn * pr format comments * clean up test file * new return type with success and failure Signed-off-by: Eric <eric.warehime@gmail.com> commit 41de83e Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 4 12:22:16 2024 -0500 add index to address read replica lag (dydxprotocol#1137) Signed-off-by: Eric <eric.warehime@gmail.com> commit 735d9a8 Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 4 11:56:59 2024 -0500 rename (dydxprotocol#1136) Signed-off-by: Eric <eric.warehime@gmail.com> commit 86617dd Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Mon Mar 4 10:43:31 2024 -0500 [CT-644] instantiate grpc stream manager (dydxprotocol#1134) * [CT-644] instantiate grpc stream manager * update type * update channel type Signed-off-by: Eric <eric.warehime@gmail.com> commit 32afd64 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 11 12:41:06 2024 -0700 Update go version in Dockerfile Signed-off-by: Eric <eric.warehime@gmail.com> commit ba27204 Author: Eric <eric.warehime@gmail.com> Date: Fri Mar 8 09:44:04 2024 -0800 Add slinky utils, use that to convert between market and currency pair commit 667a804 Author: Eric <eric.warehime@gmail.com> Date: Wed Mar 6 20:43:40 2024 -0800 Update error messages commit d53292c Author: Eric <eric.warehime@gmail.com> Date: Wed Mar 6 20:16:01 2024 -0800 Update docstrings, rename OracleClient commit daad125 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 4 10:51:23 2024 -0800 VoteExtension slinky logic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelist
Add APIs to find which insurance fund to use based on the perpetual traded
Modify usages of Insurance Fund to go through this API
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.