Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 26 files out of 84 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThe changes predominantly involve updating the Go version from Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2460 +/- ##
============================================
- Coverage 68.94% 47.17% -21.77%
============================================
Files 308 433 +125
Lines 19223 29129 +9906
============================================
+ Hits 13253 13743 +490
- Misses 5295 14604 +9309
- Partials 675 782 +107
|
gartnera
force-pushed
the
go122
branch
2 times, most recently
from
f5a4763 to
699a1bb
Compare
|
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
gartnera
requested review from
a team,
brewmaster012,
fbac,
kingpinXD,
lumtis,
skosito,
swift1337 and
ws4charlie
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
pkg/chains/conversion.go (1)
13-13: Verify range checks fornonceThe
noncevalue is not explicitly validated before calling theNonceMarkAmountfunction, which contradicts the comment#nosec G115 always in range. To ensure proper validation, add range checks fornoncebefore callingNonceMarkAmount.
pkg/chains/conversion.go: FunctionNonceMarkAmountlacksnoncevalidation.- Call sites in:
x/crosschain/types/tx_body_verification.gozetaclient/chains/bitcoin/signer/signer.gozetaclient/chains/bitcoin/observer/outbound_test.gozetaclient/chains/bitcoin/observer/outbound.goAnalysis chain
Verify range checks for
nonceThe comment
#nosec G115 always in rangeindicates that the value is always in range. Ensure that thenoncevalue is properly validated before this point.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the range check for `nonce` in the `NonceMarkAmount` function # Test: Check for the validation logic for `nonce`. Expect: Validation logic exists. rg --type go --context 5 -e 'nonce.*ParseUint' -e 'nonce.*<'Length of output: 12625
Script:
#!/bin/bash # Description: Verify the range check for `nonce` in the `pkg/chains/conversion.go` file # Test: Search for the `NonceMarkAmount` function and surrounding context to identify validation logic for `nonce`. rg --type go --context 10 -e 'func NonceMarkAmount' -e 'nonce' pkg/chains/conversion.goLength of output: 793
Script:
#!/bin/bash # Description: Verify the range check for `nonce` by searching for call sites of `NonceMarkAmount` in the codebase. # Test: Search for call sites of `NonceMarkAmount` and check for validation logic for `nonce` at those locations. rg --type go --context 10 -e 'NonceMarkAmount'Length of output: 13195
x/crosschain/client/cli/query_cctx.go (1)
68-68: Ensure additional validation forlimitThe
limitvalue is only validated for being a 32-bit unsigned integer usingstrconv.ParseUint, without any further checks on its value range or constraints. Consider adding additional validation logic to ensurelimitmeets the application's requirements.
- File:
x/crosschain/client/cli/query_cctx.go- Line: 68
Analysis chain
Verify bit size checks for
limitThe comment
#nosec G115 bit size verifiedindicates that the bit size is checked. Ensure that thelimitvalue is properly validated before this point.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the bit size check for `limit` in the `CmdPendingCctx` function # Test: Check for the validation logic for `limit`. Expect: Validation logic exists. rg --type go --context 5 -e 'limit.*ParseUint' -e 'limit.*<'Length of output: 1768
Script:
#!/bin/bash # Description: Verify additional validation checks for `limit` in the `CmdPendingCctx` function # Test: Check for any further validation logic for `limit` after parsing. Expect: Additional validation logic exists. rg --type go --context 5 -e 'limit' x/crosschain/client/cli/query_cctx.goLength of output: 804
x/fungible/keeper/evm.go (1)
Line range hint
118-118:
Reminder: Add unit tests and refactor repetitive code.The TODO comment indicates that unit tests are missing, and there is repetitive code that needs refactoring.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
Files selected for processing (55)
- .github/actions/install-dependencies/action.yml (1 hunks)
- .github/workflows/publish-release.yml (3 hunks)
- .github/workflows/sast-linters.yml (2 hunks)
- Dockerfile-localnet (2 hunks)
- Makefile (1 hunks)
- cmd/zetae2e/stress.go (1 hunks)
- cmd/zetatool/filterdeposit/btc.go (1 hunks)
- cmd/zetatool/filterdeposit/evm.go (1 hunks)
- contrib/rpctest/main.go (1 hunks)
- e2e/e2etests/test_update_bytecode_zrc20.go (1 hunks)
- e2e/runner/accounting.go (1 hunks)
- e2e/runner/evm.go (1 hunks)
- go.mod (6 hunks)
- pkg/chains/conversion.go (1 hunks)
- pkg/coin/coin.go (1 hunks)
- pkg/mempool/custom_proposal_handler.go (4 hunks)
- pkg/proofs/ethereum/proof.go (4 hunks)
- pkg/proofs/proof.go (1 hunks)
- rpc/backend/account_info.go (2 hunks)
- rpc/backend/blocks.go (3 hunks)
- rpc/backend/chain_info.go (3 hunks)
- rpc/backend/tx_info.go (12 hunks)
- rpc/backend/utils.go (2 hunks)
- rpc/namespaces/ethereum/eth/api.go (1 hunks)
- rpc/types/block.go (1 hunks)
- rpc/types/events.go (6 hunks)
- rpc/types/utils.go (3 hunks)
- scripts/gosec.sh (1 hunks)
- x/crosschain/client/cli/cli_whitelist_erc20.go (1 hunks)
- x/crosschain/client/cli/query_cctx.go (1 hunks)
- x/crosschain/client/querytests/cctx.go (3 hunks)
- x/crosschain/client/querytests/gas_price.go (3 hunks)
- x/crosschain/client/querytests/inbound_hash.go (3 hunks)
- x/crosschain/client/querytests/inbound_tracker.go (2 hunks)
- x/crosschain/client/querytests/last_block_height.go (3 hunks)
- x/crosschain/client/querytests/outbound_tracker.go (3 hunks)
- x/crosschain/keeper/cctx.go (1 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_outbound.go (1 hunks)
- x/crosschain/keeper/cctx_utils.go (1 hunks)
- x/crosschain/keeper/evm_deposit.go (1 hunks)
- x/crosschain/keeper/grpc_query_cctx.go (3 hunks)
- x/crosschain/keeper/grpc_query_cctx_rate_limit.go (6 hunks)
- x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go (1 hunks)
- x/crosschain/keeper/grpc_query_zeta_conversion_rate.go (1 hunks)
- x/crosschain/keeper/grpc_query_zeta_conversion_rate_test.go (1 hunks)
- x/crosschain/keeper/msg_server_vote_gas_price.go (1 hunks)
- x/crosschain/keeper/msg_server_vote_inbound_tx.go (1 hunks)
- x/crosschain/keeper/msg_server_vote_outbound_tx.go (1 hunks)
- x/crosschain/keeper/msg_server_whitelist_erc20.go (2 hunks)
- x/crosschain/migrations/v2/migrate.go (1 hunks)
- x/crosschain/migrations/v5/migrate.go (1 hunks)
- x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go (1 hunks)
- x/fungible/keeper/evm.go (1 hunks)
- x/fungible/keeper/gas_coin_and_pool.go (1 hunks)
- x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go (3 hunks)
Files not processed due to max files limit (6)
- x/observer/abci.go
- x/observer/client/cli/tx_update_gas_price_increase_flags.go
- x/observer/client/cli/tx_update_observer.go
- x/observer/client/querytests/chain_nonces.go
- x/observer/keeper/msg_server_reset_chain_nonces.go
- x/observer/keeper/pending_nonces.go
Files skipped from review due to trivial changes (35)
- .github/actions/install-dependencies/action.yml
- .github/workflows/publish-release.yml
- .github/workflows/sast-linters.yml
- cmd/zetae2e/stress.go
- cmd/zetatool/filterdeposit/btc.go
- cmd/zetatool/filterdeposit/evm.go
- contrib/rpctest/main.go
- e2e/e2etests/test_update_bytecode_zrc20.go
- e2e/runner/accounting.go
- go.mod
- pkg/coin/coin.go
- pkg/proofs/proof.go
- rpc/backend/chain_info.go
- rpc/backend/tx_info.go
- rpc/backend/utils.go
- rpc/namespaces/ethereum/eth/api.go
- rpc/types/block.go
- rpc/types/utils.go
- scripts/gosec.sh
- x/crosschain/client/querytests/cctx.go
- x/crosschain/client/querytests/inbound_hash.go
- x/crosschain/client/querytests/inbound_tracker.go
- x/crosschain/client/querytests/outbound_tracker.go
- x/crosschain/keeper/cctx_orchestrator_validate_outbound.go
- x/crosschain/keeper/cctx_utils.go
- x/crosschain/keeper/evm_deposit.go
- x/crosschain/keeper/grpc_query_cctx.go
- x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go
- x/crosschain/keeper/grpc_query_zeta_conversion_rate.go
- x/crosschain/keeper/grpc_query_zeta_conversion_rate_test.go
- x/crosschain/keeper/msg_server_vote_outbound_tx.go
- x/crosschain/migrations/v2/migrate.go
- x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go
- x/fungible/keeper/gas_coin_and_pool.go
- x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go
Additional context used
Path-based instructions (18)
x/crosschain/client/cli/cli_whitelist_erc20.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/chains/conversion.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/client/cli/query_cctx.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_vote_gas_price.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/client/querytests/gas_price.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/client/querytests/last_block_height.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/proofs/ethereum/proof.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/migrations/v5/migrate.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_vote_inbound_tx.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_whitelist_erc20.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.rpc/backend/account_info.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/evm.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/mempool/custom_proposal_handler.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.rpc/types/events.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/grpc_query_cctx_rate_limit.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.rpc/backend/blocks.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/fungible/keeper/evm.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (52)
x/crosschain/client/cli/cli_whitelist_erc20.go (1)
53-53: Verify range checks fordecimalsThe comment
#nosec G115 always in rangeindicates that the value is always in range. Ensure that thedecimalsvalue is properly validated before this point.Verification successful
The range check for
decimalsis properly implemented.The code ensures that the
decimalsvalue is parsed and then validated to be less than 128, which confirms that the value is within the expected range.
decimals, err := strconv.ParseUint(args[4], 10, 32)if decimals > 128 { return fmt.Errorf("decimals must be less than 128") }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the range check for `decimals` in the `CmdWhitelistERC20` function # Test: Check for the validation logic for `decimals`. Expect: Validation logic exists. rg --type go --context 5 -e 'decimals.*ParseUint' -e 'decimals.*<'Length of output: 819
x/crosschain/keeper/msg_server_vote_gas_price.go (1)
66-66: Verify positive checks formedian indexThe comment
#nosec G115 always positiveindicates that the value is always positive. Ensure that themedian indexvalue is properly validated before this point.Dockerfile-localnet (3)
2-2: Update Go version to 1.22.5The base image has been updated from
golang:1.20.14-bookwormtogolang:1.22.5-bookworm. Ensure compatibility with all dependencies and the overall build process.
25-25: Update Go version to 1.22.5The base image for
cosmovisor-buildhas been updated fromgolang:1.20.14-bookwormtogolang:1.22.5-bookworm. Ensure compatibility with all dependencies and the overall build process.
28-28: Update Go version to 1.22.5The base image for
base-runtimehas been updated fromgolang:1.20.14-bookwormtogolang:1.22.5-bookworm. Ensure compatibility with all dependencies and the overall build process.x/crosschain/keeper/cctx.go (1)
29-29: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.x/crosschain/client/querytests/gas_price.go (4)
83-83: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
98-98: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
111-111: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
118-118: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.x/crosschain/client/querytests/last_block_height.go (4)
83-83: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
98-98: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
111-111: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.
118-118: Update security annotationThe security annotation has been updated from
#nosec G701to#nosec G115. Ensure the new annotation accurately reflects the security guidelines and the code's behavior.pkg/proofs/ethereum/proof.go (4)
109-109: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the range.
136-136: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the range.
160-160: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the iterator range.
171-171: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the iterator range.
x/crosschain/migrations/v5/migrate.go (2)
65-65: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the nonce range.
67-67: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the nonce range.
x/crosschain/keeper/msg_server_vote_inbound_tx.go (1)
125-125: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the finalized height.
x/crosschain/keeper/msg_server_whitelist_erc20.go (2)
79-79: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the decimals range.
179-179: LGTM! Security annotation updated correctly.The updated annotation correctly reflects the validation of the gas limit range.
rpc/backend/account_info.go (2)
85-85: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
215-215: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.e2e/runner/evm.go (1)
186-186: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.pkg/mempool/custom_proposal_handler.go (4)
76-76: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
92-92: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
151-151: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
221-221: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.rpc/types/events.go (7)
174-174: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
231-231: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
252-252: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
280-280: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
282-282: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
302-302: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.
304-304: Update security annotation to reflect new guidelines.The security annotation is updated from
#nosec G701to#nosec G115to reflect the new guidelines for range checking.Makefile (1)
142-143: LGTM! Updated linting target.The linting target has been correctly updated from
lint-cosmos-gosectolint-gosec.x/crosschain/keeper/grpc_query_cctx_rate_limit.go (8)
54-54: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
60-60: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
72-72: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
100-100: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
262-262: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
268-268: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
294-294: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
374-374: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.rpc/backend/blocks.go (5)
200-200: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
497-497: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
499-499: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
505-505: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.
559-559: LGTM! Security annotation updated.The security annotation has been correctly updated from
#nosec G701to#nosec G115.x/fungible/keeper/evm.go (1)
133-133: Security annotation update approved.The
#nosec G115annotation indicates that the value is always in range. This update is correctly applied.
Use go 1.22 more go 1.22 upgrades upgrade gosec use upstream int overflow rule Use zeta-chain-gosec
4 participants
Description
Upgrade to go 1.22. Pretty simple on the node side, but please see zeta-chain/go-tss#19.
This is a breaking change because QUIC P2P will no longer function. We don't think anyone is using it but technically other zetaclientd operators could have configured it.
Unblocks: #2417
TODO:
strconv.ParseInt()exists.MkdirAll0o777 -> 0o750How Has This Been Tested?
Summary by CodeRabbit
Chores
G701toG115to reflect updated security guidelines.New Features
gosec.shscript for running security scans using thegosectool.Documentation