Conversation
WalkthroughThis pull request introduces support for the Venus protocol pools on the Binance Smart Chain by adding new constants, implementing a Changes
Possibly related PRs
Suggested reviewers
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
pkg/venus.go (1)
473-530: Consider removing the redundantchainIDparameter in methods; usel.chainIDinsteadThe
chainIDparameter in methods likeGenerateCalldataandValidateis redundant becausel.chainIDis already stored in theVenusOperationstruct. Usingl.chainIDdirectly within these methods reduces the risk of inconsistency and simplifies the method signatures.pkg/validate.go (1)
11-13: SimplifyIsZeroAddressfunction by comparing addresses directlyInstead of converting addresses to hexadecimal strings for comparison, you can compare the addresses directly for better performance and readability.
Apply this diff to simplify the function:
func IsZeroAddress(addr common.Address) bool { - return common.HexToAddress(zeroAddress).Hex() == addr.Hex() + return addr == common.Address{} }pkg/venus_test.go (4)
15-31: Consider moving the hardcoded contract address to a constant.The test is well-structured with both error and success cases. However, the contract address
0xd933909A4a2b7A4638903028f44D1d38ce27c352is hardcoded in multiple places. Consider moving it to a constant at the package level for better maintainability.+const ( + testVenusContractAddress = "0xd933909A4a2b7A4638903028f44D1d38ce27c352" +) func TestVenusOperation_New(t *testing.T) { t.Run("unsupported chain", func(t *testing.T) { venus, err := NewVenusOperation(getTestClient(t, ChainETH), big.NewInt(1), - common.HexToAddress("0xd933909A4a2b7A4638903028f44D1d38ce27c352")) + common.HexToAddress(testVenusContractAddress))
33-72: Add positive validation test cases.While the error cases are well covered (unsupported chain, action, and asset), consider adding positive test cases to verify that valid inputs pass validation successfully.
func TestVenusOperation_Validate(t *testing.T) { + t.Run("valid parameters", func(t *testing.T) { + venus, err := NewVenusOperation(getTestClient(t, ChainBSC), + big.NewInt(56), + common.HexToAddress(testVenusContractAddress)) + require.NoError(t, err) + + err = venus.Validate(context.Background(), big.NewInt(56), LoanSupply, TransactionParams{ + Amount: big.NewInt(1), + Asset: common.HexToAddress("0xbb4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c"), + }) + require.NoError(t, err) + })
95-174: Enhance amount comments with exact decimal representation.The comments for BNB amounts (e.g.,
// 0.1 BNB) could be more precise by including the exact decimal representation. This helps readers understand the exact value without mental conversion.- Amount: big.NewInt(100000000000000000), // 0.1 BNB + Amount: big.NewInt(100000000000000000), // 0.1 BNB (0.1 * 10^18)
195-196: Improve error handling for GetSupportedAssets.The error from
GetSupportedAssetsis checked but not used. Consider either:
- Using the returned assets in the subsequent tests, or
- Adding assertions about the expected supported assets
- _, err = venus.GetSupportedAssets(context.Background(), big.NewInt(56)) + assets, err := venus.GetSupportedAssets(context.Background(), big.NewInt(56)) require.NoError(t, err) + require.NotEmpty(t, assets, "Expected supported assets to be non-empty")Also applies to: 225-226
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pkg/constants.go(1 hunks)pkg/registry.go(1 hunks)pkg/validate.go(1 hunks)pkg/venus.go(1 hunks)pkg/venus_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: lint
🔇 Additional comments (2)
pkg/constants.go (1)
62-73: Additions align with existing naming conventionsThe addition of
VenusProtocolIsolatedPoolandVenusProtocolCorePoolconstants is appropriate and follows the existing naming conventions for protocol names.pkg/venus_test.go (1)
1-13: LGTM! Build tags and imports are properly configured.The integration test build tags are correctly formatted, and the imports include all necessary packages.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pkg/aave.go (1)
479-480: Consider documenting this pattern for Venus implementation.Since this PR aims to add Venus Isolated pools support, this source/destination pattern implementation in Aave can serve as a reference for the Venus protocol implementation. Consider documenting this pattern in the codebase to ensure consistent implementation across protocols.
tools/tokens.go (2)
14-31: Consider using constants for chain IDs.To improve maintainability and avoid magic numbers, consider defining chain IDs as constants. This is especially important as they are used in multiple places throughout the codebase.
+const ( + EthereumChainID = 1 + BNBChainID = 56 + PolygonChainID = 137 +) func main() { chains := []struct { Name string ChainID *big.Int }{ { Name: "Ethereum", - ChainID: big.NewInt(1), + ChainID: big.NewInt(EthereumChainID), }, { Name: "BNB Chain", - ChainID: big.NewInt(56), + ChainID: big.NewInt(BNBChainID), }, { Name: "Polygon", - ChainID: big.NewInt(137), + ChainID: big.NewInt(PolygonChainID), }, }
112-116: Consider platform-specific file permissions.The file permissions are hardcoded to
0644. Consider using platform-specific constants for file permissions to ensure better compatibility across different operating systems.+const defaultFileMode = 0644 -err = os.WriteFile(fileName, data, 0644) +err = os.WriteFile(fileName, data, defaultFileMode)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
generate.go(1 hunks)pkg/aave.go(1 hunks)pkg/ankr.go(1 hunks)pkg/compound.go(1 hunks)pkg/constants.go(1 hunks)pkg/lido.go(1 hunks)pkg/rocket_pool.go(1 hunks)tools/tokens.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- generate.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/constants.go
🧰 Additional context used
🪛 golangci-lint (1.62.2)
tools/tokens.go
10-10: could not import github.com/blndgs/protocol_registry/pkg (-: # github.com/blndgs/protocol_registry/pkg
pkg/registry.go:325:10: cannot use NewListaStakingOperation(client, BscChainID) (value of type *ListaStakingOperation) as Protocol value in return statement: *ListaStakingOperation does not implement Protocol (missing method IsDestination)
pkg/registry.go:333:10: cannot use NewBinanceWrappedEthOperation(client, BscChainID) (value of type *BinanceWrappedEthOperation) as Protocol value in return statement: *BinanceWrappedEthOperation does not implement Protocol (missing method IsDestination)
pkg/venus.go:339:82: cannot use c (variable of type *VenusOperation) as Protocol value in argument to registry.RegisterProtocol: *VenusOperation does not implement Protocol (missing method IsDestination))
(typecheck)
🪛 GitHub Check: lint
tools/tokens.go
[failure] 10-10:
could not import github.com/blndgs/protocol_registry/pkg (-: # github.com/blndgs/protocol_registry/pkg
🔇 Additional comments (7)
pkg/lido.go (1)
196-197: LGTM! Implementation aligns with Lido's protocol behavior.The implementation correctly identifies Lido as a destination-only protocol, which aligns with its role as a liquid staking protocol that only accepts deposits.
pkg/ankr.go (1)
206-207: LGTM! Implementation aligns with Ankr's bidirectional protocol behavior.The implementation correctly identifies Ankr as both source and destination, which matches its capability to handle both staking and unstaking operations.
pkg/rocket_pool.go (1)
294-295: LGTM! Implementation aligns with RocketPool's bidirectional protocol behavior.The implementation correctly identifies RocketPool as both source and destination, which matches its capability to handle both staking and unstaking operations.
pkg/compound.go (2)
424-425: LGTM! Implementation aligns with Compound's bidirectional protocol behavior.The implementation correctly identifies Compound as both source and destination, which matches its capability to handle both supply and withdrawal operations.
Line range hint
1-425: Missing Venus protocol implementation.While the interface implementations for existing protocols look good, the PR objective is to add Venus Isolated pools support on BNB. However, I don't see any implementation for the Venus protocol or BNB chain support in the provided files.
Let's verify if Venus-related files exist in the codebase:
Would you like me to help with implementing the Venus protocol support or creating a GitHub issue to track this task?
pkg/aave.go (1)
479-480: Implementation looks good!The implementation correctly identifies Aave operations as both source and destination, which aligns with the protocol's functionality where users can both supply assets to and withdraw assets from the protocol.
tools/tokens.go (1)
54-63: Skip processing of intentionally excluded files.The PR mentions that file
56.jsonis intentionally excluded, but the tool will still attempt to process it. Consider adding a check to skip processing of excluded files.for _, chain := range chains { + // Skip processing of intentionally excluded files + if chain.ChainID.Cmp(big.NewInt(56)) == 0 { + fmt.Printf("Skipping file for %s as it's intentionally excluded\n", chain.Name) + continue + } + fmt.Println("updating file for", chain.Name)
blewater
left a comment
blewater
left a comment
There was a problem hiding this comment.
Have you seen the linting errors?
Yes actually. PR not ready after latest changes to try fo automate the token lists for protocols . Will wrap it up in the morning |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
pkg/venus.go (1)
308-309:⚠️ Potential issueAvoid using
log.Fatalfin library code; return error insteadUsing
log.Fatalfwill terminate the entire program, which is inappropriate for library code. Instead, return the error to allow the caller to handle it appropriately.Apply this diff to fix the issue:
data, err := parsedABI.Pack("getAllPools") if err != nil { - log.Fatalf("Failed to pack contract call: %v", err) + return fmt.Errorf("Failed to pack contract call: %v", err) }tools/tokens.go (3)
32-33: 🛠️ Refactor suggestionAvoid using
panicfor configuration errors; return an error insteadUsing
panicwhen theMORALIS_API_KEYis not provided will terminate the program unexpectedly. Instead, return an error and handle it gracefully in themainfunction.Apply this diff to handle the error appropriately:
var moralisAPIKey = os.Getenv("MORALIS_API_KEY") if moralisAPIKey == "" { - panic("provide moralis api key in .env") + return "", "", 0, errors.New("MORALIS_API_KEY is not set in the environment") }And adjust the function signature to propagate the error.
106-109: 🛠️ Refactor suggestionAvoid using
panicin themainfunction; handle errors gracefullyUsing
panicin themainfunction can cause the program to exit abruptly. Instead, handle the error and exit gracefully with an appropriate message.Apply this diff to improve error handling:
if err := godotenv.Load(); err != nil { fmt.Println("Add an .env file with MORALIS_API_KEY=xyz") - panic(err) + fmt.Printf("Failed to load .env file: %v\n", err) + os.Exit(1) }This change provides a clear error message and exits the program without a stack trace.
156-158: 🛠️ Refactor suggestionAvoid using
panicfor error handling; return or log the error insteadUsing
panicwhen the registry initialization fails is not recommended. Handle the error gracefully to allow for proper cleanup or logging.Apply this diff to handle the error:
if err != nil { - panic(err.Error()) + fmt.Printf("Failed to initialize protocol registry: %v\n", err) + os.Exit(1) }This provides a user-friendly error message and exits the program cleanly.
🧹 Nitpick comments (3)
pkg/venus.go (2)
333-342: Handle errors gracefully when registering Venus poolsIn the loop where Venus pools are registered, if an error occurs for one pool, the function returns immediately. This could prevent other valid pools from being registered.
Consider collecting errors and continuing with the next pool, or at least logging the error before proceeding. Here's a possible modification:
for _, poolAddr := range pools { c, err := NewVenusOperation(client, big.NewInt(chainID), poolAddr.Comptroller) if err != nil { - return err + fmt.Printf("Failed to initialize VenusOperation for pool %s: %v\n", poolAddr.Comptroller.Hex(), err) + continue } if err := registry.RegisterProtocol(big.NewInt(chainID), poolAddr.Comptroller, c); err != nil { - return err + fmt.Printf("Failed to register protocol for pool %s: %v\n", poolAddr.Comptroller.Hex(), err) + continue } }This change allows the function to attempt to register all pools, even if some errors occur.
571-600: Check for zero address and asset support inGetBalancemethodIn the
GetBalancemethod, there is no check to verify if theaccountaddress is zero or if theassetis supported before proceeding.Consider adding validation to ensure that the
accountis not a zero address and theassetis supported:func (l *VenusOperation) GetBalance(ctx context.Context, chainID *big.Int, account, asset common.Address) (common.Address, *big.Int, error) { + if IsZeroAddress(account) { + return common.Address{}, nil, errors.New("account address cannot be zero") + } + + if !l.IsSupportedAsset(ctx, chainID, asset) { + return common.Address{}, nil, errors.New("asset not supported") + }This adds an extra layer of validation to prevent unnecessary calls and potential errors.
go.mod (1)
7-7: Evaluate the necessity of addinggithub.com/joho/godotenvdependencyThe addition of
github.com/joho/godotenv v1.5.1introduces a dependency for loading environment variables from a.envfile. Evaluate if this is necessary for the production environment or if an alternative configuration management approach would be more appropriate.If environment variables are managed differently in production, consider removing this dependency and loading configuration in a manner consistent with the application's deployment strategy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
.env.example(1 hunks)docs/00_protocol.md(1 hunks)go.mod(1 hunks)pkg/binance.go(1 hunks)pkg/listadao_stake.go(1 hunks)pkg/venus.go(1 hunks)tokens/1.json(1 hunks)tokens/137.json(2 hunks)tokens/56.json(2 hunks)tools/tokens.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.example
🧰 Additional context used
🪛 Gitleaks (8.21.2)
tokens/137.json
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/1.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
136-136: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
142-142: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (11)
pkg/listadao_stake.go (1)
207-208: LGTM! Implementation aligns with the staking operation's role.The implementation correctly identifies
ListaStakingOperationas a destination (not a source) which is consistent with its role as a staking contract.Let's verify this implementation aligns with other staking protocols:
✅ Verification successful
Implementation verified: Matches established protocol patterns
The
ListaStakingOperation's implementation aligns with similar staking protocols, particularly matching Lido's pattern (IsSource=false, IsDestination=true). All protocols consistently implementIsDestination=true, confirming this part is correct. While some protocols implementIsSource=true, having it set tofalseis a valid design choice that matches other staking implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of IsSource/IsDestination implementations across staking protocols # Search for other staking protocol implementations ast-grep --pattern 'type $protocol struct { $$$ } func ($v *$protocol) GetType() ProtocolType { return TypeStake } $$$' # Search for their IsSource/IsDestination implementations rg -A 2 'func \([^)]+\) IsSource\(\) bool' rg -A 2 'func \([^)]+\) IsDestination\(\) bool'Length of output: 2187
pkg/binance.go (1)
210-211: Implementation looks good!The added methods correctly implement the Protocol interface, indicating that BinanceWrappedEthOperation can serve as both a source and destination in transaction flows.
docs/00_protocol.md (1)
44-48: Documentation is clear and comprehensive!The added interface methods are well-documented, clearly explaining their purpose:
- IsSource(): Indicates if the protocol can be withdrawn from
- IsDestination(): Indicates if the protocol accepts supplies
tokens/56.json (2)
Line range hint
4-150: Token entries are well-structured!All token entries contain the required fields (address, name, symbol, decimals) and follow a consistent format.
Note: The static analysis warnings about API keys can be safely ignored as they are false positives triggered by the token addresses.
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
156-279: Venus isolated pools configuration looks good!The protocol configurations are consistent and well-structured:
- Each pool has correct source/destination flags
- Token lists are properly formatted
- Type is correctly set to "Loan"
Let's verify the token addresses:
✅ Verification successful
All Venus isolated pool contract addresses are valid and deployed
All checked addresses contain deployed proxy contracts with consistent bytecode patterns, confirming they are legitimate Venus isolated pool implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify token addresses on BNB chain # Check if addresses are valid contracts for addr in \ "0x3344417c9360b963ca93A4e8305361AEde340Ab9" \ "0x1b43ea8622e76627B81665B1eCeBB4867566B963" \ "0x23b4404E4E5eC5FF5a6FFb70B7d14E3FabF237B0" \ "0x33B6fa34cd23e5aeeD1B112d5988B026b8A5567d" \ "0xBE609449Eb4D76AD8545f957bBE04b596E8fC529" \ "0x9DF11376Cf28867E2B0741348044780FbB7cb1d6" do echo "Checking contract at $addr" curl -s -X POST -H "Content-Type: application/json" \ --data "{\"jsonrpc\":\"2.0\",\"method\":\"eth_getCode\",\"params\":[\"$addr\",\"latest\"],\"id\":1}" \ https://bsc-dataseed.binance.org doneLength of output: 11094
tokens/137.json (3)
11-11: LGTM! Consistent token naming and properties.The token definitions have been standardized with:
- Proper "(PoS)" suffix for Polygon-specific tokens
- Consistent naming conventions (e.g., "USD Coin" instead of "USDC")
- Appropriate decimal places for each token type
Also applies to: 17-19, 23-23, 29-31, 35-35, 41-41, 47-49, 53-55, 59-60, 65-67, 71-73, 77-79, 83-85
110-136: Verify the duplicate compound protocol entries.There are two compound protocol entries with different addresses but identical token lists (in different orders). This might be unintentional or could indicate a configuration error.
Please confirm if both compound protocol entries are necessary. If they are:
- Document the purpose of each entry
- Consider adding distinguishing names (e.g., "compound_v2", "compound_v3") or comments to clarify their roles
Line range hint
94-107: Verify token references in protocol definitions.Let's ensure all token addresses referenced in protocol.tokens arrays exist in the tokens section.
Also applies to: 116-121, 130-135
tools/tokens.go (2)
218-220: Use protocol's source and destination capabilities instead of hardcodingThe
SourceandDestinationfields are hardcoded astruefor all protocols. This may not accurately reflect each protocol's capabilities.Update the code to use the protocol's
IsSourceandIsDestinationmethods:protocolData.Protocols = append(protocolData.Protocols, tokens.Protocol{ Address: p.GetContractAddress(chain.ChainID).Hex(), Name: p.GetName(), Type: string(p.GetType()), - Source: true, - Destination: true, + Source: p.IsSource(), + Destination: p.IsDestination(), Tokens: validTokenAddrs, })This ensures that the protocol data accurately represents each protocol's capabilities.
229-236: Ensure consistent handling of native token decimalsThe code assumes that native tokens always have 18 decimals. While this is true for Ethereum and similar chains, it may not hold for all blockchains.
Please verify that the native tokens on all supported chains indeed have 18 decimals. Run the following script to confirm the decimals for native tokens:
tokens/1.json (1)
137-242:⚠️ Potential issueMissing Venus protocol configuration
The PR objectives mention adding support for Venus V4 isolated pools on BNB, but the protocol configuration is missing from this file.
Let's verify if Venus protocol configurations exist in other files:
Please add the Venus protocol configuration with appropriate isolated pool addresses and supported tokens.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
tokens/137.json (1)
Line range hint
154-155: Remove duplicate token entryThe token address
0xae78736Cd615f374D3085123A210448E74Fc6393appears twice in the aave_v3 protocol tokens list.Apply this diff to remove the duplicate entry:
"tokens": [ "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599", "0x514910771AF9Ca656af840dff83E8264EcF986CA", "0x6B175474E89094C44Da98b954EedeAC495271d0F", "0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9", "0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0", "0x83F20F44975D03b1b09e64809B757c47f942BEeA", "0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2", "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", "0xBe9895146f7AF43049ca1c1AE358B0541Ea49704", "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2", "0xCd5fE23C85820F7B72D0926FC9b05b43E359b7ee", "0xae78736Cd615f374D3085123A210448E74Fc6393", - "0xae78736Cd615f374D3085123A210448E74Fc6393", "0xdAC17F958D2ee523a2206206994597C13D831ec7", "0xf1C9acDc66974dFB6dEcB12aA385b9cD01190E38" ],🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
♻️ Duplicate comments (3)
pkg/venus.go (3)
458-458: 🛠️ Refactor suggestionAvoid hardcoding symbol comparisons to determine native tokens
Relying on a hardcoded symbol
"vWBNB_LiquidStakedBNB"to determine if a token is native is fragile and may lead to errors if the symbol changes or if new tokens are added. Consider using a more reliable method, such as checking against a known list of native token addresses or utilizing token metadata.Apply this diff to improve the determination of native tokens:
v.poolMarkets[addr.Hex()] = struct { address string isNative bool }{ address: underlyingAddr.Hex(), - isNative: symbol == "vWBNB_LiquidStakedBNB", + isNative: strings.EqualFold(underlyingAddr.Hex(), nativeDenomAddress), }This change compares the token's address to the known native token address, which is more reliable than comparing symbols.
484-501:⚠️ Potential issueEnsure 'calldata' is not overwritten in 'LoanSupply' action
In the
LoanSupplycase of theGenerateCalldatamethod, thecalldatavariable may be unintentionally overwritten due to multiple conditions being true simultaneously. Specifically, ifparams.Recipientis not zero andparams.Assetis a native token, thecalldataformintBehalfcould be overwritten bywrapAndSupply, leading to incorrect transaction data.Consider restructuring the conditions to ensure that only one
calldatais generated based on mutually exclusive conditions. Apply this refactor:switch action { case LoanSupply: + if IsNativeToken(params.Asset) { + calldata, err = l.parsedABI.Pack("wrapAndSupply", params.Sender) + if err != nil { + return "", err + } + } else if !IsZeroAddress(params.Recipient) { + calldata, err = l.parsedABI.Pack("mintBehalf", params.Recipient, params.Amount) + if err != nil { + return "", err + } + } else { + calldata, err = l.parsedABI.Pack("mint", params.Amount) + if err != nil { + return "", err + } + } - - calldata, err = l.parsedABI.Pack("mint", params.Amount) - if err != nil { - return "", err - } - - if !IsZeroAddress(params.Recipient) { - calldata, err = l.parsedABI.Pack("mintBehalf", params.Recipient, params.Amount) - if err != nil { - return "", err - } - } - - if IsNativeToken(params.Asset) { - calldata, err = l.parsedABI.Pack("wrapAndSupply", params.Sender) - if err != nil { - return "", err - } - }This ensures that
calldatais assigned correctly based on the conditions without being overwritten.
505-522:⚠️ Potential issueEnsure 'calldata' is not overwritten in 'LoanWithdraw' action
Similarly, in the
LoanWithdrawcase, thecalldatavariable may be overwritten due to multiple conditions being true. This could result in incorrect calldata being used for the transaction.Refactor the conditions to prevent unintended overwriting:
case LoanWithdraw: + if IsNativeToken(params.Asset) { + calldata, err = l.parsedABI.Pack("redeemAndUnwrap", params.Amount) + if err != nil { + return "", err + } + } else if !IsZeroAddress(params.Recipient) { + calldata, err = l.parsedABI.Pack("redeemUnderlyingBehalf", params.Recipient, params.Amount) + if err != nil { + return "", err + } + } else { + calldata, err = l.parsedABI.Pack("redeemUnderlying", params.Amount) + if err != nil { + return "", err + } + } - - calldata, err = l.parsedABI.Pack("redeemUnderlying", params.Amount) - if err != nil { - return "", err - } - - if !IsZeroAddress(params.Recipient) { - calldata, err = l.parsedABI.Pack("redeemUnderlyingBehalf", params.Recipient, params.Amount) - if err != nil { - return "", err - } - } - - if IsNativeToken(params.Asset) { - calldata, err = l.parsedABI.Pack("redeemAndUnwrap", params.Amount) - if err != nil { - return "", err - } - }This modification ensures that the
calldatais generated correctly based on the asset type and recipient without conflict.
🧹 Nitpick comments (6)
pkg/venus.go (1)
315-316: Use provided context instead ofcontext.Background()in contract callsIn several places, the code uses
context.Background()when making contract calls. This ignores the context passed through the function parameters, potentially missing cancellation signals or deadlines.Consider using the provided
ctxin your contract calls to ensure proper context propagation:result, err := client.CallContract(context.Background(), callMsg, nil) + result, err := client.CallContract(ctx, callMsg, nil)Repeat this change for the following lines:
- Line 394:
result, err := v.client.CallContract(context.Background(), msg, nil)
result, err := v.client.CallContract(ctx, msg, nil)
- Line 420:
result, err := v.client.CallContract(context.Background(), msg, nil)
result, err := v.client.CallContract(ctx, msg, nil)
- Line 441:
result, err = v.client.CallContract(context.Background(), msg, nil)
result, err = v.client.CallContract(ctx, msg, nil)To facilitate this, you'll need to modify the signatures of the functions
registerVenusPoolsandgetSupportedAssetsto accept acontext.Contextparameter and pass it accordingly.Also applies to: 394-395, 420-421, 441-442
tokens/token_impl.go (2)
84-86: Compare addresses usingcommon.Addresstypes instead of hex stringsConverting addresses to strings for comparison may lead to case sensitivity issues and is less efficient. Comparing
common.Addresstypes directly is more reliable and efficient.Refactor the code to compare
common.Addressvalues:addr := common.HexToAddress(address) for _, token := range data.Tokens { - if token.TokenAddress == addr { + if common.HexToAddress(token.TokenAddress) == addr { return &token, nil } }This change ensures that addresses are compared in their native binary form, avoiding potential issues with string representations.
103-106: Usecommon.Addressfor protocol address comparisonSimilar to the previous suggestion, comparing protocol addresses as
common.Addresstypes improves reliability.Apply this diff to refactor the comparison:
addr := common.HexToAddress(address) for _, protocol := range data.Protocols { - if protocol.Address == addr { + if common.HexToAddress(protocol.Address) == addr { return &protocol, nil } }This ensures consistent and accurate address comparisons for protocols.
tokens/token_impl_test.go (1)
164-166: Consistent protocol naming in testsThe protocol name has been changed from
"AaveV3"to"aave_v3". Ensure that this change is intentional and consistent throughout the codebase.If the naming convention has been standardized to lowercase with underscores, make sure all references to protocol names follow this format.
tools/tokens.go (2)
36-46: Move chain mappings to configuration.The chain ID to network name mappings should be externalized to configuration for better maintainability.
+type ChainConfig struct { + ID int64 + Network string + MoralisID string +} + +var chainConfigs = []ChainConfig{ + {ID: 1, Network: "Ethereum", MoralisID: "eth"}, + {ID: 56, Network: "BNB Chain", MoralisID: "bsc"}, + {ID: 137, Network: "Polygon", MoralisID: "polygon"}, +} + func getTokenMetadata(ctx context.Context, tokenAddr string, chainID *big.Int) (string, string, int, error) { - var chainStr string - switch chainID.Int64() { - case 1: - chainStr = "eth" - case 56: - chainStr = "bsc" - case 137: - chainStr = "polygon" - default: + var config *ChainConfig + for _, c := range chainConfigs { + if c.ID == chainID.Int64() { + config = &c + break + } + } + if config == nil { return "", "", 0, fmt.Errorf("unsupported chain ID: %d", chainID) }
208-214: Improve token validation with retries.The current implementation lacks retry mechanism for temporary API failures.
+func withRetry(ctx context.Context, fn func() error) error { + backoff := time.Second + maxRetries := 3 + for i := 0; i < maxRetries; i++ { + err := fn() + if err == nil { + return nil + } + if i == maxRetries-1 { + return err + } + time.Sleep(backoff) + backoff *= 2 + } + return fmt.Errorf("max retries exceeded") +} + - // Validate token using Moralis - _, _, _, err := getTokenMetadata(ctx, addrStr, chain.ChainID) - if err == nil { - validTokens[addrStr] = true - } else { - fmt.Printf("Skipping invalid token %s on chain %s: %v\n", addrStr, chain.Name, err) - } + err := withRetry(ctx, func() error { + _, _, _, err := getTokenMetadata(ctx, addrStr, chain.ChainID) + if err != nil { + return fmt.Errorf("token validation failed: %v", err) + } + validTokens[addrStr] = true + return nil + }) + if err != nil { + log.Printf("Skipping invalid token %s on chain %s after retries: %v", addrStr, chain.Name, err) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pkg/aave.go(2 hunks)pkg/venus.go(1 hunks)tokens/1.json(1 hunks)tokens/137.json(1 hunks)tokens/56.json(2 hunks)tokens/token_impl.go(3 hunks)tokens/token_impl_test.go(3 hunks)tools/tokens.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/aave.go
🧰 Additional context used
🪛 Gitleaks (8.21.2)
tokens/1.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/137.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
136-136: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
142-142: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (7)
tokens/token_impl_test.go (2)
74-76: Ensure test cases reflect updated token countsThe expected token counts in the tests have been updated. Verify that these counts match the actual number of tokens in the data files to prevent test failures due to mismatches.
If the token counts are dynamic or subject to change, consider calculating the expected counts based on the data rather than hardcoding them.
104-105: Validate updated protocol counts in testsThe expected protocol counts have changed. Ensure that these values accurately reflect the data to maintain test reliability.
Similar to the token counts, if the protocol counts are not fixed, consider deriving them programmatically from the data.
tools/tokens.go (2)
233-240: 🛠️ Refactor suggestionUse protocol capabilities instead of hardcoding source/destination flags.
The current implementation assumes all protocols support both source and destination operations.
protocolData.Protocols = append(protocolData.Protocols, tokens.Protocol{ Address: p.GetContractAddress(chain.ChainID).Hex(), Name: p.GetName(), Type: string(p.GetType()), - Source: true, - Destination: true, + Source: p.IsSource(), + Destination: p.IsDestination(), Tokens: validTokenAddrs, })Likely invalid or redundant comment.
160-177: 🛠️ Refactor suggestionImprove configuration management and error handling.
The current implementation has several issues:
- Hard-coded RPC URLs
- Use of panic for error handling
- No graceful shutdown mechanism
+type Config struct { + RPCURLs map[int64]string + Timeout time.Duration +} + +func loadConfig() (*Config, error) { + return &Config{ + RPCURLs: map[int64]string{ + 1: os.Getenv("ETH_RPC_URL"), + 56: os.Getenv("BSC_RPC_URL"), + 137: os.Getenv("POLYGON_RPC_URL"), + }, + Timeout: 5 * time.Minute, + }, nil +} + func main() { + config, err := loadConfig() + if err != nil { + log.Fatalf("Failed to load config: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), config.Timeout) + defer cancel() + + // Handle shutdown signals + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) + go func() { + <-sigCh + cancel() + }() + - registry, err := pkg.NewProtocolRegistry([]pkg.ChainConfig{ - { - ChainID: big.NewInt(1), - RPCURL: "https://rpc.ankr.com/eth", - }, - // ... - }) + var chainConfigs []pkg.ChainConfig + for chainID, rpcURL := range config.RPCURLs { + chainConfigs = append(chainConfigs, pkg.ChainConfig{ + ChainID: big.NewInt(chainID), + RPCURL: rpcURL, + }) + } + + registry, err := pkg.NewProtocolRegistry(chainConfigs) if err != nil { - panic(err.Error()) + log.Fatalf("Failed to create registry: %v", err) }Likely invalid or redundant comment.
tokens/137.json (1)
58-61: Fix incorrect token symbol formatThe symbol field contains the full name instead of the token symbol.
Apply this diff to fix the token symbol:
"token_address": "0x2791Bca1f2de4661ED88A30C99A7a9449Aa84174", "name": "USD Coin (PoS)", - "symbol": "USDC", + "symbol": "USDC.e", "decimals": 6🧰 Tools
🪛 Gitleaks (8.21.2)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/1.json (1)
1-245: LGTM with previously mentioned fixesThe changes look good, but please apply the same fixes mentioned in the review of
tokens/137.json:
- Fix incorrect token symbol formats
- Remove duplicate token entries
- Differentiate between protocol instances with the same name
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json (1)
205-302: LGTM! Well-structured Venus isolated poolsThe Venus isolated pools are well-organized with clear token associations and consistent protocol definitions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tools/tokens.go (1)
37-46: Improve chain validation maintainability.Consider using a map for chain mappings to make it more maintainable and efficient.
+var chainIDToMoralisChain = map[int64]string{ + 1: "eth", + 56: "bsc", + 137: "polygon", +} + func getTokenMetadata(ctx context.Context, tokenAddr string, chainID *big.Int) (string, string, int, error) { - var chainStr string - switch chainID.Int64() { - case 1: - chainStr = "eth" - case 56: - chainStr = "bsc" - case 137: - chainStr = "polygon" - default: + chainStr, ok := chainIDToMoralisChain[chainID.Int64()] + if !ok { return "", "", 0, fmt.Errorf("unsupported chain ID: %d", chainID) }tokens/56.json (2)
83-84: Fix typo in token name.The token name contains a typo: "Destablecoin" should be "Stablecoin".
- "name": "Hay Destablecoin", + "name": "Hay Stablecoin",
205-302: Consider adding pool identifiers to Venus isolated pool names.All Venus isolated pools share the same name "venus_isolated_pool". Consider adding identifiers to differentiate between pools, e.g., based on their primary token or purpose.
Example:
- "name": "venus_isolated_pool", + "name": "venus_isolated_pool_ankr", // For pool with ANKR token
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pkg/compound.go(1 hunks)tokens/1.json(1 hunks)tokens/137.json(1 hunks)tokens/56.json(2 hunks)tools/tokens.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/compound.go
🧰 Additional context used
🪛 Gitleaks (8.21.2)
tokens/1.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/137.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
136-136: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
142-142: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
tools/tokens.go (4)
58-60: 🛠️ Refactor suggestionAdd timeout to HTTP client.
The current implementation creates a new HTTP client for each request without timeout settings, which could lead to hanging requests.
- client := &http.Client{} + client := &http.Client{ + Timeout: 10 * time.Second, + }Likely invalid or redundant comment.
31-34:⚠️ Potential issueReplace panic with proper error handling for API key.
Using
panicfor configuration issues is not production-ready. Consider returning an error instead.- if moralisAPIKey == "" { - panic("provide moralis api key in .env") - } + if moralisAPIKey == "" { + return "", "", 0, fmt.Errorf("MORALIS_API_KEY environment variable is not set") + }Likely invalid or redundant comment.
248-255:⚠️ Potential issueUse protocol-specific source/destination capabilities.
The code currently hardcodes both
SourceandDestinationtotruefor all protocols, which may not accurately reflect each protocol's capabilities.protocolData.Protocols = append(protocolData.Protocols, tokens.Protocol{ Address: p.GetContractAddress(chain.ChainID).Hex(), Name: p.GetName(), Type: string(p.GetType()), - Source: true, - Destination: true, + Source: p.IsSource(), + Destination: p.IsDestination(), Tokens: validTokenAddrs, })Likely invalid or redundant comment.
175-188: 🛠️ Refactor suggestionImprove RPC URL configuration management.
RPC URLs should be configurable through environment variables or a config file instead of being hardcoded.
+type ChainRPCConfig struct { + ChainID *big.Int + RPCURL string +} + +func loadChainConfigs() ([]pkg.ChainConfig, error) { + configs := make([]pkg.ChainConfig, 0) + for _, chain := range []struct{ id int64 }{ + {id: 1}, // Ethereum + {id: 56}, // BSC + {id: 137}, // Polygon + } { + rpcURL := os.Getenv(fmt.Sprintf("RPC_URL_%d", chain.id)) + if rpcURL == "" { + return nil, fmt.Errorf("RPC_URL_%d environment variable is not set", chain.id) + } + configs = append(configs, pkg.ChainConfig{ + ChainID: big.NewInt(chain.id), + RPCURL: rpcURL, + }) + } + return configs, nil +} - registry, err := pkg.NewProtocolRegistry([]pkg.ChainConfig{ - { - ChainID: big.NewInt(1), - RPCURL: "https://rpc.ankr.com/eth", - }, - { - ChainID: big.NewInt(56), - RPCURL: "https://rpc.ankr.com/bsc", - }, - { - ChainID: big.NewInt(137), - RPCURL: "https://rpc.ankr.com/polygon", - }, - }) + configs, err := loadChainConfigs() + if err != nil { + return fmt.Errorf("failed to load chain configs: %w", err) + } + registry, err := pkg.NewProtocolRegistry(configs)Likely invalid or redundant comment.
tokens/137.json (1)
111-123: Differentiate between Compound protocol instances.Two protocols are named differently ("Compound USDC Pool" and "Compound USDT pool") but share identical token lists. This could indicate either:
- A copy-paste error in the token lists, or
- Intentional configuration where both pools support the same tokens
Please verify the token lists are correct for each pool.
Also applies to: 124-136
✅ Verification successful
Token configuration for Compound pools is correct
The identical token lists for both Compound pools appear intentional. Each pool operates with a different base asset (USDC vs USDT) while accepting the same set of collateral tokens (WMATIC, WBTC, WETH, stMATIC, and MaticX). The distinct names sufficiently differentiate their purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if these are the correct token lists for each Compound pool echo "Checking token addresses in Compound pools..." for addr in "0xF25212E676D1F7F89Cd72fFEe66158f541246445" "0xaeB318360f27748Acb200CE616E389A6C9409a07"; do echo "Pool address: $addr" # Use ast-grep to find any references to these pools in the codebase ast-grep --pattern "address.*$addr" doneLength of output: 704
Script:
#!/bin/bash # Check for token addresses and any documentation echo "Searching for token addresses and pool documentation..." # First, let's get the content of the entire file to see both pools cat tokens/137.json # Then search for any documentation or references to these pools echo -e "\nSearching for documentation or references..." rg -i "compound (usdc|usdt) pool" --type txt --type md # Search for token addresses to verify their association echo -e "\nChecking token associations..." for token in "0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270" "0x1BFD67037B42Cf73acF2047067bd4F2C47D9BfD6" "0x3A58a54C066FdC0f2D55FC9C89F0415C92eBf3C4" "0x7ceB23fD6bC0adD59E62ac25578270cFf1b9f619" "0xfa68FB4628DFF1028CFEc22b4162FCcd0d45efb6"; do echo "Token: $token" rg "$token" --type json doneLength of output: 7855
tokens/1.json (1)
58-62: Fix incorrect token symbol format.The symbol field contains the full name instead of the token symbol.
"token_address": "0xbf5495Efe5DB9ce00f80364C8B423567e58d2110", "name": "ezETH", - "symbol": "Renzo Restaked ETH", + "symbol": "ezETH", "decimals": 18🧰 Tools
🪛 Gitleaks (8.21.2)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json (1)
205-219: Verify Venus isolated pool configurations.Multiple Venus isolated pools have been added with different token lists. Please verify:
- Each pool address is correct and matches the Venus V4 documentation
- Token lists are correctly assigned to each pool
- No duplicate token entries across pools that shouldn't share tokens
Also applies to: 220-229, 230-241, 242-253, 254-268, 269-281, 282-292, 293-302
✅ Verification successful
Venus isolated pool configurations are correctly structured
The verification confirms that:
- All 8 Venus isolated pools have consistent configuration structure
- Each pool has a unique address and a distinct set of supported tokens
- Common tokens (e.g., USDT) appearing in multiple pools is expected and correct
- All pools are properly configured as bidirectional loan markets
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify Venus isolated pool configurations echo "Checking Venus isolated pool addresses and tokens..." # Get all Venus pool addresses venus_pools=($(rg -o '"address": "[^"]*".*"name": "venus_isolated_pool"' -A 5)) # Check for duplicate tokens across pools for pool in "${venus_pools[@]}"; do echo "Pool address: $pool" # Extract and check token lists rg -A 20 "\"address\": \"$pool\"" | rg "tokens" doneLength of output: 262
Script:
#!/bin/bash # Search for Venus isolated pool configurations echo "Venus Isolated Pool Configurations:" rg -A 15 '"name": "venus_isolated_pool"' tokens/56.json echo -e "\nChecking for duplicate tokens across pools:" # Extract all token addresses rg -o '"tokens": \[[^\]]*\]' tokens/56.jsonLength of output: 3407
adelowo
force-pushed
the
venus_protocol
branch
from
2e6e415 to
9171108
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
pkg/venus.go (3)
458-458: 🛠️ Refactor suggestionAvoid hardcoding symbol comparisons to determine native tokens
Relying on hardcoded symbol strings like
"vWBNB_LiquidStakedBNB"to determine if a token is native is fragile and may lead to errors if the symbol changes or if new tokens are added. Consider comparing the token's address to a known native token address for a more reliable method.Apply this diff to improve the determination of native tokens:
v.poolMarkets[addr.Hex()] = struct { address string isNative bool }{ address: underlyingAddr.Hex(), - isNative: symbol == "vWBNB_LiquidStakedBNB", + isNative: strings.EqualFold(underlyingAddr.Hex(), nativeDenomAddress), }
484-501:⚠️ Potential issueEnsure correct calldata generation without unintended overwrites in
LoanSupplyIn the
GenerateCalldatamethod, within theLoanSupplycase, thecalldatavariable may be unintentionally overwritten if multiple conditions are true simultaneously. Specifically, ifparams.Recipientis not zero andparams.Assetis a native token, thecalldatagenerated formintBehalfcould be overwritten bywrapAndSupply, leading to incorrect transaction data.Restructure the conditions to ensure that only one calldata is generated based on mutually exclusive scenarios:
switch action { case LoanSupply: + if IsNativeToken(params.Asset) { + calldata, err = l.parsedABI.Pack("wrapAndSupply", params.Sender) + if err != nil { + return "", err + } + } else if !IsZeroAddress(params.Recipient) { + calldata, err = l.parsedABI.Pack("mintBehalf", params.Recipient, params.Amount) + if err != nil { + return "", err + } + } else { + calldata, err = l.parsedABI.Pack("mint", params.Amount) + if err != nil { + return "", err + } + } - - calldata, err = l.parsedABI.Pack("mint", params.Amount) - if err != nil { - return "", err - } - - if !IsZeroAddress(params.Recipient) { - calldata, err = l.parsedABI.Pack("mintBehalf", params.Recipient, params.Amount) - if err != nil { - return "", err - } - } - - if IsNativeToken(params.Asset) { - calldata, err = l.parsedABI.Pack("wrapAndSupply", params.Sender) - if err != nil { - return "", err - } - }This refactoring ensures that
calldatais assigned correctly based on exclusive conditions, preventing unintended overwrites.
505-522:⚠️ Potential issueEnsure correct calldata generation without unintended overwrites in
LoanWithdrawSimilarly, in the
LoanWithdrawcase, thecalldatavariable may be overwritten due to multiple conditions being true. This can cause incorrect calldata for the intended operation.Restructure the conditions to ensure mutual exclusivity:
case LoanWithdraw: + if IsNativeToken(params.Asset) { + calldata, err = l.parsedABI.Pack("redeemAndUnwrap", params.Amount) + if err != nil { + return "", err + } + } else if !IsZeroAddress(params.Recipient) { + calldata, err = l.parsedABI.Pack("redeemUnderlyingBehalf", params.Recipient, params.Amount) + if err != nil { + return "", err + } + } else { + calldata, err = l.parsedABI.Pack("redeemUnderlying", params.Amount) + if err != nil { + return "", err + } + } - - calldata, err = l.parsedABI.Pack("redeemUnderlying", params.Amount) - if err != nil { - return "", err - } - - if !IsZeroAddress(params.Recipient) { - calldata, err = l.parsedABI.Pack("redeemUnderlyingBehalf", params.Recipient, params.Amount) - if err != nil { - return "", err - } - } - - if IsNativeToken(params.Asset) { - calldata, err = l.parsedABI.Pack("redeemAndUnwrap", params.Amount) - if err != nil { - return "", err - } - }This ensures that the correct calldata is generated without overwriting previous values.
tokens/1.json (1)
58-61:⚠️ Potential issueFix incorrect token symbol
The
symbolfield contains the full name instead of the token symbol. It should be updated to use the correct token symbolezETHfor consistency and correctness.Apply this diff to correct the symbol:
{ "token_address": "0xbf5495Efe5DB9ce00f80364C8B423567e58d2110", "name": "ezETH", - "symbol": "Renzo Restaked ETH", + "symbol": "ezETH", "decimals": 18 },🧰 Tools
🪛 Gitleaks (8.21.2)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🧹 Nitpick comments (4)
tokens/token_impl.go (2)
84-86: Consider adding address format validation.The standardization of addresses using
common.HexToAddressis good, but consider validating the input address format before processing. Invalid addresses will be zero-padded without error.+ if !common.IsHexAddress(address) { + return nil, fmt.Errorf("invalid address format: %s", address) + } addr := common.HexToAddress(address).Hex()
103-104: Remove unnecessary blank line and consider extracting address validation.
- The blank line after the address conversion can be removed.
- Since both
GetTokenByAddressandGetProtocolByAddressperform the same address handling, consider extracting this logic into a helper function.// Add this helper function + func validateAndNormalizeAddress(address string) (string, error) { + if !common.IsHexAddress(address) { + return "", fmt.Errorf("invalid address format: %s", address) + } + return common.HexToAddress(address).Hex(), nil + } // Use it in both methods - addr := common.HexToAddress(address).Hex() + addr, err := validateAndNormalizeAddress(address) + if err != nil { + return nil, err + }pkg/compound.go (1)
419-433: Consider optimizing address comparison and improving error handling.The current implementation has a few areas for improvement:
- Pre-compute lowercase addresses as constants to avoid repeated
strings.ToLowercalls- Consider differentiating between Polygon and mainnet USDC pools in the name
- Add chain validation to ensure the pool exists on the current chain
Consider this optimization:
const ( CompoundV3USDCPool = "0xc3d688b66703497daa19211eedff47f25384cdc3" CompoundV3ETHPool = "0xa17581a9e3356d9a858b789d68b4d866e593ae94" CompoundV3PolygonUSDCPool = "0xF25212E676D1F7F89Cd72fFEe66158f541246445" CompoundV3PolygonUSDTPool = "0xaeB318360f27748Acb200CE616E389a6C9409a07" + + // Lowercase addresses for efficient comparison + CompoundV3USDCPoolLower = "0xc3d688b66703497daa19211eedff47f25384cdc3" + CompoundV3ETHPoolLower = "0xa17581a9e3356d9a858b789d68b4d866e593ae94" + CompoundV3PolygonUSDCPoolLower = "0xf25212e676d1f7f89cd72ffee66158f541246445" + CompoundV3PolygonUSDTPoolLower = "0xaeb318360f27748acb200ce616e389a6c9409a07" ) func (l *CompoundOperation) GetName() string { + // Convert contract address to lowercase once + contractAddr := strings.ToLower(l.contract.Hex()) + + // Validate if the pool exists on the current chain + protocols, ok := poolMaps[l.chainID.Int64()] + if !ok { + return Compound + } + + found := false + for _, addr := range protocols { + if strings.ToLower(addr) == contractAddr { + found = true + break + } + } + + if !found { + return Compound + } - switch strings.ToLower(l.contract.Hex()) { - case strings.ToLower(CompoundV3ETHPool): + switch contractAddr { + case CompoundV3ETHPoolLower: return "Compound ETH pool" - case strings.ToLower(CompoundV3PolygonUSDCPool), - strings.ToLower(CompoundV3USDCPool): - return "Compound USDC Pool" - case strings.ToLower(CompoundV3PolygonUSDTPool): + case CompoundV3USDCPoolLower: + return "Compound USDC Pool (Mainnet)" + case CompoundV3PolygonUSDCPoolLower: + return "Compound USDC Pool (Polygon)" + case CompoundV3PolygonUSDTPoolLower: return "Compound USDT pool" default: return Compound } }tokens/137.json (1)
4-73: Ensure consistent placement of "(PoS)" in token namesThe placement of "(PoS)" in token names is inconsistent. Some tokens have "(PoS)" as a suffix, while others have it as a prefix. For clarity and consistency, consider standardizing the placement.
For example:
- Use suffix placement for all tokens:
"name": "Aave (PoS)""name": "Dai Stablecoin (PoS)""name": "Tether USD (PoS)"Or
- Use prefix placement for all tokens:
"name": "(PoS) Aave""name": "(PoS) Dai Stablecoin""name": "(PoS) Tether USD"Applying consistent naming conventions improves readability and maintainability.
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
.env.example(1 hunks)generate.go(1 hunks)go.mod(1 hunks)pkg/aave.go(2 hunks)pkg/compound.go(1 hunks)pkg/venus.go(1 hunks)tokens/1.json(1 hunks)tokens/137.json(1 hunks)tokens/56.json(2 hunks)tokens/token_impl.go(3 hunks)tokens/token_impl_test.go(3 hunks)tools/tokens.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .env.example
- go.mod
- generate.go
- pkg/aave.go
- tokens/token_impl_test.go
🧰 Additional context used
🪛 Gitleaks (8.21.2)
tokens/1.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
124-124: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/137.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
76-76: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tokens/56.json
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
130-130: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
136-136: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
142-142: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (10)
tokens/token_impl.go (2)
10-10: LGTM! Good choice using go-ethereum's common package.Using the standard Ethereum address handling library ensures consistent and reliable address formatting.
84-86: Verify address format compatibility with existing JSON files.The new address standardization might affect lookups if existing JSON files contain addresses in non-standard formats.
Also applies to: 103-106
✅ Verification successful
Address format compatibility verified successfully
All addresses in the JSON files are already in standard Ethereum format (0x-prefixed, 40 hex chars). The address standardization change is safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check address formats in JSON files # Expected: All addresses should be valid Ethereum addresses # Find all JSON files and check address formats for file in $(fd -e json); do echo "Checking $file..." # Extract and validate token addresses jq -r '.tokens[] | .token_address' "$file" | while read addr; do if [[ $addr =~ ^0x[a-fA-F0-9]{40}$ ]]; then continue else echo "Invalid token address format in $file: $addr" fi done # Extract and validate protocol addresses jq -r '.protocols[] | .address' "$file" | while read addr; do if [[ $addr =~ ^0x[a-fA-F0-9]{40}$ ]]; then continue else echo "Invalid protocol address format in $file: $addr" fi done doneLength of output: 9036
pkg/compound.go (1)
438-439: Add documentation for interface methods.Please add documentation explaining the purpose of
IsSourceandIsDestinationmethods. Also, verify if all Compound operations should indeed be both source and destination.Let's verify the source/destination roles by checking other protocol implementations:
✅ Verification successful
Add documentation for interface methods
The implementation of
IsSource()andIsDestination()inCompoundOperationis correct and consistent with other major protocols like Aave and RocketPool. However, please add documentation to explain:
IsSource()- indicates if the protocol can initiate operationsIsDestination()- indicates if the protocol can receive operations🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if other protocols implement similar source/destination roles # and verify their implementations for consistency # Search for IsSource implementations echo "Checking IsSource implementations:" rg -A 1 'func.*IsSource\(\).*bool.*{' --type go # Search for IsDestination implementations echo "Checking IsDestination implementations:" rg -A 1 'func.*IsDestination\(\).*bool.*{' --type goLength of output: 518
Script:
#!/bin/bash # Search for IsSource and IsDestination implementations with proper regex escaping echo "=== IsSource implementations ===" rg -A 2 'func.*IsSource\(\).*bool.*\{' --type go echo -e "\n=== IsDestination implementations ===" rg -A 2 'func.*IsDestination\(\).*bool.*\{' --type go echo -e "\n=== Looking for interface definition ===" rg 'type.*interface.*\{.*(?:IsSource|IsDestination).*\}' --type go -A 5Length of output: 2411
Script:
#!/bin/bash # Look for documentation or interface definition with broader context echo "=== Searching for interface or documentation ===" rg -B 3 -A 1 'IsSource\(\s*\)\s*bool' --type go echo -e "\n=== Checking for comments or docs ===" rg -B 2 'interface.*{.*Source' --type go -A 5Length of output: 3252
tools/tokens.go (5)
105-136: LGTM!The function correctly implements sorting and deduplication logic for both tokens and protocols.
175-192: 🛠️ Refactor suggestionImprove configuration management and error handling.
Several improvements can be made to this section:
- RPC URLs should be configurable through environment variables or config file
- Error handling should be more graceful than panic
- Chain configurations are duplicated with the previous chain definitions
Consider this approach:
+type Config struct { + Chains []ChainConfig +} + +func loadConfig() (*Config, error) { + // Load from environment variables or config file +} + func main() { + config, err := loadConfig() + if err != nil { + fmt.Printf("Failed to load config: %v\n", err) + os.Exit(1) + } + - registry, err := pkg.NewProtocolRegistry([]pkg.ChainConfig{ - { - ChainID: big.NewInt(1), - RPCURL: "https://rpc.ankr.com/eth", - }, - // ... - }) + registry, err := pkg.NewProtocolRegistry(config.Chains) if err != nil { - panic(err.Error()) + fmt.Printf("Failed to initialize registry: %v\n", err) + os.Exit(1) }Likely invalid or redundant comment.
31-34:⚠️ Potential issueReplace panic with proper error handling for missing API key.
Using
panicfor configuration issues is not a good practice. Consider returning an error instead.Apply this diff to improve error handling:
- if moralisAPIKey == "" { - panic("provide moralis api key in .env") - } + if moralisAPIKey == "" { + return "", "", 0, fmt.Errorf("MORALIS_API_KEY environment variable is not set") + }Likely invalid or redundant comment.
140-143:⚠️ Potential issueReplace panic with proper error handling for missing .env file.
Using
panicfor configuration issues is not a good practice. Consider returning an error instead.Apply this diff to improve error handling:
+func run() error { if err := godotenv.Load(); err != nil { - fmt.Println("add an .env file with MORALIS_API_KEY=xyz") - panic(err) + return fmt.Errorf("failed to load .env file (ensure MORALIS_API_KEY is set): %w", err) } + return nil +} + +func main() { + if err := run(); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } +}Likely invalid or redundant comment.
48-62: 🛠️ Refactor suggestionImprove HTTP client implementation.
The current implementation creates a new HTTP client for each request and lacks timeout settings.
Apply this diff to improve the HTTP client implementation:
+var httpClient = &http.Client{ + Timeout: 10 * time.Second, +} + func getTokenMetadata(ctx context.Context, tokenAddr string, chainID *big.Int) (string, string, int, error) { - client := &http.Client{} - resp, err := client.Do(req) + resp, err := httpClient.Do(req)Likely invalid or redundant comment.
tokens/56.json (2)
Line range hint
4-150: LGTM!The token configurations are well-structured and consistent. All token addresses are valid Ethereum addresses.
🧰 Tools
🪛 Gitleaks (8.21.2)
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
112-112: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
157-302: 🛠️ Refactor suggestionReview protocol capabilities instead of hardcoding Source/Destination flags.
The code currently sets both
SourceandDestinationtotruefor all protocols, which might not be accurate. These flags should be determined by the protocol's capabilities.Apply this diff to improve protocol capabilities:
protocolData.Protocols = append(protocolData.Protocols, tokens.Protocol{ Address: p.GetContractAddress(chain.ChainID).Hex(), Name: p.GetName(), Type: string(p.GetType()), - Source: true, - Destination: true, + Source: p.IsSource(), + Destination: p.IsDestination(), Tokens: tokenAddrs, })Likely invalid or redundant comment.
| GetContractAddress(chainID *big.Int) common.Address | ||
|
|
||
| // Can this protocol be withdrawn from or be the source state of an intent? | ||
| IsSource() bool |
There was a problem hiding this comment.
Source is an overloaded term in our stack. Maybe CanSourceFunds tokens or assets or funds
| }) | ||
| } | ||
|
|
||
| func TestVenusOperation_Validate(t *testing.T) { |
There was a problem hiding this comment.
Maybe we should discuss adding a Pause/Utilization/Health % API to avoid failed operations or funds loss.
2 participants
This adds support for all Venus V4 isolated pools
I have also a cli tool to auto generate the
chain_id.json files. It uses Moralis to check if we support the token priceson the solver feeder, if not supported, it will not be included in the json file
Summary by CodeRabbit
Release Notes
New Features
Improvements
Testing
Chores
godotenv.