Feat/sync with master v1.56.2 #298
Conversation
…master fix/EIP712_wrapper_v1_for_master
… feat/release_v1_56
…e pre-commit validation workflow error
Feat/release v1.56
…always in lowercase
…always in lowercase
…ple_script fix/fix_stream_prices_example_script
fix/add_notional_quantization
* fix: fixed the manual TX generation and signing to receive the gas price * fix: solve issues in pre-commit workflow caused by using deprecated components * fix: solve issues in pre-commit workflow caused by using deprecated components
… feat/sync_with_master_v1_56_2
WalkthroughThis pull request updates several GitHub Actions workflows, linter configurations, and build targets while refactoring parts of the core client and example code. Changes include modifications to cache and lint settings, upgrading action versions, and standardizing type usage in chain transaction functions. In addition, file handling in examples has been updated and import statements reordered for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChainClient
participant BlockchainNode
User->>ChainClient: Request transaction build (with messages, gasPrice)
ChainClient->>ChainClient: BuildSignedTx (using sdkclient.Context)
ChainClient->>ChainClient: Validate & prepare transaction
User->>ChainClient: Request broadcast (with broadcastMode)
ChainClient->>BlockchainNode: BroadcastSignedTx(txBytes, broadcastMode)
BlockchainNode-->>ChainClient: Return transaction response
ChainClient-->>User: Deliver BroadcastTxResponse
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/pre-commit.yml (1)
11-11: Caution: Using a branch reference for checkout action
The update from a fixed version (previously v3) to usingactions/checkout@mastermeans the workflow will always pull the latest commit from the master branch of the action. This can lead to unanticipated changes if the master branch is updated with breaking changes. Please verify that using a branch reference instead of a stable version tag aligns with your reproducibility and stability requirements..golangci.yml (3)
31-35: New Revive Rules Added
New rules for revive have been introduced:
- add-constant is now explicitly disabled.
- line-length-limit is enforced with an argument of 140.
Please verify that these new rules (and their configurations) reflect your intended code style guidelines. It might also be helpful to add inline comments or documentation if these rules are not self-explanatory to all team members.
39-42: Expanded gocritic Enabled Tags
Additional tags have been added undergocritic.enabled-tags:diagnostic,opinionated, andexperimental. While these options can provide a deeper analysis, they might also increase noise from lint warnings. Please confirm that the inclusion of these tags is intentional and that the team is prepared to handle any additional feedback generated by these checks.
48-59: New Issues Section for Linting
A newissuessection has been added with exclusions and limits:
- Excludes revive issues that mention
ALL_CAPSunder specified linters.- Excludes directories such as
chain,exchange,injective_data,proto, andclient/keyring/testdata.- Sets
max-issues-per-linterto 0, which typically means no limit.
Please double-check that these exclusions and the configuration align with your team's strategy for managing lint issues, ensuring that important warnings are not inadvertently suppressed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/golangci-lint.yml(1 hunks).github/workflows/pre-commit.yml(1 hunks).golangci.yml(1 hunks)Makefile(1 hunks)client/chain/chain.go(13 hunks)client/chain/chain_test_support.go(2 hunks)client/core/market.go(3 hunks)examples/chain/8_OfflineSigning/example.go(5 hunks)examples/exchange/oracle/1_StreamPrices/example.go(2 hunks)typeddata/typed_data.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
client/chain/chain_test_support.go (1)
exchange/exchange_rpc/pb/goadesign_goagen_injective_exchange_rpc.pb.go (3)
BroadcastTxResponse(854-876)BroadcastTxResponse(891-891)BroadcastTxResponse(906-908)
🔇 Additional comments (41)
examples/exchange/oracle/1_StreamPrices/example.go (2)
8-8: Logical addition of the strings package.The strings package is now required for the new functionality using strings.ToLower() on line 78.
78-78: Good improvement to normalize Oracle type to lowercase.Converting the Oracle type to lowercase ensures consistent case formatting when making the API call to
StreamPrices. This change improves robustness by standardizing the input format regardless of how the Oracle type is provided in the market data.typeddata/typed_data.go (1)
18-22: Reorganization of import statements.This change is purely organizational, moving several Cosmos SDK imports from the end of the import block to the beginning. This sort of reordering improves readability and likely aligns with the project's code style guidelines.
Makefile (4)
154-157: Good addition of a standard lint target.This new target sets the GOPROXY environment variable to 'direct' and runs the linter against the dev branch, helping to maintain code quality. The '--new-from-rev=dev' flag specifically focuses on changes made since branching from dev, which keeps the linting process targeted and efficient.
158-161: Useful addition of a last-commit linting target.This target allows developers to quickly validate their most recent commit without checking the entire codebase, providing a fast feedback loop for code quality checks.
162-165: Important addition of master branch linting comparison.This target is particularly valuable for pull requests like this one that sync with the master branch, as it specifically identifies linting issues introduced since the master branch.
166-166: Updated .PHONY declaration to include new targets.Properly marking these targets as .PHONY ensures that Make will run the commands even if files with the same names exist, which is good practice for task-oriented targets like these.
.github/workflows/golangci-lint.yml (1)
34-34: Improved CI configuration to focus on new issues.Changing
only-new-issuestotrueconfigures the GitHub Actions workflow to report only linting issues introduced in the current pull request, rather than all pre-existing issues. This focuses code review attention on recent changes and prevents overwhelming developers with legacy issues that weren't part of their modifications..github/workflows/pre-commit.yml (1)
18-18: Update Pre-commit Action Version
The action has been upgraded topre-commit/action@v3.0.1, which should provide improved features and fixes over the previous version. This change appears beneficial; please ensure that any breaking changes introduced in v3.0.1 have been reviewed..golangci.yml (3)
26-26: Revive Configuration Key Update
The configuration key has been updated toenable-all-rules: true(line 26). This change appears to be in line with the new settings for revive. Confirm that this modification is compatible with your current tooling and that all team members are aware of the new configuration syntax.
44-45: Updated gocritic Disabled Checks
DisablingsingleCaseSwitchanddeferInLoopcan help reduce false positives or less useful warnings. Verify that these checks are intentionally disabled and that their exclusion does not hide real issues in your codebase.
61-62: Lint Run Configuration Update
The newrunsection now specifiestests: false, which will bypass linting of test files during analysis. Confirm that skipping tests is the desired behavior for your linting process and that this change will not allow problematic test code to be overlooked.client/core/market.go (3)
45-46: Consistent change to use Ceil() for notional value calculationsThe change ensures the notional value is rounded up to the next integer when converting to chain format, which guarantees sufficient funds are available for operations. This is a prudent modification that prevents potential transaction failures due to insufficient funds.
133-134: Consistent rounding approach for derivative market notional valuesThe modification to use
Ceil()aligns with the changes in other market types, ensuring consistent handling of notional values across different market categories. This harmonized approach prevents potential discrepancies in transaction processing.
230-231: Appropriate ceiling rounding for binary option market notional valuesThe implementation now correctly rounds up notional values using
Ceil(), which matches the approach used in other market types. This consistency is important for maintaining predictable behavior across different market structures.examples/chain/8_OfflineSigning/example.go (8)
6-7: Added JSON encoding support for improved response handlingThe addition of the JSON encoding import supports the improved response formatting implemented later in the file.
21-22: Modernized file handling with os.WriteFileReplaced deprecated ioutil function with the recommended os.WriteFile, which simplifies the code and follows current best practices.
25-26: Simplified file reading with os.ReadFileUpdated to use the more concise os.ReadFile function instead of manual file opening and reading, reducing code complexity.
29-29: Updated network configuration to testnetChanged from "devnet" to "testnet" with "lb" parameter, which appears to be part of aligning the example with current network configurations.
78-78: Updated marketId to current valid valueThe market ID has been updated, likely to reference a currently valid market on the testnet environment.
99-99: Added dynamic gas price parameter for transaction buildingNow using client.DefaultGasPrice instead of hardcoded values, which provides more flexibility and follows current API requirements for the BuildSignedTx method.
110-112: Enhanced transaction broadcasting with flexible broadcast modeUpdated to use BroadcastSignedTx with explicit broadcast mode parameter (BROADCAST_MODE_SYNC), which provides more control over how transactions are processed by the network.
116-120: Improved response handling with structured outputAdded detailed transaction response output including hash and code, with proper JSON formatting for better readability. This makes the example more useful for debugging and understanding transaction results.
client/chain/chain_test_support.go (2)
80-80: Updated BuildSignedTx signature with gasPrice parameterAdded gasPrice parameter to the BuildSignedTx method signature to match the changes in the actual implementation, ensuring the mock client remains compatible with code that uses the updated API.
92-94: Added BroadcastSignedTx method to support new transaction broadcasting approachImplemented the new BroadcastSignedTx method in the mock client, which supports the flexible broadcast mode parameter and aligns with updates to the broadcasting API in the real client implementation.
client/chain/chain.go (16)
20-46: Import statements updated for consistency and improved organization.The updated imports maintain consistent naming conventions and better organization by:
- Using
sdkclientalias for the Cosmos SDK client package- Adding insecure GRPC credentials import
- Organizing InjectiveLabs imports appropriately
These changes align with the architectural changes throughout the file.
74-75: Interface method updated to use standardized SDK client types.The return type for
ClientContext()has been changed fromclient.Contexttosdkclient.Contextto standardize on Cosmos SDK client types across the codebase.
78-79: Interface method parameter type updated for consistency.Parameter type for
SimulateMsghas been changed fromclient.Contexttosdkclient.Contextfor consistency with other method signatures that use the Cosmos SDK client types.
88-89: New interface method added for flexible transaction broadcasting.A new
BroadcastSignedTxmethod has been added to support different broadcast modes, allowing more flexibility in how transactions are broadcast to the network.
313-314: Updated struct field type for consistency with interface.The
ctxfield in thechainClientstruct has been updated fromclient.Contexttosdkclient.Contextto match the interface changes and maintain type consistency throughout the codebase.
359-360: Constructor parameter type updated to match struct field type.The
NewChainClientfunction now acceptssdkclient.Contextinstead ofclient.Contextto match the updated struct field type, maintaining type consistency throughout the codebase.
558-560: Implementation return type updated to match interface.The return type of
ClientContext()implementation has been updated fromclient.Contexttosdkclient.Contextto match the interface changes and maintain type consistency.
703-728: Updated method signature and implementation for SimulateMsg.The parameter type for
SimulateMsghas been changed fromclient.Contexttosdkclient.Contextwhile maintaining the same functionality. This change ensures type consistency across the codebase.
730-738: BuildSignedTx implementation updated with gas price handling.The method has been enhanced to:
- Accept the new
gasPriceparameter- Create a gas price string with the appropriate denomination
- Set the gas price on the transaction factory
This provides more explicit control over transaction fees.
740-782: Updated buildSignedTx with sdkclient parameter type.The parameter type has been updated from
client.Contexttosdkclient.Contextwhile preserving the same functionality, ensuring type consistency across the codebase.
784-822: Refactored SyncBroadcastSignedTx to use the new BroadcastSignedTx method.The implementation now delegates to the new
BroadcastSignedTxmethod withBROADCAST_MODE_SYNCmode, reducing code duplication and improving maintainability.
805-807: Updated error handling to use sdkclient.CheckCometError.Error handling has been updated to use
sdkclient.CheckCometErrorinstead ofclient.CheckCometErrorto align with the updated import conventions.
824-841: Implemented AsyncBroadcastSignedTx to use the new BroadcastSignedTx method.The implementation now delegates to the new
BroadcastSignedTxmethod withBROADCAST_MODE_ASYNCmode, reducing code duplication and improving code maintainability.
828-841: Added new BroadcastSignedTx method for flexible transaction broadcasting.This new method centralizes the broadcasting logic and supports different broadcast modes through the
broadcastModeparameter. This abstraction reduces code duplication in theSyncBroadcastSignedTxandAsyncBroadcastSignedTxmethods.
690-692: Updated error handling in SyncBroadcastMsg.Error handling has been updated to use
sdkclient.CheckCometErrorinstead ofclient.CheckCometErrorto align with the updated import conventions.
85-86:✅ Verification successful
Interface method signature extended with gas price parameter.
The
BuildSignedTxmethod now includes agasPriceparameter which allows more explicit control over transaction fees. This is a breaking change that requires updates in dependent code.Please verify that all callers of this method have been updated to include the new gas price parameter.
🏁 Script executed:
#!/bin/bash # Search for calls to BuildSignedTx to ensure they all include the new gasPrice parameter rg "BuildSignedTx\(" --type goLength of output: 693
Confirmed Update of Gas Price Parameter Usage
The
BuildSignedTxinterface now robustly includes thegasPriceparameter, and our search verifies that all callers across the codebase—such as inexamples/chain/8_OfflineSigning/example.go, as well as the production and test support implementations—have been updated accordingly. No further modifications to dependent code are necessary.
devbranch withmasterwith the latest changes in v1.56.2 releaseSummary by CodeRabbit
Chores
Refactor