Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request systematically pre-allocates slice capacity across 24 files throughout the codebase. Instead of initializing empty or nil slices and letting them grow dynamically, changes specify initial capacity using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
e2e/e2etests/test_crosschain_swap.go (1)
86-96: Incomplete preallocation — one reallocation still occurs at line 96.The capacity
len(r.ZEVMSwapAppAddr.Bytes())(20 bytes) only covers the firstappend. Whenmemobytesis appended at line 96 the slice must reallocate. Computingmemobytesfirst allows a fully precise capacity hint.♻️ Proposed refactor to eliminate the remaining reallocation
- msg := make([]byte, 0, len(r.ZEVMSwapAppAddr.Bytes())) - msg = append(msg, r.ZEVMSwapAppAddr.Bytes()...) - memobytes, err := r.ZEVMSwapApp.EncodeMemo( + memobytes, err := r.ZEVMSwapApp.EncodeMemo( &bind.CallOpts{}, r.BTCZRC20Addr, []byte(r.GetBtcAddress().EncodeAddress()), ) require.NoError(r, err) r.Logger.Info("memobytes(%d) %x", len(memobytes), memobytes) + msg := make([]byte, 0, len(r.ZEVMSwapAppAddr.Bytes())+len(memobytes)) + msg = append(msg, r.ZEVMSwapAppAddr.Bytes()...) msg = append(msg, memobytes...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/e2etests/test_crosschain_swap.go` around lines 86 - 96, The slice preallocation for msg only uses len(r.ZEVMSwapAppAddr.Bytes()) so appending memobytes forces a reallocation; call r.ZEVMSwapApp.EncodeMemo(...) first to get memobytes and err, then create msg with capacity len(r.ZEVMSwapAppAddr.Bytes())+len(memobytes), append the address bytes and memobytes into msg, and keep the require.NoError and r.Logger.Info usage around the EncodeMemo call; update references to memobytes, msg, r.ZEVMSwapAppAddr and r.ZEVMSwapApp.EncodeMemo accordingly.x/crosschain/types/key_inbound_hash_to_cctx.go (1)
14-16: LGTM on capacity; minor style note on single-byte append.The capacity
len(inboundHashBytes)+1is exact and avoids any reallocation. On Line 16, spreading a slice literal for a single byte is unnecessary.♻️ Proposed simplification
- key = append(key, []byte("/")...) + key = append(key, '/')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/crosschain/types/key_inbound_hash_to_cctx.go` around lines 14 - 16, The code builds key with capacity len(inboundHashBytes)+1 then appends inboundHashBytes and appends a single-byte separator using append(key, []byte("/")...), which is stylistically suboptimal; replace the spread of a single-byte slice with append(key, '/') to append the single byte directly (locate where variable key is created and the append of inboundHashBytes and the separator in the function that handles inboundHashBytes).x/crosschain/keeper/foreign_coins.go (1)
11-14: Capacity hint is a lower bound — may underestimate for chains with multiple foreign coins.
GetAllForeignCoinsForChaincan return more than one coin per chain (e.g., a gas token plus registered ERC20 tokens), solen(chains)is a lower bound on the final slice size and reallocations may still occur. This is still a meaningful improvement over a nil slice. If profiling ever identifies this as a hot path, a pre-sizing pass or summation overGetAllForeignCoinsForChaincounts could eliminate remaining reallocations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/crosschain/keeper/foreign_coins.go` around lines 11 - 14, The capacity hint for fCoins (created with make([]fungibleModuleTypes.ForeignCoins, 0, len(chains))) underestimates when GetAllForeignCoinsForChain returns multiple coins per chain; update the pre-sizing so it won't assume one coin per chain — either perform a short pre-pass summing len(k.fungibleKeeper.GetAllForeignCoinsForChain(ctx, chain.ChainId)) for each chain to compute the exact capacity before make, or append into a nil slice (or simple zero-cap slice) if you prefer simplicity; locate and change the allocation around the fCoins variable and calls to k.fungibleKeeper.GetAllForeignCoinsForChain(ctx, chain.ChainId) in foreign_coins.go.pkg/contracts/solana/gateway_message.go (1)
352-388:MsgExecute.Hash()andMsgExecuteSPL.Hash()still usevar message []byte— consider a partial preallocation for consistency.While the
prealloclinter with its defaultsimple: truesetting does not flag these methods (their capacity depends on a conditional sender size and runtime-variablemsg.data/remainingAccounts), all otherHash()implementations in the same file now preallocate. A base capacity covering the deterministic portion would align the file and reduce the initial growth cycle on the hot path.♻️ Suggested partial preallocation for
MsgExecute.Hash()func (msg *MsgExecute) Hash() [32]byte { - var message []byte buff := make([]byte, 8) + // base: identifier(9) + instruction(1) + 3×uint64(24) + to(32) + max-sender(32) + data + accounts×32 + baseCapacity := len(InstructionIdentifier) + 1 + 3*len(buff) + len(msg.to) + 32 + len(msg.data) + len(msg.remainingAccounts)*32 + message := make([]byte, 0, baseCapacity)Apply the same pattern to
MsgExecuteSPL.Hash(), addinglen(msg.mintAccount)+len(msg.recipientAta)to the base capacity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/contracts/solana/gateway_message.go` around lines 352 - 388, The Hash methods should preallocate the message slice with a deterministic base capacity to avoid repeated growth: in MsgExecute.Hash() create message with make([]byte, 0, N) where N includes len(InstructionIdentifier)+1 (instruction byte)+8*3 (chainID, nonce, amount)+len(msg.to.Bytes())+max sender size (or the larger of the two sender encodings) plus any fixed-size remainingAccounts count estimate per account if known; do the same for MsgExecuteSPL.Hash() but add len(msg.mintAccount)+len(msg.recipientAta) to the base capacity; update both functions to use the preallocated slice before appending.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.golangci.yml:
- Around line 97-102: Remove the blanket revive exclusions under
linters.exclusions.rules that silence "var-naming: avoid package names that
conflict with Go standard library" and "var-naming: avoid meaningless package
names"; instead restore the full canonical message for the stdlib conflict entry
("avoid package names that conflict with Go standard library package names") and
reconfigure revive to handle package-name collisions via its option (e.g., set
skipPackageNameCollisionWithGoStd = true in revive config) or apply path-scoped
exclusions only for the specific offending directories, or fix offending package
names in code; ensure you remove the global text-based suppressions and replace
them with either the revive rule option, targeted exclusions, or inline
//nolint:revive on specific files.
---
Nitpick comments:
In `@e2e/e2etests/test_crosschain_swap.go`:
- Around line 86-96: The slice preallocation for msg only uses
len(r.ZEVMSwapAppAddr.Bytes()) so appending memobytes forces a reallocation;
call r.ZEVMSwapApp.EncodeMemo(...) first to get memobytes and err, then create
msg with capacity len(r.ZEVMSwapAppAddr.Bytes())+len(memobytes), append the
address bytes and memobytes into msg, and keep the require.NoError and
r.Logger.Info usage around the EncodeMemo call; update references to memobytes,
msg, r.ZEVMSwapAppAddr and r.ZEVMSwapApp.EncodeMemo accordingly.
In `@pkg/contracts/solana/gateway_message.go`:
- Around line 352-388: The Hash methods should preallocate the message slice
with a deterministic base capacity to avoid repeated growth: in
MsgExecute.Hash() create message with make([]byte, 0, N) where N includes
len(InstructionIdentifier)+1 (instruction byte)+8*3 (chainID, nonce,
amount)+len(msg.to.Bytes())+max sender size (or the larger of the two sender
encodings) plus any fixed-size remainingAccounts count estimate per account if
known; do the same for MsgExecuteSPL.Hash() but add
len(msg.mintAccount)+len(msg.recipientAta) to the base capacity; update both
functions to use the preallocated slice before appending.
In `@x/crosschain/keeper/foreign_coins.go`:
- Around line 11-14: The capacity hint for fCoins (created with
make([]fungibleModuleTypes.ForeignCoins, 0, len(chains))) underestimates when
GetAllForeignCoinsForChain returns multiple coins per chain; update the
pre-sizing so it won't assume one coin per chain — either perform a short
pre-pass summing len(k.fungibleKeeper.GetAllForeignCoinsForChain(ctx,
chain.ChainId)) for each chain to compute the exact capacity before make, or
append into a nil slice (or simple zero-cap slice) if you prefer simplicity;
locate and change the allocation around the fCoins variable and calls to
k.fungibleKeeper.GetAllForeignCoinsForChain(ctx, chain.ChainId) in
foreign_coins.go.
In `@x/crosschain/types/key_inbound_hash_to_cctx.go`:
- Around line 14-16: The code builds key with capacity len(inboundHashBytes)+1
then appends inboundHashBytes and appends a single-byte separator using
append(key, []byte("/")...), which is stylistically suboptimal; replace the
spread of a single-byte slice with append(key, '/') to append the single byte
directly (locate where variable key is created and the append of
inboundHashBytes and the separator in the function that handles
inboundHashBytes).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.golangci.ymlapp/app.gocmd/zetacore_utils/main.gocmd/zetatool/cctx/cctx_details.gocmd/zetatool/cli/tss_balances.goe2e/e2etests/test_crosschain_swap.goe2e/e2etests/test_solana_withdraw_and_call_alt.goe2e/e2etests/test_spl_withdraw_and_call_alt.goe2e/runner/e2etest.goe2e/runner/setup_bitcoin.goe2e/runner/setup_solana.goe2e/runner/solana.goe2e/runner/solana_gateway_upgrade.goe2e/utils/zetacore.gopkg/contracts/solana/gateway_message.gopkg/proofs/bitcoin/bitcoin_spv.gosimulation/state.gox/crosschain/keeper/foreign_coins.gox/crosschain/types/key_inbound_hash_to_cctx.gox/crosschain/types/keys.gox/fungible/types/key_foreign_coins.gozetaclient/config/config.go
skosito
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix all lint issues
How Has This Been Tested?
Note
Low Risk
Mechanical lint-driven changes (slice preallocation and minor formatting) with no intended behavior changes; low risk aside from potential subtle differences if any code relied on nil vs empty slices.
Overview
Primarily resolves lint findings by replacing nil/empty slice declarations with
make(..., 0, cap)preallocations across CLI tools, E2E tests, Solana/Bitcoin helpers, simulation genesis builders, and module key/keeper utilities.Updates
.golangci.ymlto ignore tworevivevar-namingwarnings, and makes a few small mechanical tweaks (e.g., preallocating proposal handler list inapp/app.go, formatting long function calls) without changing behavior.Written by Cursor Bugbot for commit b94b621. Configure here.
Summary by CodeRabbit
Chores
Refactor
Style