-
Notifications
You must be signed in to change notification settings - Fork 53
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/chain_upgrade_v1_15 #294
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
… feat/chain_upgrade_v1_15
WalkthroughThis pull request updates multiple configurations and code components. The CI/CD workflow is adjusted with a new pre-commit action version and linting settings. Repository cloning commands in the Makefile are modified to target different branches. In the core exchange logic, a new boolean parameter is introduced to specify currency formatting, and additional validation for a fixed gas flag is added. Cosmetic improvements are also applied to various protobuf and Go files. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant EC as ExchangeClient
participant S as Portfolio Service
C->>EC: GetAccountPortfolioBalances(ctx, accountAddress, usd)
EC->>S: Build Request (include usd flag)
S-->>EC: Return portfolio balances
EC-->>C: Deliver portfolio response
sequenceDiagram
participant U as User/Caller
participant P as Params Validator
participant V as validateFixedGasFlag
U->>P: Validate Params (with FixedGasEnabled)
P->>V: Check FixedGasEnabled value type
V-->>P: Return validation result (ok/error)
P-->>U: Validation outcome
Possibly related PRs
Suggested reviewers
Poem
🪧 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
|
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: 1
🧹 Nitpick comments (3)
.golangci.yml (2)
26-35
: Revise Revive Linter Configurations
The configuration underlinters-settings.revive
now uses the keyenable-all-rules
(replacing the previousenableAllRules
), and new rule entries have been added—such as disablingadd-constant
and settingline-length-limit
to 140. These changes provide finer control over linting behavior. Please verify that these settings align with your project's style guidelines.
36-45
: Updated gocritic Linter Settings
Thegocritic
section now includes additional enabled tags (diagnostic
,opinionated
, andexperimental
) and disables checks forsingleCaseSwitch
anddeferInLoop
. These updates can help catch a broader range of issues but may also increase the noise level. It would be good to monitor the lint results and adjust if necessary.client/exchange/exchange.go (1)
963-967
: Implementation correctly includes new USD parameterThe method implementation has been properly updated to match the interface change, passing the
usd
parameter to the request structure via theUsd
field.The parameter name
usd
is somewhat ambiguous. Consider using a more descriptive name likeconvertToUsd
orformatInUsd
to make the intent clearer:-func (c *exchangeClient) GetAccountPortfolioBalances(ctx context.Context, accountAddress string, usd bool) (*portfolioExchangePB.AccountPortfolioBalancesResponse, error) { +func (c *exchangeClient) GetAccountPortfolioBalances(ctx context.Context, accountAddress string, formatInUsd bool) (*portfolioExchangePB.AccountPortfolioBalancesResponse, error) { req := &portfolioExchangePB.AccountPortfolioBalancesRequest{ AccountAddress: accountAddress, - Usd: usd, + Usd: formatInUsd, }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
chain/exchange/types/exchange.pb.go
is excluded by!**/*.pb.go
chain/permissions/types/query.pb.go
is excluded by!**/*.pb.go
chain/tokenfactory/types/tx.pb.go
is excluded by!**/*.pb.go
exchange/accounts_rpc/pb/goadesign_goagen_injective_accounts_rpc.pb.go
is excluded by!**/*.pb.go
exchange/auction_rpc/pb/goadesign_goagen_injective_auction_rpc.pb.go
is excluded by!**/*.pb.go
exchange/campaign_rpc/pb/goadesign_goagen_injective_campaign_rpc.pb.go
is excluded by!**/*.pb.go
exchange/explorer_rpc/pb/goadesign_goagen_injective_explorer_rpc.pb.go
is excluded by!**/*.pb.go
exchange/portfolio_rpc/pb/goadesign_goagen_injective_portfolio_rpc.pb.go
is excluded by!**/*.pb.go
exchange/spot_exchange_rpc/pb/goadesign_goagen_injective_spot_exchange_rpc.pb.go
is excluded by!**/*.pb.go
📒 Files selected for processing (17)
.github/workflows/pre-commit.yml
(1 hunks).golangci.yml
(1 hunks)Makefile
(1 hunks)chain/exchange/types/params.go
(2 hunks)client/core/market.go
(3 hunks)client/exchange/exchange.go
(2 hunks)client/exchange/exchange_test_support.go
(1 hunks)examples/exchange/oracle/1_StreamPrices/example.go
(2 hunks)examples/exchange/portfolio/1_AccountPortfolio/example.go
(1 hunks)proto/injective/exchange/v1beta1/exchange.proto
(1 hunks)proto/injective/exchange/v1beta1/query.proto
(1 hunks)proto/injective/permissions/v1beta1/permissions.proto
(2 hunks)proto/injective/permissions/v1beta1/query.proto
(7 hunks)proto/injective/permissions/v1beta1/tx.proto
(3 hunks)proto/injective/tokenfactory/v1beta1/authorityMetadata.proto
(1 hunks)proto/injective/tokenfactory/v1beta1/tx.proto
(3 hunks)typeddata/typed_data.go
(1 hunks)
🔇 Additional comments (35)
typeddata/typed_data.go (1)
18-22
: Import Reorganization Enhances Readability
The newly added import lines (forsdkmath
,codec
,codectypes
,cosmtypes
, andlegacytx
) have been moved to the upper part of the import block. This improves the logical grouping of external dependencies and makes it easier to locate related imports. Please verify that the new ordering does not impact any implicit dependency order..github/workflows/pre-commit.yml (1)
18-18
: Updated Pre-commit Action Version
The workflow now usespre-commit/action@v3.0.1
instead of an older version. This update should bring in improved performance and bug fixes. Please ensure that the updated version behaves as expected with the rest of your CI configuration.Makefile (2)
4-4
: Updated Repository Branch for injective-indexer
The clone command for the injective-indexer repository now specifies branchv1.14.25
instead of an earlier release candidate. This change likely reflects a move toward a more stable version in line with the protocol upgrade.
7-7
: Updated Repository Branch for injective-core
The clone command now uses themaster
branch for injective-core. Please confirm that shifting to themaster
branch is intentional given the chain upgrade requirements and that any downstream dependencies are compatible with this update..golangci.yml (2)
48-59
: Issues Section Configuration for Linters
A newissues
section has been introduced with exclusions (e.g., for therevive
linter with text conditionALL_CAPS
) and specified directories to ignore. Settingmax-issues-per-linter
to 0 might be useful to ensure that no issue goes unreported. Please review these exclusions to ensure they do not hide any critical issues.
60-62
: Configured Run Section in GolangCI
Therun
section explicitly setstests: false
which can speed up linting by skipping test file checks. Verify that bypassing tests during linting won’t lead to missing important issues in test code.proto/injective/permissions/v1beta1/permissions.proto (4)
12-13
: Improved code formatting for better readability.The
contract_hook
field declaration has been split across two lines, enhancing code readability while maintaining the same functionality.
15-17
: Improved alignment of comments for better readability.The repeated fields declarations have been reformatted to consistently align the comments, making the code more visually organized.
19-20
: Improved code formatting for field declaration.The
policy_manager_capabilities
field declaration has been split across two lines for better readability, consistent with other formatting changes in this file.
93-93
: Consolidated message declaration.The
RoleIDs
message has been reformatted to a single line, making the simple structure more concise.proto/injective/tokenfactory/v1beta1/authorityMetadata.proto (1)
19-20
: Improved code formatting for better readability.The
admin_burn_allowed
field declaration has been split across two lines, enhancing code readability while maintaining the same functionality.proto/injective/exchange/v1beta1/exchange.proto (1)
178-180
:✅ Verification successful
Added new feature for fixed gas consumption control.
A new boolean parameter
fixed_gas_enabled
has been added to theParams
message to control whether the message server will consume a fixed gas amount for certain message types. This addition is part of the chain upgrade v1.15 and enhances gas management capabilities.
🏁 Script executed:
#!/bin/bash # Check for corresponding parameter validation in Go code rg -A 5 "fixed_gas_enabled" --type goLength of output: 994
Fix: Confirmed implementation of fixed gas consumption control
The new boolean field
fixed_gas_enabled
added to the Params message inproto/injective/exchange/v1beta1/exchange.proto
is correctly implemented. Verification shows that:
- The generated file
chain/exchange/types/exchange.pb.go
properly reflects the new field.- The parameter validation in
chain/exchange/types/params.go
includes the expected error handling forfixed_gas_enabled
.These changes are consistent with the v1.15 chain upgrade and deliver the intended enhancement in gas management.
examples/exchange/oracle/1_StreamPrices/example.go (2)
8-8
: Added import for string manipulation.The
strings
package has been imported to support the case normalization of the oracle type.
78-78
:✅ Verification successful
Added case normalization for oracle type.
The code now converts the oracle type to lowercase before passing it to the
StreamPrices
function, ensuring consistent behavior regardless of the case used in the source data.
🏁 Script executed:
#!/bin/bash # Check if StreamPrices implementation expects lowercase oracle type rg -A 5 "func.*StreamPrices" --type goLength of output: 22047
Oracle Type Normalization Verified
The change converting the oracle type to lowercase in the example confirms that the value passed to the
StreamPrices
function is normalized. Our verification shows that the receiving functions (e.g. inclient/exchange/exchange.go
) simply forward the provided oracle type without any additional case handling, so this normalization is both necessary and correctly applied.
- Ensures consistent behavior by converting
market.OracleType
usingstrings.ToLower
(seeexamples/exchange/oracle/1_StreamPrices/example.go
, line 78).- Confirms that neither the gRPC client implementation nor the corresponding protobuf methods perform internal case normalization.
examples/exchange/portfolio/1_AccountPortfolio/example.go (1)
22-22
:✅ Verification successful
Method signature updated with new boolean parameter
The
GetAccountPortfolioBalances
method now accepts a third boolean parameter (true
) which appears to be for including inactive balances in the response. This change aligns with the updated method signature in the client package.
🏁 Script executed:
#!/bin/bash # Verify that all calls to GetAccountPortfolioBalances have been updated with the new parameter rg "GetAccountPortfolioBalances\(ctx, .*\)" --type goLength of output: 196
Action: Verified Updated Boolean Parameter Usage in GetAccountPortfolioBalances
The call to
GetAccountPortfolioBalances
inexamples/exchange/portfolio/1_AccountPortfolio/example.go
now correctly passes the new boolean parameter (true
), aligning with the updated client package signature. The shell script verification confirms that this is the only occurrence and that it has been updated accordingly. No further changes are required.proto/injective/tokenfactory/v1beta1/tx.proto (4)
47-48
: Formatting improvement for boolean field declarationThe
allow_admin_burn
field declaration has been reformatted to split across multiple lines for better readability.
58-59
: Documentation format improvedThe comment for the
MsgMint
message has been reformatted for better readability without changing its meaning.
122-123
: Formatting improvement for boolean field declarationThe
should_disable
field declaration has been reformatted to split across multiple lines for better readability.
125-126
: Formatting improvement for field declarationThe
admin_burn_disabled
field declaration has been reformatted to split across multiple lines for better readability.proto/injective/permissions/v1beta1/tx.proto (4)
26-27
: Comment formatting improvedThe comment for the commented-out
DeleteNamespace
RPC method has been reformatted to split across multiple lines for better readability.
63-64
: Field declaration formatting improvedThe
contract_hook
field declaration and its comment have been reformatted to split across multiple lines for better readability.
66-70
: Field declaration formatting improvedThe field declarations and comments for
role_permissions
,role_managers
, andpolicy_manager_capabilities
have been reformatted for better alignment and readability.
83-84
: Field declaration formatting improvedThe
role_actors_to_revoke
field declaration has been reformatted to split across multiple lines for better readability.proto/injective/permissions/v1beta1/query.proto (2)
22-24
: RPC method documentation and formatting improvedMultiple RPC method documentation and HTTP option assignments have been reformatted for better readability. This improves code consistency without changing any functionality.
Also applies to: 32-34, 38-39, 43-45, 51-53, 59-61, 67-70, 74-76, 82-84, 91-94, 96-99, 106-108
120-122
: Message definition formatting improvedMultiple message definitions, field declarations, and comments have been reformatted for better readability and consistency. These changes are purely cosmetic and don't affect functionality.
Also applies to: 124-126, 138-138, 168-168, 187-187, 191-193, 195-198, 201-203, 207-207, 222-224, 226-228
client/exchange/exchange_test_support.go (1)
295-297
: Updated mock method signature to match interface changes.The
GetAccountPortfolioBalances
method signature has been updated to include a newusd
boolean parameter, maintaining consistency with changes in the actual client implementation. This ensures the mock implementation correctly matches the interface requirements.chain/exchange/types/params.go (3)
225-225
: Added newFixedGasEnabled
parameter in DefaultParams.This new boolean field has been properly added to the parameters struct with a default value of
false
. This parameter will likely control whether fixed gas consumption is enabled for certain message types in the chain upgrade.
303-306
: Added validation for the newFixedGasEnabled
parameter.The validation logic for the new parameter has been properly integrated into the existing validation flow, ensuring that the parameter is checked before being applied.
311-316
: Added helper function forFixedGasEnabled
parameter validation.The new
validateFixedGasFlag
function checks that the parameter is of typebool
and returns an appropriate error message if validation fails. This follows the existing pattern for parameter validation in this file.client/core/market.go (3)
45-46
: Improved notional value handling with ceiling function in SpotMarket.Changed the
NotionalToChainFormat
implementation to useCeil()
on the chain formatted value, ensuring the result is rounded up to the nearest whole number. This provides more predictable behavior for notional values, potentially preventing issues with fractional values.
133-134
: Improved notional value handling with ceiling function in DerivativeMarket.Similar to the SpotMarket change, the
NotionalToChainFormat
implementation now usesCeil()
for more predictable behavior with notional values.
230-231
: Improved notional value handling with ceiling function in BinaryOptionMarket.Consistent with the other market types, the
NotionalToChainFormat
implementation now usesCeil()
for notional values, ensuring uniform handling across all market types.proto/injective/exchange/v1beta1/query.proto (2)
1527-1531
: Adjusted formatting of the QueryMarketBalanceResponse message.Minor formatting change that doesn't affect functionality - the opening bracket for field options was moved to the same line as the field declaration for better readability.
1535-1535
: Simplified QueryMarketBalancesResponse message formatting.The message definition was reformatted to have the field declaration on a single line, improving readability without changing functionality.
client/exchange/exchange.go (1)
89-89
:✅ Verification successful
Interface method updated with new USD parameter
The
GetAccountPortfolioBalances
method signature has been enhanced to include a new boolean parameterusd
. This is a breaking change to the interface that will require clients to update their method calls.Let's check for existing usages of this method that will need to be updated:
🏁 Script executed:
#!/bin/bash # Find all calls to GetAccountPortfolioBalances that may need to be updated rg "\.GetAccountPortfolioBalances\s*\(" --type goLength of output: 192
Breaking Change Verified: Interface Method Updated Correctly
The updated signature of
GetAccountPortfolioBalances
now includes the new booleanusd
parameter, and our search confirms that at least one call site (inexamples/exchange/portfolio/1_AccountPortfolio/example.go
) has already been updated to pass the additional argument. No outdated usages were found.
@@ -11,15 +11,15 @@ import ( | |||
|
|||
func main() { | |||
// select network: local, testnet, mainnet | |||
network := common.LoadNetwork("testnet", "lb") | |||
network := common.LoadNetwork("devnet", "lb") |
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.
💡 Verification agent
🧩 Analysis chain
Network environment changed from testnet to devnet
The example now uses "devnet" instead of "testnet". This change might impact users who are following this example and expecting it to work with the testnet environment.
🏁 Script executed:
#!/bin/bash
# Check for other examples that might still be using testnet
grep -r "LoadNetwork(\"testnet\"" examples/ --include="*.go"
Length of output: 27526
Network environment inconsistency in portfolio example
The file examples/exchange/portfolio/1_AccountPortfolio/example.go
now uses:
network := common.LoadNetwork("devnet", "lb")
while almost all other examples still call the function with "testnet"
. This inconsistency may confuse users who expect a uniform testnet environment across examples. Please clarify whether the portfolio example should genuinely target devnet (in which case update its documentation to explain the deviation) or modify it to use testnet for consistency with the rest of the examples.
- Verify if the change in this file was intentional.
- If intentional, add a comment or update the documentation to highlight the different network usage.
- If not, update the code to use
"testnet"
as in the other examples.
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: 0
🧹 Nitpick comments (1)
ethereum/util/noncecache.go (1)
118-120
: Redundant locking pattern for nextNonce readThe code acquires a read lock to read
nextNonce
immediately after readingprevNonce
without a lock. This seems unnecessary as both are reading the samen.nonces[account]
value, and there's already a lock (lock.Lock()
) held on line 114 which should protect the account's nonce.Consider removing these redundant read locks:
prevNonce, prevOk := n.nonces[account] -n.mux.RLock() nextNonce, nextOk := n.nonces[account] -n.mux.RUnlock()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (5)
.github/workflows/golangci-lint.yml
(1 hunks)Makefile
(2 hunks)ethereum/util/amounts.go
(1 hunks)ethereum/util/noncecache.go
(1 hunks)go.mod
(5 hunks)
🔇 Additional comments (11)
ethereum/util/noncecache.go (2)
122-125
: Logic check seems counterintuitiveThe condition
!prevOk && nextOk
checks if the nonce wasn't initially found but then suddenly exists. This scenario seems unlikely given the lock acquisition flow, as you're already holding the account's lock. This would only happen if another goroutine modified the map while this method is executing, which shouldn't be possible with the current locking strategy.Please verify this edge case is actually possible or consider removing this check if it's redundant.
126-145
: Improved control flow in Sync methodThe refactored code has significantly improved the control flow by:
- Flattening the nested structure
- Adding clear early returns for error cases
- Simplified error handling with immediate unlocks and returns
- Proper handling of nonce overflow with appropriate locking
The changes make the code more readable and maintainable.
ethereum/util/amounts.go (1)
43-44
: Improved code clarity by simplifying control flow.The removal of the
else
block simplifies the code structure without changing the behavior. This is a good refactoring as it reduces nesting, making the code more readable.Makefile (4)
4-4
: Updated injective-indexer reference to newer version.The branch reference has been updated from
v1.14.1-RC.6
tov1.14.25
, which aligns with the chain upgrade mentioned in the PR objectives. This ensures the SDK is using the latest compatible indexer code.
7-7
: Changed injective-core branch to master.Switching from a specific version (
v1.14.0
) tomaster
might introduce unexpected changes as the master branch can be unstable. Consider using a specific version or tag for reproducible builds.Would it be more appropriate to use a specific version tag aligned with v1.15 instead of the
master
branch to ensure stability? This would prevent potential breaking changes from being introduced from the master branch.
154-165
: Added new linting targets.The addition of these linting targets enhances the project's quality assurance process by providing multiple options for checking code quality:
lint
: Checks new issues compared to the dev branchlint-last-commit
: Checks new issues in the latest commitlint-master
: Checks new issues compared to the master branchThese additions are beneficial for maintaining code quality during the chain upgrade process.
166-166
: Updated .PHONY to include new targets.This correctly marks the new linting targets as phony, which is necessary for Make to understand these targets don't represent files.
.github/workflows/golangci-lint.yml (1)
34-34
: Changed linting to only show new issues.Setting
only-new-issues
totrue
improves the PR review process by focusing only on issues introduced in the current changes rather than reporting pre-existing issues. This change works well with the new linting targets added in the Makefile.go.mod (3)
9-9
: Updated multiple dependency versions.The updates to
cosmossdk.io/math
,filippo.io/edwards25519
,github.com/cosmos/ics23/go
, andgoogle.golang.org/genproto
are part of the chain upgrade process. These updates likely include bug fixes and new features that are needed for the v1.15 chain upgrade.Also applies to: 48-48, 89-89, 219-219
5-5
:❓ Verification inconclusive
Downgraded Go toolchain version.
The toolchain has been downgraded from
go1.23.5
togo1.23.1
. This might be for compatibility reasons, but it's worth confirming this won't miss any critical bug fixes from the newer toolchain version.
🌐 Web query:
What are the differences between Go 1.23.1 and Go 1.23.5 in terms of bug fixes or security patches?
💡 Result:
Go versions 1.23.1 and 1.23.5 differ significantly in security patches and incremental bug fixes, with 1.23.5 addressing critical vulnerabilities discovered after 1.23.1's release:
Security Fixes
Go 1.23.1 (2024-09-05):
Patched vulnerabilities in:
encoding/gob
(deserialization risks)go/build/constraint
(build system manipulation)go/parser
(code injection vectors)[1]Go 1.23.5 (2025-01-16):
Fixed two high-severity CVEs:
- CVE-2024-45341: IPv6 zone ID bypass in
crypto/x509
URI constraints, allowing unauthorized certificate validation[2][8]- CVE-2024-45336:
net/http
client improperly retained sensitive headers (like Authorization) after cross-domain redirect chains[2][8]Key Bug Fix Differences
Version Compiler/Runtime Fixes Standard Library Fixes 1.23.1 - Compiler stability
- Runtime memory managementdatabase/sql
,go/types
,os
,runtime/trace
[1]1.23.5 - Compiler optimizations
- Reduced PGO overheadnet
package improvementstime.Timer
GC behavior changes[4][7]Notable Improvements in Later Versions
- 1.23.2 (Oct 2024): Fixed
maps
,os/exec
, andtime
package issues[1]- 1.23.3 (Nov 2024): Addressed linker stability and
syscall
handling[1]- 1.23.4 (Dec 2024): Enhanced
trace
command and syscall reliability[1]The 1.23.5 release also introduced stricter
//go:linkname
directive checks[4][6] and optimized garbage collection for unstopped timers[4][7], features not present in 1.23.1. Organizations handling private PKIs or sensitive HTTP headers should prioritize upgrading to 1.23.5 due to its security-critical fixes[2][8].Citations:
- 1: https://go.dev/doc/devel/release
- 2: https://groups.google.com/g/golang-announce/c/sSaUhLA-2SI
- 3: https://github.com/go-gitea/gitea/releases
- 4: https://tip.golang.org/doc/go1.23
- 5: https://security.snyk.io/package/linux/debian:unstable/golang-1.23
- 6: https://withcodeexample.com/introducing-go-123-whats-new-in-the-latest-release/
- 7: https://antonz.org/go-1-23/
- 8: https://www.tenable.com/plugins/nessus/214539
- 9: https://tip.golang.org/doc/go1.22
- 10: https://changelog.com/gotime/325
- 11: https://devblogs.microsoft.com/go/go-1-23-5-1-and-1-22-11-1-microsoft-builds-now-available/
- 12: https://techdocs.broadcom.com/us/en/vmware-tanzu/platform/tanzu-platform-for-cloud-foundry/6-0/tpcf/runtime-rn.html
- 13: https://blog.stackademic.com/a-first-look-at-unique-pkg-in-go-1-23-6a43a2377b68
- 14: https://devblogs.microsoft.com/go/tag/security/
- 15: https://mariocarrion.com/2024/09/02/what-is-new-in-go-1-23.html
- 16: Update go to 1.23.5 for CVE-2024-45336 and CVE-2024-45341 evanw/esbuild#4056
- 17: https://dev.to/conquerym/go-123s-game-changing-unique-package-supercharge-your-data-deduplication-46n8
- 18: Update Go to 1.23.5/1.22.11 etcd-io/etcd#19210
Action Required: Confirm Compatibility and Security Implications of Downgrade
The Go toolchain version in
go.mod
has been changed togo1.23.1
, effectively downgrading from the previously usedgo1.23.5
. Notably, version 1.23.5 includes critical security patches and bug fixes—such as fixes for CVE-2024-45341 (IPv6 zone ID bypass incrypto/x509
) and CVE-2024-45336 (sensitive header handling innet/http
)—that are absent in 1.23.1. Although the downgrade may be motivated by compatibility reasons, please verify that this change does not expose the project to security vulnerabilities or other issues addressed in the newer release.
- Confirm the downgrade is intentional and that any necessary compensating mitigations are in place.
- Evaluate whether the compatibility benefits outweigh the security and bug fix improvements provided in Go 1.23.5.
237-244
:✅ Verification successful
Updated replace directives for core dependencies.
The updates to the replace directives for major components like
wasmd
,cosmos-sdk
, andibc-go
are critical changes for the chain upgrade. These changes align with the PR objectives of updating protocol definitions for the v1.15 chain upgrade.Pay special attention to these changes during testing, as they represent significant updates to core dependencies that could affect compatibility and behavior.
🏁 Script executed:
#!/bin/bash # Check for any breaking changes in the updated dependencies # Check if there are any issues with the dependency changes go mod tidy go mod verifyLength of output: 18027
Action Required: Core Dependency Updates Verified
The updated dependency replace directives in
go.mod
(lines 237–244) have been successfully verified. Runninggo mod tidy
andgo mod verify
produced no errors, confirming that the dependency updates forwasmd
,cosmos-sdk
,ibc-go
, and others are correct and aligned with the v1.15 chain upgrade objectives.
- The new replace directives replace:
github.com/CosmWasm/wasmd
withgithub.com/InjectiveLabs/wasmd v0.53.2-inj.2
github.com/bandprotocol/bandchain-packet
withgithub.com/InjectiveLabs/bandchain-packet v0.0.4-inj-1
github.com/cometbft/cometbft
withgithub.com/InjectiveLabs/cometbft v0.38.17-inj-0
github.com/cosmos/cosmos-sdk
withgithub.com/InjectiveLabs/cosmos-sdk v0.50.9-inj-4
- And similar updates for the IBC modules.
Please continue to monitor for any behavioral changes during integration testing.
… example for the new GetEipBaseFee request in txfees module
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: 1
🧹 Nitpick comments (5)
Makefile (2)
95-99
: Review New txfees and txfees/osmosis Copy Commands
New targets for copying protocol types related totxfees
have been added. The commands create the directory and copy both.pb.go
andcodec.go
files forchain/txfees/types
, while only.pb.go
files are copied forchain/txfees/osmosis/types
. Please ensure the absence of acodec.go
file for the osmosis variant is intentional and consistent with the expected structure of those types.
159-170
: New Linting Targets and GOPROXY Usage
The PR introduces three new linting targets (lint
,lint-last-commit
, andlint-master
), each settingGOPROXY=direct
and specifying different revision flags (--new-from-rev=dev
,HEAD~
, andmaster
). Ensure that these revision references and environment settings are aligned with your CI/CD process. Additionally, consider whether the repeated export ofGOPROXY=direct
for each target could be consolidated for better maintainability.proto/injective/txfees/v1beta1/tx.proto (1)
19-30
: Message definition is well-structured with proper annotations.The
MsgUpdateParams
message follows best practices:
- Uses the cosmos.msg.v1.signer annotation to specify the authority
- Properly documents the authority field as governance account
- Includes a non-nullable params field with appropriate comments explaining all parameters must be supplied
- Uses amino.name for compatibility
The comment on line 26 mentions "ocr parameters" which seems to be a copy-paste error since this is for txfees.
- // params defines the ocr parameters to update. + // params defines the txfees parameters to update.examples/chain/txfees/query/1_GetEipBaseFee/example.go (1)
59-68
: EIP base fee query implementation is concise and correct.The code creates a background context and uses it to fetch the EIP base fee. The error handling is appropriate, and the result is nicely formatted as JSON for readability.
However, there's a minor issue: the error from
json.MarshalIndent
on line 66 is discarded. While this is unlikely to fail for simple responses, it's best practice to handle all potential errors.- str, _ := json.MarshalIndent(res, "", " ") - fmt.Print(string(str)) + str, err := json.MarshalIndent(res, "", " ") + if err != nil { + fmt.Printf("Error marshaling response: %v\n", err) + return + } + fmt.Println(string(str))proto/injective/txfees/v1beta1/query.proto (1)
41-42
: Defined EipBaseFee request and response messagesThe
QueryEipBaseFeeRequest
(empty) andQueryEipBaseFeeResponse
messages are properly defined. The response includes abase_fee
field of typeEipBaseFee
.Note that unlike the
params
field inQueryParamsResponse
, thebase_fee
field is not marked as non-nullable. This is consistent with the design but allows for potential nil responses. Consider if this is the intended behavior.Consider adding
[(gogoproto.nullable) = false]
to thebase_fee
field inQueryEipBaseFeeResponse
to ensure a non-nil response, consistent with the params pattern.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (7)
chain/exchange/types/query.pb.go
is excluded by!**/*.pb.go
chain/txfees/osmosis/types/query.pb.go
is excluded by!**/*.pb.go
chain/txfees/types/genesis.pb.go
is excluded by!**/*.pb.go
chain/txfees/types/query.pb.go
is excluded by!**/*.pb.go
chain/txfees/types/tx.pb.go
is excluded by!**/*.pb.go
chain/txfees/types/txfees.pb.go
is excluded by!**/*.pb.go
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (15)
Makefile
(3 hunks)chain/txfees/types/codec.go
(1 hunks)client/chain/chain.go
(5 hunks)client/chain/chain_test_support.go
(2 hunks)client/chain/context.go
(4 hunks)examples/chain/exchange/query/58_MarketBalance/example.go
(1 hunks)examples/chain/txfees/query/1_GetEipBaseFee/example.go
(1 hunks)go.mod
(10 hunks)injective_data/ofac.json
(1 hunks)proto/injective/exchange/v1beta1/query.proto
(1 hunks)proto/injective/txfees/v1beta1/genesis.proto
(1 hunks)proto/injective/txfees/v1beta1/query.proto
(1 hunks)proto/injective/txfees/v1beta1/tx.proto
(1 hunks)proto/injective/txfees/v1beta1/txfees.proto
(1 hunks)proto/osmosis/txfees/v1beta1/query.proto
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- proto/injective/exchange/v1beta1/query.proto
- go.mod
🧰 Additional context used
🧬 Code Definitions (3)
chain/txfees/types/codec.go (3)
chain/txfees/types/tx.pb.go (5) (5)
MsgUpdateParams
(34-41)MsgUpdateParams
(45-45)MsgUpdateParams
(46-48)init
(126-129)init
(131-131)chain/txfees/types/txfees.pb.go (6) (6)
Params
(31-45)Params
(49-49)Params
(50-52)init
(108-111)init
(113-115)init
(116-118)chain/txfees/types/genesis.pb.go (2) (2)
init
(70-72)init
(74-76)
client/chain/chain.go (1)
chain/txfees/types/query.pb.go (13) (13)
QueryParamsResponse
(108-111)QueryParamsResponse
(115-115)QueryParamsResponse
(116-118)QueryEipBaseFeeResponse
(189-191)QueryEipBaseFeeResponse
(195-195)QueryEipBaseFeeResponse
(196-198)QueryClient
(289-295)QueryParamsRequest
(71-72)QueryParamsRequest
(76-76)QueryParamsRequest
(77-79)QueryEipBaseFeeRequest
(153-154)QueryEipBaseFeeRequest
(158-158)QueryEipBaseFeeRequest
(159-161)
client/chain/context.go (5)
chain/txfees/types/codec.go (1) (1)
RegisterInterfaces
(16-22)chain/exchange/types/codec.go (1) (1)
RegisterInterfaces
(85-176)chain/wasmx/types/codec.go (1) (1)
RegisterInterfaces
(34-57)chain/permissions/types/codec.go (1) (1)
RegisterInterfaces
(21-30)chain/tokenfactory/types/codec.go (1) (1)
RegisterInterfaces
(25-37)
🪛 Buf (1.47.2)
proto/injective/txfees/v1beta1/genesis.proto
4-4: import "injective/txfees/v1beta1/txfees.proto": file does not exist
(COMPILE)
proto/injective/txfees/v1beta1/tx.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/injective/txfees/v1beta1/txfees.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/osmosis/txfees/v1beta1/query.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/injective/txfees/v1beta1/query.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
🔇 Additional comments (41)
chain/txfees/types/codec.go (4)
3-9
: Import statements look good and follow standard Cosmos SDK practices.All necessary imports are included for codec registration, including the core Cosmos SDK codec packages and the authz codec for integration with authorization functionality.
11-14
: RegisterLegacyAminoCodec implementation follows standard Cosmos SDK patterns.The function correctly registers the concrete types
MsgUpdateParams
andParams
with the LegacyAmino codec using the appropriate type paths. This enables proper serialization and deserialization of these types in the Cosmos SDK ecosystem.
16-22
: RegisterInterfaces implementation is correct and follows SDK conventions.The function properly registers:
MsgUpdateParams
as an implementation of thesdk.Msg
interface- Message service descriptors using the standard
msgservice
packageThis ensures that the message can be properly routed and processed within the Cosmos SDK message handling system.
24-36
: ModuleCdc declaration and initialization follow best practices.The initialization sequence:
- Creates a new LegacyAmino codec instance
- Registers all necessary types with the module's codec
- Registers SDK and authz types for compatibility with those systems
- Properly seals the codec to prevent runtime modifications
This implementation ensures proper serialization compatibility across the Cosmos SDK ecosystem.
Makefile (3)
4-4
: Verify Branch Consistency for injective-indexer Clone
The updated clone command now uses branchv1.14.25
for theinjective-indexer
repository. Please confirm that this branch is the intended stable target for the chain upgrade and that it aligns with your release strategy.
7-7
: Reassess Branch Selection for injective-core Clone
The repositoryinjective-core
is now cloned from themaster
branch instead of a versioned branch. Verify that usingmaster
is appropriate for your development and deployment workflow, as it could include unanticipated changes compared to a fixed release branch.
171-171
: Update PHONY Declarations
The.PHONY
declaration has been updated to include the new linting targets. This ensures that these make targets are not mistaken for files, which is a best practice.proto/injective/txfees/v1beta1/genesis.proto (3)
1-9
: The file structure follows Proto3 standards correctly.The Genesis state definition is well-organized with proper package naming, imports, and Go package option.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "injective/txfees/v1beta1/txfees.proto": file does not exist
(COMPILE)
9-9
: GenesisState message is well-defined.The GenesisState message has a single non-nullable Params field, which is appropriate for genesis configuration.
4-4
:✅ Verification successful
Verify import file exists.
The static analysis tool indicates that
injective/txfees/v1beta1/txfees.proto
might not exist. Ensure this file is part of your PR or already exists in the repository.
🏁 Script executed:
#!/bin/bash # Check if the imported proto file exists find proto -name "txfees.proto" | grep "injective/txfees/v1beta1"Length of output: 107
Import File Verified
The static analysis concern has been addressed. The file
proto/injective/txfees/v1beta1/txfees.proto
exists in the repository as confirmed by your shell script output. No changes are needed.🧰 Tools
🪛 Buf (1.47.2)
4-4: import "injective/txfees/v1beta1/txfees.proto": file does not exist
(COMPILE)
injective_data/ofac.json (1)
161-164
: New OFAC addresses added correctly.The addition of new addresses to the OFAC list follows the existing pattern and maintains the JSON array structure.
proto/injective/txfees/v1beta1/txfees.proto (3)
1-10
: Proto3 configuration is well-structured.The file correctly defines package, imports, and options following Proto3 standards.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
11-67
: Well-structured transaction fee parameters.The Params message is comprehensive, with appropriate field types and Gogoproto annotations:
- Gas limits and thresholds use uint64
- Decimal values use LegacyDec custom type
- Boolean for feature flags
- All fields have proper YAML tags and nullable settings
This provides a complete configuration for transaction fee management.
4-4
:❓ Verification inconclusive
Verify import file exists.
The static analysis tool indicates that
gogoproto/gogo.proto
might not exist. Ensure this file is properly imported in your build environment.
🏁 Script executed:
#!/bin/bash # Check if the gogoproto import is available find . -name "gogo.proto" | grep "gogoproto"Length of output: 42
Manual Verification Required: Confirm
gogoproto/gogo.proto
ImportThe search script did not locate any file named
gogo.proto
within a directory containing "gogoproto". Please verify manually that this dependency is correctly provided in your build environment. If the file is expected to be resolved via an external dependency or vendor setup, consider documenting that behavior. Otherwise, please add or correct the missing file.🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
examples/chain/exchange/query/58_MarketBalance/example.go (1)
56-58
: Improved error handling.Added proper error checking for the chainClient creation, which prevents the application from continuing with an invalid client. This is a good defensive programming practice.
proto/injective/txfees/v1beta1/tx.proto (4)
1-11
: Proto file structure and imports look good.The file correctly defines the package with a proper Go package option. The imports include all necessary proto dependencies for the service implementation.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
12-17
: Service definition follows protocol standards.The
Msg
service is properly defined with the cosmos.msg.v1.service option and includes a single RPC method for updating parameters. This follows standard practices for governance-controlled parameter updates in Cosmos SDK modules.
32-32
: Response message structure is appropriate.The empty response message follows standard Cosmos SDK patterns for update operations where no return data is needed.
4-4
:❓ Verification inconclusive
Verify that the gogoproto import can be resolved.
Static analysis reports that "gogoproto/gogo.proto" file does not exist. This could be a configuration issue with the analyzer or a genuine missing dependency.
🏁 Script executed:
#!/bin/bash # Check if the import can be resolved in the project fd -t f "gogo.proto" --exec echo "Found: {}" # Check if there are other proto files using this import rg -l "import \"gogoproto/gogo.proto\"" --type proto | grep -v "txfees/v1beta1/tx.proto" | head -5Length of output: 171
Below is an updated shell script to further investigate the resolution of the
gogoproto/gogo.proto
import. The previous command failed due to an unrecognized
file type ("proto"). Please run the following script to check for the existence of the file
and to search for its import across all.proto
files:#!/bin/bash # Find the file anywhere in the repository find . -type f -path "*/gogoproto/gogo.proto" # Search through all .proto files for the import reference to gogoproto/gogo.proto rg -g "*.proto" 'import "gogoproto/gogo.proto"'Once you have the output of this script, we can determine whether the dependency is missing
or if the issue lies with the configuration of the static analyzer.Next Steps:
• Verify that the file is present or intentionally omitted in a different dependency package.
• Confirm whether the import inproto/injective/txfees/v1beta1/tx.proto
is still valid or can be removed.🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
examples/chain/txfees/query/1_GetEipBaseFee/example.go (3)
1-15
: Well-structured imports and package declaration.The import statements are properly organized and include all necessary packages for the example. The use of qualified imports (with aliases) for packages that might have naming conflicts is a good practice.
16-35
: Client initialization follows best practices.The code correctly initializes the network configuration and establishes a connection to the Tendermint RPC client with proper error handling. The
InitCosmosKeyring
function is called with all required parameters, including a hardcoded private key for demonstration purposes.Note that the hardcoded private key on line 29 should never be used in production code. Since this is an example, it's acceptable, but please verify this is not a real private key with actual funds.
37-57
: Proper chain client setup with error handling.The code correctly creates a client context and initializes the chain client with appropriate error handling. The use of
panic
is acceptable in example code to simplify error handling.proto/osmosis/txfees/v1beta1/query.proto (4)
1-9
: Proto file structure and imports look good.The file correctly defines the package with a proper Go package option that aligns with the Injective Core project structure, despite being in the osmosis namespace.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
10-15
: Query service is properly defined with REST endpoint mapping.The
Query
service is correctly defined with a single RPC methodGetEipBaseFee
that includes the appropriate HTTP mapping using Google API annotations to define the REST endpoint.
17-25
: Well-structured request and response messages.The request is appropriately defined as an empty message since no parameters are needed. The response includes a well-documented
base_fee
field with proper annotations for GoGo protobuf serialization and Cosmos SDK compatibility.
4-4
:✅ Verification successful
Verify that the gogoproto import can be resolved.
Similar to the other proto file, static analysis reports that "gogoproto/gogo.proto" file does not exist. This could be a configuration issue with the analyzer or a genuine missing dependency.
🏁 Script executed:
#!/bin/bash # Check for other proto files using this import pattern successfully fd -t f "proto$" -x grep -l "import \"gogoproto/gogo.proto\"" {} \; | grep -v "txfees/v1beta1" | head -3 # Check if this is a common pattern in the codebase rg -t proto "option go_package.*injective-core" | head -3Length of output: 316
Gogoproto Import Verified – No Action Required
After verifying the repository, we found that the same import statement is used in several other proto files (e.g.,
./proto/injective/wasmx/v1/events.proto
,proposal.proto
, andquery.proto
). This indicates that the import is intentional and that the dependency is incorporated elsewhere in the build configuration. The static analysis warning appears to be due to the analyzer’s proto file settings rather than an actual missing dependency.🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
client/chain/chain_test_support.go (2)
31-31
: Import added correctly.The import for the txfeestypes package is properly added to the existing import block.
831-838
: Mock implementations for TxFees module methods.Two new methods are correctly added to the MockChainClient:
FetchTxFeesParams
- Returns mock transaction fee parametersFetchEipBaseFee
- Returns mock EIP base fee responseThese implementations match the pattern of other mock methods in the file and return empty responses with nil errors.
client/chain/chain.go (5)
46-47
: New txfees module import added to the client packageThe addition of the
txfeestypes "github.com/InjectiveLabs/sdk-go/chain/txfees/types"
import supports the chain upgrade to v1.15 by bringing in transaction fee-related types. This import is necessary for the new TxFees module functionality being integrated in this PR.
305-307
: Added new TxFees module query methods to the ChainClient interfaceTwo new methods have been added to the
ChainClient
interface to support the TxFees module:
FetchTxFeesParams
to retrieve transaction fee parametersFetchEipBaseFee
to retrieve the Ethereum Improvement Proposal (EIP) base feeThese additions properly extend the client's capabilities to interact with the new TxFees functionality introduced in the v1.15 chain upgrade.
353-353
: Added txfeesQueryClient to chainClient structA new field
txfeesQueryClient
has been added to thechainClient
struct to hold the query client for the TxFees module. This is consistent with how other module query clients are handled in this struct.
455-455
: Initialized txfeesQueryClient in NewChainClient constructorThe
txfeesQueryClient
is properly initialized in theNewChainClient
constructor, following the same pattern used for other module query clients. This ensures the client is ready to use when a new chain client is created.
2666-2679
: Implemented TxFees module query methodsThe implementation of
FetchTxFeesParams
andFetchEipBaseFee
methods follows the established pattern for other query methods in this client. They:
- Create a properly formed request object
- Execute the query using the common
ExecuteCall
function- Return the response and any error that occurred
This implementation is clean, consistent with existing code, and properly handles error propagation.
proto/injective/txfees/v1beta1/query.proto (5)
1-9
: New protocol buffer file for TxFees moduleThis new file establishes the protocol definitions for the TxFees module as part of the v1.15 chain upgrade. It properly defines:
- Package name
injective.txfees.v1beta1
- Required imports
- Go package path for code generation
The structure follows standard protobuf practices for Cosmos SDK modules.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
10-23
: Defined gRPC Query service with two methodsThe
Query
service definition includes two methods:
Params
: Returns the TxFees module's parametersGetEipBaseFee
: Returns the current fee market EIP feeBoth methods have appropriate HTTP endpoint mappings using Google API annotations, making them accessible via both gRPC and REST. This follows the standard practice for Cosmos SDK modules.
25-31
: Defined EipBaseFee message with appropriate annotationsThe
EipBaseFee
message is defined with a single fieldbase_fee
that uses:
- Appropriate YAML tag for serialization
- Custom type mapping to
cosmossdk.io/math.LegacyDec
for precise decimal operations- Non-nullable constraint to ensure the field is always present
These annotations ensure proper handling of decimal values in the blockchain context where precision is crucial.
33-39
: Defined Query/Params request and response messagesThe
QueryParamsRequest
(empty, as is standard for params queries) andQueryParamsResponse
messages are properly defined. The response includes a non-nullableparams
field which ensures that a valid params object is always returned.
4-4
:❓ Verification inconclusive
Addressing the gogoproto import static analysis warning
The static analysis tool flagged the import
gogoproto/gogo.proto
as potentially missing. This is likely a false positive in the analysis tool's configuration, as this is a standard import in the Cosmos SDK ecosystem.To verify this won't cause issues during compilation:
🏁 Script executed:
#!/bin/bash # Check if the protobuf compilation works correctly with the gogoproto import cd "$(git rev-parse --show-toplevel)" && \ echo "Checking if protoc can find gogoproto imports..." && \ find . -name "buf.yaml" -exec dirname {} \; | sort | head -1 | xargs -I{} sh -c "cd {} && buf build --path proto/injective/txfees/v1beta1/query.proto"Length of output: 498
Action Required: Manual Verification of the gogoproto Import
The static analysis warning on importing"gogoproto/gogo.proto"
inproto/injective/txfees/v1beta1/query.proto
appears to be a false positive, given its standard use in the Cosmos SDK. However, our automated verification command did not successfully target any.proto
files (it returned "Failure: no .proto files were targeted"), which means we cannot conclusively confirm that the build is correctly processing this file.Please take the following steps:
- Confirm Module Inclusion: Verify that
proto/injective/txfees/v1beta1/query.proto
is part of your Buf module and is correctly listed (e.g., by runningbuf ls-files | grep 'proto/injective/txfees/v1beta1/query.proto'
).- Manual Build Verification: Re-run your protobuf compilation manually (or adjust your Buf command options) to check that the import
"gogoproto/gogo.proto"
resolves correctly during the build.Once these steps are manually verified, you can safely disregard the static analysis warning.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
client/chain/context.go (3)
6-9
: Added necessary imports for evidence, feegrant, and upgrade typesNew imports have been added for Cosmos SDK modules that are required for the chain upgrade:
evidencetypes
for evidence handlingfeegranttypes
for fee grantsupgradetypes
for chain upgradesThese imports support the expanded functionality in v1.15.
43-54
: Updated imports for Injective modulesThe import statements have been reorganized to improve clarity and import the necessary txfees module. This follows good practices by keeping related imports grouped together.
90-91
: Added TxFees module interface registration in NewTxConfigThe
txfeestypes.RegisterInterfaces(interfaceRegistry)
call ensures that the transaction fee types are properly registered with the interface registry. This is necessary for the codec to correctly serialize and deserialize TxFees module messages.
permissions.RegisterInterfaces(interfaceRegistry) | ||
txfeestypes.RegisterInterfaces(interfaceRegistry) | ||
interfaceRegistry.RegisterImplementations((*sdk.Msg)(nil), | ||
&txfeestypes.QueryEipBaseFeeResponse{}, | ||
) |
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.
Added interface registrations in NewClientContext
The changes include:
- Adding
permissions.RegisterInterfaces(interfaceRegistry)
- Adding
txfeestypes.RegisterInterfaces(interfaceRegistry)
- Explicitly registering
QueryEipBaseFeeResponse
as an implementation ofsdk.Msg
The first two changes are appropriate, but there's a potential issue with the third change:
Query response types like QueryEipBaseFeeResponse
should not be registered as implementations of sdk.Msg
. The sdk.Msg
interface is specifically for transaction messages that can be included in blocks, not for query responses. This registration could lead to unexpected behavior when the codec tries to process these messages.
Recommended fix:
- interfaceRegistry.RegisterImplementations((*sdk.Msg)(nil),
- &txfeestypes.QueryEipBaseFeeResponse{},
- )
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
permissions.RegisterInterfaces(interfaceRegistry) | |
txfeestypes.RegisterInterfaces(interfaceRegistry) | |
interfaceRegistry.RegisterImplementations((*sdk.Msg)(nil), | |
&txfeestypes.QueryEipBaseFeeResponse{}, | |
) | |
permissions.RegisterInterfaces(interfaceRegistry) | |
txfeestypes.RegisterInterfaces(interfaceRegistry) |
…of using the default value
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: 2
🧹 Nitpick comments (48)
examples/chain/exchange/11_MsgCreateDerivativeMarketOrder/example.go (1)
132-135
: Redundant gas price adjustment.This second gas price adjustment appears redundant as:
- The gas fee was already retrieved at line 123 before this adjustment, so this won't affect the displayed gas fee
- The transaction was already queued for broadcasting at line 114
The gas price adjustment should only be needed once before transaction broadcast.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/auction/1_MsgBid/example.go (3)
93-96
: Redundant gas price setting after transaction is queuedThis second gas price adjustment occurs after the transaction has already been queued for broadcasting (line 76), so it has no effect on the current transaction. If this is intended to prepare for potential future transactions in the same session, please add a clarifying comment.
fmt.Println("gas fee:", gasFee, "INJ") +// Reset gas price for any subsequent transactions gasPrice = chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice)
59-62
: Consider adding error handling for gas price operationsThe calls to
CurrentChainGasPrice()
andSetGasPrice()
don't include error handling. While these methods might not return errors in the current implementation, adding error handling would make the code more robust against future changes.
59-62
: Consider making the gas price adjustment factor configurableThe 10% adjustment factor (1.1) is hardcoded. Consider making this value configurable or at least defined as a constant at the top of the file for easier maintenance.
+const ( + // GasPriceAdjustmentFactor ensures gas price remains valid if it changes between request and broadcast + GasPriceAdjustmentFactor = 1.1 +) // ... gasPrice = chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) +gasPrice = int64(float64(gasPrice) * GasPriceAdjustmentFactor) chainClient.SetGasPrice(gasPrice)examples/chain/authz/2_MsgExec/example.go (1)
142-146
: Consider removing redundant gas price settingThis second gas price adjustment appears redundant since it occurs after the transaction has already been queued (line 126) and the gas fee has already been retrieved (line 134). Changes to gas price at this point won't affect the already processed transaction.
fmt.Println("gas fee:", gasFee, "INJ") - -gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/authz/3_MsgRevoke/example.go (1)
88-92
: Consider removing redundant gas price settingThis second gas price adjustment appears redundant since it occurs after the transaction has already been queued (line 72) and the gas fee has already been retrieved (line 80). Changes to gas price at this point won't affect the already processed transaction.
fmt.Println("gas fee:", gasFee, "INJ") - -gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/authz/1_MsgGrant/example.go (1)
96-100
: Consider removing redundant gas price settingThis second gas price adjustment appears redundant since it occurs after the transaction has already been queued (line 80) and the gas fee has already been retrieved (line 88). Changes to gas price at this point won't affect the already processed transaction.
fmt.Println("gas fee:", gasFee, "INJ") - -gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/distribution/2_MsgWithdrawDelegatorReward/example.go (1)
84-87
: Unnecessary gas price adjustment at the end of function.This second gas price adjustment occurs after the transaction has been processed and the gas fee has been displayed. Since no further transactions are being sent after this point, this adjustment doesn't serve any purpose in the current implementation.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/distribution/3_WithdrawValidatorCommission/example.go (1)
78-81
: Redundant gas price adjustment after transaction completion.This second gas price adjustment is performed after the transaction has been broadcasted and the response has been printed. Since no further transactions are sent afterward, this adjustment doesn't affect any operations within the current function scope.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/ibc/transfer/1_MsgTransfer/example.go (1)
92-95
: Unnecessary gas price adjustment at function exit.This second gas price adjustment occurs after the transaction has already been broadcasted and the response has been printed. Since no further blockchain operations are performed after this point, this adjustment has no effect on the current execution flow.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go (1)
94-97
: Redundant gas price adjustment at the end of function.This second gas price adjustment occurs after the transaction has been completed and the response has been displayed. Since there are no subsequent transactions in this function, this adjustment doesn't affect any operations and can be safely removed.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/tokenfactory/4_MsgSetDenomMetadata/example.go (1)
103-106
: Consider if second gas price adjustment is necessary.The second adjustment of gas price at the end of the function might be unnecessary since there are no further transactions being broadcasted after this point. The adjustment here won't affect any operations in the current execution flow.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/exchange/17_MsgIncreasePositionMargin/example.go (1)
89-92
: Consider if second gas price adjustment is necessary.The second gas price adjustment occurs after all operations are completed (after printing the gas fee). Since there are no further transactions being sent after this point, this adjustment won't have any effect on the current execution.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/exchange/2_MsgWithdraw/example.go (1)
91-94
: Consider if second gas price adjustment is necessary.The second adjustment of gas price at the end of the function appears unnecessary since there are no further transactions being broadcasted after this point. Consider removing this code block if it doesn't serve any purpose in the current execution flow.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go (2)
91-94
: Consider if second gas price adjustment is necessary.The second adjustment of gas price at the end of the function appears unnecessary since there are no further transactions being broadcasted after this point. Consider removing this code block if it doesn't serve any purpose in the current execution flow.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)
57-94
: Consider extracting gas price adjustment logic to a reusable function.The gas price adjustment logic is duplicated across multiple files in this PR. Consider extracting this logic into a reusable helper function within the
chainClient
to improve maintainability and reduce duplication.For example, you could add a method to the
chainClient
:// AdjustGasPrice retrieves the current gas price and applies a safety margin // to account for price fluctuations between request time and broadcast time. func (c *chainClient) AdjustGasPrice(safetyFactor float64) { gasPrice := c.CurrentChainGasPrice() adjustedGasPrice := int64(float64(gasPrice) * safetyFactor) c.SetGasPrice(adjustedGasPrice) }Then in the examples, you could simply call:
chainClient.AdjustGasPrice(1.1)examples/chain/exchange/4_MsgInstantPerpetualMarketLaunch/example.go (1)
95-98
: Redundant gas price adjustment at the end of the functionThis second gas price adjustment appears unnecessary since the transaction has already been broadcasted at line 86. While this doesn't cause any functional issues, it has no effect as there are no subsequent transactions after this point.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/permissions/3_MsgUpdateActorRoles/example.go (1)
98-101
: Redundant gas price adjustment at the end of the functionThis second gas price adjustment appears unnecessary since the transaction has already been broadcasted at line 89. While this doesn't cause any functional issues, it has no effect as there are no subsequent transactions after this point.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/exchange/8_MsgCancelSpotOrder/example.go (1)
87-90
: Consider removing redundant gas price adjustmentThis second gas price adjustment appears unnecessary since:
- The transaction has already been queued at line 70
- The gas fee has already been retrieved at line 78
- There are no subsequent transactions after this point
While this doesn't cause functional issues, it has no effect on the program's execution.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/tokenfactory/3_MsgBurn/example.go (1)
80-83
: Redundant gas price adjustment at the end of the functionThis second gas price adjustment appears unnecessary since the transaction has already been broadcasted at line 71. While this doesn't cause any functional issues, it has no effect as there are no subsequent transactions after this point.
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)examples/chain/exchange/7_MsgCreateSpotMarketOrder/example.go (1)
129-132
: Consider consolidating duplicate gas price adjustment code.This is the second identical instance of gas price adjustment in this file. Consider extracting this logic into a helper function to avoid duplication and improve maintainability.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustChainGasPrice(chainClient)And add the helper function:
func adjustChainGasPrice(chainClient *chainclient.ChainClient) { gasPrice := chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice) }examples/chain/distribution/4_FundCommunityPool/example.go (1)
81-84
: Consider consolidating duplicate gas price adjustment code.This is the second identical instance of gas price adjustment in this file. Consider extracting this logic into a helper function to avoid duplication and improve maintainability.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustChainGasPrice(chainClient)And add the helper function:
func adjustChainGasPrice(chainClient *chainclient.ChainClient) { gasPrice := chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice) }examples/chain/permissions/4_MsgClaimVoucher/example.go (1)
80-83
: Consider consolidating duplicate gas price adjustment code.This is the second identical instance of gas price adjustment in this file. Consider extracting this logic into a helper function to avoid duplication and improve maintainability.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustChainGasPrice(chainClient)And add the helper function:
func adjustChainGasPrice(chainClient *chainclient.ChainClient) { gasPrice := chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice) }examples/chain/exchange/24_MsgUpdateSpotMarket/example.go (2)
91-94
: Consider consolidating duplicate gas price adjustment code.This is the second identical instance of gas price adjustment in this file. Consider extracting this logic into a helper function to avoid duplication and improve maintainability.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustChainGasPrice(chainClient)And add the helper function:
func adjustChainGasPrice(chainClient *chainclient.ChainClient) { gasPrice := chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice) }
1-96
: Consider creating a shared utility for gas price adjustment across example files.Since this same gas price adjustment logic is being implemented in multiple example files, consider creating a shared utility function in a common package that all example files can import and use.
For example, you could create a utility function in a common utils package:
// File: examples/chain/utils/gas.go package utils import ( chainclient "github.com/InjectiveLabs/sdk-go/client/chain" ) // AdjustGasPrice fetches the current chain gas price and adds a 10% buffer // to ensure the transaction remains valid if gas prices change func AdjustGasPrice(chainClient *chainclient.ChainClient) { gasPrice := chainClient.CurrentChainGasPrice() // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted gasPrice = int64(float64(gasPrice) * 1.1) chainClient.SetGasPrice(gasPrice) }Then in each example file:
import ( // other imports "github.com/InjectiveLabs/sdk-go/examples/chain/utils" ) // In main() utils.AdjustGasPrice(chainClient)This would make the code more maintainable and reduce duplication across the codebase.
examples/chain/13_BroadcastMsgWithoutSimulation/example.go (1)
108-111
: Consider removing unnecessary gas price adjustment.This gas price adjustment at the end of the function doesn't serve any purpose since there are no further transactions being broadcasted. Consider removing these lines to improve code clarity.
examples/chain/distribution/1_SetWithdrawAddress/example.go (1)
79-82
: Consider removing unnecessary gas price adjustment.This gas price adjustment at the end of the function doesn't serve any purpose since there are no further transactions being broadcasted. The transaction has already been sent on line 70 and the response has been printed. Consider removing these lines to improve code clarity.
examples/chain/4_MsgBatchCreateSpotLimitOrders/example.go (1)
130-133
: Consider removing unnecessary gas price adjustment.This gas price adjustment at the end of the function doesn't serve any purpose since there are no further transactions being broadcasted after this point. The transaction has already been queued on line 112 and the gas fee has been displayed. Consider removing these lines to improve code clarity.
examples/chain/exchange/23_MsgDecreasePositionMargin/example.go (1)
83-86
: Consider removing unnecessary gas price adjustment.This gas price adjustment at the end of the function doesn't serve any purpose since there are no further transactions being broadcasted. The transaction has already been sent on line 74 and the response has been printed. Consider removing these lines to improve code clarity.
examples/chain/exchange/5_MsgInstantExpiryFuturesMarketLaunch/example.go (1)
96-99
: Consider removing redundant gas price adjustment.This second gas price adjustment occurs after the transaction has already been broadcast, so it won't affect the current transaction. If this is intended for subsequent transactions (not shown in this example), consider adding a comment explaining this purpose or removing it if unnecessary.
examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go (1)
83-86
: Evaluate necessity of post-broadcast gas price adjustment.This gas price adjustment happens after the transaction has already been broadcast, so it doesn't affect the current transaction. Unless there are subsequent transactions in real-world implementation, consider removing this redundant code.
examples/chain/exchange/16_MsgLiquidatePosition/example.go (1)
109-112
: Consider purpose of post-broadcast gas price adjustment.Since this gas price adjustment occurs after transaction broadcast, it has no effect on the current transaction. If this pattern is being consistently applied across examples for a specific reason (e.g., preparing for subsequent transactions), consider adding clarifying documentation.
examples/chain/6_MsgRegisterAsDMM/example.go (1)
84-87
: Evaluate necessity of post-operation gas price adjustment.This gas price adjustment occurs after the transaction has been queued and the gas fee displayed, so it has no visible effect in this example. Consider whether this is intended for future operations not shown in the example or if it can be removed.
examples/chain/staking/1_MsgDelegate/example.go (1)
89-92
: Consider refactoring duplicated gas price adjustment logicThis is a duplicate of the gas price adjustment logic at lines 59-62. Since this adjustment happens after displaying the gas fee and no subsequent transactions are being sent, it appears to be redundant. Consider either adding a comment explaining its purpose or extracting this repeated logic into a helper function if it's intentionally needed in both places.
+ // Helper function to adjust gas price with buffer + func adjustGasPrice(chainClient *chainclient.ChainClient) { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } + // In the main function, replace both occurrences with: + adjustGasPrice(chainClient)examples/chain/2_MsgBatchCancelSpotOrders/example.go (1)
102-105
: Consider refactoring duplicated gas price adjustment logicThis block duplicates the gas price adjustment logic from lines 58-61. Since no further transactions are sent after this point, this adjustment may be unnecessary. Consider refactoring this repeated pattern across your examples by extracting it into a helper function or adding comments to explain why it's needed after transaction broadcasting.
+ // Helper function to adjust gas price with buffer + func adjustGasPrice(chainClient *chainclient.ChainClient) { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } + // In the main function, replace both occurrences with: + adjustGasPrice(chainClient)examples/chain/8_OfflineSigning/example.go (1)
130-133
: Consider refactoring duplicated gas price adjustment logicThis block duplicates the gas price adjustment from lines 73-76. Since there are no more transactions being sent after this point, it appears unnecessary. Consider either:
- Removing this second adjustment if it serves no purpose
- Adding a comment explaining why it's needed
- Extracting both occurrences into a helper function to improve maintainability
+ // Helper function to adjust gas price with buffer + func adjustGasPrice(chainClient *chainclient.ChainClient) { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + }examples/chain/exchange/27_MsgActivateStakeGrant/example.go (1)
77-80
: Consider refactoring duplicated gas price adjustment logicThis is a duplicate of the gas price logic at lines 57-60. Since there are no more transactions after displaying the response, this second adjustment appears unnecessary. Consider either removing it, explaining its presence with a comment, or refactoring both occurrences into a helper function to improve maintainability across all your example files.
+ // Helper function to adjust gas price with buffer + func adjustGasPrice(chainClient *chainclient.ChainClient) { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } + // In the main function, replace both occurrences with: + adjustGasPrice(chainClient)examples/chain/tokenfactory/5_MsgChangeAdmin/example.go (1)
77-80
: Consider refactoring the duplicated gas price adjustment logic.The gas price adjustment logic appears twice in this file with identical implementation. Consider extracting this into a helper function to improve maintainability and reduce code duplication.
func main() { + // Helper function to adjust and set gas price + adjustGasPrice := func() { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } network := common.LoadNetwork("testnet", "lb") // ... existing code ... - gasPrice := chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() // ... existing code ... - gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() }examples/chain/3_MsgBatchCancelDerivativeOrders/example.go (1)
102-105
: Consider refactoring the duplicated gas price adjustment logic.The gas price adjustment logic appears twice in this file with identical implementation. Consider extracting this into a helper function to improve maintainability and reduce code duplication.
func main() { + // Helper function to adjust and set gas price + adjustGasPrice := func() { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } network := common.LoadNetwork("testnet", "lb") // ... existing code ... - gasPrice := chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() // ... existing code ... - gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() }examples/chain/exchange/1_MsgDeposit/example.go (1)
88-91
: Consider refactoring the duplicated gas price adjustment logic.The gas price adjustment logic appears twice in this file with identical implementation. Consider extracting this into a helper function to improve maintainability and reduce code duplication.
func main() { + // Helper function to adjust and set gas price + adjustGasPrice := func() { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } network := common.LoadNetwork("testnet", "lb") // ... existing code ... - gasPrice := chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() // ... existing code ... - gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() }examples/chain/exchange/12_MsgCancelDerivativeOrder/example.go (1)
87-90
: Consider refactoring the duplicated gas price adjustment logic.The gas price adjustment logic appears twice in this file with identical implementation. Consider extracting this into a helper function to improve maintainability and reduce code duplication.
func main() { + // Helper function to adjust and set gas price + adjustGasPrice := func() { + gasPrice := chainClient.CurrentChainGasPrice() + // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted + gasPrice = int64(float64(gasPrice) * 1.1) + chainClient.SetGasPrice(gasPrice) + } network := common.LoadNetwork("testnet", "lb") // ... existing code ... - gasPrice := chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() // ... existing code ... - gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice) + adjustGasPrice() }examples/chain/permissions/2_MsgUpdateNamespace/example.go (1)
117-121
: Consider consolidating duplicate gas price adjustment logicThis second gas price adjustment block is identical to the one at lines 57-60. Since you're already setting the gas price before the transaction is sent, this second adjustment may be unnecessary unless you're planning to send more transactions afterward.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/permissions/1_MsgCreateNamespace/example.go (1)
139-142
: Consider consolidating duplicate gas price adjustment logicThis second gas price adjustment block is identical to the one at lines 56-59. The second adjustment appears unnecessary unless you're planning to send additional transactions afterward, as the first adjustment already handles gas price for the current transaction.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/exchange/14_MsgSubaccountTransfer/example.go (1)
91-94
: Consider the necessity of the second gas price adjustmentSince this example is using QueueBroadcastMsg at line 74 and the transaction is already in the queue before reaching this second adjustment, this second gas price update won't affect the already queued transaction. Consider removing it if it doesn't serve a specific purpose.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/exchange/15_MsgExternalTransfer/example.go (1)
91-94
: Second gas price adjustment may be unnecessarySince the transaction has already been queued at line 74 before this second gas price adjustment, updating the gas price here won't affect the already queued transaction. Consider removing this block unless it serves a specific purpose for subsequent operations.
- gasPrice = chainClient.CurrentChainGasPrice() - // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted - gasPrice = int64(float64(gasPrice) * 1.1) - chainClient.SetGasPrice(gasPrice)examples/chain/wasmx/1_MsgExecuteContractCompat/example.go (1)
94-98
: Unnecessary gas price adjustment after transaction broadcast.The gas price adjustment here occurs after the transaction has already been broadcast with
AsyncBroadcastMsg
, so it won't affect the already submitted transaction.Unless you're preparing for a subsequent transaction in the same function (which isn't present), this adjustment can be removed:
-gasPrice = chainClient.CurrentChainGasPrice() -// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted -gasPrice = int64(float64(gasPrice) * 1.1) -chainClient.SetGasPrice(gasPrice)client/chain/chain_test_support.go (1)
845-847
: Implemented minimal gas price setter for interface compliance.The current implementation is a no-op as indicated by the comment. For more robust testing:
Consider storing the gas price value to allow tests to verify it was set correctly:
+// Track gas price for test verification +var mockGasPrice int64 + func (c *MockChainClient) SetGasPrice(gasPrice int64) { - // do nothing + mockGasPrice = gasPrice }client/chain/chain.go (1)
2768-2787
: New methods: CurrentChainGasPrice and SetGasPrice
These allow dynamic retrieval and configuration of gas price. Consider validating against negative or zero values inSetGasPrice
.func (c *chainClient) SetGasPrice(gasPrice int64) { + if gasPrice <= 0 { + c.logger.Warnf("Received non-positive gasPrice %d, ignoring it and using default", gasPrice) + return + } gasPrices := fmt.Sprintf("%v%s", gasPrice, injectiveclient.InjDenom) c.txFactory = c.txFactory.WithGasPrices(gasPrices) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (59)
Makefile
(3 hunks)auth_vote/authz_vote.go
(2 hunks)client/chain/chain.go
(8 hunks)client/chain/chain_test_support.go
(2 hunks)examples/chain/13_BroadcastMsgWithoutSimulation/example.go
(2 hunks)examples/chain/1_LocalOrderHash/example.go
(2 hunks)examples/chain/2_MsgBatchCancelSpotOrders/example.go
(2 hunks)examples/chain/3_MsgBatchCancelDerivativeOrders/example.go
(2 hunks)examples/chain/4_MsgBatchCreateSpotLimitOrders/example.go
(2 hunks)examples/chain/5_MsgBatchCreateDerivativeLimitOrders/example.go
(2 hunks)examples/chain/6_MsgRegisterAsDMM/example.go
(3 hunks)examples/chain/8_OfflineSigning/example.go
(2 hunks)examples/chain/auction/1_MsgBid/example.go
(3 hunks)examples/chain/authz/1_MsgGrant/example.go
(3 hunks)examples/chain/authz/2_MsgExec/example.go
(2 hunks)examples/chain/authz/3_MsgRevoke/example.go
(3 hunks)examples/chain/bank/1_MsgSend/example.go
(3 hunks)examples/chain/bank/2_MsgMultiSend/example.go
(3 hunks)examples/chain/distribution/1_SetWithdrawAddress/example.go
(3 hunks)examples/chain/distribution/2_MsgWithdrawDelegatorReward/example.go
(3 hunks)examples/chain/distribution/3_WithdrawValidatorCommission/example.go
(3 hunks)examples/chain/distribution/4_FundCommunityPool/example.go
(3 hunks)examples/chain/exchange/10_MsgCreateDerivativeLimitOrder/example.go
(2 hunks)examples/chain/exchange/11_MsgCreateDerivativeMarketOrder/example.go
(2 hunks)examples/chain/exchange/12_MsgCancelDerivativeOrder/example.go
(3 hunks)examples/chain/exchange/13_MsgInstantBinaryOptionsMarketLaunch/example.go
(2 hunks)examples/chain/exchange/14_MsgSubaccountTransfer/example.go
(3 hunks)examples/chain/exchange/15_MsgExternalTransfer/example.go
(3 hunks)examples/chain/exchange/16_MsgLiquidatePosition/example.go
(2 hunks)examples/chain/exchange/17_MsgIncreasePositionMargin/example.go
(3 hunks)examples/chain/exchange/1_MsgDeposit/example.go
(3 hunks)examples/chain/exchange/21_MsgRewardsOptOut/example.go
(3 hunks)examples/chain/exchange/23_MsgDecreasePositionMargin/example.go
(3 hunks)examples/chain/exchange/24_MsgUpdateSpotMarket/example.go
(2 hunks)examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go
(2 hunks)examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go
(2 hunks)examples/chain/exchange/27_MsgActivateStakeGrant/example.go
(2 hunks)examples/chain/exchange/2_MsgWithdraw/example.go
(2 hunks)examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go
(2 hunks)examples/chain/exchange/4_MsgInstantPerpetualMarketLaunch/example.go
(2 hunks)examples/chain/exchange/5_MsgInstantExpiryFuturesMarketLaunch/example.go
(2 hunks)examples/chain/exchange/6_MsgCreateSpotLimitOrder/example.go
(2 hunks)examples/chain/exchange/7_MsgCreateSpotMarketOrder/example.go
(2 hunks)examples/chain/exchange/8_MsgCancelSpotOrder/example.go
(2 hunks)examples/chain/exchange/9_MsgBatchUpdateOrders/example.go
(2 hunks)examples/chain/ibc/transfer/1_MsgTransfer/example.go
(2 hunks)examples/chain/oracle/1_MsgRelayPriceFeedPrice/example.go
(2 hunks)examples/chain/peggy/1_MsgSendToEth/example.go
(2 hunks)examples/chain/permissions/1_MsgCreateNamespace/example.go
(2 hunks)examples/chain/permissions/2_MsgUpdateNamespace/example.go
(3 hunks)examples/chain/permissions/3_MsgUpdateActorRoles/example.go
(2 hunks)examples/chain/permissions/4_MsgClaimVoucher/example.go
(2 hunks)examples/chain/staking/1_MsgDelegate/example.go
(2 hunks)examples/chain/tokenfactory/1_CreateDenom/example.go
(2 hunks)examples/chain/tokenfactory/2_MsgMint/example.go
(2 hunks)examples/chain/tokenfactory/3_MsgBurn/example.go
(2 hunks)examples/chain/tokenfactory/4_MsgSetDenomMetadata/example.go
(3 hunks)examples/chain/tokenfactory/5_MsgChangeAdmin/example.go
(3 hunks)examples/chain/wasmx/1_MsgExecuteContractCompat/example.go
(3 hunks)
🧰 Additional context used
🧬 Code Definitions (55)
examples/chain/distribution/3_WithdrawValidatorCommission/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/tokenfactory/1_CreateDenom/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/6_MsgRegisterAsDMM/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/5_MsgBatchCreateDerivativeLimitOrders/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/2_MsgBatchCancelSpotOrders/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/staking/1_MsgDelegate/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/3_MsgBatchCancelDerivativeOrders/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/permissions/2_MsgUpdateNamespace/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/distribution/4_FundCommunityPool/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/4_MsgInstantPerpetualMarketLaunch/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/peggy/1_MsgSendToEth/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/bank/2_MsgMultiSend/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/9_MsgBatchUpdateOrders/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/10_MsgCreateDerivativeLimitOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/distribution/1_SetWithdrawAddress/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/ibc/transfer/1_MsgTransfer/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/tokenfactory/5_MsgChangeAdmin/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/1_MsgDeposit/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/2_MsgWithdraw/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/23_MsgDecreasePositionMargin/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/15_MsgExternalTransfer/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/tokenfactory/3_MsgBurn/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/5_MsgInstantExpiryFuturesMarketLaunch/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/8_MsgCancelSpotOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/distribution/2_MsgWithdrawDelegatorReward/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/6_MsgCreateSpotLimitOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/authz/1_MsgGrant/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/8_OfflineSigning/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/wasmx/1_MsgExecuteContractCompat/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/11_MsgCreateDerivativeMarketOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
auth_vote/authz_vote.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/27_MsgActivateStakeGrant/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/bank/1_MsgSend/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/permissions/3_MsgUpdateActorRoles/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/oracle/1_MsgRelayPriceFeedPrice/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/14_MsgSubaccountTransfer/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/13_MsgInstantBinaryOptionsMarketLaunch/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/4_MsgBatchCreateSpotLimitOrders/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/1_LocalOrderHash/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/authz/3_MsgRevoke/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/auction/1_MsgBid/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/21_MsgRewardsOptOut/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/tokenfactory/4_MsgSetDenomMetadata/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/7_MsgCreateSpotMarketOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/12_MsgCancelDerivativeOrder/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/authz/2_MsgExec/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/exchange/16_MsgLiquidatePosition/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
client/chain/chain_test_support.go (1)
chain/txfees/types/query.pb.go (6)
QueryParamsResponse
(108-111)QueryParamsResponse
(115-115)QueryParamsResponse
(116-118)QueryEipBaseFeeResponse
(189-191)QueryEipBaseFeeResponse
(195-195)QueryEipBaseFeeResponse
(196-198)
examples/chain/tokenfactory/2_MsgMint/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
client/chain/chain.go (1)
chain/txfees/types/query.pb.go (13)
QueryParamsResponse
(108-111)QueryParamsResponse
(115-115)QueryParamsResponse
(116-118)QueryEipBaseFeeResponse
(189-191)QueryEipBaseFeeResponse
(195-195)QueryEipBaseFeeResponse
(196-198)QueryClient
(289-295)QueryParamsRequest
(71-72)QueryParamsRequest
(76-76)QueryParamsRequest
(77-79)QueryEipBaseFeeRequest
(153-154)QueryEipBaseFeeRequest
(158-158)QueryEipBaseFeeRequest
(159-161)
examples/chain/exchange/17_MsgIncreasePositionMargin/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
examples/chain/permissions/1_MsgCreateNamespace/example.go (1)
client/chain/chain.go (1)
chainClient
(319-364)
🔇 Additional comments (96)
examples/chain/exchange/11_MsgCreateDerivativeMarketOrder/example.go (1)
61-64
: Good addition of gas price adjustment logic.Adding a 10% buffer to the gas price is a robust defensive programming practice. This accounts for potential gas price fluctuations between request time and transaction broadcast time, helping to prevent transaction failures due to insufficient gas.
examples/chain/exchange/10_MsgCreateDerivativeLimitOrder/example.go (1)
60-63
: Improvement: Added dynamic gas price adjustmentThis change enhances transaction reliability by adding a 10% buffer to the current chain gas price. This approach is more flexible than using a fixed gas price and helps ensure transactions don't fail due to gas price fluctuations between price retrieval and transaction broadcast.
examples/chain/auction/1_MsgBid/example.go (2)
8-14
: Imports refactoring looks goodThe code now properly imports specific packages with clear aliases, improving code readability by making the origin of types more explicit. This follows good Go practices of organizing imports with external dependencies first, followed by internal packages.
59-62
: Dynamic gas price handling is a good improvementDynamically retrieving the current chain gas price and adding a 10% buffer is a better approach than using a fixed default value. This ensures the transaction remains valid even if gas prices fluctuate between preparation and broadcast time.
examples/chain/authz/2_MsgExec/example.go (1)
78-81
: Gas price adjustment improves transaction reliabilityThe addition of dynamic gas price calculation is a good improvement over using static default values. The 1.1x multiplier provides a safety margin to ensure transactions have sufficient gas even if prices fluctuate between retrieval and broadcast time.
examples/chain/authz/3_MsgRevoke/example.go (2)
10-12
: Import reorganization looks goodThe removal of unnecessary imports and reordering of the remaining imports improves code cleanliness.
57-60
: Gas price adjustment approach is consistentThe implementation of dynamic gas price retrieval and adjustment is consistent with the pattern used across other files in this PR, which is good for maintainability.
examples/chain/authz/1_MsgGrant/example.go (2)
8-11
: Import cleanup looks goodThe removal of unnecessary imports and reordering of the remaining imports improves code cleanliness.
56-59
: Dynamic gas price handling is a good improvementSwitching from static default gas prices to dynamically retrieving and adjusting current chain gas prices is a good approach for ensuring transactions have sufficient gas, even as network conditions change.
examples/chain/exchange/13_MsgInstantBinaryOptionsMarketLaunch/example.go (1)
58-61
: Appropriate gas price adjustment to ensure transaction validity.The gas price adjustment logic is a good addition as it helps ensure transactions remain valid even if gas prices fluctuate between creation and broadcasting. The 10% increase provides a reasonable buffer.
examples/chain/distribution/2_MsgWithdrawDelegatorReward/example.go (1)
57-60
: Improved gas price handling to ensure transaction validity.The code now dynamically retrieves the current chain gas price and increases it by 10% to account for potential price fluctuations between retrieval time and transaction broadcast. This is a more robust approach compared to using static gas prices.
examples/chain/distribution/3_WithdrawValidatorCommission/example.go (1)
57-60
: Dynamic gas price handling implemented correctly.The code now dynamically retrieves the current chain gas price and increases it by 10% to ensure the transaction remains valid even if gas prices fluctuate slightly. This is a more robust approach than using static default gas prices.
examples/chain/ibc/transfer/1_MsgTransfer/example.go (1)
59-62
: Improved gas price handling with dynamic adjustment.The implementation now dynamically retrieves and adjusts the gas price based on current chain conditions, rather than using static default values. The 10% increase provides a buffer against price fluctuations during transaction preparation and broadcasting.
examples/chain/exchange/3_MsgInstantSpotMarketLaunch/example.go (1)
57-60
: Effective dynamic gas price management implemented.The code now retrieves the current gas price from the blockchain and applies a 10% increase to ensure it remains valid during transaction broadcasting. This approach is more resilient to gas price fluctuations compared to using static default values.
examples/chain/tokenfactory/4_MsgSetDenomMetadata/example.go (2)
8-10
: Imports added to support gas price adjustments.The addition of the
banktypes
import is appropriate as it's needed for thebanktypes.DenomUnit
andbanktypes.Metadata
used in the function. The rpchttp import is also properly used for client initialization.
57-60
: Gas price adjustment implemented correctly.This addition dynamically retrieves and adjusts the current chain gas price by a 10% margin to handle potential gas price fluctuations between request and broadcast times. This is a good improvement over using static gas prices.
examples/chain/exchange/17_MsgIncreasePositionMargin/example.go (1)
58-61
: Gas price adjustment implemented correctly.This addition dynamically retrieves and adjusts the gas price by multiplying it by 1.1 (10% increase) to handle potential fluctuations. This is a good improvement over using static gas prices.
examples/chain/exchange/2_MsgWithdraw/example.go (1)
60-64
: Gas price adjustment implemented correctly.This addition dynamically retrieves and adjusts the current chain gas price by a 10% margin to handle potential gas price fluctuations between request and broadcast times. This is a good improvement over using static gas prices.
examples/chain/exchange/25_MsgUpdateDerivativeMarket/example.go (1)
57-60
: Gas price adjustment implemented correctly.This addition dynamically retrieves and adjusts the current chain gas price by a 10% margin to handle potential gas price fluctuations between request and broadcast times. This is a good improvement over using static gas prices.
examples/chain/peggy/1_MsgSendToEth/example.go (2)
59-63
: Dynamic gas price adjustment implementedThis addition improves transaction reliability by adjusting the gas price to account for potential fluctuations between when the price is requested and when the transaction is broadcasted. The 10% buffer (multiplying by 1.1) provides a safety margin to ensure transactions don't fail due to gas price changes.
97-101
: Secondary gas price adjustment before completionGood practice to re-adjust the gas price after the transaction has been processed. This ensures that any subsequent transactions (if they were to be executed after this point) would also use an up-to-date gas price with the same safety buffer.
examples/chain/exchange/9_MsgBatchUpdateOrders/example.go (2)
62-65
: Dynamic gas price adjustment implementedThe addition of gas price adjustment logic improves transaction reliability by accounting for potential fluctuations in gas prices. The 10% buffer provides a safety margin to ensure the transaction doesn't fail due to changing network conditions.
155-158
: Secondary gas price adjustment before completionThis second adjustment ensures that the gas price is up-to-date after the transaction has been processed. While this specific adjustment won't affect the current transaction (which has already been queued), it ensures the client has the latest gas price for any potential subsequent operations.
examples/chain/exchange/6_MsgCreateSpotLimitOrder/example.go (2)
59-63
: Dynamic gas price adjustment implementedThe addition of gas price adjustment logic improves transaction reliability. By retrieving the current chain gas price and adding a 10% buffer, the code ensures that transactions will still be valid even if gas prices fluctuate between request and broadcast time.
130-133
: Secondary gas price adjustment before completionThis second adjustment ensures the
chainClient
has the most up-to-date gas price with buffer before the example completes. While not directly used in this example after this point, it maintains consistency with other examples and ensures the client is left in a proper state.examples/chain/tokenfactory/1_CreateDenom/example.go (2)
55-59
: Dynamic gas price adjustment implementedThe addition of gas price adjustment logic improves transaction reliability by accounting for potential fluctuations in gas prices. The 10% buffer provides a safety margin to ensure the transaction doesn't fail due to changing network conditions.
77-80
: Secondary gas price adjustment before completionThis second adjustment ensures that the gas price is up-to-date after the transaction has been processed. This is good practice for maintaining the client state, even though the example ends shortly after this adjustment.
examples/chain/exchange/4_MsgInstantPerpetualMarketLaunch/example.go (1)
58-61
: Gas price adjustment added for transaction reliabilityThis change introduces dynamic gas price calculation with a safety margin to ensure transactions remain valid even if gas prices fluctuate between request time and broadcast time. The 10% buffer (multiplying by 1.1) provides a sensible cushion for network congestion.
examples/chain/permissions/3_MsgUpdateActorRoles/example.go (1)
58-61
: Gas price adjustment added for transaction reliabilityThis change introduces dynamic gas price calculation with a safety margin to ensure transactions remain valid even if gas prices fluctuate between request time and broadcast time. The 10% buffer (multiplying by 1.1) provides a sensible cushion for network congestion.
examples/chain/exchange/8_MsgCancelSpotOrder/example.go (1)
57-60
: Gas price adjustment added for transaction reliabilityThis change introduces dynamic gas price calculation with a safety margin to ensure transactions remain valid even if gas prices fluctuate between request time and broadcast time. The 10% buffer (multiplying by 1.1) provides a sensible cushion for network congestion.
examples/chain/tokenfactory/3_MsgBurn/example.go (1)
58-61
: Gas price adjustment added for transaction reliabilityThis change introduces dynamic gas price calculation with a safety margin to ensure transactions remain valid even if gas prices fluctuate between request time and broadcast time. The 10% buffer (multiplying by 1.1) provides a sensible cushion for network congestion.
examples/chain/exchange/7_MsgCreateSpotMarketOrder/example.go (1)
60-63
: Improved gas price handling for transaction stability.The addition of dynamic gas price adjustment is a good improvement. By fetching the current chain gas price and adding a 10% buffer, you ensure the transaction remains valid even if gas prices fluctuate between request and broadcast time.
examples/chain/distribution/4_FundCommunityPool/example.go (1)
59-62
: Improved gas price handling for transaction stability.The addition of dynamic gas price adjustment is a good improvement. By fetching the current chain gas price and adding a 10% buffer, you ensure the transaction remains valid even if gas prices fluctuate between request and broadcast time.
examples/chain/permissions/4_MsgClaimVoucher/example.go (1)
58-61
: Improved gas price handling for transaction stability.The addition of dynamic gas price adjustment is a good improvement. By fetching the current chain gas price and adding a 10% buffer, you ensure the transaction remains valid even if gas prices fluctuate between request and broadcast time.
examples/chain/exchange/24_MsgUpdateSpotMarket/example.go (1)
57-60
: Improved gas price handling for transaction stability.The addition of dynamic gas price adjustment is a good improvement. By fetching the current chain gas price and adding a 10% buffer, you ensure the transaction remains valid even if gas prices fluctuate between request and broadcast time.
examples/chain/13_BroadcastMsgWithoutSimulation/example.go (1)
64-67
: Approve the dynamic gas price adjustment approach.Using
CurrentChainGasPrice()
with a 1.1 multiplier is a more robust approach than using static gas prices, as it accounts for potential fluctuations between requesting and broadcasting transactions.examples/chain/distribution/1_SetWithdrawAddress/example.go (1)
57-60
: Approve the dynamic gas price adjustment approach.Using
CurrentChainGasPrice()
with a 1.1 multiplier is a more robust approach than using static gas prices, as it accounts for potential fluctuations between requesting and broadcasting transactions.examples/chain/4_MsgBatchCreateSpotLimitOrders/example.go (1)
62-65
: Approve the dynamic gas price adjustment approach.Using
CurrentChainGasPrice()
with a 1.1 multiplier is a more robust approach than using static gas prices, as it accounts for potential fluctuations between requesting and broadcasting transactions.examples/chain/exchange/23_MsgDecreasePositionMargin/example.go (1)
58-61
: Approve the dynamic gas price adjustment approach.Using
CurrentChainGasPrice()
with a 1.1 multiplier is a more robust approach than using static gas prices, as it accounts for potential fluctuations between requesting and broadcasting transactions.examples/chain/exchange/5_MsgInstantExpiryFuturesMarketLaunch/example.go (1)
58-61
: Improved gas price handling for transaction reliability.The implementation dynamically retrieves the current chain gas price and applies a 10% buffer, which is a good practice to ensure transactions remain valid even if gas prices fluctuate between retrieval and broadcast time.
examples/chain/exchange/26_MsgAuthorizeStakeGrants/example.go (1)
58-61
: LGTM: Dynamic gas price adjustment enhances transaction reliability.The implementation correctly retrieves the current chain gas price and adds a 10% buffer to handle potential gas price fluctuations during transaction submission.
examples/chain/exchange/16_MsgLiquidatePosition/example.go (1)
60-63
: LGTM: Proper dynamic gas price implementation.This approach of retrieving the current gas price and adding a 10% margin ensures the transaction remains valid despite potential gas price fluctuations between retrieval and broadcast.
examples/chain/6_MsgRegisterAsDMM/example.go (1)
57-60
: LGTM: Improved dynamic gas price handling.This implementation properly retrieves and adjusts the current chain gas price with a buffer, which is more robust than using static gas prices.
examples/chain/staking/1_MsgDelegate/example.go (1)
59-62
: Good enhancement to dynamically adjust gas pricesThe implementation now dynamically fetches the current chain gas price and adjusts it by a 10% buffer, making transactions more resilient to gas price fluctuations that may occur between price retrieval and transaction broadcast.
examples/chain/2_MsgBatchCancelSpotOrders/example.go (1)
58-61
: Good implementation of dynamic gas price adjustmentThe code now fetches the current chain gas price and applies a safety buffer of 10% to account for potential fluctuations between when the price is requested and when the transaction is broadcasted. This is a more robust approach than using a static gas price.
examples/chain/8_OfflineSigning/example.go (1)
73-76
: Good enhancement to dynamically adjust gas pricesThe implementation now dynamically fetches the current chain gas price and adjusts it by a 10% buffer, which is more resilient to market conditions than using a static gas price. This is particularly important for offline signing, as there may be a significant time gap between transaction creation and broadcasting.
examples/chain/exchange/27_MsgActivateStakeGrant/example.go (1)
57-60
: Good enhancement to dynamically adjust gas pricesThe implementation now dynamically fetches the current chain gas price and adds a 10% buffer, making transactions more resilient to gas price fluctuations that may occur between price retrieval and transaction broadcast.
examples/chain/1_LocalOrderHash/example.go (2)
60-63
: Dynamic gas price adjustment - good approachThe implementation properly retrieves the current chain gas price and applies a 10% buffer to account for potential fluctuations between price retrieval and transaction broadcast time. This is a significant improvement over using static gas prices.
137-140
: Gas price adjustment before transaction completionThis second adjustment ensures the gas price remains appropriate even after the delay from the previous operations. Consistent with the pattern applied across the codebase.
examples/chain/bank/1_MsgSend/example.go (3)
12-14
: Import cleanup looks goodThe imports have been properly adjusted, removing unnecessary dependencies while maintaining the required ones for the new gas price management approach.
58-61
: Dynamic gas price adjustment implemented correctlyThe implementation properly retrieves the current chain gas price and applies a 10% buffer to account for potential fluctuations between price retrieval and transaction broadcast time. This replaces the previous static gas price approach.
90-93
: Second gas price adjustment ensures accuracyApplying a second gas price adjustment before completing execution ensures the transaction uses up-to-date gas prices even after potential delays from previous operations.
auth_vote/authz_vote.go (2)
61-64
: Dynamic gas price adjustment implemented correctlyThe implementation properly retrieves the current chain gas price and applies a 10% buffer to account for potential fluctuations between price retrieval and transaction broadcast time. This replaces the previous static approach that used
txFactory.WithGasPrices
.
104-107
: Second gas price adjustment ensures accuracyApplying a second gas price adjustment after the transaction broadcast ensures any subsequent operations use up-to-date gas prices even after potential delays.
examples/chain/exchange/21_MsgRewardsOptOut/example.go (3)
8-12
: Import cleanup looks goodThe imports have been properly adjusted, removing unnecessary dependencies while maintaining the required ones for the new gas price management approach.
57-60
: Dynamic gas price adjustment implemented correctlyThe implementation properly retrieves the current chain gas price and applies a 10% buffer to account for potential fluctuations between price retrieval and transaction broadcast time.
84-87
: Second gas price adjustment maintains consistencyApplying a second gas price adjustment before completing execution ensures the gas price remains appropriate even after the delay from previous operations, consistent with the pattern applied across other examples.
examples/chain/oracle/1_MsgRelayPriceFeedPrice/example.go (2)
58-61
: Improved dynamic gas price handling introduced.This change implements a more robust gas price management strategy by dynamically retrieving the current chain gas price and applying a 10% buffer. This approach ensures transaction validity even when gas prices fluctuate between request time and broadcast time.
83-86
: Second gas price adjustment ensures latest values are used.Adding another gas price adjustment before transaction broadcast is a good practice to ensure the most up-to-date gas prices are used, especially when there might be delays between client initialization and transaction submission.
examples/chain/tokenfactory/2_MsgMint/example.go (2)
58-61
: Improved dynamic gas price handling introduced.This change implements a more robust gas price management strategy by dynamically retrieving the current chain gas price and applying a 10% buffer. This approach ensures transaction validity even when gas prices fluctuate between request time and broadcast time.
80-83
: Second gas price adjustment ensures latest values are used.Adding another gas price adjustment after response processing is a good practice to ensure the most up-to-date gas prices are used for any subsequent operations.
examples/chain/bank/2_MsgMultiSend/example.go (3)
12-14
: Import organization improved.The reorganization of imports removes the unused client package and keeps related imports grouped together, improving code clarity.
61-64
: Improved dynamic gas price handling introduced.This change implements a more robust gas price management strategy by dynamically retrieving the current chain gas price and applying a 10% buffer. This approach ensures transaction validity even when gas prices fluctuate between request time and broadcast time.
117-120
: Second gas price adjustment ensures latest values are used.Adding another gas price adjustment after displaying the gas fee is a good practice to ensure the most up-to-date gas prices are used for any subsequent operations.
examples/chain/5_MsgBatchCreateDerivativeLimitOrders/example.go (2)
62-65
: Improved dynamic gas price handling introduced.This change implements a more robust gas price management strategy by dynamically retrieving the current chain gas price and applying a 10% buffer. This approach ensures transaction validity even when gas prices fluctuate between request time and broadcast time.
135-138
: Second gas price adjustment ensures latest values are used.Adding another gas price adjustment after displaying the gas fee is a good practice to ensure the most up-to-date gas prices are used for any subsequent operations.
examples/chain/tokenfactory/5_MsgChangeAdmin/example.go (2)
8-9
: Import reorganization looks good.The addition of the direct rpchttp import is correctly positioned with proper spacing.
56-60
: Improved gas price handling approach.This change implements a dynamic gas price strategy by retrieving the current chain gas price and applying a 10% buffer. This approach is more robust than using a static default gas price as it adapts to network conditions.
examples/chain/3_MsgBatchCancelDerivativeOrders/example.go (1)
58-61
: Improved gas price handling approach.This change implements a dynamic gas price strategy by retrieving the current chain gas price and applying a 10% buffer. This approach is more robust than using a static default gas price as it adapts to network conditions.
examples/chain/exchange/1_MsgDeposit/example.go (2)
9-10
: Import reorganization looks good.The reorganization of the imports enhances code readability while maintaining proper grouping of the imports.
Also applies to: 14-14
57-61
: Improved gas price handling approach.This change implements a dynamic gas price strategy by retrieving the current chain gas price and applying a 10% buffer. This approach is more robust than using a static default gas price as it adapts to network conditions.
examples/chain/exchange/12_MsgCancelDerivativeOrder/example.go (3)
8-9
: Import reorganization looks good.The reorganization of the imports enhances code readability while maintaining proper grouping of the imports.
Also applies to: 12-12
57-61
: Improved gas price handling approach.This change implements a dynamic gas price strategy by retrieving the current chain gas price and applying a 10% buffer. This approach is more robust than using a static default gas price as it adapts to network conditions.
1-92
:✅ Verification successful
Verify the second gas price adjustment is necessary.
The second gas price adjustment occurs after the transaction has already been sent (
QueueBroadcastMsg
) and the gas fee has been retrieved and displayed. Since this appears to be at the end of the program execution, it's unclear whether this adjustment serves any purpose.
🏁 Script executed:
#!/bin/bash # Description: Search for similar pattern of gas price adjustments at the end of example files # Look for files that make a second gas price adjustment at the end rg -A 3 "fmt.Println\(\"gas fee:\"" --glob="examples/chain/**/*.go" | rg -A 5 "gasPrice = chainClient.CurrentChainGasPrice\(\)"Length of output: 11849
Clarification on the Gas Price Update
After reviewing the code and checking across multiple examples, it appears that the second gas price adjustment in this file is intentional. Although the update occurs after retrieving and printing the gas fee—meaning it has no impact on further actions within this one-off execution—it is consistent with the pattern used in other examples (e.g., MsgRegisterAsDMM, MsgDelegate, MsgSendToEth, etc.). This pattern is likely meant to demonstrate how to refresh the gas price for potential subsequent transactions in a longer-running client session.
No changes are required in this snippet if the goal is to showcase the complete process, including future-proofing the client's state. However, if this example is intended to be a minimal, self-contained demo where no further transactions follow, you might consider removing the final adjustment to avoid potential confusion.
examples/chain/permissions/2_MsgUpdateNamespace/example.go (2)
8-9
: Import changes align with updated dependenciesThe code now properly imports
rpchttp
from the cometbft package instead of using the client import. This change is consistent with the chain upgrade mentioned in the PR objectives.
57-61
: Gas price adjustment improves transaction reliabilityAdding dynamic gas price adjustment (10% increase) is a good practice to ensure transactions remain valid even if gas prices fluctuate between request and broadcast time. This helps prevent transaction failures due to insufficient gas.
examples/chain/permissions/1_MsgCreateNamespace/example.go (1)
56-60
: Gas price adjustment improves transaction reliabilityAdding dynamic gas price adjustment with a 10% buffer is a good practice to handle gas price fluctuations between request and broadcast time. This makes transactions more reliable in volatile network conditions.
examples/chain/exchange/14_MsgSubaccountTransfer/example.go (2)
8-15
: Cleaned up import structureThe import structure has been improved by removing unnecessary dependencies and organizing imports more clearly. The
rpchttp
import from cometbft is now properly included, which aligns with the chain upgrade.
59-63
: Gas price adjustment improves transaction reliabilityAdding dynamic gas price adjustment with a 10% buffer is a good practice that ensures transactions remain valid despite gas price fluctuations. This is particularly important for QueueBroadcastMsg which might be executed with some delay.
examples/chain/exchange/15_MsgExternalTransfer/example.go (2)
8-15
: Simplified import structureThe import structure has been improved by removing unnecessary dependencies and organizing imports more logically. The
rpchttp
import is now properly included, aligning with the chain upgrade requirements.
59-63
: Gas price adjustment enhances transaction reliabilityAdding dynamic gas price adjustment with a 10% buffer before sending the transaction is a good practice to handle fluctuations in gas prices. This helps ensure the transaction doesn't fail due to insufficient gas when using QueueBroadcastMsg.
examples/chain/wasmx/1_MsgExecuteContractCompat/example.go (2)
8-9
: LGTM: Adding necessary import for HTTP client.The import for cometbft's HTTP client is needed for the
tmClient
instantiation used to interact with the blockchain.
56-60
: Good improvement: Dynamic gas price adjustment for transaction reliability.This code retrieves the current chain gas price and increases it by 10% to ensure the transaction remains valid even if the gas price changes slightly between request and broadcast time. This is a robust defensive approach.
Makefile (5)
3-4
: Version upgrade for injective-indexer dependency.The branch for cloning injective-indexer has been updated from v1.14.1-RC.6 to v1.14.25, which aligns with the chain upgrade objective.
6-7
: Important branch update for injective-core dependency.The branch for injective-core has been updated from v1.14.0 to release/v1.15.x, which is a key part of the v1.15 chain upgrade.
95-99
: Added new txfees module support for v1.15 chain upgrade.These changes add support for the new txfees module by:
- Creating directory structures for txfees types
- Copying proto-generated files and codecs from injective-core
- Including osmosis-specific txfees types
This addition is crucial for the v1.15 chain upgrade requirements.
159-170
: Added code quality tooling with linting targets.These new targets improve the development workflow by adding:
lint
: Compares against dev branchlint-last-commit
: Only checks changes in the most recent commitlint-master
: Compares against master branchEach command sets GOPROXY=direct to ensure consistent dependency resolution.
171-171
: Updated PHONY declaration with new targets.Added the new lint targets to the PHONY declaration, ensuring Make won't confuse these targets with files of the same name.
client/chain/chain_test_support.go (3)
31-33
: Added imports for new TxFees functionality.These imports are necessary for the new transaction fee methods being implemented:
- txfeestypes for accessing transaction fee data structures
- injectiveclient for accessing default gas prices
832-839
: Added mock TxFees query methods for testing.These new methods provide mock implementations for transaction fee queries:
- FetchTxFeesParams: Returns mock parameters about transaction fees
- FetchEipBaseFee: Returns mock EIP-1559 style base fee data
These align with the v1.15 chain upgrade features for gas price management.
841-843
: Implemented gas price retrieval for testing.This method provides a fixed return value using the DefaultGasPrice constant, allowing tests to have consistent gas price behavior.
client/chain/chain.go (5)
18-18
: New imports for transaction fee management, distribution, IBC functionalities, and more
These newly introduced imports appear consistent and purposeful, with no duplicates detected.Also applies to: 23-23, 27-27, 32-32, 38-38, 42-42, 44-48
306-312
: Extended interface for TxFees module and gas price queries
The new methodsFetchTxFeesParams
,FetchEipBaseFee
,CurrentChainGasPrice
, andSetGasPrice
cleanly integrate into the existingChainClient
interface.
1928-1937
: New method: FetchChainSubaccountPositionInMarket
Implementation follows the established pattern of building the request and invokingcommon.ExecuteCall
. No issues found.
1938-1947
: New method: FetchChainSubaccountEffectivePositionInMarket
Similarly structured to other fetch methods, ensuring a concise request/response flow.
2670-2683
: New TxFees queries: FetchTxFeesParams and FetchEipBaseFee
Both methods properly construct and execute requests against the TxFees query client. No correctness or performance concerns.
|
||
gasPrice = chainClient.CurrentChainGasPrice() | ||
// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted | ||
gasPrice = int64(float64(gasPrice) * 1.1) | ||
chainClient.SetGasPrice(gasPrice) |
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.
Unnecessary duplicate gas price adjustment
This second gas price adjustment occurs after the transaction has already been queued for broadcasting (line 113) and after retrieving the gas fee (line 121). Modifying the gas price at this point won't affect the already submitted transaction.
Consider removing this duplicate adjustment code block, as it has no effect on the completed transaction and could cause confusion.
gasPrice = chainClient.CurrentChainGasPrice() | ||
// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted | ||
gasPrice = int64(float64(gasPrice) * 1.1) | ||
chainClient.SetGasPrice(gasPrice) |
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.
🛠️ Refactor suggestion
Remove redundant gas price adjustment at the end of the function.
This second gas price adjustment appears unnecessary as it happens after the transaction is already broadcasted (line 87) and there are no subsequent transactions in this example that would benefit from it.
- gasPrice = chainClient.CurrentChainGasPrice()
- // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted
- gasPrice = int64(float64(gasPrice) * 1.1)
- chainClient.SetGasPrice(gasPrice)
If this adjustment is intended for other transactions that might follow in a real-world scenario, consider adding a comment explaining its purpose or creating a reusable helper function to reduce code duplication.
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
gasPrice = chainClient.CurrentChainGasPrice() | |
// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted | |
gasPrice = int64(float64(gasPrice) * 1.1) | |
chainClient.SetGasPrice(gasPrice) |
* 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
* (fix) Applied the fixes for EIP712 JSON unmarshalling to the master branch * fix: update pre-commit/action from v2.0.2 to v3.0.1 to ry to solve the pre-commit validation workflow error * fix: Added configuration in .golangci.yml file to exclude some dirs * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: Added quantization in the functions that convert notional values to chain format * fix/fix_manual_tx_generation (#296) * 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
…always in lowercase
…always in lowercase
… example for the new GetEipBaseFee request in txfees module
…of using the default value
…veLabs/sdk-go into feat/chain_upgrade_v1_15
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
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: 1
🧹 Nitpick comments (7)
.golangci.yml (2)
27-35
: Revive Rules Enhancements
The addition of new rules—including the configuration forvar-naming
, disablingadd-constant
, and setting a line-length limit of 140 characters—appears well considered. Please verify that these settings align with your project's style guide and do not conflict with any legacy formatting rules.
36-47
: Gocritic Settings Update Review
The updated enabled-tags now include a comprehensive list (performance, diagnostic, style, opinionated, experimental), and the disabled checks (singleCaseSwitch
,deferInLoop
, along withhugeParam
andunnamedResult
) are clearly laid out. Confirm that these selections reflect your desired linting behavior and reduce false positives without omitting essential checks.examples/chain/8_OfflineSigning/example.go (1)
120-124
: Improved response formatting.The transaction response is now printed in a more readable format with both specific fields and a complete JSON representation.
Consider adding error handling for the JSON marshaling operation:
- str, _ := json.MarshalIndent(response, "", " ") + str, err := json.MarshalIndent(response, "", " ") + if err != nil { + fmt.Printf("Error marshaling response: %v\n", err) + return + } fmt.Print(string(str))client/chain/chain.go (4)
86-86
: Added gasPrice parameter in BuildSignedTx.
Including a dedicated gasPrice parameter is a helpful improvement. Consider validating thatgasPrice >= 0
before using it.
740-749
: Refined BuildSignedTx logic to incorporate custom gasPrice.
The usage ofWithGas(initialGas)
and a formatted string for gas prices is clear. Consider adding a quick check against negative gasPrice values.
2771-2785
: CurrentChainGasPrice method.
Falling back to a default gas price on errors is sensible. As a small refinement, consider checking that truncated values are non-negative.
2787-2791
: SetGasPrice method.
Simple and effective approach to override TxFactory’s gas prices. Consider guarding for negative values.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (6)
.github/workflows/pre-commit.yml
(1 hunks).golangci.yml
(1 hunks)client/chain/chain.go
(19 hunks)client/chain/chain_test_support.go
(4 hunks)examples/chain/8_OfflineSigning/example.go
(5 hunks)typeddata/typed_data.go
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- typeddata/typed_data.go
- client/chain/chain_test_support.go
🧰 Additional context used
🧬 Code Definitions (2)
examples/chain/8_OfflineSigning/example.go (1)
client/chain/markets_assistant.go (1)
NewMarketsAssistant
(161-166)
client/chain/chain.go (3)
chain/txfees/types/query.pb.go (6)
QueryParamsResponse
(108-111)QueryParamsResponse
(115-115)QueryParamsResponse
(116-118)QueryEipBaseFeeResponse
(189-191)QueryEipBaseFeeResponse
(195-195)QueryEipBaseFeeResponse
(196-198)client/chain/tx_factory.go (1)
NewTxFactory
(9-19)client/common/api_request_assistant.go (1)
ExecuteCall
(13-22)
🔇 Additional comments (27)
.github/workflows/pre-commit.yml (2)
11-11
: Action Version Update: Verify Checkout Action Usage
Usingactions/checkout@master
instead of a specific semantic version (e.g. v3) may lead to unexpected changes over time. Please confirm that using themaster
branch is intentional and aligns with your stability requirements.
18-18
: Pre-commit Action Version Update Approved
The update topre-commit/action@v3.0.1
reflects the latest release. Ensure that integration tests are run to confirm compatibility with your workflow..golangci.yml (3)
25-26
: Revive Configuration Key Update
Renaming toenable-all-rules: true
ensures compatibility with the latest configuration standards for the revive linter. This update is correctly applied.
48-60
: New Issues Configuration Block
The newly added issues section—with exclusions for specific rules (text matching ALL_CAPS) and directories (e.g., proto, chain, exchange, etc.) and settingmax-issues-per-linter
to 0—is a robust approach to filtering linting outcomes. Please verify that the excluded directories accurately reflect your repository structure and that no unintended directories are omitted from linting.
61-63
: Linter Run Configuration
Disabling tests in the linter run by settingtests: false
is acceptable if tests are executed independently. Confirm that this decision integrates well within your CI/CD pipeline.examples/chain/8_OfflineSigning/example.go (7)
6-6
: Imports updated appropriately.The addition of the JSON package is necessary for the new response formatting, and using the more specific
txtypes
import improves code clarity.Also applies to: 11-11
21-21
: Good replacement of deprecated ioutil package.The code now correctly uses the
os
package's direct file operations rather than the deprecatedioutil
package. This follows modern Go best practices.Also applies to: 25-25
29-29
: Network configuration update.The network has been changed from "devnet" to "testnet" with a "lb" parameter, which aligns with the chain upgrade mentioned in the PR objectives.
70-74
: Gas price adjustment logic added.The code now dynamically retrieves the current chain gas price and adjusts it by a factor of 1.1 to account for potential fluctuations between requesting and broadcasting. This is a good practice for ensuring transactions don't fail due to gas price changes.
82-82
: Market ID updated.The market ID has been updated, likely to match an available market in the testnet environment.
103-103
: Gas price handling improved.Now using
client.DefaultGasPrice
from the SDK instead of a hardcoded value, which provides better consistency across applications using the SDK.
114-115
: Transaction broadcasting updated.The comment now provides more information about broadcasting options, and the code uses
BroadcastSignedTx
withBROADCAST_MODE_SYNC
, which provides more immediate feedback on transaction acceptance.client/chain/chain.go (15)
20-48
: Imports updated cleanly.
These newly introduced and renamed imports (e.g.,sdkclient
for the Cosmos SDK client) look consistent with the rest of the file.
75-75
: Switched to sdkclient.Context.
Changing the returned type tosdkclient.Context
aligns with the new usage throughout the file.
79-79
: Signature updated for SimulateMsg.
Replacingclient.Context
withsdkclient.Context
is coherent with the interface changes.
89-89
: Refined BroadcastSignedTx signature.
The updated signature for broadcasting looks fine and integrates well with the rest of the code.
307-313
: Introduced TxFees module methods.
These new interface methods (e.g., FetchTxFeesParams, FetchEipBaseFee) plus gas price helpers are a clean addition, enabling better fee management.
321-321
: Replaced client.Context with sdkclient.Context in chainClient.
This maintains consistency with other changes and avoids naming clashes.
360-360
: Added txfeesQueryClient field.
The new field seamlessly matches the functionality introduced in FetchTxFeesParams and FetchEipBaseFee.
368-368
: Constructor now accepts sdkclient.Context.
Shifting the constructor to acceptsdkclient.Context
keeps the context usage consistent.
713-713
: SimulateMsg now uses sdkclient.Context.
This matches the interface adjustments and is implemented clearly in line with the rest of the code.
795-795
: Refactored SyncBroadcastSignedTx call.
Delegating toBroadcastSignedTx
in synchronous mode is straightforward and consistent.
815-815
: New usage of CheckCometError.
Usingsdkclient.CheckCometError
helps parse CometBFT errors accurately.
835-836
: AsyncBroadcastSignedTx refactor.
Now succinctly reusingBroadcastSignedTx
with async mode is a neat improvement.
838-841
: New BroadcastSignedTx implementation.
Extracting this into its own function provides clarity and reusability for different broadcast modes.
1931-1931
: FetchChainSubaccountPositionInMarket signature updated.
This query function remains straightforward; no issues noted.
1941-1941
: FetchChainSubaccountEffectivePositionInMarket signature updated.
Likewise, the structure and usage are consistent with other query patterns.
gasPrice = chainClient.CurrentChainGasPrice() | ||
// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted | ||
gasPrice = int64(float64(gasPrice) * 1.1) | ||
chainClient.SetGasPrice(gasPrice) |
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.
🛠️ Refactor suggestion
Remove duplicate gas price adjustment code.
The gas price adjustment logic at the end of the function appears to be unnecessary duplication since it's already done at the beginning (lines 70-74) and there's no further use of the gas price after this point.
- gasPrice = chainClient.CurrentChainGasPrice()
- // adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted
- gasPrice = int64(float64(gasPrice) * 1.1)
- chainClient.SetGasPrice(gasPrice)
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
gasPrice = chainClient.CurrentChainGasPrice() | |
// adjust gas price to make it valid even if it changes between the time it is requested and the TX is broadcasted | |
gasPrice = int64(float64(gasPrice) * 1.1) | |
chainClient.SetGasPrice(gasPrice) |
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: 1
🧹 Nitpick comments (3)
injective_data/ofac.json (1)
71-74
: Addition of New OFAC EntriesThe three new entries added at lines 72–74:
"0xb9436d76e8fe08859d042e41b4a21c85715e1176"
"0x7bc5cb059f21553af489d2b2df3d40aaae9b44e8"
"0x430ab3c698b3210548b6ac9f72936b43b15ebe9b"
align with the update described in the PR objectives. Please confirm that these addresses have been vetted against the latest compliance and OFAC sanctions data for the chain upgrade v1.15.
chain/wasmx/types/authz.go (1)
57-61
: Consider adding documentation about this hot-patch solution.The comment indicates this is a "hot-patch", suggesting this might be a temporary solution. Adding more detailed documentation explaining the need for this patch would help future maintainers understand the rationale behind this approach. If this is indeed temporary, consider adding a TODO comment with details about the intended permanent solution.
- if res.Updated != nil { // hot-patch the updated authorization type back to "Compat" version + // Hot-patch the updated authorization type back to "Compat" version. + // This is necessary because the SDK's authz module returns the standard + // wasmtypes.ContractExecutionAuthorization, but our code expects the compat version. + // TODO: Consider a more robust solution in the future that doesn't rely on type assertions. + if res.Updated != nil {proto/injective/txfees/v1beta1/txfees.proto (1)
11-64
: Review of Params Message Structure
The newParams
message is well-defined and comprehensive. The fields are sequentially numbered and use custom types (i.e.,cosmossdk.io/math.LegacyDec
) where high precision for financial computations is needed. In addition, each field is annotated with YAML tags, which is useful for configuration management. Please ensure that these definitions align with the chain upgrade v1.15 requirements.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (2)
chain/txfees/types/txfees.pb.go
is excluded by!**/*.pb.go
exchange/trading_rpc/pb/goadesign_goagen_injective_trading_rpc.pb.go
is excluded by!**/*.pb.go
📒 Files selected for processing (4)
Makefile
(3 hunks)chain/wasmx/types/authz.go
(1 hunks)injective_data/ofac.json
(1 hunks)proto/injective/txfees/v1beta1/txfees.proto
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
🪛 Buf (1.47.2)
proto/injective/txfees/v1beta1/txfees.proto
4-4: import "amino/amino.proto": file does not exist
(COMPILE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint
🔇 Additional comments (3)
injective_data/ofac.json (1)
1-70
: JSON Array Structure and Retained Entry CheckThe JSON array structure remains well-formatted. Notably, the entry at line 71 (
"0x8aa07899eb940f40e514b8effdb3b6af5d1cf7bb"
) appears with a tilde but, per the PR summary, it is intended to be retained. Please verify that marking it as changed is intentional and that no unintended modifications occurred.proto/injective/txfees/v1beta1/txfees.proto (2)
8-10
: Verify Go Package Options
The go_package option is set to"github.com/InjectiveLabs/injective-core/injective-chain/modules/txfees/types"
and the gogoproto registration option is enabled. These settings appear to be correct for ensuring proper package resolution on the Go side.
1-7
:❓ Verification inconclusive
Check Import Paths and File Existence
Static analysis indicates that the import on line 4 (import "amino/amino.proto";
) does not point to an existing file. Please verify whether the file is located at the expected path or update the import statement accordingly to avoid compilation issues.
Below is a shell script to search for the file
amino.proto
(and its expected relative path) to verify whether the import is valid:
🏁 Script executed:
#!/bin/bash # Recursively search for any file named "amino.proto" in the repository to verify its existence. fd "amino.proto"Length of output: 16
Attention: Verification Needed for
amino.proto
Import PathStatic analysis suggests that the file referenced by
import "amino/amino.proto";
inproto/injective/txfees/v1beta1/txfees.proto
was not found in the repository. Based on our search output, please confirm one of the following:
- If
amino.proto
is intended to be part of the repository, verify its location and update the import path accordingly.- If the file is provided as an external dependency or has been relocated/removed, adjust or remove the import statement to avoid compilation issues.
Please perform a manual verification to ensure that this change aligns with your dependency management and build process.
🧰 Tools
🪛 Buf (1.47.2)
4-4: import "amino/amino.proto": file does not exist
(COMPILE)
res, err := wasmAuth.Accept(goCtx, wasmMsg) | ||
|
||
if res.Updated != nil { // hot-patch the updated authorization type back to "Compat" version | ||
res.Updated = &ContractExecutionCompatAuthorization{ | ||
Grants: res.Updated.(*wasmtypes.ContractExecutionAuthorization).Grants, | ||
} | ||
} | ||
|
||
return res, err |
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.
🛠️ Refactor suggestion
Add type assertion safety check to prevent potential runtime panic.
The current implementation assumes that res.Updated
will always be of type *wasmtypes.ContractExecutionAuthorization
when it's not nil. If a different type is returned, this will cause a runtime panic. Consider adding a type assertion check using the comma-ok idiom before proceeding with the conversion.
if res.Updated != nil { // hot-patch the updated authorization type back to "Compat" version
- res.Updated = &ContractExecutionCompatAuthorization{
- Grants: res.Updated.(*wasmtypes.ContractExecutionAuthorization).Grants,
- }
+ if updatedAuth, ok := res.Updated.(*wasmtypes.ContractExecutionAuthorization); ok {
+ res.Updated = &ContractExecutionCompatAuthorization{
+ Grants: updatedAuth.Grants,
+ }
+ }
}
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
res, err := wasmAuth.Accept(goCtx, wasmMsg) | |
if res.Updated != nil { // hot-patch the updated authorization type back to "Compat" version | |
res.Updated = &ContractExecutionCompatAuthorization{ | |
Grants: res.Updated.(*wasmtypes.ContractExecutionAuthorization).Grants, | |
} | |
} | |
return res, err | |
res, err := wasmAuth.Accept(goCtx, wasmMsg) | |
if res.Updated != nil { // hot-patch the updated authorization type back to "Compat" version | |
if updatedAuth, ok := res.Updated.(*wasmtypes.ContractExecutionAuthorization); ok { | |
res.Updated = &ContractExecutionCompatAuthorization{ | |
Grants: updatedAuth.Grants, | |
} | |
} | |
} | |
return res, err |
Summary by CodeRabbit
New Features
Chores