feat(ante): Add support for custom decimals #2907
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces significant updates to the codebase, particularly in the integration of the Cosmos SDK and EVM. Key changes include the removal of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (20)
app/ante/evm/06_account_verification.go (2)
28-28: Approve comment change with a minor suggestion.The updated comment is more precise and aligns better with the code's behavior. It clearly states the requirement that only Externally Owned Accounts (EOA) are allowed to send transactions.
To further improve clarity, consider expanding the acronym EOA in the comment:
- // Only EOA are allowed to send transactions. + // Only Externally Owned Accounts (EOA) are allowed to send transactions.This expansion would be helpful for developers who might not be familiar with the acronym.
Update
VerifyAccountBalanceto Use Bank WrapperThe shell script results show that while custom decimal support and the bank wrapper are implemented in the codebase, the
VerifyAccountBalancefunction inapp/ante/evm/06_account_verification.go(lines 25-31) directly usesaccountKeeperinstead of thebankWrapper. To align with the PR objectives and ensure consistent handling of account balances, please refactor this function to utilize thebankWrapper.
- File:
app/ante/evm/06_account_verification.go- Lines: 25-31
🔗 Analysis chain
Line range hint
1-51: Verify implementation of custom decimal support and bank wrapper usage.The current implementation of
VerifyAccountBalancedoesn't seem to directly address the PR objective of adding support for custom decimals. Additionally, it's not clear if the bank wrapper is being used as mentioned in the linked issue ENG-2636.Consider the following points:
Custom Decimal Support: The function uses
sdkmath.NewIntFromBigInt(account.Balance), which suggests that balances are handled as big integers. This could potentially support custom decimals, but it's not explicit. Could you clarify how custom decimal support is implemented?Bank Wrapper Usage: The PR objectives mention using the bank wrapper instead of the bank keeper for EVM operations. However, this function seems to use
accountKeeperdirectly. Is there a plan to integrate the bank wrapper here?To verify the implementation of custom decimal support and bank wrapper usage across the codebase, you can run the following script:
This will help identify if and where custom decimal support and bank wrapper usage are implemented in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for custom decimal support and bank wrapper usage # Test 1: Search for custom decimal handling echo "Searching for custom decimal handling:" rg --type go -i "custom.*decimal|decimal.*support" # Test 2: Check for bank wrapper usage echo "Checking for bank wrapper usage:" rg --type go -i "bank.*wrapper|wrapper.*bank" # Test 3: Look for potential places where custom decimals might be relevant echo "Potential places for custom decimal handling:" rg --type go "NewIntFromBigInt|NewDecFromBigInt"Length of output: 16064
x/evm/types/tx_data.go (5)
42-42: Improved method documentation, consider minor enhancement.The updated comment for the
Fee()method provides clearer information about its purpose. This aligns well with the PR objective of improving documentation.Consider adding a brief mention of how this fee is calculated (e.g., based on gas price and gas limit) to make the documentation even more informative.
44-46: Excellent addition of theCost()method.The new
Cost()method is a valuable addition that aligns well with the PR objectives. It provides a clear way to calculate the total transaction cost, which is crucial for accurate transaction handling in the ante handler.Consider adding a brief note about the return type (e.g., "Returns a *big.Int representing the total cost") to make the documentation even more precise.
49-52: ImprovedEffectiveGasPrice()documentation and addition ofEffectiveFee()method.The updated comment for
EffectiveGasPrice()and the newEffectiveFee()method are valuable improvements that align with the PR objectives. They provide clearer information and additional functionality for fee calculations.For the
EffectiveFee()method, consider adding a brief explanation of how it differs from theFee()method to avoid potential confusion.
77-80: Excellent addition of thefee()helper function.The new
fee()function provides a standardized way to calculate transaction fees, which is crucial for consistent fee handling across the codebase. The implementation correctly usesbig.Intfor precise calculations.Consider adding a comment explaining that this function returns a new
big.Intto clarify that it doesn't modify the inputgasPrice.
Line range hint
83-91: Valuable addition of thecost()helper function.The new
cost()function is a well-implemented helper for calculating the total transaction cost. It correctly handles cases where the value might be nil and aligns perfectly with the PR objective of enhancing cost calculations for the ante handler.Consider adding a brief note about the return type (e.g., "Returns a new *big.Int representing the total cost") to make the documentation even more precise.
app/ante/evm/05_signature_verification.go (2)
61-62: Approve comment addition with a minor suggestion.The added comment provides valuable clarity on the function's behavior, specifically explaining how the
Fromfield is set. This improves code documentation and maintainability.Consider slightly rewording the comment for better clarity:
-// The function set the field from of the given message equal to the sender -// computed from the signature of the Ethereum transaction. +// This function sets the 'From' field of the given message to the sender address +// computed from the signature of the Ethereum transaction.
Discrepancy Between PR Objectives and Code Changes
Upon reviewing the diffs in the relevant files, the changes primarily involve minor comment updates and small function adjustments. There is no substantial evidence of implementations related to adding support for custom decimals or significant updates to the ante handler as outlined in the PR objectives.
- Custom Decimals Support: No changes were found that introduce or modify the handling of custom decimals.
- Ante Handler Updates: While there are minor adjustments in ante handler-related files, they do not align with the stated intent to update the ante handler for EVM checks.
🔗 Analysis chain
Line range hint
1-95: Verify alignment with PR objectives.The changes in this file, while improving documentation, don't appear to directly address the stated PR objectives of adding support for custom decimals or updating the ante handler for EVM checks.
Could you please clarify how these changes relate to the PR objectives? Are there additional files or changes that implement the custom decimal support?
To verify if there are other relevant changes, we can run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for files that might contain changes related to custom decimals or ante handler updates # Test 1: Search for files mentioning "custom decimal" or "ante handler" echo "Files potentially related to custom decimals or ante handler:" rg -i -g '!*.md' 'custom decimal|ante handler' # Test 2: Search for changes in other ante handler files echo "Changes in other ante handler files:" fd -e go -p 'ante' | xargs rg '^[+-]' -N --headingLength of output: 4412
Script:
#!/bin/bash # Description: Inspect diffs in files related to custom decimals and ante handler updates # Files potentially related to custom decimals or ante handler identified from previous script FILES=( x/evm/types/tx_data.go x/evm/wrappers/bank.go x/evm/keeper/keeper.go x/evm/keeper/state_transition.go x/evm/keeper/grpc_query.go server/start.go server/config/toml.go server/config/migration/v0.50-app.toml types/indexer.pb.go server/config/config.go testutil/integration/evmos/utils/genesis.go rpc/types/utils.go proto/ethermint/types/v1/indexer.proto scripts/changelog_checker/test_check_changelog.py indexer/kv_indexer.go api/ethermint/types/v1/indexer.pulsar.go app/ante/evm/05_signature_verification.go app/ante/evm/01_setup_ctx.go app/ante/evm/suite_test.go app/ante/evm/eth_benchmark_test.go app/ante/evm/12_emit_event.go app/ante/cosmos.go app/ante/ante.go app/ante/cosmos/eip712.go ) echo "Inspecting diffs for custom decimals and ante handler updates:" for file in "${FILES[@]}"; do echo "Diff for $file:" git diff origin/main -- "$file" doneLength of output: 9243
app/ante/cosmos/min_price.go (1)
24-25: LGTM! Consider updating the documentation.The renaming of
feesKeepertofeemarketKeeperimproves clarity and aligns with the PR objectives. This change enhances readability without altering functionality.Consider updating the struct's documentation to reflect this change and briefly explain the role of
feemarketKeeper.x/evm/keeper/fees.go (1)
45-45: Approved: Improved documentation and alignment with custom decimal supportThe changes in this function are well-aligned with the PR objectives:
- The updated function comment now accurately reflects that the function deducts fees, not the full gas cost.
- The new comment provides valuable information about the use of
bankWrapperfor decimal conversion, which is crucial for supporting custom decimal base tokens.- The implementation addresses the issue ENG-2636 by using
bankWrapperinstead of directly calling the bank keeper.These improvements enhance the code's clarity and maintainability.
Consider updating the error message in the
DeductFeescall to reflect that it's deducting fees, not the full gas cost:- return errorsmod.Wrapf(err, "failed to deduct full gas cost %s from the user %s balance", fees, from) + return errorsmod.Wrapf(err, "failed to deduct fees %s from the user %s balance", fees, from)This change would make the error message consistent with the updated function comment and its actual behavior.
Also applies to: 57-60
x/evm/types/scaling.go (4)
28-34: LGTM! Consider adding a parameter description.The implementation of
ConvertAmountTo18Decimalsis correct and consistent with the existing conversion functions. It properly uses theGetEVMCoinDecimals()method to obtain the conversion factor.Consider adding a brief description of the
amtparameter in the function comment:// ConvertAmountTo18Decimals converts the given amount into an 18 decimals // representation. // Parameters: // - amt: The amount to convert, as an sdkmath.Int // Returns: // - The converted amount with 18 decimal places func ConvertAmountTo18Decimals(amt sdkmath.Int) sdkmath.Int { // ... (existing implementation) }
44-50: LGTM! Consider adding a parameter description.The implementation of
ConvertAmountTo18DecimalsBigIntis correct and consistent with the other conversion functions. It properly uses theGetEVMCoinDecimals()method to obtain the conversion factor and handles the*big.Inttype appropriately.Consider adding a brief description of the
amtparameter in the function comment:// ConvertAmountTo18DecimalsBigInt converts the given amount into an 18 decimals // representation. // Parameters: // - amt: The amount to convert, as a *big.Int // Returns: // - The converted amount with 18 decimal places as a *big.Int func ConvertAmountTo18DecimalsBigInt(amt *big.Int) *big.Int { // ... (existing implementation) }
84-101: LGTM! Consider a minor optimization.The implementation of
ConvertCoinsTo18Decimalsis correct and consistent with the existingConvertCoinsFrom18Decimalsfunction. It properly identifies EVM coins and applies the conversion only to those coins.Consider a minor optimization to avoid unnecessary allocations when no conversion is needed:
func ConvertCoinsTo18Decimals(coins sdk.Coins) sdk.Coins { evmDenom := GetEVMCoinDenom() evmCoinDecimals := GetEVMCoinDecimals() // Check if conversion is needed needsConversion := false for _, coin := range coins { if coin.Denom == evmDenom { needsConversion = true break } } if !needsConversion { return coins } convertedCoins := make(sdk.Coins, len(coins)) for i, coin := range coins { if coin.Denom == evmDenom { newAmount := coin.Amount.Mul(evmCoinDecimals.ConversionFactor()) coin = sdk.Coin{Denom: evmDenom, Amount: newAmount} } convertedCoins[i] = coin } return convertedCoins }This optimization avoids creating a new slice and copying coins when no conversion is actually needed.
Line range hint
1-102: Summary: Changes align well with PR objectives and enhance EVM coin handling.The new functions
ConvertAmountTo18Decimals,ConvertAmountTo18DecimalsBigInt, andConvertCoinsTo18Decimalseffectively enhance the support for custom decimal base tokens within the EVM checks, aligning well with the PR objectives. These additions provide a comprehensive set of tools for converting between different decimal representations across various data types.The implementations are consistent with existing functions, maintain good coding practices, and integrate seamlessly with the current codebase. The changes contribute to improving the ante handler's capabilities in handling custom decimal tokens, which is relevant to the bank wrapper operations mentioned in the linked issue ENG-2636.
To further improve the robustness of the system:
- Consider adding unit tests for these new functions to ensure their correctness across various scenarios.
- Evaluate if any other parts of the codebase, particularly in the ante handler, need to be updated to utilize these new functions for consistent handling of custom decimal tokens.
- Update relevant documentation to reflect these new capabilities in handling custom decimal base tokens within the EVM checks.
app/ante/evm/04_validate.go (1)
110-113: LGTM: Proper handling of custom decimals in fee validationThe implementation correctly converts the fee amount to 18 decimals before comparison, ensuring proper handling of custom decimal base tokens. This aligns well with the PR objectives.
Consider updating the error message to include both the original and converted amounts for easier debugging:
- return errorsmod.Wrapf(errortypes.ErrInvalidRequest, "invalid AuthInfo Fee Amount (%s != %s)", convertedAmount, txFee) + return errorsmod.Wrapf(errortypes.ErrInvalidRequest, "invalid AuthInfo Fee Amount (original: %s, converted: %s != expected: %s)", txFeeInfo.Amount, convertedAmount, txFee)x/evm/types/dynamic_fee_tx.go (1)
278-278: Consistent improvement in EffectiveFee calculationThe change from
tx.GasLimittotx.GetGas()is consistent with the earlier modification in theFeemethod. This enhances encapsulation and maintains consistency across the struct's methods.For improved clarity and maintainability, consider extracting the gas calculation into a separate method, e.g.,
calculateFee(price, gas *big.Int) *big.Int. This would further reduce duplication betweenFeeandEffectiveFeemethods.CHANGELOG.md (1)
71-71: Minor formatting suggestionThe line exceeds the recommended maximum length of 120 characters. Consider breaking it into multiple lines for better readability.
🧰 Tools
🪛 Markdownlint
71-71: Expected: 120; Actual: 126
Line length(MD013, line-length)
app/ante/evm/01_setup_ctx.go (1)
47-49: Improve the formatting of the steps in the comment for clarity.Use consistent numbering without indentation to make the steps easier to read.
Apply this diff:
-// 1. Set an empty gas config for both KV and transient store. -// 2. Set an infinite gas meter. +// 1. Set an empty gas config for both KV and transient store. +// 2. Set an infinite gas meter.app/ante/evm/mono.go (1)
260-262: Clarify the safety check comment for account nil checkThe comment on lines 260-262 could be clearer. Consider rephrasing it for better understanding.
Apply this diff to improve the comment clarity:
- // safety check: shouldn't happen since if account is nil it is set in - // account balance verification step. + // Safety check: Account should not be nil since it is set during + // the account balance verification step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
- CHANGELOG.md (1 hunks)
- app/ante/cosmos/min_price.go (2 hunks)
- app/ante/evm/01_setup_ctx.go (1 hunks)
- app/ante/evm/02_mempool_fee.go (2 hunks)
- app/ante/evm/04_validate.go (3 hunks)
- app/ante/evm/05_signature_verification.go (1 hunks)
- app/ante/evm/06_account_verification.go (1 hunks)
- app/ante/evm/08_vesting.go (3 hunks)
- app/ante/evm/08_vesting_test.go (1 hunks)
- app/ante/evm/09_gas_consume.go (0 hunks)
- app/ante/evm/mono.go (10 hunks)
- x/evm/keeper/fees.go (2 hunks)
- x/evm/types/dynamic_fee_tx.go (2 hunks)
- x/evm/types/scaling.go (3 hunks)
- x/evm/types/tx_data.go (2 hunks)
- x/evm/types/utils.go (1 hunks)
💤 Files with no reviewable changes (1)
- app/ante/evm/09_gas_consume.go
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
71-71: Expected: 120; Actual: 126
Line length(MD013, line-length)
🔇 Additional comments (29)
app/ante/evm/02_mempool_fee.go (1)
24-25: Improved error message clarity.The updated error message format provides clearer information by directly comparing the provided fee with the required fee. This change enhances the user experience by making the error more understandable.
x/evm/types/tx_data.go (1)
Line range hint
1-91: Overall excellent enhancements to fee and cost calculations.The changes in this file significantly improve the capabilities for fee and cost calculations, aligning perfectly with the PR objectives. The new methods and functions are well-implemented and documented, providing valuable tools for the ante handler and overall transaction processing.
Key improvements:
- Enhanced documentation for existing methods.
- Addition of
Cost()method for total transaction cost calculation.- New
EffectiveFee()method for calculating user-willing fees.- Helper functions
fee()andcost()for standardized calculations.These changes will greatly contribute to more accurate and flexible handling of custom decimal tokens within EVM checks.
app/ante/cosmos/min_price.go (3)
31-31: LGTM! Consistent with struct field renaming.The update to the
NewMinGasPriceDecoratorfunction correctly initializes the renamedfeemarketKeeperfield. This change maintains consistency with the struct definition while preserving the function's signature and behavior.
Line range hint
1-95: Summary: Consistent renaming enhances clarity without functional changes.The changes in this file consistently rename
feesKeepertofeemarketKeeper, improving code clarity and aligning with the PR objectives. This renaming affects theMinGasPriceDecoratorstruct, its constructor, and theAnteHandlemethod. The changes are purely nomenclature-based and do not introduce any functional modifications.Regarding the PR objectives:
- The changes contribute to enhancing the ante handler by improving clarity in token handling, specifically in relation to fee market operations.
- While these changes don't directly address the use of the bank wrapper mentioned in ENG-2636, they do improve the overall code quality in the ante package, which is relevant to EVM operations.
40-40: LGTM! Verify consistent usage across the codebase.The update in the
AnteHandlemethod correctly uses the renamedfeemarketKeeperfield to retrieve the minimum gas price parameters. This change maintains consistency with the struct field renaming.To ensure consistency across the codebase, please run the following script:
✅ Verification successful
LGTM! The renaming from
feesKeepertofeemarketKeeperhas been successfully applied and is consistent across the codebase.
- All instances of
feemarketKeeperare correctly used inapp/ante/cosmos/min_price.go.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of feemarketKeeper instead of feesKeeper # Test: Search for any remaining instances of feesKeeper # Expect: No results, indicating complete renaming rg --type go 'feesKeeper' # Test: Confirm usage of feemarketKeeper # Expect: Results showing consistent usage of feemarketKeeper rg --type go 'feemarketKeeper'Length of output: 312
x/evm/keeper/fees.go (2)
39-39: Approved: Improved error message consistency and efficiencyThe change in the error message is a good improvement. It now uses the
costvariable directly instead of callingtxData.Cost()again, which is consistent with the comparison made earlier in the function. This change enhances code readability and potentially improves performance by avoiding an unnecessary function call.
Line range hint
1-124: Summary: Changes align well with PR objectives and improve code qualityThe modifications in this file successfully address the PR objectives and the linked issue ENG-2636:
- The changes in
DeductTxCostsFromUserBalancesupport custom decimal base tokens by utilizing thebankWrapperfor proper decimal conversion.- The use of
bankWrapperinstead of directly calling the bank keeper aligns with the objective of checking the bank wrapper in ante (issue ENG-2636).- The code improvements enhance clarity, efficiency, and maintainability.
Overall, these changes represent a positive step towards better handling of custom decimal tokens and improved integration with the bank wrapper in the EVM module.
x/evm/types/utils.go (1)
98-107: Approved: Function signature and error handling improvementsThe simplification of the
UnpackEthMsgfunction signature and streamlined error handling are good improvements. They make the function more focused and easier to use.Clarification needed: Relation to PR objectives
Could you please clarify how this change relates to the PR objectives of adding support for custom decimals and using the bank wrapper instead of the bank keeper? The connection is not immediately apparent from the code changes.
Suggestion: Add explanatory comment
Consider adding a comment explaining why the sender's address retrieval was removed. This would help future developers understand the reasoning behind this change.
Verify: Impact of removing sender's address
The removal of the sender's address retrieval might affect other parts of the codebase. Let's verify if there are any potential impacts:
Please review the results of this script to ensure that all usages of
UnpackEthMsghave been updated accordingly and that there are no remaining dependencies on thefromaddress that was previously returned.app/ante/evm/04_validate.go (4)
15-16: LGTM: Improved function descriptionThe updated comment provides a clearer and more accurate description of the
ValidateMsgfunction's purpose, explicitly mentioning the check for a nil "from" address.
44-45: LGTM: Clearer error condition descriptionThe updated comment provides a more concise and clear description of when errors are returned in the
checkDisabledCreateCallfunction, improving code readability.
101-104: LGTM: New function for fee validation with custom decimals supportThe addition of the
CheckTxFeefunction and its descriptive comment aligns well with the PR objectives. It explicitly handles the validation of transaction fees while considering custom decimal representations, which is a key aspect of the feature being implemented.
Line range hint
1-121: Overall LGTM: Successfully implemented custom decimal support in fee validationThe changes in this file successfully implement support for custom decimal base tokens in EVM checks, aligning perfectly with the PR objectives. The new
CheckTxFeefunction and its implementation provide a robust mechanism for validating transaction fees while considering custom decimal representations. The code is well-commented and the logic is sound, ensuring proper handling of different decimal bases in the ante handler.These changes contribute significantly to addressing the linked issue ENG-2636, as they enhance the integration of custom token handling within the EVM context. Great job on implementing this feature!
x/evm/types/dynamic_fee_tx.go (2)
263-263: Improved encapsulation in Fee calculationThe change from
tx.GasLimittotx.GetGas()enhances encapsulation and adheres to best practices. This modification promotes consistency across the codebase and simplifies future maintenance.
Line range hint
1-283: Summary: Improved encapsulation and consistency in DynamicFeeTxThe changes in this file focus on enhancing encapsulation and consistency within the
DynamicFeeTxstruct methods. By usingtx.GetGas()instead of directly accessingtx.GasLimit, the code now adheres better to object-oriented principles and Go best practices.These modifications align well with the "minor refactor" mentioned in the PR objectives. While they don't directly address the support for custom decimal base tokens, they do improve the overall code quality and maintainability. This refactoring lays a solid foundation for future enhancements, potentially making it easier to implement custom decimal support in subsequent changes.
The changes are subtle but impactful, improving the codebase without altering core functionality. This approach minimizes the risk of introducing bugs while setting the stage for future improvements.
app/ante/evm/08_vesting_test.go (1)
163-163: LGTM! Verify consistency across the codebase.The change from
unitNetwork.App.BankKeepertounitNetwork.App.EvmKeeperaligns with the PR objective to use the bank wrapper instead of the bank keeper for EVM operations. This addresses the goal outlined in the linked issue ENG-2636.To ensure consistency, please run the following script to check for any remaining instances of
BankKeeperusage in EVM-related contexts:If the script returns any results, consider updating those instances to use
EvmKeeperas well, if appropriate.CHANGELOG.md (1)
71-71: LGTM! New changelog entries added correctly.The new entries for the ante handler and EVM module improvements have been added correctly to the "Unreleased" section of the changelog.
🧰 Tools
🪛 Markdownlint
71-71: Expected: 120; Actual: 126
Line length(MD013, line-length)
app/ante/evm/01_setup_ctx.go (1)
58-58: Verify the context used inResetTransientGasUsed.
evmKeeper.ResetTransientGasUsedis called with the originalctx, notnewCtx. Please verify if this is intentional or if it should usenewCtxto ensure consistency with the modified context.Run the following script to check how
ResetTransientGasUsedis used elsewhere:✅ Verification successful
To verify the context parameter used in
ResetTransientGasUsed, let's examine its implementation.
To gain insight into how
ResetTransientGasUsedis being used and determine whetherctxornewCtxshould be passed, let's examine the surrounding code.
To determine whether
ResetTransientGasUsedshould usectxornewCtx, let's examine hownewCtxis derived inapp/ante/evm/01_setup_ctx.go.
Verified: The use of
ctxinResetTransientGasUsedis intentional and correct.No changes are necessary as
ResetTransientGasUsedappropriately uses the original context to manage the transient gas state.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of ResetTransientGasUsed and assess the context parameter. rg --type go 'ResetTransientGasUsed\('Length of output: 254
Script:
#!/bin/bash # Description: Retrieve the implementation of the ResetTransientGasUsed function. ast-grep --lang go --pattern 'func (k Keeper) ResetTransientGasUsed(ctx sdk.Context) { $$$ }' x/evm/keeper/keeper.goLength of output: 377
Script:
#!/bin/bash # Description: Retrieve the surrounding code of ResetTransientGasUsed call in app/ante/evm/01_setup_ctx.go. rg -C 5 'ResetTransientGasUsed\(ctx\)' app/ante/evm/01_setup_ctx.goLength of output: 409
Script:
#!/bin/bash # Description: Retrieve the code where newCtx is defined and assigned in app/ante/evm/01_setup_ctx.go. rg -C 10 'newCtx' app/ante/evm/01_setup_ctx.goLength of output: 1659
app/ante/evm/08_vesting.go (11)
8-9: Addition ofcommonpackage import is appropriateThe import of
"github.com/ethereum/go-ethereum/common"is necessary for address conversion functions used in the code.
23-24: Improved documentation forEthVestingExpenseTracker.TotalThe updated comments enhance the clarity of the
Totalfield in theEthVestingExpenseTrackerstruct.
33-33: Clarification onnewExpensesrepresentationThe comment specifies that
newExpensesis expected to be in an 18-decimal representation, which provides clarity for function usage.
36-39: Updated function signature to useEVMKeeperandnewExpensesThe function
CheckVestingnow acceptsevmKeeper EVMKeeperandnewExpenses *big.Intinstead ofbankKeeperandaddedExpense. This change aligns with the goal of supporting custom decimal tokens and improves consistency with EVM operations.
50-50: PassingevmKeeperinstead ofbankKeepertoUpdateAccountExpensesThe
evmKeeperis now passed toUpdateAccountExpenses, replacingbankKeeper. This change is consistent with the modifications inUpdateAccountExpensesand supports retrieving balances in 18-decimal representation.
64-65: Updated function documentation forUpdateAccountExpensesThe comments for
UpdateAccountExpenseshave been updated to reflect the changes in parameters and return value, enhancing clarity.
68-71: Modified function signature to useEVMKeeperandnewExpensesThe function
UpdateAccountExpensesnow acceptsevmKeeper EVMKeeperandnewExpenses *big.Int, aligning with the changes inCheckVestingand supporting custom decimal tokens.
78-78: Correctly updatingexpenses.TotalwithnewExpensesThe total expenses are updated by adding
newExpenses, ensuring accurate tracking of the total value spent.
82-86: Retrieving account balance usingEVMKeeperin 18-decimal representationUsing
evmKeeper.GetBalanceensures that the account balance is obtained in 18-decimal format, which is necessary for consistency with EVM token standards.
Line range hint
101-102: ConvertinglockedBalance.Amountto 18 decimalsThe conversion of
lockedBalance.Amountto 18-decimal representation ensures consistency when calculating spendable balances.
109-110: SettingTotalandSpendableinEthVestingExpenseTrackerInitializing the
TotalwithnewExpensesand settingSpendabletospendableValuecorrectly prepares the tracker for expense validation.app/ante/evm/mono.go (1)
327-329: Ensure accurate update of transaction gas limitsIn lines 327-329, you're updating the transaction gas limit by adding the gas specified in the current message. Ensure that this accumulation correctly reflects the total gas limit for all messages in the transaction.
Confirm that
decUtils.TxGasLimitaccurately represents the cumulative gas across all messages.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: stepit <48993133+0xstepit@users.noreply.github.com>
|
After addressing Tom’s comments we should be ready to merge. Awesome work @0xstepit |
Description
Closes: ENG-2636
Summary by CodeRabbit
New Features
Improvements
eth_headerByHashRPC call.Bug Fixes