Conversation
- Add chains.go with SupportedEVMChains, IsThorchainSupported, IsMayachainSupported - Add evm_test.go for chain support tests - Add provider_evm.go for MayaChain Arbitrum swaps
- Add Cronos and Zksync to rpc config struct - Add them to networkConfigs for worker initialization - Update SupportedEVMChains to include all 10 EVM chains - Add IsDirectSendSupported helper for chains without swap routing - Update tests to cover all EVM chains
- Add supportedSwapChains list for chains with cross-chain swap support - Swaps: ETH, ARB, AVAX, BSC, BASE, BTC, SOL, XRP, ZEC - Sends: All 14 chains including Blast, Cronos, Optimism, Polygon, zkSync - Update swap spec to only show swap-capable chains in UI
WalkthroughAdds support for CronosChain and Zksync EVM networks in worker configuration, introduces EVM chain support helper functions with comprehensive tests, implements MayaChain EVM provider for Arbitrum-compatible swaps, and refactors chain support distinctions between send and swap capabilities. Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Fix all issues with AI Agents 🤖
In @internal/mayachain/provider_evm.go:
- Around line 103-132: The swap flow in provider_evm.go lacks structured logs;
add structured logging using the provider's logger (e.g., p.logger or p.log)
around the key steps so observability is consistent: log an Info when swap
starts after p.validateEvm with fields chain=from.Chain,
asset=from.AssetID.Hex(), amount=from.Amount; log Info after parseMayaNetwork
with fromMayaNet; log Info after resolving fromAsset (makeMayaAsset or native
branch) with fromAsset; log Info after getTokenDecimals with tokenDecimals and
after convertDecimals with mayaAmount and exactAmount; on any error returns
(validateEvm, parseMayaNetwork, makeMayaAsset, getTokenDecimals) use
p.logger.WithError(err).Error including the same structured fields and the
error; use appropriate levels (Info for successes, Error for failures) and
include contextual messages like "swap initiated", "resolved asset", "token
decimals retrieved", and "amount converted".
- Around line 202-228: The errors returned when building the tx (from
p.sdk.MakeTx) and when fetching token decimals (p.getTokenDecimals) lack chain
context; update the fmt.Errorf calls to include the provider's chain identifier
(e.g., p.chainID or p.chainName) so messages read like "failed to build tx for
chain %s: %w" and "failed to get target token decimals for chain %s: %w" (use
the actual field name on the provider struct), leaving other logic (parsing
expectedOut and convertDecimals) unchanged.
- Around line 27-34: The NewProviderEvm constructor does not validate its inputs
(client, rpc, sdk) which can cause panics later; update NewProviderEvm to check
each parameter (client, rpc, sdk) for nil and return nil (or panic with a clear
message) when any are nil, and document this behavior; specifically modify the
NewProviderEvm function that constructs a *ProviderEvm to perform nil checks for
client, rpc, and sdk before creating the ProviderEvm instance and handle the
error case consistently with project conventions.
- Around line 36-48: In ProviderEvm.validateEvm, the error returns from
parseMayaNetwork do not include which chain failed; update the two error returns
(the parseMayaNetwork calls for from.Chain and to.Chain) to include the chain
identifier (from.Chain and to.Chain respectively) in the fmt.Errorf message so
the returned errors read like "unsupported 'from' chain for MayaChain: %s: %w"
(or similar) referencing the failing chain value alongside the wrapped error
from parseMayaNetwork.
- Around line 139-149: The quote returned by p.client.getQuote (assigned to
variable quote) is used without validating essential fields; add explicit checks
after the getQuote call to ensure quote.Router, quote.InboundAddress, quote.Memo
are non-empty strings and quote.ExpectedAmountOut and quote.DustThreshold are
positive/non-zero (or within expected ranges), and if any validation fails
return a descriptive error (e.g., fmt.Errorf("invalid quote: missing Router") )
rather than proceeding; update the error handling in the block that currently
returns "failed to get quote" so it validates these fields before using them in
subsequent transaction building (referencing variables quote and
quoteSwapRequest to locate the code).
- Around line 98-105: Add explicit input validation in ProviderEvm.MakeTx to
ensure from.Amount is non-nil and > 0 before further processing: check if
from.Amount == nil or from.Amount.Cmp(big.NewInt(0)) <= 0 and return an error
(e.g., fmt.Errorf("invalid swap: from.Amount must be non-nil and positive"))
rather than proceeding; place this check at the start of MakeTx (before or
immediately after calling validateEvm) so callers like MakeTx and any downstream
logic never receive a nil/zero amount.
- Around line 50-70: Update the error returned from ProviderEvm.getTokenDecimals
to include the token address and the chain identifier used by the provider: when
the evm.CallReadonly call returns an error, wrap it with a message that includes
tokenAddress.Hex() and the ProviderEvm chain identifier (e.g., p.chain,
p.chainID or p.rpc endpoint field) so the log shows which token and chain
failed; modify the fmt.Errorf call in getTokenDecimals to embed those values
alongside the original error.
- Around line 156-164: The comparison uses mayaAmount.Uint64() which can panic
for values > uint64 max; replace that with a safe big.Int comparison: create a
big.Int thresholdBig := new(big.Int).SetUint64(dustThreshold) (or use
big.NewInt(0) then SetUint64) and change the condition to if
mayaAmount.Cmp(thresholdBig) < 0 { ... } so you avoid calling Uint64 on large
values and keep the same error return using from.Amount, mayaAmount.String(),
and exactAmount.String().
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/worker/main.go (1)
216-219: Add MayaChain EVM provider to the provider list to enable Arbitrum swap support.The MayaChain EVM provider is implemented in
internal/mayachain/provider_evm.goand explicitly supports ETH and ARB chains, but it's not included in the provider list at lines 216–219. Perinternal/evm/chains.go, Arbitrum is exclusively supported by MayaChain for swaps. ThemayachainClientis already initialized in the same file, making integration straightforward. Addmayachain.NewProviderEvm(mayachainClient, evmRpc, evmSdk)to the provider array to complete the advertised MayaChain Arbitrum swap functionality.
🧹 Nitpick comments (1)
internal/recurring/spec.go (1)
32-44: Clarify the swap routing comment to include all providers and chains.The comment mentions only "THORChain (ETH, BSC, BASE, AVAX) or MayaChain (ARB)" but
supportedSwapChainsincludes additional chains (BTC, SOL, XRP, ZEC) whose routing mechanisms are not documented. Consider updating the comment to reflect the complete routing picture:
- THORChain: ETH, BSC, BASE, AVAX, BTC, XRP, ZEC
- MayaChain: ARB
- Jupiter: SOL
🔎 Proposed documentation improvement
-// supportedSwapChains lists chains that support cross-chain swaps -// via THORChain (ETH, BSC, BASE, AVAX) or MayaChain (ARB) +// supportedSwapChains lists chains that support cross-chain swaps via: +// - THORChain: ETH, BSC, BASE, AVAX, BTC, XRP, ZEC +// - MayaChain: ARB +// - Jupiter: SOL (same-chain swaps only) var supportedSwapChains = []common.Chain{ - common.Ethereum, // THORChain - common.Arbitrum, // MayaChain - common.Avalanche, // THORChain - common.BscChain, // THORChain - common.Base, // THORChain - common.Bitcoin, // THORChain - common.Solana, // Jupiter - common.XRP, // THORChain - common.Zcash, // THORChain + common.Ethereum, + common.Arbitrum, + common.Avalanche, + common.BscChain, + common.Base, + common.Bitcoin, + common.Solana, + common.XRP, + common.Zcash, }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/worker/main.gointernal/evm/chains.gointernal/evm/evm_test.gointernal/mayachain/provider_evm.gointernal/recurring/spec.gointernal/recurring/spec_swap.go
🧰 Additional context used
📓 Path-based instructions (6)
internal/evm/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/evm/**/*.go: For Send operations in EVM chains, implement native token transfers using standard value transfers (21,000 gas) and ERC20 token transfers usingtransfer(address,uint256)function (65,000 gas)
For Swap operations, automatically handle approval checking and transaction building for ERC20 tokens
Use Keccak256 hashing for public key to Ethereum address conversion in EVM chain operations
When adding new swap providers (like 1inch), ensure they integrate cleanly with the existing EVM provider abstraction layer
Files:
internal/evm/chains.gointernal/evm/evm_test.go
internal/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/**/*.go: Include chain identification in error messages and logs for multi-chain operations
Use PostgreSQL withgithub.com/jackc/pgx/v5for policy and scheduler storage persistence
Use structured logging with configurable levels and include DataDog integration for metrics and monitoring across all services
Files:
internal/evm/chains.gointernal/recurring/spec_swap.gointernal/recurring/spec.gointernal/mayachain/provider_evm.gointernal/evm/evm_test.go
internal/recurring/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/recurring/**/*.go: Automatically detect whether an operation is Send or Swap based on the criteria: Send if (fromChain == toChain AND fromAsset == toAsset AND fromAddress != toAddress), otherwise Swap
Validate DCA parameters against JSON schema in the recipe configuration before policy creation
Files:
internal/recurring/spec_swap.gointernal/recurring/spec.go
internal/recurring/spec.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/recurring/spec.go: Use lowercase chain names for resource path construction in policy rules (e.g.,ethereum.swap,arbitrum.swap,bitcoin.send)
Dynamically generate policy rules and resource patterns inspec.gofor all supported chains rather than hardcoding chain-specific logic
Implement Send operations with maxTxsPerWindow=1 rate limiting (no approvals needed) and Swap operations with chain-specific maxTxsPerWindow values (EVM: 2, Solana: 8, XRP: 1)
Support the official send interface format{chain}.send (asset, from_address, amount, to_address, memo)for policy compliance
Files:
internal/recurring/spec.go
cmd/*/main.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/*/main.go: In Go service main files, use graceful shutdown handling via theinternal/graceful/package for clean service termination
Use environment-based configuration viagithub.com/kelseyhightower/envconfigfor all service configurations (database, Redis, blockchain RPCs, vault, DataDog)
Expose Prometheus metrics at/metricsendpoint for all services with port 8081 (Server), 8082 (Scheduler), 8083 (Worker), 8084 (TX Indexer)
Build services using CGO_ENABLED=0 GOOS=linux GOARCH=amd64 for cross-platform compatibility
Files:
cmd/worker/main.go
cmd/worker/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use
github.com/hibiken/asynqwith Redis for task consumption and job processing in the Worker service
Files:
cmd/worker/main.go
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/evm/**/*.go : When adding new swap providers (like 1inch), ensure they integrate cleanly with the existing EVM provider abstraction layer
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Ensure all code changes work across all supported chains (Ethereum, Arbitrum, Avalanche, BNB Chain, Base, Blast, Optimism, Polygon, Bitcoin, Solana, XRP) through testing
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Consider multi-chain implications when making code changes that affect operation support or configuration patterns
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/evm/**/*.go : Use Keccak256 hashing for public key to Ethereum address conversion in EVM chain operations
Applied to files:
internal/evm/chains.gointernal/recurring/spec.gointernal/mayachain/provider_evm.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/**/*.go : Include chain identification in error messages and logs for multi-chain operations
Applied to files:
internal/evm/chains.gointernal/recurring/spec.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/evm/**/*.go : When adding new swap providers (like 1inch), ensure they integrate cleanly with the existing EVM provider abstraction layer
Applied to files:
internal/evm/chains.gointernal/recurring/spec_swap.gointernal/recurring/spec.gocmd/worker/main.gointernal/mayachain/provider_evm.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/evm/**/*.go : For Send operations in EVM chains, implement native token transfers using standard value transfers (21,000 gas) and ERC20 token transfers using `transfer(address,uint256)` function (65,000 gas)
Applied to files:
internal/evm/chains.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/recurring/spec.go : Dynamically generate policy rules and resource patterns in `spec.go` for all supported chains rather than hardcoding chain-specific logic
Applied to files:
internal/evm/chains.gointernal/recurring/spec_swap.gointernal/recurring/spec.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Ensure all code changes work across all supported chains (Ethereum, Arbitrum, Avalanche, BNB Chain, Base, Blast, Optimism, Polygon, Bitcoin, Solana, XRP) through testing
Applied to files:
internal/evm/chains.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/recurring/**/*.go : Automatically detect whether an operation is Send or Swap based on the criteria: Send if (fromChain == toChain AND fromAsset == toAsset AND fromAddress != toAddress), otherwise Swap
Applied to files:
internal/evm/chains.gointernal/recurring/spec_swap.gointernal/recurring/spec.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/evm/**/*.go : For Swap operations, automatically handle approval checking and transaction building for ERC20 tokens
Applied to files:
internal/evm/chains.gointernal/recurring/spec_swap.gointernal/recurring/spec.gointernal/mayachain/provider_evm.gointernal/evm/evm_test.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/recurring/spec.go : Support the official send interface format `{chain}.send (asset, from_address, amount, to_address, memo)` for policy compliance
Applied to files:
internal/evm/chains.gointernal/recurring/spec.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/recurring/spec.go : Use lowercase chain names for resource path construction in policy rules (e.g., `ethereum.swap`, `arbitrum.swap`, `bitcoin.send`)
Applied to files:
internal/recurring/spec_swap.gointernal/recurring/spec.go
📚 Learning: 2025-12-16T21:31:34.971Z
Learnt from: CR
Repo: vultisig/app-recurring PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T21:31:34.971Z
Learning: Applies to internal/recurring/spec.go : Implement Send operations with maxTxsPerWindow=1 rate limiting (no approvals needed) and Swap operations with chain-specific maxTxsPerWindow values (EVM: 2, Solana: 8, XRP: 1)
Applied to files:
internal/recurring/spec.gointernal/evm/evm_test.go
🧬 Code graph analysis (1)
internal/evm/evm_test.go (1)
internal/evm/chains.go (3)
IsThorchainSupported(25-32)IsMayachainSupported(48-55)SupportedEVMChains(8-21)
🔇 Additional comments (18)
internal/recurring/spec.go (1)
115-121: LGTM!The new helper function correctly converts the
supportedSwapChainslist to string representations, maintaining consistency with the existinggetSupportedChainStrings()pattern.internal/recurring/spec_swap.go (2)
302-302: LGTM!Correctly updated to use
getSupportedSwapChainStrings(), ensuring the swap recipe configuration only advertises chains that support cross-chain swaps.
326-326: LGTM!Properly updated to iterate over
supportedSwapChainswhen building supported resources, ensuring resource patterns are only generated for swap-capable chains.cmd/worker/main.go (2)
194-197: LGTM!Cronos and zkSync chains are correctly added to the network configuration list with their respective RPC URLs.
359-362: LGTM!The RPC configuration struct is correctly extended with Cronos and Zksync fields to support the new chains.
internal/evm/chains.go (4)
7-21: LGTM!The function correctly returns all 10 supported EVM chains, including the newly added Cronos and zkSync, matching the PR objectives.
23-32: LGTM!The THORChain support check correctly identifies the four EVM chains (ETH, BSC, BASE, AVAX) that can route swaps through THORChain, as documented in the comment.
34-45: LGTM!Correctly identifies that all 10 EVM chains support direct send operations, which aligns with the PR's separation of send vs swap capabilities.
47-55: LGTM!Accurately identifies Arbitrum as the sole EVM chain supported by MayaChain for swap routing, which is a key feature of this PR.
internal/evm/evm_test.go (6)
13-35: LGTM!Comprehensive test coverage for THORChain support validation, including positive cases (ETH, BSC, BASE, AVAX) and negative cases (Arbitrum, Optimism, Polygon, Solana, Bitcoin). The comment on Line 22 correctly clarifies that Arbitrum is MayaChain-exclusive.
37-59: LGTM!Thorough test coverage for MayaChain support, correctly validating that only Arbitrum returns true while all other chains return false. Comments on Lines 42-43 helpfully document the routing distinctions.
61-83: LGTM!The test properly validates that
SupportedEVMChains()returns exactly 10 chains and explicitly checks for the presence of all expected chains including the newly added Cronos and zkSync.Based on learnings, this test ensures all code changes work across all supported EVM chains through comprehensive testing.
85-94: LGTM!Clean and simple mock implementation that provides full control over the
Providerinterface behavior for testing swap service logic.
96-168: LGTM!Excellent test coverage for the swap service's provider selection logic, covering all critical scenarios:
- Best provider selection based on amountOut
- Error handling when no providers are available
- Fallback to working providers when some fail
- Proper error reporting when all providers fail
170-199: LGTM!Comprehensive validation of the chain-to-provider routing expectations for all 10 EVM chains, clearly documenting which chains route through THORChain, MayaChain, or support only direct sends. This test serves as both validation and documentation of the routing architecture.
internal/mayachain/provider_evm.go (3)
72-94: LGTM! Decimal conversion logic is correct.The bidirectional conversion correctly handles scaling up and down, returning both the converted amount and the exact amount that matches the quote. This prevents precision loss issues.
178-200: Missing ERC20 approval checking for token swaps.When swapping ERC20 tokens (lines 191-199), the code doesn't check if the router has sufficient allowance or build an approval transaction if needed. Based on learnings, swap operations should automatically handle approval checking and transaction building for ERC20 tokens.
The current implementation will fail if users haven't pre-approved the router contract. Consider adding:
- Check current allowance for the router
- Build and return an approval transaction if allowance is insufficient
- Document that approval must be handled separately, or return multiple transactions
Based on learnings for files matching
internal/evm/**/*.go.⛔ Skipped due to learnings
Learnt from: CR Repo: vultisig/app-recurring PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-16T21:31:34.971Z Learning: Applies to internal/evm/**/*.go : For Swap operations, automatically handle approval checking and transaction building for ERC20 tokensLearnt from: CR Repo: vultisig/app-recurring PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-16T21:31:34.971Z Learning: Applies to internal/evm/**/*.go : When adding new swap providers (like 1inch), ensure they integrate cleanly with the existing EVM provider abstraction layerLearnt from: CR Repo: vultisig/app-recurring PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-16T21:31:34.971Z Learning: Applies to internal/evm/**/*.go : For Send operations in EVM chains, implement native token transfers using standard value transfers (21,000 gas) and ERC20 token transfers using `transfer(address,uint256)` function (65,000 gas)Learnt from: CR Repo: vultisig/app-recurring PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-16T21:31:34.971Z Learning: Applies to internal/recurring/**/*.go : Automatically detect whether an operation is Send or Swap based on the criteria: Send if (fromChain == toChain AND fromAsset == toAsset AND fromAddress != toAddress), otherwise Swap
131-132: Clarify or correct the CACAO decimal specification.The comment claims "MayaChain uses 10 decimals for CACAO (native)" but the code treats native tokens as 18 decimals (standard EVM), then converts to 8 for quotes. Either the comment is misleading about CACAO's actual decimal precision, or the code needs to differentiate between CACAO and other assets by checking the token address and applying appropriate conversion.
| // NewProviderEvm creates a new MayaChain EVM provider | ||
| func NewProviderEvm(client *Client, rpc *ethclient.Client, sdk *evm.SDK) *ProviderEvm { | ||
| return &ProviderEvm{ | ||
| client: client, | ||
| rpc: rpc, | ||
| sdk: sdk, | ||
| } | ||
| } |
There was a problem hiding this comment.
Add nil checks for constructor parameters.
The constructor doesn't validate that client, rpc, or sdk are non-nil. If any of these are nil, subsequent method calls will panic.
🔎 Proposed fix to add nil validation
func NewProviderEvm(client *Client, rpc *ethclient.Client, sdk *evm.SDK) *ProviderEvm {
+ if client == nil {
+ panic("client cannot be nil")
+ }
+ if rpc == nil {
+ panic("rpc cannot be nil")
+ }
+ if sdk == nil {
+ panic("sdk cannot be nil")
+ }
return &ProviderEvm{
client: client,
rpc: rpc,
sdk: sdk,
}
}📝 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.
| // NewProviderEvm creates a new MayaChain EVM provider | |
| func NewProviderEvm(client *Client, rpc *ethclient.Client, sdk *evm.SDK) *ProviderEvm { | |
| return &ProviderEvm{ | |
| client: client, | |
| rpc: rpc, | |
| sdk: sdk, | |
| } | |
| } | |
| // NewProviderEvm creates a new MayaChain EVM provider | |
| func NewProviderEvm(client *Client, rpc *ethclient.Client, sdk *evm.SDK) *ProviderEvm { | |
| if client == nil { | |
| panic("client cannot be nil") | |
| } | |
| if rpc == nil { | |
| panic("rpc cannot be nil") | |
| } | |
| if sdk == nil { | |
| panic("sdk cannot be nil") | |
| } | |
| return &ProviderEvm{ | |
| client: client, | |
| rpc: rpc, | |
| sdk: sdk, | |
| } | |
| } |
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 27-34, The NewProviderEvm
constructor does not validate its inputs (client, rpc, sdk) which can cause
panics later; update NewProviderEvm to check each parameter (client, rpc, sdk)
for nil and return nil (or panic with a clear message) when any are nil, and
document this behavior; specifically modify the NewProviderEvm function that
constructs a *ProviderEvm to perform nil checks for client, rpc, and sdk before
creating the ProviderEvm instance and handle the error case consistently with
project conventions.
| func (p *ProviderEvm) validateEvm(from evm_swap.From, to evm_swap.To) error { | ||
| _, err := parseMayaNetwork(from.Chain) | ||
| if err != nil { | ||
| return fmt.Errorf("unsupported 'from' chain for MayaChain: %w", err) | ||
| } | ||
|
|
||
| _, err = parseMayaNetwork(to.Chain) | ||
| if err != nil { | ||
| return fmt.Errorf("unsupported 'to' chain for MayaChain: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Include chain identification in error messages.
Error messages at lines 39 and 44 don't include the specific chain that failed validation. As per coding guidelines, error messages for multi-chain operations should include chain identification.
🔎 Proposed fix to include chain names in errors
func (p *ProviderEvm) validateEvm(from evm_swap.From, to evm_swap.To) error {
_, err := parseMayaNetwork(from.Chain)
if err != nil {
- return fmt.Errorf("unsupported 'from' chain for MayaChain: %w", err)
+ return fmt.Errorf("unsupported 'from' chain %s for MayaChain: %w", from.Chain, err)
}
_, err = parseMayaNetwork(to.Chain)
if err != nil {
- return fmt.Errorf("unsupported 'to' chain for MayaChain: %w", err)
+ return fmt.Errorf("unsupported 'to' chain %s for MayaChain: %w", to.Chain, err)
}
return nil
}As per coding guidelines for files matching internal/**/*.go.
📝 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.
| func (p *ProviderEvm) validateEvm(from evm_swap.From, to evm_swap.To) error { | |
| _, err := parseMayaNetwork(from.Chain) | |
| if err != nil { | |
| return fmt.Errorf("unsupported 'from' chain for MayaChain: %w", err) | |
| } | |
| _, err = parseMayaNetwork(to.Chain) | |
| if err != nil { | |
| return fmt.Errorf("unsupported 'to' chain for MayaChain: %w", err) | |
| } | |
| return nil | |
| } | |
| func (p *ProviderEvm) validateEvm(from evm_swap.From, to evm_swap.To) error { | |
| _, err := parseMayaNetwork(from.Chain) | |
| if err != nil { | |
| return fmt.Errorf("unsupported 'from' chain %s for MayaChain: %w", from.Chain, err) | |
| } | |
| _, err = parseMayaNetwork(to.Chain) | |
| if err != nil { | |
| return fmt.Errorf("unsupported 'to' chain %s for MayaChain: %w", to.Chain, err) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 36-48, In
ProviderEvm.validateEvm, the error returns from parseMayaNetwork do not include
which chain failed; update the two error returns (the parseMayaNetwork calls for
from.Chain and to.Chain) to include the chain identifier (from.Chain and
to.Chain respectively) in the fmt.Errorf message so the returned errors read
like "unsupported 'from' chain for MayaChain: %s: %w" (or similar) referencing
the failing chain value alongside the wrapped error from parseMayaNetwork.
| func (p *ProviderEvm) getTokenDecimals(ctx context.Context, tokenAddress common.Address) (uint8, error) { | ||
| if bytes.Equal(tokenAddress.Bytes(), evm.ZeroAddress.Bytes()) { | ||
| return 18, nil | ||
| } | ||
|
|
||
| erc20Contract := erc20.NewErc20() | ||
| decimals, err := evm.CallReadonly( | ||
| ctx, | ||
| p.rpc, | ||
| erc20Contract, | ||
| tokenAddress, | ||
| erc20Contract.PackDecimals(), | ||
| erc20Contract.UnpackDecimals, | ||
| nil, | ||
| ) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("failed to get token decimals: %w", err) | ||
| } | ||
|
|
||
| return decimals, nil | ||
| } |
There was a problem hiding this comment.
Include token address and chain in error message.
The error message at line 66 lacks context about which token and chain failed. This makes debugging multi-chain operations difficult.
🔎 Proposed fix to add context
- return 0, fmt.Errorf("failed to get token decimals: %w", err)
+ return 0, fmt.Errorf("failed to get token decimals for %s: %w", tokenAddress.Hex(), err)As per coding guidelines for files matching internal/**/*.go.
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 50-70, Update the error
returned from ProviderEvm.getTokenDecimals to include the token address and the
chain identifier used by the provider: when the evm.CallReadonly call returns an
error, wrap it with a message that includes tokenAddress.Hex() and the
ProviderEvm chain identifier (e.g., p.chain, p.chainID or p.rpc endpoint field)
so the log shows which token and chain failed; modify the fmt.Errorf call in
getTokenDecimals to embed those values alongside the original error.
| func (p *ProviderEvm) MakeTx( | ||
| ctx context.Context, | ||
| from evm_swap.From, | ||
| to evm_swap.To, | ||
| ) (*big.Int, []byte, error) { | ||
| if err := p.validateEvm(from, to); err != nil { | ||
| return nil, nil, fmt.Errorf("invalid swap: %w", err) | ||
| } |
There was a problem hiding this comment.
Add input validation for swap parameters.
The function doesn't validate that from.Amount is non-nil and positive before processing. This could lead to panics or incorrect behavior.
🔎 Proposed fix to add input validation
func (p *ProviderEvm) MakeTx(
ctx context.Context,
from evm_swap.From,
to evm_swap.To,
) (*big.Int, []byte, error) {
+ if from.Amount == nil || from.Amount.Sign() <= 0 {
+ return nil, nil, fmt.Errorf("invalid amount for chain %s: must be positive", from.Chain)
+ }
if err := p.validateEvm(from, to); err != nil {
return nil, nil, fmt.Errorf("invalid swap: %w", err)
}📝 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.
| func (p *ProviderEvm) MakeTx( | |
| ctx context.Context, | |
| from evm_swap.From, | |
| to evm_swap.To, | |
| ) (*big.Int, []byte, error) { | |
| if err := p.validateEvm(from, to); err != nil { | |
| return nil, nil, fmt.Errorf("invalid swap: %w", err) | |
| } | |
| func (p *ProviderEvm) MakeTx( | |
| ctx context.Context, | |
| from evm_swap.From, | |
| to evm_swap.To, | |
| ) (*big.Int, []byte, error) { | |
| if from.Amount == nil || from.Amount.Sign() <= 0 { | |
| return nil, nil, fmt.Errorf("invalid amount for chain %s: must be positive", from.Chain) | |
| } | |
| if err := p.validateEvm(from, to); err != nil { | |
| return nil, nil, fmt.Errorf("invalid swap: %w", err) | |
| } |
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 98-105, Add explicit input
validation in ProviderEvm.MakeTx to ensure from.Amount is non-nil and > 0 before
further processing: check if from.Amount == nil or
from.Amount.Cmp(big.NewInt(0)) <= 0 and return an error (e.g.,
fmt.Errorf("invalid swap: from.Amount must be non-nil and positive")) rather
than proceeding; place this check at the start of MakeTx (before or immediately
after calling validateEvm) so callers like MakeTx and any downstream logic never
receive a nil/zero amount.
| if err := p.validateEvm(from, to); err != nil { | ||
| return nil, nil, fmt.Errorf("invalid swap: %w", err) | ||
| } | ||
|
|
||
| fromMayaNet, err := parseMayaNetwork(from.Chain) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("unsupported from chain: %w", err) | ||
| } | ||
|
|
||
| var fromAsset string | ||
| if bytes.Equal(from.AssetID.Bytes(), evm.ZeroAddress.Bytes()) { | ||
| nativeSymbol, er := from.Chain.NativeSymbol() | ||
| if er != nil { | ||
| return nil, nil, fmt.Errorf("failed to get native symbol: %w", er) | ||
| } | ||
| fromAsset = string(fromMayaNet) + "." + nativeSymbol | ||
| } else { | ||
| fromAsset, err = makeMayaAsset(ctx, p.client, from.Chain, from.AssetID.Hex()) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to resolve from asset: %w", err) | ||
| } | ||
| } | ||
|
|
||
| tokenDecimals, err := p.getTokenDecimals(ctx, from.AssetID) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to get token decimals: %w", err) | ||
| } | ||
|
|
||
| // MayaChain uses 10 decimals for CACAO (native) and 8 decimals for most assets | ||
| mayaAmount, exactAmount := convertDecimals(from.Amount, tokenDecimals, 8) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add structured logging for swap operations.
The function performs critical swap operations but lacks structured logging. As per coding guidelines, structured logging with configurable levels should be used across all services for observability.
Consider adding log statements at key points:
- Swap initiation with chain and amount details
- Quote received with expected output
- Transaction built successfully
- Any validation failures
As per coding guidelines for files matching internal/**/*.go.
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 103-132, The swap flow in
provider_evm.go lacks structured logs; add structured logging using the
provider's logger (e.g., p.logger or p.log) around the key steps so
observability is consistent: log an Info when swap starts after p.validateEvm
with fields chain=from.Chain, asset=from.AssetID.Hex(), amount=from.Amount; log
Info after parseMayaNetwork with fromMayaNet; log Info after resolving fromAsset
(makeMayaAsset or native branch) with fromAsset; log Info after getTokenDecimals
with tokenDecimals and after convertDecimals with mayaAmount and exactAmount; on
any error returns (validateEvm, parseMayaNetwork, makeMayaAsset,
getTokenDecimals) use p.logger.WithError(err).Error including the same
structured fields and the error; use appropriate levels (Info for successes,
Error for failures) and include contextual messages like "swap initiated",
"resolved asset", "token decimals retrieved", and "amount converted".
| quote, err := p.client.getQuote(ctx, quoteSwapRequest{ | ||
| FromAsset: fromAsset, | ||
| ToAsset: toAsset, | ||
| Amount: mayaAmount.String(), | ||
| Destination: to.Address, | ||
| StreamingInterval: defaultStreamingInterval, | ||
| StreamingQuantity: defaultStreamingQuantity, | ||
| }) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to get quote: %w", err) | ||
| } |
There was a problem hiding this comment.
Validate quote response fields before use.
The quote response fields (Router, InboundAddress, Memo, ExpectedAmountOut, DustThreshold) are used without validation. If any of these are empty or invalid, it could lead to incorrect transactions or panics.
🔎 Proposed fix to validate quote fields
quote, err := p.client.getQuote(ctx, quoteSwapRequest{
FromAsset: fromAsset,
ToAsset: toAsset,
Amount: mayaAmount.String(),
Destination: to.Address,
StreamingInterval: defaultStreamingInterval,
StreamingQuantity: defaultStreamingQuantity,
})
if err != nil {
- return nil, nil, fmt.Errorf("failed to get quote: %w", err)
+ return nil, nil, fmt.Errorf("failed to get quote for %s to %s: %w", from.Chain, to.Chain, err)
}
+
+ if quote.Router == "" || quote.InboundAddress == "" || quote.Memo == "" {
+ return nil, nil, fmt.Errorf("invalid quote response: missing required fields for %s to %s swap", from.Chain, to.Chain)
+ }📝 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.
| quote, err := p.client.getQuote(ctx, quoteSwapRequest{ | |
| FromAsset: fromAsset, | |
| ToAsset: toAsset, | |
| Amount: mayaAmount.String(), | |
| Destination: to.Address, | |
| StreamingInterval: defaultStreamingInterval, | |
| StreamingQuantity: defaultStreamingQuantity, | |
| }) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to get quote: %w", err) | |
| } | |
| quote, err := p.client.getQuote(ctx, quoteSwapRequest{ | |
| FromAsset: fromAsset, | |
| ToAsset: toAsset, | |
| Amount: mayaAmount.String(), | |
| Destination: to.Address, | |
| StreamingInterval: defaultStreamingInterval, | |
| StreamingQuantity: defaultStreamingQuantity, | |
| }) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to get quote for %s to %s: %w", from.Chain, to.Chain, err) | |
| } | |
| if quote.Router == "" || quote.InboundAddress == "" || quote.Memo == "" { | |
| return nil, nil, fmt.Errorf("invalid quote response: missing required fields for %s to %s swap", from.Chain, to.Chain) | |
| } |
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 139-149, The quote returned
by p.client.getQuote (assigned to variable quote) is used without validating
essential fields; add explicit checks after the getQuote call to ensure
quote.Router, quote.InboundAddress, quote.Memo are non-empty strings and
quote.ExpectedAmountOut and quote.DustThreshold are positive/non-zero (or within
expected ranges), and if any validation fails return a descriptive error (e.g.,
fmt.Errorf("invalid quote: missing Router") ) rather than proceeding; update the
error handling in the block that currently returns "failed to get quote" so it
validates these fields before using them in subsequent transaction building
(referencing variables quote and quoteSwapRequest to locate the code).
| if mayaAmount.Uint64() < dustThreshold { | ||
| return nil, nil, fmt.Errorf( | ||
| "amount %s (8-decimal: %s, exact: %s) below dust threshold %d", | ||
| from.Amount.String(), | ||
| mayaAmount.String(), | ||
| exactAmount.String(), | ||
| dustThreshold, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix potential panic in dust threshold comparison.
Line 156 calls mayaAmount.Uint64() which will panic if mayaAmount exceeds uint64 max value (2^64 - 1). For tokens with 18 decimals, amounts above ~18.4 ETH would cause a panic.
🔎 Proposed fix using big.Int comparison
dustThreshold, err := strconv.ParseUint(quote.DustThreshold, 10, 64)
if err != nil {
return nil, nil, fmt.Errorf("failed to parse dust threshold: %w", err)
}
- if mayaAmount.Uint64() < dustThreshold {
+ dustThresholdBig := new(big.Int).SetUint64(dustThreshold)
+ if mayaAmount.Cmp(dustThresholdBig) < 0 {
return nil, nil, fmt.Errorf(
- "amount %s (8-decimal: %s, exact: %s) below dust threshold %d",
+ "amount %s (8-decimal: %s, exact: %s) below dust threshold %s for chain %s",
from.Amount.String(),
mayaAmount.String(),
exactAmount.String(),
- dustThreshold,
+ dustThresholdBig.String(),
+ from.Chain,
)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 156-164, The comparison uses
mayaAmount.Uint64() which can panic for values > uint64 max; replace that with a
safe big.Int comparison: create a big.Int thresholdBig :=
new(big.Int).SetUint64(dustThreshold) (or use big.NewInt(0) then SetUint64) and
change the condition to if mayaAmount.Cmp(thresholdBig) < 0 { ... } so you avoid
calling Uint64 on large values and keep the same error return using from.Amount,
mayaAmount.String(), and exactAmount.String().
| unsignedTx, err := p.sdk.MakeTx( | ||
| ctx, | ||
| from.Address, | ||
| routerAddr, | ||
| value, | ||
| data, | ||
| 0, // nonceOffset | ||
| ) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to build tx: %w", err) | ||
| } | ||
|
|
||
| expectedOut, ok := new(big.Int).SetString(quote.ExpectedAmountOut, 10) | ||
| if !ok { | ||
| return nil, nil, fmt.Errorf("failed to parse expected amount out: %s", quote.ExpectedAmountOut) | ||
| } | ||
|
|
||
| targetTokenAddr := common.HexToAddress(to.AssetID) | ||
| targetDecimals, err := p.getTokenDecimals(ctx, targetTokenAddr) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to get target token decimals: %w", err) | ||
| } | ||
|
|
||
| realExpectedOut, _ := convertDecimals(expectedOut, 8, targetDecimals) | ||
|
|
||
| return realExpectedOut, unsignedTx, nil | ||
| } |
There was a problem hiding this comment.
Include chain identification in transaction building errors.
Error messages at lines 211 and 222 lack chain context, making debugging difficult in a multi-chain environment.
🔎 Proposed fix to add chain context
unsignedTx, err := p.sdk.MakeTx(
ctx,
from.Address,
routerAddr,
value,
data,
0, // nonceOffset
)
if err != nil {
- return nil, nil, fmt.Errorf("failed to build tx: %w", err)
+ return nil, nil, fmt.Errorf("failed to build tx for chain %s: %w", from.Chain, err)
}
expectedOut, ok := new(big.Int).SetString(quote.ExpectedAmountOut, 10)
if !ok {
- return nil, nil, fmt.Errorf("failed to parse expected amount out: %s", quote.ExpectedAmountOut)
+ return nil, nil, fmt.Errorf("failed to parse expected amount out for chain %s: %s", to.Chain, quote.ExpectedAmountOut)
}
targetTokenAddr := common.HexToAddress(to.AssetID)
targetDecimals, err := p.getTokenDecimals(ctx, targetTokenAddr)
if err != nil {
- return nil, nil, fmt.Errorf("failed to get target token decimals: %w", err)
+ return nil, nil, fmt.Errorf("failed to get target token decimals for %s on chain %s: %w", to.AssetID, to.Chain, err)
}As per coding guidelines for files matching internal/**/*.go.
📝 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.
| unsignedTx, err := p.sdk.MakeTx( | |
| ctx, | |
| from.Address, | |
| routerAddr, | |
| value, | |
| data, | |
| 0, // nonceOffset | |
| ) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to build tx: %w", err) | |
| } | |
| expectedOut, ok := new(big.Int).SetString(quote.ExpectedAmountOut, 10) | |
| if !ok { | |
| return nil, nil, fmt.Errorf("failed to parse expected amount out: %s", quote.ExpectedAmountOut) | |
| } | |
| targetTokenAddr := common.HexToAddress(to.AssetID) | |
| targetDecimals, err := p.getTokenDecimals(ctx, targetTokenAddr) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to get target token decimals: %w", err) | |
| } | |
| realExpectedOut, _ := convertDecimals(expectedOut, 8, targetDecimals) | |
| return realExpectedOut, unsignedTx, nil | |
| } | |
| unsignedTx, err := p.sdk.MakeTx( | |
| ctx, | |
| from.Address, | |
| routerAddr, | |
| value, | |
| data, | |
| 0, // nonceOffset | |
| ) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to build tx for chain %s: %w", from.Chain, err) | |
| } | |
| expectedOut, ok := new(big.Int).SetString(quote.ExpectedAmountOut, 10) | |
| if !ok { | |
| return nil, nil, fmt.Errorf("failed to parse expected amount out for chain %s: %s", to.Chain, quote.ExpectedAmountOut) | |
| } | |
| targetTokenAddr := common.HexToAddress(to.AssetID) | |
| targetDecimals, err := p.getTokenDecimals(ctx, targetTokenAddr) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to get target token decimals for %s on chain %s: %w", to.AssetID, to.Chain, err) | |
| } | |
| realExpectedOut, _ := convertDecimals(expectedOut, 8, targetDecimals) | |
| return realExpectedOut, unsignedTx, nil | |
| } |
🤖 Prompt for AI Agents
In @internal/mayachain/provider_evm.go around lines 202-228, The errors returned
when building the tx (from p.sdk.MakeTx) and when fetching token decimals
(p.getTokenDecimals) lack chain context; update the fmt.Errorf calls to include
the provider's chain identifier (e.g., p.chainID or p.chainName) so messages
read like "failed to build tx for chain %s: %w" and "failed to get target token
decimals for chain %s: %w" (use the actual field name on the provider struct),
leaving other logic (parsing expectedOut and convertDecimals) unchanged.
| {common.CronosChain, cfg.Rpc.Cronos.URL}, | ||
| {common.Optimism, cfg.Rpc.Optimism.URL}, | ||
| {common.Polygon, cfg.Rpc.Polygon.URL}, | ||
| {common.Zksync, cfg.Rpc.Zksync.URL}, |
There was a problem hiding this comment.
Please add new config values in deploy dir
Key Commits:
feat(evm): add MayaChain EVM provider and chain support helpers
feat(evm): add Cronos and zkSync chain support
feat(swap): separate supported chains for swaps vs sends
Changes:
Added all 10 EVM chains + Cronos/zkSync to worker
MayaChain Arbitrum swap support
Separate chain lists: 14 chains for sends, 9 for swaps (prevents users selecting unsupported swap chains)
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.