-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat/add_exchange_messages_from_v1_13 #232
Conversation
…d in the chain upgrade v1.13. Added unit tests and example scripts
WalkthroughThe recent updates enhance the trading functionalities by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant Blockchain
User->>Application: Initiate market update
Application->>Blockchain: Load configuration
Blockchain-->>Application: Configuration loaded
Application->>Blockchain: Construct MsgUpdateSpotMarket
Application->>Blockchain: Broadcast update
Blockchain-->>Application: Confirmation of update
Application-->>User: Display success message
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 Configuration 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.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (14)
- .github/workflows/run-tests.yml (1 hunks)
- client/chain/markets_assistant.go (7 hunks)
- client/chain/markets_assistant_test_support.go (3 hunks)
- client/core/market.go (9 hunks)
- client/core/market_test.go (10 hunks)
- client/metadata/assets/devnet-1.ini (33 hunks)
- client/metadata/assets/devnet.ini (33 hunks)
- client/metadata/assets/testnet.ini (62 hunks)
- examples/chain/exchange/23_MsgDecreasePositionMargin/example.go (1 hunks)
- examples/chain/exchange/24_MsgUpdateSpotMarket/example.go (1 hunks)
- examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go (1 hunks)
- examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go (1 hunks)
- examples/chain/exchange/27_MsgActivateStakeGrant/example.go (1 hunks)
- examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go (2 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/run-tests.yml
Additional comments not posted (105)
examples/chain/exchange/27_MsgActivateStakeGrant/example.go (4)
1-15
: LGTM!The package declaration and imports are appropriate for the functionality provided.
17-36
: LGTM!The network, Tendermint client, and Cosmos keyring initialization are correct and handle errors appropriately.
38-46
: LGTM!The client context initialization is correct and handles errors appropriately.
50-58
: LGTM!The chain client initialization is correct and handles errors appropriately.
examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go (4)
1-17
: LGTM!The package declaration and imports are appropriate for the functionality provided.
19-38
: LGTM!The network, Tendermint client, and Cosmos keyring initialization are correct and handle errors appropriately.
40-48
: LGTM!The client context initialization is correct and handles errors appropriately.
50-58
: LGTM!The chain client initialization is correct and handles errors appropriately.
examples/chain/exchange/23_MsgDecreasePositionMargin/example.go (4)
1-17
: LGTM!The package declaration and imports are appropriate for the functionality provided.
19-38
: LGTM!The network, Tendermint client, and Cosmos keyring initialization are correct and handle errors appropriately.
40-48
: LGTM!The client context initialization is correct and handles errors appropriately.
50-58
: LGTM!The chain client initialization is correct and handles errors appropriately.
examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go (3)
77-77
: Initialization ofminNotional
looks good.The
minNotional
variable is correctly initialized usingmath.LegacyMustNewDecFromStr("1")
.
83-83
: Computation ofchainMinNotional
looks good.The
chainMinNotional
variable is correctly computed by multiplyingminNotional
with the decimal representation of thequoteToken
's decimals.
92-92
: Addition ofMinNotional
to theMsgInstantSpotMarketLaunch
struct looks good.The
MinNotional
field is correctly added to theMsgInstantSpotMarketLaunch
struct.examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go (1)
1-102
: New example file for updating a derivative market looks good.The file correctly initializes
minNotional
and uses it in theMsgUpdateDerivativeMarket
struct. The overall structure and logic are consistent with other examples.examples/chain/exchange/24_MsgUpdateSpotMarket/example.go (1)
1-104
: New example file for updating a spot market looks good.The file correctly initializes
minNotional
and uses it in theMsgUpdateSpotMarket
struct. The overall structure and logic are consistent with other examples.client/chain/markets_assistant_test_support.go (3)
91-91
: LGTM! Verify consistency ofMinNotional
usage.The addition of the
MinNotional
field with a value of "1000000" looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.Verification successful
Verified: Consistent usage of
MinNotional
field across the codebase.The
MinNotional
field is used consistently across various files and contexts, including protobuf definitions, Go structs, and functions. No discrepancies or inconsistencies were found in its usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `MinNotional` usage across the codebase. # Test: Search for the `MinNotional` field usage. Expect: Consistent usage of the `MinNotional` field. rg --type go 'MinNotional'Length of output: 17899
69-69
: LGTM! Verify consistency ofMinNotional
usage.The addition of the
MinNotional
field with a value of "1000000" looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.Verification successful
Verified: Consistent usage of
MinNotional
field.The addition of the
MinNotional
field with a value of "1000000" inclient/chain/markets_assistant_test_support.go
is consistent with its usage across the codebase. The field is used appropriately in various market-related contexts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `MinNotional` usage across the codebase. # Test: Search for the `MinNotional` field usage. Expect: Consistent usage of the `MinNotional` field. rg --type go 'MinNotional'Length of output: 17899
133-133
: LGTM! Verify consistency ofMinNotional
usage.The addition of the
MinNotional
field with a value of "1000000" looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.Verification successful
LGTM! Verify consistency of
MinNotional
usage.The addition of the
MinNotional
field with a value of "1000000" looks good. The usage ofMinNotional
is consistent across the codebase and aligns with the intended functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `MinNotional` usage across the codebase. # Test: Search for the `MinNotional` field usage. Expect: Consistent usage of the `MinNotional` field. rg --type go 'MinNotional'Length of output: 17899
client/core/market.go (9)
42-48
: LGTM!The implementation of the
NotionalToChainFormat
function forSpotMarket
looks correct and aligns with the intended functionality.
59-62
: LGTM!The implementation of the
NotionalFromChainFormat
function forSpotMarket
looks correct and aligns with the intended functionality.
72-74
: LGTM!The implementation of the
NotionalFromExtendedChainFormat
function forSpotMarket
looks correct and aligns with the intended functionality.
129-135
: LGTM!The implementation of the
NotionalToChainFormat
function forDerivativeMarket
looks correct and aligns with the intended functionality.
147-148
: LGTM!The refactoring of the
MarginFromChainFormat
function forDerivativeMarket
to use theNotionalFromChainFormat
method looks correct and aligns with the intended functionality.
150-152
: LGTM!The implementation of the
NotionalFromChainFormat
function forDerivativeMarket
looks correct and aligns with the intended functionality.
167-169
: LGTM!The implementation of the
NotionalFromExtendedChainFormat
function forDerivativeMarket
looks correct and aligns with the intended functionality.
22-22
: LGTM! Verify consistency ofMinNotional
usage.The addition of the
MinNotional
field to theSpotMarket
struct looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.Verification successful
LGTM! Verify consistency of
MinNotional
usage.The addition of the
MinNotional
field to theSpotMarket
struct looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `MinNotional` usage across the codebase. # Test: Search for the `MinNotional` field usage. Expect: Consistent usage of the `MinNotional` field. rg --type go 'MinNotional'Length of output: 17899
92-92
: LGTM! Verify consistency ofMinNotional
usage.The addition of the
MinNotional
field to theDerivativeMarket
struct looks good. Ensure that this change is consistent across the codebase and aligns with the intended functionality.client/chain/markets_assistant.go (2)
76-79
: LGTM!The implementation of the
minNotional
field handling in theNewMarketsAssistant
function looks correct and aligns with the intended functionality.Also applies to: 104-104, 125-125
199-199
: LGTM!The implementation of the
minNotional
field handling in theNewMarketsAssistantInitializedFromChain
function looks correct and aligns with the intended functionality.Also applies to: 212-212, 243-243, 261-261
client/core/market_test.go (4)
21-21
: LGTM!The
MinNotional
field has been correctly initialized in theSpotMarket
structure.Also applies to: 34-34
49-49
: LGTM!The
MinNotional
field has been correctly initialized in theDerivativeMarket
structure.Also applies to: 67-67
99-109
: LGTM!The test functions correctly validate the conversion of notional values to and from chain format for
SpotMarket
.Also applies to: 132-141
229-239
: LGTM!The test functions correctly validate the conversion of notional values to and from chain format for
DerivativeMarket
.Also applies to: 273-282
client/metadata/assets/devnet-1.ini (19)
9-9
: Verify themin_notional
value.The
min_notional
value is set to0
. Verify if this is the expected value.
19-19
: LGTM!The
min_notional
value is appropriately set to1000000
.
29-29
: LGTM!The
min_notional
value is appropriately set to1000000
.
39-39
: LGTM!The
min_notional
value is appropriately set to1000000
.
49-49
: LGTM!The
min_notional
value is appropriately set to1000000
.
59-59
: LGTM!The
min_notional
value is appropriately set to1000000
.
69-69
: LGTM!The
min_notional
value is appropriately set to1000000
.
79-79
: LGTM!The
min_notional
value is appropriately set to1000000
.
89-89
: LGTM!The
min_notional
value is appropriately set to1000000
.
99-99
: LGTM!The
min_notional
value is appropriately set to1000000
.
109-109
: LGTM!The
min_notional
value is appropriately set to1000000
.
119-119
: LGTM!The
min_notional
value is appropriately set to1000000
.
129-129
: LGTM!The
min_notional
value is appropriately set to1000000
.
139-139
: LGTM!The
min_notional
value is appropriately set to1000000
.
149-149
: LGTM!The
min_notional
value is appropriately set to1000000
.
159-159
: LGTM!The
min_notional
value is appropriately set to1000000
.
169-169
: LGTM!The
min_notional
value is appropriately set to1000000
.
179-179
: LGTM!The
min_notional
value is appropriately set to10000000000000000
.
189-189
: LGTM!The
min_notional
value is appropriately set to10000000000000000
.client/metadata/assets/devnet.ini (20)
9-9
: Verify the correctness ofmin_notional
value.The
min_notional
value is set to0
. Ensure this is intentional and correct for the given trading pair.
19-19
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
29-29
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
39-39
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
49-49
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
59-59
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
69-69
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
79-79
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
89-89
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
99-99
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
109-109
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
119-119
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
129-129
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
139-139
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
149-149
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
159-159
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
169-169
: LGTM!The
min_notional
value is set to1000000
, which seems appropriate.
179-179
: Verify the correctness ofmin_notional
value.The
min_notional
value is set to10000000000000000
, which is significantly higher than other entries. Ensure this is intentional and correct for the given trading pair.
189-189
: Verify the correctness ofmin_notional
value.The
min_notional
value is set to10000000000000000
, which is significantly higher than other entries. Ensure this is intentional and correct for the given trading pair.
199-199
: Verify the correctness ofmin_notional
value.The
min_notional
value is set to10000000000000000
, which is significantly higher than other entries. Ensure this is intentional and correct for the given trading pair.client/metadata/assets/testnet.ini (31)
9-9
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
19-19
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
29-29
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
39-39
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
49-49
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
59-59
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
69-69
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
79-79
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
89-89
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
99-99
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
109-109
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
119-119
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
129-129
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
139-139
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
149-149
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
159-159
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
169-169
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
179-179
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
189-189
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
199-199
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
209-209
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
219-219
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
229-229
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
239-239
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
249-249
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
259-259
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
269-269
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
279-279
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
289-289
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
299-299
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
309-309
: LGTM!The addition of
min_notional = 0
is consistent with the PR objectives.
msg := &exchangetypes.MsgActivateStakeGrant{ | ||
Sender: senderAddress.String(), | ||
Granter: "inj1hkhdaj2a2clmq5jq6mspsggqs32vynpk228q3r", | ||
} | ||
|
||
// AsyncBroadcastMsg, SyncBroadcastMsg, QueueBroadcastMsg | ||
response, err := chainClient.AsyncBroadcastMsg(msg) | ||
|
||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
str, _ := json.MarshalIndent(response, "", " ") | ||
fmt.Print(string(str)) | ||
} |
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.
Improve error handling in message broadcasting.
Instead of using panic
, consider logging the error to provide more context and avoid crashing the application.
if err != nil {
fmt.Fprintf(os.Stderr, "Error broadcasting message: %v\n", err)
return
}
grantAuthorization := &exchangetypes.GrantAuthorization{ | ||
Grantee: "inj1hkhdaj2a2clmq5jq6mspsggqs32vynpk228q3r", | ||
Amount: math.NewIntWithDecimal(1, 18), | ||
} | ||
|
||
msg := &exchangetypes.MsgAuthorizeStakeGrants{ | ||
Sender: senderAddress.String(), | ||
Grants: []*exchangetypes.GrantAuthorization{grantAuthorization}, | ||
} | ||
|
||
// AsyncBroadcastMsg, SyncBroadcastMsg, QueueBroadcastMsg | ||
response, err := chainClient.AsyncBroadcastMsg(msg) | ||
|
||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
str, _ := json.MarshalIndent(response, "", " ") | ||
fmt.Print(string(str)) |
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.
Improve error handling in message broadcasting.
Instead of using panic
, consider logging the error to provide more context and avoid crashing the application.
if err != nil {
fmt.Fprintf(os.Stderr, "Error broadcasting message: %v\n", err)
return
}
defaultSubaccountID := chainClient.DefaultSubaccount(senderAddress) | ||
|
||
msg := &exchangetypes.MsgDecreasePositionMargin{ | ||
Sender: senderAddress.String(), | ||
MarketId: "0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | ||
SourceSubaccountId: defaultSubaccountID.String(), | ||
DestinationSubaccountId: defaultSubaccountID.String(), | ||
Amount: math.LegacyMustNewDecFromStr("100000000"), //100 USDT | ||
} | ||
|
||
// AsyncBroadcastMsg, SyncBroadcastMsg, QueueBroadcastMsg | ||
response, err := chainClient.AsyncBroadcastMsg(msg) | ||
|
||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
str, _ := json.MarshalIndent(response, "", " ") | ||
fmt.Print(string(str)) |
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.
Improve error handling in message broadcasting.
Instead of using panic
, consider logging the error to provide more context and avoid crashing the application.
if err != nil {
fmt.Fprintf(os.Stderr, "Error broadcasting message: %v\n", err)
return
}
min_notional
field to all market classes.min_notional
as part of the markets and tokens initialization in AsyncClientSolves CHAIN-171
Summary by CodeRabbit
New Features
min_notional
parameter across various market configurations to enforce minimum trade sizes, enhancing liquidity management.Bug Fixes
Documentation