[CP-447] refactor support for exchange v2 #315
Merged
Conversation
This file contains hidden or 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
…2 markets to allow users to load either human readable or chain format markets
…v1.16.16. Updated queries with requests or response changes
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 87 files out of 297 files are above the max files limit of 200. You can disable this status message by setting the ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors market support to enable both Exchange V1 and V2 usage by introducing a new ChainClientV2, updating the MarketsAssistant to handle human-readable V2 markets, and bumping proto and SDK dependencies to v1.16+.
- Added
ChainClientV2and human-readable markets assistant constructors - Updated MarketsAssistant internals and tests to initialize and query V2 markets
- Bumped proto definitions and SDK clone targets to core v1.16.0-beta.3 and indexer v1.16.16
Reviewed Changes
Copilot reviewed 356 out of 356 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| client/chain/markets_assistant.go | Refactored to support V1/V2 initialization methods |
| client/chain/markets_assistant_test.go | Expanded tests for both V1 and human-readable V2 |
| chain/exchange/types/v2/params.go | Updated default V2 parameter MaxDerivativeOrderSideCount |
| chain/types/chain_id.go | Added validation and parsing for chain ID formats |
| README.md | Documented selection between Exchange V1 and V2 |
Comments suppressed due to low confidence (3)
client/chain/markets_assistant.go:28
- Storing binary option markets as
core.DerivativeMarketmay confuse consumers; change this to the appropriatecore.BinaryOptionMarketV2type (and update related methods) to match the domain model.
binaryOptionMarkets map[string]core.DerivativeMarket
client/chain/markets_assistant_test.go:137
- The test uses strings.Split but the import for "strings" is missing; add
import "strings"to the test file.
symbols := strings.Split(injUsdtSpotMarketInfo.Ticker, "/")
auth_vote/authz_vote.go:102
- [nitpick] Using a tab for indentation in
json.MarshalIndentmay conflict with project styling; consider using a consistent space-based indent (e.g., two or four spaces).
str, _ := json.MarshalIndent(response, "", "\t")
| ) | ||
|
|
||
| var ( | ||
| regexChainID = `[a-z]*` |
There was a problem hiding this comment.
Current regex allows an empty prefix before the dash; change [a-z]* to [a-z]+ to require at least one letter in the chain name.
Suggested change
| regexChainID = `[a-z]*` | |
| regexChainID = `[a-z]+` |
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.
Solves CP-447