-
Notifications
You must be signed in to change notification settings - Fork 1
Problem: Cannot send to EVM address #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new utility class for converting Ethereum addresses between hexadecimal and Bech32 formats was added. The token sending logic was updated to automatically convert receiver addresses from hex to Bech32 if needed, using this new utility. Supporting imports for the converter and chain prefix were introduced. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
lib/application/wallet/utils.dart (3)
21-21: Add type annotation for the prefix parameter.The
prefixparameter is missing a type annotation, which reduces code clarity and IDE support.- static String ethAddressToBech32(String ethAddress, prefix) { + static String ethAddressToBech32(String ethAddress, String prefix) {
43-43: Add type annotation and improve documentation.The
prefixparameter is missing both type annotation and documentation.- /// Parameters: - /// - bech32Address: The Bech32-encoded Ethereum address. - /// + /// Parameters: + /// - bech32Address: The Bech32-encoded Ethereum address. + /// - prefix: The Bech32 prefix used for decoding. + ///- static String bech32ToEthAddress(String bech32Address, prefix) { + static String bech32ToEthAddress(String bech32Address, String prefix) {
7-52: Consider adding input validation and error handling.The class lacks comprehensive input validation for edge cases such as null inputs, empty strings, or malformed addresses. Consider adding validation at the beginning of each method.
Example for enhanced validation:
static String ethAddressToBech32(String ethAddress, String prefix) { + if (ethAddress.isEmpty || prefix.isEmpty) { + throw ArgumentError("Address and prefix cannot be empty"); + } + final cleanHex = AddrDecUtils.validateAndRemovePrefix( ethAddress, CoinsConf.ethereum.params.addrPrefix! ); // ... rest of method }lib/application/wallet/wallet_bloc.dart (1)
45-50: Consider using a local variable instead of mutating the parameter.Mutating the
receiverparameter can be confusing and makes the code harder to debug. Consider using a local variable for clarity.- if (receiver.startsWith("0x")) { - receiver = EthBech32Converter.ethAddressToBech32( - receiver, - chainAddressPrefix - ); - } + String processedReceiver = receiver; + if (receiver.startsWith("0x")) { + try { + processedReceiver = EthBech32Converter.ethAddressToBech32( + receiver, + chainAddressPrefix + ); + } catch (e) { + throw ArgumentError("Failed to convert Ethereum address: $e"); + } + } final message = MsgSend( fromAddress: CosmosBaseAddress(sender), - toAddress: CosmosBaseAddress(receiver), + toAddress: CosmosBaseAddress(processedReceiver), amount: [Coin(denom: denom, amount: amount)] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/application/wallet/utils.dart(1 hunks)lib/application/wallet/wallet_bloc.dart(2 hunks)
🔇 Additional comments (3)
lib/application/wallet/utils.dart (1)
1-6: LGTM: Well-organized imports.The imports are properly organized and include all necessary dependencies for Bech32 encoding, Ethereum address handling, and hex conversion.
lib/application/wallet/wallet_bloc.dart (2)
2-2: LGTM: Proper imports for new functionality.The imports correctly include the new utility class and constants needed for address conversion.
Also applies to: 4-4
45-50: chainAddressPrefix Verified and Imported Correctly
ThechainAddressPrefixconstant is defined inlib/infrastructure/constants.dart("mantra") and properly imported intolib/application/wallet/wallet_bloc.dart. No further changes needed.
| assert( | ||
| hexEncoded.length == EthAddrConst.addrLen, | ||
| "Invalid Ethereum address length: ${hexEncoded.length}, expected: ${EthAddrConst.addrLen}" | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace assertion with proper exception handling.
Similar to the previous method, use proper exception handling instead of assertions for production reliability.
- assert(
- hexEncoded.length == EthAddrConst.addrLen,
- "Invalid Ethereum address length: ${hexEncoded.length}, expected: ${EthAddrConst.addrLen}"
- );
+ if (hexEncoded.length != EthAddrConst.addrLen) {
+ throw ArgumentError(
+ "Invalid Ethereum address length: ${hexEncoded.length}, expected: ${EthAddrConst.addrLen}"
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert( | |
| hexEncoded.length == EthAddrConst.addrLen, | |
| "Invalid Ethereum address length: ${hexEncoded.length}, expected: ${EthAddrConst.addrLen}" | |
| ); | |
| if (hexEncoded.length != EthAddrConst.addrLen) { | |
| throw ArgumentError( | |
| "Invalid Ethereum address length: ${hexEncoded.length}, expected: ${EthAddrConst.addrLen}" | |
| ); | |
| } |
🤖 Prompt for AI Agents
In lib/application/wallet/utils.dart around lines 46 to 49, replace the assert
statement that checks the Ethereum address length with proper exception handling
by throwing an exception when the length is invalid. This ensures the check is
enforced in production environments where assertions may be disabled.
| final cleanHex = AddrDecUtils.validateAndRemovePrefix( | ||
| ethAddress, CoinsConf.ethereum.params.addrPrefix! | ||
| ); | ||
| assert( | ||
| cleanHex.length == EthAddrConst.addrLen, | ||
| "Invalid Ethereum address length: ${cleanHex.length}, expected: ${EthAddrConst.addrLen}" | ||
| ); | ||
| final hexAddressBytes = hex.decode(cleanHex); | ||
| return Bech32Encoder.encode(prefix, hexAddressBytes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider replacing assertions with proper exception handling.
Using assertions for input validation can be problematic in production builds where assertions may be disabled. Consider throwing specific exceptions instead for better error handling and debugging.
- assert(
- cleanHex.length == EthAddrConst.addrLen,
- "Invalid Ethereum address length: ${cleanHex.length}, expected: ${EthAddrConst.addrLen}"
- );
+ if (cleanHex.length != EthAddrConst.addrLen) {
+ throw ArgumentError(
+ "Invalid Ethereum address length: ${cleanHex.length}, expected: ${EthAddrConst.addrLen}"
+ );
+ }🤖 Prompt for AI Agents
In lib/application/wallet/utils.dart around lines 23 to 31, replace the
assertion that checks the Ethereum address length with proper exception
handling. Instead of using assert, throw a specific exception (e.g.,
ArgumentError) when the address length is invalid to ensure the validation works
in production and provides clear error feedback.
| if (receiver.startsWith("0x")) { | ||
| receiver = EthBech32Converter.ethAddressToBech32( | ||
| receiver, | ||
| chainAddressPrefix | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for address conversion.
The address conversion could fail due to invalid input addresses, but there's no error handling around the conversion. This could cause the entire sendToken operation to fail unexpectedly.
- if (receiver.startsWith("0x")) {
- receiver = EthBech32Converter.ethAddressToBech32(
- receiver,
- chainAddressPrefix
- );
- }
+ if (receiver.startsWith("0x")) {
+ try {
+ receiver = EthBech32Converter.ethAddressToBech32(
+ receiver,
+ chainAddressPrefix
+ );
+ } catch (e) {
+ throw ArgumentError("Failed to convert Ethereum address: $e");
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (receiver.startsWith("0x")) { | |
| receiver = EthBech32Converter.ethAddressToBech32( | |
| receiver, | |
| chainAddressPrefix | |
| ); | |
| } | |
| if (receiver.startsWith("0x")) { | |
| try { | |
| receiver = EthBech32Converter.ethAddressToBech32( | |
| receiver, | |
| chainAddressPrefix | |
| ); | |
| } catch (e) { | |
| throw ArgumentError("Failed to convert Ethereum address: $e"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/application/wallet/wallet_bloc.dart around lines 45 to 50, the code
converts an Ethereum address to Bech32 format without error handling, which may
cause the sendToken operation to fail if the input address is invalid. Wrap the
conversion call in a try-catch block to catch any exceptions thrown during
conversion, and handle the error gracefully by logging it or emitting an error
state to prevent the entire operation from failing unexpectedly.
Summary by CodeRabbit