precompiles/gov/errors.go (1)

32-35: Consider aligning error constant name with its message

The error constant ErrInvalidCoin uses singular form while its message refers to "coins" (plural). Consider either:

• Renaming to ErrInvalidCoins to match the message, or
• Updating the message to "invalid coin %s" for consistency

-	// ErrInvalidCoin invalid coin.
-	ErrInvalidCoin = "invalid coins %s "
+	// ErrInvalidCoins invalid coins.
+	ErrInvalidCoins = "invalid coins %s "

precompiles/gov/events.go (1)

27-38: Consider standardizing the field naming convention.

The ProposalId field in both structs uses non-standard Go naming convention. Consider renaming to ProposalID to follow Go's convention for abbreviations.

Apply this diff:

-	ProposalId uint64         `json:"proposalId"` //nolint:revive,stylecheck
+	ProposalID uint64         `json:"proposalId"` //nolint:revive,stylecheck

precompiles/gov/IGov.sol (2)

128-150: Consider enhancing method documentation with validation requirements.

While the methods are well-structured, the documentation could be enhanced by specifying:

• Parameter validation requirements (e.g., valid proposal ID ranges, non-zero deposit amounts)
• Conditions under which the operations might fail
• Access control requirements (if any)

Example enhancement for the deposit method:

 /// @dev deposit defines a method to add a deposit to a proposal
 /// @param depositor The address of the depositor
 /// @param proposalId The proposal id
 /// @param amount The deposit amount
 /// @return success Whether the transaction was successful or not
+/// @notice The proposal must be in the deposit period
+/// @notice The amount must be non-zero and in accepted denomination

---

Line range hint 1-150: Consider architectural improvements for better maintainability and standardization.

The interface could benefit from the following architectural improvements:

1. Consider splitting into sub-interfaces (e.g., IGovTransactions, IGovQueries) for better separation of concerns
2. Consider implementing or aligning with governance standards like ERC-3000 for better interoperability
3. Consider adding events for error cases to improve observability

precompiles/gov/events_test.go (2)

172-244: Consider adding error test cases for Deposit events

While the happy path test case is well implemented, consider adding test cases for common error scenarios such as:

• Invalid deposit amount
• Non-existent proposal ID
• Insufficient funds

---

246-313: Consider adding error test cases for CancelProposal events

The success path is well tested, but consider adding test cases for:

• Non-existent proposal
• Unauthorized cancellation (non-proposer attempt)
• Already canceled proposal

precompiles/gov/tx_test.go (1)

399-486: Enhance proposal cancellation verification

While the test cases cover the basic scenarios, the success case could be more comprehensive:

Consider enhancing the success test case to verify:

1. Events emitted during cancellation
2. State changes before and after cancellation
3. Any refunds or side effects

 func() {
+    // Verify proposal exists before cancellation
+    found, err := s.network.App.GovKeeper.Proposals.Has(s.network.GetContext(), proposalID)
+    s.Require().NoError(err)
+    s.Require().True(found)
+
+    // Get initial state if needed (e.g., deposits, votes)
+    initialDeposits, err := s.network.App.GovKeeper.GetDeposits(ctx, proposalID)
+    s.Require().NoError(err)
+
     found, err := s.network.App.GovKeeper.Proposals.Has(s.network.GetContext(), proposalID)
     s.Require().NoError(err)
     s.Require().False(found)
+
+    // Verify any expected side effects (e.g., deposit refunds)
+    // TODO: Add specific assertions based on the expected behavior
 }

precompiles/gov/integration_test.go (2)

213-235: Consider adding error test cases for CancelProposal.

While the happy path is well tested, consider adding test cases for important error scenarios:

• Attempting to cancel a proposal by a non-proposer
• Attempting to cancel a non-existent proposal
• Attempting to cancel a proposal that's already in a final state (e.g., passed, rejected)

---

237-271: Enhance deposit test coverage and improve maintainability.

1. Consider adding test cases for important error scenarios:

   • Attempting to deposit with insufficient balance
   • Attempting to deposit to a non-existent proposal
   • Attempting to deposit with invalid coin denominations
   • Attempting to deposit when proposal is in final state

2. The initial deposit amount (100 wei) should be defined as a constant at the package level for better maintainability and clarity.

 // Add at the package level
+const (
+    // InitialProposalDeposit represents the initial deposit amount for test proposals
+    InitialProposalDeposit = 100
+)

 // Update the test to use the constant
-expectedAmount := big.NewInt(0).Add(big.NewInt(100), depositAmount)
+expectedAmount := big.NewInt(0).Add(big.NewInt(InitialProposalDeposit), depositAmount)

precompiles/gov/tx.go (2)

115-117: Refactor redundant origin checks into a helper function

The logic for checking whether the origin matches the depositor address is repeated across methods. Refactoring this into a helper function enhances code maintainability and reduces duplication.

For example, implement a helper function:

func (p Precompile) validateOrigin(origin, caller, address common.Address, errMsg string) error {
    isContractCaller := caller == address && caller != origin
    if !isContractCaller && origin != address {
        return fmt.Errorf(errMsg, origin.String(), address.String())
    }
    return nil
}

And then use it in your methods.

---

156-159: Refactor repeated origin check logic into a helper function

The origin check logic is repeated in multiple methods (Vote, VoteWeighted, Deposit, CancelProposal). Refactoring this logic into a reusable helper function will improve code readability and maintainability.

You can create a helper function as suggested earlier and replace the origin checks in your methods.

precompiles/gov/types.go (3)

Line range hint 273-299: Improve error handling in VotesOutput.FromResponse method

In the FromResponse method for VotesOutput, when an error occurs during address conversion, the function returns nil without providing error context. This may make debugging difficult.

Consider returning an error to provide more context:

 func (vo *VotesOutput) FromResponse(res *v1.QueryVotesResponse) (*VotesOutput, error) {
     vo.Votes = make([]WeightedVote, len(res.Votes))
     for i, v := range res.Votes {
         hexAddr, err := utils.Bech32ToHexAddr(v.Voter)
         if err != nil {
-            return nil
+            return nil, fmt.Errorf("failed to convert voter address: %w", err)
         }
         // Existing code...
     }
     // Existing code...
-    return vo
+    return vo, nil
 }

Adjust the function signature and handle the error accordingly.

---

Line range hint 402-421: Enhance error handling in DepositOutput.FromResponse method

Similar to the previous comment, if there's an error converting the depositor address, returning nil without context can hinder debugging.

Modify the method to return an error with more information:

 func (do *DepositOutput) FromResponse(res *v1.QueryDepositResponse) (*DepositOutput, error) {
     hexDepositor, err := utils.Bech32ToHexAddr(res.Deposit.Depositor)
     if err != nil {
-        return nil
+        return nil, fmt.Errorf("failed to convert depositor address: %w", err)
     }
     // Existing code...
-    return do
+    return do, nil
 }

Update the function signature and handle the error accordingly.

---

Line range hint 584-620: Consistent error handling in ProposalsOutput.FromResponse method

In the loop, if an error occurs during address conversion, the method returns nil. It's better to return an error to provide context.

Apply this diff to improve error handling:

 func (po *ProposalsOutput) FromResponse(res *v1.QueryProposalsResponse) (*ProposalsOutput, error) {
     po.Proposals = make([]ProposalData, len(res.Proposals))
     for i, p := range res.Proposals {
         // Existing code...
 
         proposer, err := utils.Bech32ToHexAddr(p.Proposer)
         if err != nil {
-            return nil
+            return nil, fmt.Errorf("failed to convert proposer address: %w", err)
         }
 
         po.Proposals[i] = ProposalData{
             // Existing code...
         }
     }
 
     // Existing code...
-    return po
+    return po, nil
 }

Ensure the function signature reflects the error return value.

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check usage of new error constants in the codebase

# Search for usage of new error constants
echo "Checking usage of new error constants:"
rg "ErrDifferentOriginProposer|ErrDifferentOriginDepositor|ErrInvalidProposer|ErrInvalidCoin" \
  --type go \
  -B 2 -A 2

# Look for potential error handling patterns we should match
echo -e "\nChecking existing error handling patterns in gov package:"
rg "return.*Err.*Origin|return.*Err.*Invalid" \
  --type go \
  -g 'precompiles/gov/*.go' \
  -B 2 -A 2

#!/bin/bash
# Description: Check for the method constant definitions
# Expected: Should find constant definitions for DepositMethod and CancelProposalMethod

# Search for method constant definitions
echo "Searching for method constant definitions..."
rg -A 1 'const \(\s*DepositMethod|CancelProposalMethod\s*=\s*"'

#!/bin/bash
# Let's also check for DepositMethod constant and verify all method constants used in the switch case
echo "Searching for all method constants..."
rg "const.*(?:Vote|Deposit|CancelProposal).*Method.*=" -A 1

echo -e "\nVerifying usage in switch case..."
rg "case.*(?:Vote|Deposit|CancelProposal).*Method.*:" -A 1

#!/bin/bash
# Let's check the tx.go file for all method constant definitions to ensure completeness
echo "Checking tx.go for all method constants..."
rg "^const \(" -A 15 precompiles/gov/tx.go

#!/bin/bash
# Description: Check for the implementation of Deposit and CancelProposal methods
# Expected: Should find method implementations in the same package

# Search for Deposit method implementation
echo "Searching for Deposit method implementation..."
ast-grep --pattern 'func (p Precompile) Deposit(ctx sdk.Context, origin common.Address, contract *vm.Contract, stateDB *storetypes.KVStore, method *abi.Method, args []interface{}) '

# Search for CancelProposal method implementation
echo "Searching for CancelProposal method implementation..."
ast-grep --pattern 'func (p Precompile) CancelProposal(ctx sdk.Context, origin common.Address, contract *vm.Contract, stateDB *storetypes.KVStore, method *abi.Method, args []interface{}) '

#!/bin/bash
# Search for Deposit and CancelProposal method implementations with a more flexible pattern
echo "Searching for Deposit method..."
ast-grep --pattern 'func $_ Deposit($_) {
  $$$
}'

echo -e "\nSearching for CancelProposal method..."
ast-grep --pattern 'func $_ CancelProposal($_) {
  $$$
}'

# Fallback to ripgrep for additional context
echo -e "\nSearching with ripgrep..."
rg "func.*(?:Deposit|CancelProposal).*\(" -A 2

event CancelProposal(uint64 indexed proposalId, address indexed proposer);

#!/bin/bash
# Description: Verify CancelProposal event definition
# Search for the CancelProposal event definition in Solidity files
rg -A 2 'event\s+CancelProposal\s*\(' --type solidity

# Check if there are any non-indexed parameters that should be packed as data
ast-grep --pattern 'event CancelProposal($_) { $$$ }'

#!/bin/bash
# Let's try searching for the event definition in all files since the .sol extension might not be recognized
rg -A 2 'event\s+CancelProposal\s*\('

# Also search for the ABI definition which would show the event structure
rg -A 5 'EventTypeCancelProposal\s*=' 

# Let's also check the interface or contract definition that might contain this event
rg -A 10 'interface.*Gov'

#!/bin/bash
# Description: Verify event signature matches Solidity definition
# Search for the Deposit event definition in Solidity files
rg -A 2 'event\s+Deposit\s*\(' --type solidity

#!/bin/bash
# Search for Deposit event definition in all files, focusing on contract interfaces and definitions
rg -A 2 'event\s+Deposit\s*\(' 

# Also search specifically in contract-related directories that might contain the interface
fd -e sol . contracts precompiles interfaces 2>/dev/null

# Backup search for ABI definitions that might contain event definitions
fd -g "*abi*.go" . | xargs rg "EventTypeDeposit"

#!/bin/bash
# Let's find the ABI definition for the Deposit event to compare with implementation
rg -B 2 -A 2 'EventTypeDeposit.*=|const.*EventTypeDeposit' precompiles/gov/

# Also get the ABI construction to see event definition
ast-grep --pattern 'abi.NewEvent($$$)'

#!/bin/bash
# Let's find the ABI method that constructs the events
rg -A 10 'Methods.*Events.*=.*map\[string\]abi.Event' precompiles/gov/

#!/bin/bash
# Search for the ABI definition in the gov precompile
rg -A 20 'var.*ABI.*=.*abi\.JSON' precompiles/gov/

# Also check the contract interface definition again but with more context
rg -B 2 -A 5 'event\s+Deposit' precompiles/gov/IGov.sol

#!/bin/bash
# Description: Check test coverage for the new governance transactions

# Look for all governance-related test files
fd -e go test$ precompiles/gov/ -x

# Check for any TODOs or missing test coverage
rg -i "todo|fixme|xxx" precompiles/gov/

#!/bin/bash
# Let's try a different approach to check test coverage

# Find all test files in the gov precompile directory
fd ".*_test.go$" precompiles/gov/

# Check for test functions related to Deposit and CancelProposal
rg "func.*Test(Deposit|CancelProposal)" precompiles/gov/

# Look for the implementation of these methods to ensure all cases are covered
rg "func.*(Deposit|CancelProposal)" precompiles/gov/ --type go -g '!*_test.go'

#!/bin/bash
# Description: Verify consistent usage of Coin[] type and function naming patterns

# Check for consistent usage of Coin[] type in other governance functions
ast-grep --pattern 'struct Coin[] {
  $$$
}'

# Check for consistent function naming patterns
ast-grep --pattern 'function $FNAME($$$) {
  $$$
}'

#!/bin/bash
# Let's verify the ABI functions with more targeted commands

# Check for consistent usage of Coin[] type in governance functions
rg -U "struct Coin\[\].*?\{.*?\}" --multiline

# Check for consistent function patterns in governance ABI
rg -U "\"name\": \"(cancelProposal|deposit)\".*?\"type\": \"function\"" --multiline -A 20 precompiles/gov/abi.json

# Look for any related governance function implementations
rg -U "function (cancelProposal|deposit)" --multiline

#!/bin/bash
# Let's examine the governance interface and implementation more closely

# Check the full interface definitions in IGov.sol
cat precompiles/gov/IGov.sol

# Look for any test files that might show usage patterns
rg -g "*.sol" -g "*.js" -l "cancelProposal|deposit.*proposalId"

#!/bin/bash
# Description: Analyze the code for the G115 linter warning and determine if suppression is appropriate.

# Run gosec linter on the file
gosec -no-fail -include=G115 precompiles/gov/tx.go

#!/bin/bash
# Let's analyze the context around the Unix() call to understand its usage

# First, let's see the full context of the file
rg -A 5 -B 5 "Unix\(\)" precompiles/gov/tx.go

# Also check for any related test files or documentation
fd "tx_test.go|README" precompiles/gov/

# Look for other similar Unix() usages in the codebase
rg "Unix\(\)" precompiles/

precompiles/gov/errors.go (1)

34-35: Fix inconsistency in error message

There's a mismatch between the constant name and its error message:

• Constant name uses singular form: ErrInvalidCoin
• Error message uses plural form: "invalid coins %s"

Consider maintaining consistency by either:

1. Changing the constant to ErrInvalidCoins to match the message, or
2. Changing the message to "invalid coin %s" to match the constant

precompiles/gov/gov.go (1)

105-108: Consider comprehensive testing scenarios.

Given that these governance operations can have significant impact on the chain state:

1. Ensure thorough testing of gas consumption patterns for these new operations
2. Consider adding integration tests that verify the interaction between these methods and the underlying governance module
3. Add test cases for error conditions, especially around proposal state transitions

Would you like assistance in designing these test scenarios?

Also applies to: 148-148

precompiles/gov/events.go (1)

27-38: LGTM: Event structs are well-defined

The structs are properly defined with appropriate types and JSON tags. Consider adding a period at the end of struct documentation comments for consistency with Go documentation standards.

-// EventDeposit is the event type emitted when a deposit is made to a proposal
+// EventDeposit is the event type emitted when a deposit is made to a proposal.
-// EventCancelProposal is the event type emitted when a proposal is canceled
+// EventCancelProposal is the event type emitted when a proposal is canceled.

precompiles/gov/tx.go (2)

128-136: Enhance documentation for EVM state handling.

The EVM state consistency handling is crucial but could use more detailed documentation. Consider expanding the comment to explain:

1. Why scaling to 18 decimals is necessary
2. What problems could occur without this handling
3. The relationship between bank keeper and EVM stateDB

-    // NOTE: This ensures that the changes in the bank keeper are correctly mirrored to the EVM stateDB
-    // when calling the precompile from a smart contract
-    // This prevents the stateDB from overwriting the changed balance in the bank keeper when committing the EVM state.
-    // Need to scale the amount to 18 decimals for the EVM balance change entry
+    // IMPORTANT: EVM State Consistency Handling
+    // When this precompile is called from a smart contract:
+    // 1. The bank keeper modifies the actual balance
+    // 2. These changes must be mirrored in the EVM stateDB to prevent overwrites
+    // 3. All EVM amounts must be scaled to 18 decimals (EVM standard)
+    // 4. Without this handling, the stateDB would overwrite the bank keeper changes
+    //    during the EVM state commitment phase

---

171-175: Document return value structure.

The method returns multiple values packed into the ABI output:

1. A boolean success flag
2. Cancellation timestamp
3. Cancellation height

Consider adding a comment documenting these return values and their significance.

+    // Pack multiple return values into ABI output:
+    // - bool: success flag indicating the proposal was successfully canceled
+    // - uint64: Unix timestamp when the proposal was canceled
+    // - uint64: block height when the proposal was canceled
     return method.Outputs.Pack(
         true,
         uint64(resp.CanceledTime.Unix()),
         resp.CanceledHeight,
     )

precompiles/gov/IGov.sol (2)

129-138: Consider adding validation requirements to deposit documentation.

While the method signature is well-structured, it would be helpful to document any validation requirements for the deposit amount (e.g., minimum/maximum values, token types accepted) to guide implementers and users.

---

140-150: Enhance cancelProposal documentation with cancellation conditions.

The method is well-structured, but consider adding documentation about:

• Under what conditions a proposal can be canceled
• Any timing restrictions on cancellation
• Whether the proposer is the only one who can cancel

precompiles/gov/events_test.go (3)

155-156: LGTM - Explicit struct initialization improves readability

The explicit use of gov.WeightedVoteOption in struct initialization improves code clarity.

Consider using the field names for even better readability:

-gov.WeightedVoteOption{Option: 1, Weight: "0.70"},
+gov.WeightedVoteOption{
+    Option: 1,
+    Weight: "0.70",
+},

---

172-244: Consider adding more test cases for deposit events

The test structure is solid, but consider adding these test cases for better coverage:

1. Negative test cases:
   • Invalid deposit amount (zero or negative)
   • Non-existent proposal ID
   • Unauthorized depositor
2. Edge cases:
   • Maximum deposit amount
   • Multiple deposit denominations

---

246-313: Consider adding more test cases for cancel proposal events

The test follows good practices but could be enhanced with additional test cases:

1. Negative test cases:
   • Non-existent proposal
   • Unauthorized cancellation attempt (non-proposer)
   • Already canceled proposal
2. Edge cases:
   • Proposal in different states (voting period, passed, rejected)

precompiles/gov/tx_test.go (2)

293-294: Consider varying gas limits for different test cases

All test cases use the same gas limit of 200000. Consider adjusting gas limits based on the complexity of each operation to ensure proper gas estimation.

---

283-486: Consider enhancing test infrastructure

The test suite could benefit from:

1. Helper functions for common setup operations (e.g., proposal creation)
2. Shared constants for common values
3. Table-driven test utilities for better maintainability

This would reduce code duplication and make tests more maintainable.

precompiles/gov/types.go (8)

Line range hint 211-246: Potential index out of range panic when accessing method.Inputs[2]

In the function NewMsgVoteWeighted, accessing method.Inputs[2] without verifying that method.Inputs has at least three elements could cause an index out of range panic. Ensure that method.Inputs has sufficient length before accessing its elements.

Apply this diff to add the necessary check:

 func NewMsgVoteWeighted(method *abi.Method, args []interface{}) (*v1.MsgVoteWeighted, common.Address, WeightedVoteOptions, error) {
 	if len(args) != 4 {
 		return nil, common.Address{}, WeightedVoteOptions{}, fmt.Errorf(cmn.ErrInvalidNumberOfArgs, 4, len(args))
 	}

+	if len(method.Inputs) <= 2 {
+		return nil, common.Address{}, WeightedVoteOptions{}, fmt.Errorf("method.Inputs has insufficient length")
+	}

 	voterAddress, ok := args[0].(common.Address)
 	if !ok || voterAddress == (common.Address{}) {
 		return nil, common.Address{}, WeightedVoteOptions{}, fmt.Errorf(ErrInvalidVoter, args[0])

---

Line range hint 273-302: Inconsistent error handling in FromResponse method

In the VotesOutput.FromResponse method, if there's an error converting the Bech32 address to hex (utils.Bech32ToHexAddr), the function returns nil without signaling the error. This can lead to silent failures downstream. Consider modifying the method to return an error alongside the VotesOutput to properly handle error cases.

Apply this diff to modify the function signature and error handling:

-func (vo *VotesOutput) FromResponse(res *v1.QueryVotesResponse) *VotesOutput {
+func (vo *VotesOutput) FromResponse(res *v1.QueryVotesResponse) (*VotesOutput, error) {
 	vo.Votes = make([]WeightedVote, len(res.Votes))
 	for i, v := range res.Votes {
 		hexAddr, err := utils.Bech32ToHexAddr(v.Voter)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 		// ... rest of the code ...
 	}
 	// ... rest of the code ...
-	return vo
+	return vo, nil
 }

---

Line range hint 326-345: Consistent error handling in VoteOutput.FromResponse method

Similar to the previous comment, in VoteOutput.FromResponse, the method returns nil in case of an error without propagating it. Consider updating the method to return an error for proper error handling.

Apply this diff to modify the function signature and error handling:

-func (vo *VoteOutput) FromResponse(res *v1.QueryVoteResponse) *VoteOutput {
+func (vo *VoteOutput) FromResponse(res *v1.QueryVoteResponse) (*VoteOutput, error) {
 	hexVoter, err := utils.Bech32ToHexAddr(res.Vote.Voter)
 	if err != nil {
-		return nil
+		return nil, err
 	}
 	// ... rest of the code ...
-	return vo
+	return vo, nil
 }

---

Line range hint 402-420: Ensure proper error propagation in DepositOutput.FromResponse

In the DepositOutput.FromResponse method, errors are not propagated. Modify the method to return an error along with the output to handle failures explicitly.

Apply this diff:

-func (do *DepositOutput) FromResponse(res *v1.QueryDepositResponse) *DepositOutput {
+func (do *DepositOutput) FromResponse(res *v1.QueryDepositResponse) (*DepositOutput, error) {
 	hexDepositor, err := utils.Bech32ToHexAddr(res.Deposit.Depositor)
 	if err != nil {
-		return nil
+		return nil, err
 	}
 	// ... rest of the code ...
-	return do
+	return do, nil
 }

---

Line range hint 422-450: Consistent error handling in DepositsOutput.FromResponse

The DepositsOutput.FromResponse method should also return errors when address conversion fails to avoid silent failures.

Apply this diff:

-func (do *DepositsOutput) FromResponse(res *v1.QueryDepositsResponse) *DepositsOutput {
+func (do *DepositsOutput) FromResponse(res *v1.QueryDepositsResponse) (*DepositsOutput, error) {
 	do.Deposits = make([]DepositData, len(res.Deposits))
 	for i, d := range res.Deposits {
 		hexDepositor, err := utils.Bech32ToHexAddr(d.Depositor)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 		// ... rest of the code ...
 	}
 	// ... rest of the code ...
-	return do
+	return do, nil
 }

---

Line range hint 514-536: Potential issue with casting ProposalStatus

In ParseProposalsArgs, casting input.ProposalStatus directly to v1.ProposalStatus without validation could result in invalid status values. Consider adding validation to ensure input.ProposalStatus corresponds to a valid v1.ProposalStatus enum value.

Apply this diff to include validation:

 	var input ProposalsInput
 	if err := method.Inputs.Copy(&input, args); err != nil {
 		return nil, fmt.Errorf("error while unpacking args to ProposalsInput: %s", err)
 	}

+	if _, ok := v1.ProposalStatus_name[int32(input.ProposalStatus)]; !ok {
+		return nil, fmt.Errorf("invalid ProposalStatus value: %d", input.ProposalStatus)
+	}

 	voter := ""

---

Line range hint 542-583: Ensure error propagation in ProposalOutput.FromResponse

In the ProposalOutput.FromResponse method, errors are not propagated when address conversion fails. Modify the method to return an error for proper error handling.

Apply this diff:

-func (po *ProposalOutput) FromResponse(res *v1.QueryProposalResponse) *ProposalOutput {
+func (po *ProposalOutput) FromResponse(res *v1.QueryProposalResponse) (*ProposalOutput, error) {
 	// ... existing code ...
 	proposer, err := utils.Bech32ToHexAddr(res.Proposal.Proposer)
 	if err != nil {
-		return nil
+		return nil, err
 	}
 	// ... rest of the code ...
-	return po
+	return po, nil
 }

---

Line range hint 584-627: Consistent error handling in ProposalsOutput.FromResponse

The ProposalsOutput.FromResponse method should propagate errors instead of returning nil to avoid silent failures.

Apply this diff:

-func (po *ProposalsOutput) FromResponse(res *v1.QueryProposalsResponse) *ProposalsOutput {
+func (po *ProposalsOutput) FromResponse(res *v1.QueryProposalsResponse) (*ProposalsOutput, error) {
 	// ... existing code ...
 		proposer, err := utils.Bech32ToHexAddr(p.Proposer)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 	// ... rest of the code ...
-	return po
+	return po, nil
 }

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check if all new error constants are used in the codebase
# and verify no direct string error messages are used instead

# Search for usage of new error constants
echo "Checking usage of new error constants..."
rg "ErrDifferentOriginProposer|ErrDifferentOriginDepositor|ErrInvalidProposer|ErrInvalidCoin" --type go

# Search for potential hardcoded error messages that should use these constants
echo "Checking for potential hardcoded error messages..."
rg -i "(?:invalid (?:proposer|coins?)|(?:origin|tx).*(?:proposer|depositor))" --type go

#!/bin/bash
# Check for the usage of other error constants defined in the file
# and their corresponding error handling in transaction handlers

# Search for usage of remaining error constants
echo "Checking usage of other error constants..."
rg "ErrInvalidVoter|ErrInvalidProposalID|ErrInvalidPageRequest|ErrInvalidOption|ErrInvalidMetadata|ErrInvalidWeightedVoteOptions|ErrInvalidWeightedVoteOption|ErrInvalidWeightedVoteOptionType|ErrInvalidWeightedVoteOptionWeight|ErrInvalidDepositor" --type go

# Check the transaction handlers implementation
echo "Checking transaction handlers implementation..."
ast-grep --pattern 'func ($x *Precompile) $method($$$) ($$$) {
  $$$
}'  precompiles/gov/tx.go

#!/bin/bash
# Description: Verify that the method constants for Deposit and CancelProposal are defined

# Search for the constant definitions
rg -A 1 'const \(.*DepositMethod.*=.*\)' 
rg -A 1 'const \(.*CancelProposalMethod.*=.*\)'

#!/bin/bash
# Let's try a broader search for these method constants
# First, search for any mentions of these methods
rg "DepositMethod|CancelProposalMethod" -B 2 -A 2

# Also search for const blocks in the same file
rg "const \(" -B 1 -A 10 precompiles/gov/gov.go

#!/bin/bash
# Search for other Unix timestamp conversions in governance code
rg -A 2 "Unix\(\)" --type go

isOriginDepositor := contract.CallerAddress == origin
isContractDepositor := contract.CallerAddress == depositorAddr && !isOriginDepositor
if !isContractDepositor && origin != depositorAddr {
    return nil, fmt.Errorf(ErrDifferentOriginDepositor, origin.String(), depositorAddr.String())
}

#!/bin/bash
# Search for authorization checks in the implementation
ast-grep --pattern 'if ($proposer == $_) { $$$
}'

# Search for proposal state validation
ast-grep --pattern 'if ($proposal.status == $_) { $$$
}'

#!/bin/bash
# Let's search for the implementation file first
fd "gov" --type f --extension go --extension sol

# Then search for cancelProposal implementation
rg -l "cancelProposal"

# Search for any mentions of proposal cancellation logic
rg -i "cancel.*proposal"

#!/bin/bash
# Let's examine the implementation in tx.go
rg -A 20 "func \(p Precompile\) CancelProposal" precompiles/gov/tx.go

# Also check the message construction
rg -A 10 "func NewMsgCancelProposal" precompiles/gov/types.go

#!/bin/bash
# Description: Verify the implementation of Deposit and CancelProposal transactions
# in the governance precompile

# Check for the transaction definitions
rg -A 5 "Deposit.*CancelProposal" precompiles/gov

# Check for any test files
fd -g "*_test.go" | xargs rg "TestDeposit|TestCancelProposal"

#!/bin/bash
# Let's verify the implementation details of these methods

# Check the method definitions and implementations
ast-grep --pattern 'func (p *Precompile) Deposit($$$) error {
  $$$
}'

ast-grep --pattern 'func (p *Precompile) CancelProposal($$$) error {
  $$$
}'

# Check the method declarations in the interface
rg -A 2 "DepositMethod.*=|CancelProposalMethod.*=" precompiles/gov

# Check for event definitions
rg "Deposit.*Event|CancelProposal.*Event" precompiles/gov

#!/bin/bash
# Let's check the actual implementation of these transactions

# Check the transaction implementations
rg -B 2 -A 10 "^func.*Deposit\(" precompiles/gov/tx.go
rg -B 2 -A 10 "^func.*CancelProposal\(" precompiles/gov/tx.go

# Check the test implementations to understand the expected behavior
rg -B 2 -A 10 "^func.*TestDeposit\(" precompiles/gov/tx_test.go
rg -B 2 -A 10 "^func.*TestCancelProposal\(" precompiles/gov/tx_test.go

precompiles/gov/events_test.go (3)

155-156: Consider using idiomatic struct initialization

The struct initialization can be more concise using field names without repeating the type.

-				gov.WeightedVoteOption{Option: 1, Weight: "0.70"},
-				gov.WeightedVoteOption{Option: 2, Weight: "0.30"},
+				{Option: 1, Weight: "0.70"},
+				{Option: 2, Weight: "0.30"},

---

172-244: Consider adding error test cases

The test only covers the success path. Consider adding test cases for:

• Invalid proposal ID
• Insufficient deposit amount
• Invalid deposit denomination

---

246-313: Consider adding error test cases

The test only covers the success path. Consider adding test cases for:

• Invalid proposal ID
• Non-proposer attempting to cancel
• Already canceled proposal

Configuration used: CodeRabbit UI
Review profile: CHILL

-				s.Require().Equal(uint64(ctx.BlockHeight()), log.BlockNumber) //nolint:gosec // G115
+				s.Require().Equal(log.BlockNumber, uint64(ctx.BlockHeight())) //nolint:gosec // G115

-				s.Require().Equal(log.BlockNumber, uint64(ctx.BlockHeight())) //nolint:gosec // G115
+				s.Require().Equal(uint64(ctx.BlockHeight()), log.BlockNumber) //nolint:gosec // G115

precompiles/common/errors.go (1)

31-32: Consider enhancing the error message with more context.

The error message is quite generic and doesn't follow the pattern of other validation errors in this file that provide more detailed context. Consider making it more specific and adding format specifiers to include validation failure details.

-	// ErrInvalidMethodInputs is raised when the method does not have the expected inputs defined.
-	ErrInvalidMethodInputs = "invalid method inputs"
+	// ErrInvalidMethodInputs is raised when the method inputs fail validation.
+	ErrInvalidMethodInputs = "invalid method inputs: %v"

precompiles/vesting/types.go (2)

105-107: Good addition of input validation, but consider making the check more precise

The validation check prevents potential panic when accessing method.Inputs[3] and [4] later in the function. However, since we know exactly how many inputs we need, we could make the check more precise.

Consider updating the check to explicitly validate for exactly 5 inputs:

-if len(method.Inputs) <= 4 {
+if len(method.Inputs) != 5 {
   return nil, common.Address{}, common.Address{}, nil, nil, fmt.Errorf(cmn.ErrInvalidMethodInputs)
}

This would better align with the function's requirements, as it uses indices 3 and 4 specifically for lockup and vesting periods.

---

105-107: Consider enhancing the error message

The generic ErrInvalidMethodInputs could be more helpful if it specified the expected number of inputs.

Consider using a more descriptive error message:

-return nil, common.Address{}, common.Address{}, nil, nil, fmt.Errorf(cmn.ErrInvalidMethodInputs)
+return nil, common.Address{}, common.Address{}, nil, nil, fmt.Errorf("invalid number of method inputs: expected 5, got %d", len(method.Inputs))

precompiles/gov/types.go (2)

Line range hint 408-426: Consider adding error context in FromResponse methods

The FromResponse methods for DepositOutput and DepositsOutput silently return nil on Bech32 address conversion errors. Consider adding error context to help with debugging.

 func (do *DepositOutput) FromResponse(res *v1.QueryDepositResponse) *DepositOutput {
     hexDepositor, err := utils.Bech32ToHexAddr(res.Deposit.Depositor)
     if err != nil {
-        return nil
+        return fmt.Errorf("failed to convert depositor address: %w", err)
     }

Also applies to: 428-456

---

Line range hint 548-589: Consider implementing pagination limit

The FromResponse methods for ProposalOutput and ProposalsOutput handle large datasets. Consider implementing pagination limits to prevent potential memory issues with large result sets.

Also applies to: 590-642

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check how the new error constant is used and compare with similar validation patterns

# Check where the new error is used
echo "Checking usage of ErrInvalidMethodInputs:"
rg "ErrInvalidMethodInputs" -B 2 -A 2

# Check similar validation patterns in governance precompile
echo -e "\nChecking similar validation patterns in governance code:"
rg "invalid.*method|method.*invalid" --type go -g "precompiles/gov/**"

#!/bin/bash
# Check error patterns in precompiles
echo "Checking error patterns in precompiles:"
rg "ErrInvalid|invalid" --type go -g "precompiles/**/*.go" -B 1 -A 1

# Check error string formats
echo -e "\nChecking error string formats:"
rg '= ".*invalid.*"' --type go -g "precompiles/**/*.go"

# Check error handling patterns in input validation
echo -e "\nChecking input validation patterns:"
ast-grep --pattern 'if len($_.Inputs) $_ {
  return $$$, fmt.Errorf($_)
}'

-	if len(method.Inputs) <= 1 {
-		return common.Address{}, nil, fmt.Errorf(cmn.ErrInvalidMethodInputs)
-	}
+	// Option 1: Match expected args length
+	if len(method.Inputs) != 2 {
+		return common.Address{}, nil, fmt.Errorf("invalid number of method inputs: expected 2, got %d", len(method.Inputs))
+	}
+
+	// Option 2: Remove redundant check if it's not necessary

#!/bin/bash
# Search for the ABI definition or interface that defines this authorization method
rg -A 5 'transfer.*authorization.*\(.*common\.Address.*ICS20Allocation'

#!/bin/bash
# Let's try to find the method definition and its usage in a broader search
rg -A 5 'checkTransferAuthzArgs'

# Also search for any transfer authorization related code
rg -A 5 'TransferAuthorization'

# Look for the actual method definition in the file
cat precompiles/ics20/types.go

-	if len(method.Inputs) <= 6 {
-		return nil, common.Address{}, fmt.Errorf(cmn.ErrInvalidMethodInputs)
-	}
+	// Option 1: Match expected args length
+	if len(method.Inputs) != 9 {
+		return nil, common.Address{}, fmt.Errorf("invalid number of method inputs: expected 9, got %d", len(method.Inputs))
+	}
+
+	// Option 2: Remove redundant check if it's not necessary

precompiles/gov/tx.go (2)

133-142: Consider documenting the state consistency logic

The state consistency handling for non-origin depositors is crucial but complex. Consider adding a more detailed comment explaining why this scaling is necessary and what could happen if it's not done correctly.

 if !isOriginDepositor {
-    // NOTE: This ensures that the changes in the bank keeper are correctly mirrored to the EVM stateDB
-    // when calling the precompile from a smart contract
-    // This prevents the stateDB from overwriting the changed balance in the bank keeper when committing the EVM state.
-    // Need to scale the amount to 18 decimals for the EVM balance change entry
+    // IMPORTANT: State Consistency Handling
+    // 1. When called from a smart contract, we need to mirror bank keeper changes in the EVM stateDB
+    // 2. This prevents the stateDB from overwriting the bank keeper's balance changes during EVM state commit
+    // 3. The amount must be scaled to 18 decimals to match EVM's decimal precision
+    // 4. Without this scaling, there would be precision loss in the EVM state

---

97-182: Consider extracting common authorization patterns

The authorization logic for checking contract vs origin addresses is repeated across all methods (Vote, VoteWeighted, Deposit, CancelProposal). Consider extracting this into a helper function to improve maintainability and reduce duplication.

Example helper function:

func (p Precompile) validateOrigin(
    contract *vm.Contract,
    origin, expectedAddr common.Address,
    errMsg string,
) error {
    isContractCaller := contract.CallerAddress == expectedAddr && contract.CallerAddress != origin
    if !isContractCaller && origin != expectedAddr {
        return fmt.Errorf(errMsg, origin.String(), expectedAddr.String())
    }
    return nil
}

🧰 Tools

🪛 GitHub Check: Run golangci-lint

[failure] 113-113:
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)

precompiles/gov/tx_test.go (1)

370-371: Improve clarity of initial deposit value

The magic number 100 should be defined as a constant with a descriptive name to improve readability and maintainability.

+const initialDeposit = 100
 // 100 is the initial deposit
-s.Require().Equal(math.NewInt(1e18).AddRaw(100), deposits[0].Amount[0].Amount.BigInt())
+s.Require().Equal(math.NewInt(1e18).AddRaw(initialDeposit), deposits[0].Amount[0].Amount.BigInt())

precompiles/gov/types.go (2)

112-158: LGTM: Well-structured deposit message creation with comprehensive validation

The implementation includes thorough validation of all input parameters and proper conversion of coin amounts.

Consider improving error messages for better debugging:

-		return nil, common.Address{}, fmt.Errorf(ErrInvalidDeposit)
+		return nil, common.Address{}, fmt.Errorf("%s: empty deposit amount", ErrInvalidDeposit)

-			return nil, common.Address{}, fmt.Errorf(ErrInvalidDeposit)
+			return nil, common.Address{}, fmt.Errorf("%s: non-positive amount %v", ErrInvalidDeposit, c.Amount)

🧰 Tools

🪛 GitHub Check: Run golangci-lint

[failure] 138-138:
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)

---

[failure] 144-144:
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)

---

160-182: Add validation for proposal ID

The implementation looks good but could benefit from additional validation.

Consider adding validation for non-zero proposal ID:

 	proposalID, ok := args[1].(uint64)
 	if !ok {
 		return nil, common.Address{}, fmt.Errorf(ErrInvalidProposalID, args[1])
 	}
+	if proposalID == 0 {
+		return nil, common.Address{}, fmt.Errorf("%s: proposal ID cannot be zero", ErrInvalidProposalID)
+	}

precompiles/gov/integration_test.go (1)

259-325: Well-structured test cases for Deposit!

The test cases effectively cover both success and error scenarios, with proper verification of deposit amounts. Good use of the initialDeposit constant for maintainability.

Consider adding a test case for depositing after the deposit end time:

It("should fail when depositing after deposit end time", func() {
    // Advance past the deposit end time
    proposal, _ := s.network.App.GovKeeper.Proposals.Get(s.network.GetContext(), proposalID)
    ctx := s.network.GetContext()
    ctx = ctx.WithBlockTime(proposal.DepositEndTime.Add(time.Hour))
    s.network.App.GovKeeper.Proposals.Set(ctx, proposalID, proposal)

    depositAmount := big.NewInt(1e18)
    coins := []cmn.Coin{{Denom: evmtypes.GetEVMCoinDenom(), Amount: depositAmount}}
    callArgs.Args = []interface{}{
        s.keyring.GetAddr(0),
        proposalID,
        coins,
    }

    depositCheck := defaultLogCheck.WithErrContains("deposit period ended")

    _, _, err := s.factory.CallContractAndCheckLogs(s.keyring.GetPrivKey(0), txArgs, callArgs, depositCheck)
    Expect(err).To(BeNil())
})

Configuration used: CodeRabbit UI
Review profile: CHILL

 func() {
+    // Verify proposal exists before cancellation
+    proposal, err := s.network.App.GovKeeper.Proposals.Get(ctx, proposalID)
+    s.Require().NoError(err)
+    s.Require().NotNil(proposal)
+
+    // Verify proposal is cancelled
     found, err := s.network.App.GovKeeper.Proposals.Has(s.network.GetContext(), proposalID)
     s.Require().NoError(err)
     s.Require().False(found)
 }

 	weightedOptions := make([]*v1.WeightedVoteOption, len(options))
 	for i, option := range options {
+		if _, ok := v1.VoteOption_name[int32(option.Option)]; !ok {
+			return nil, common.Address{}, WeightedVoteOptions{}, fmt.Errorf("invalid VoteOption: %d", option.Option)
+		}
 		weightedOptions[i] = &v1.WeightedVoteOption{
 			Option: v1.VoteOption(option.Option),
 			Weight: option.Weight,
 		}
 	}

precompiles/gov/tx.go (1)

155-171: Consider additional validation for proposal cancellation

While the basic validation is in place, consider adding checks for:

1. Proposal existence
2. Proposal status (ensure it can be cancelled)
3. Cancellation timeframe validation (if there are time-based restrictions)

 msg, proposerAddr, err := NewMsgCancelProposal(args)
 if err != nil {
     return nil, err
 }
+
+// Add proposal existence check
+proposal, err := p.govKeeper.GetProposal(ctx, msg.ProposalId)
+if err != nil {
+    return nil, fmt.Errorf("proposal %d not found", msg.ProposalId)
+}
+
+// Add status check
+if !proposal.Status.IsCancellable() {
+    return nil, fmt.Errorf("proposal %d cannot be cancelled in status %s", msg.ProposalId, proposal.Status)
+}

precompiles/gov/tx_test.go (1)

288-377: Consider adding more test cases

The test suite could benefit from additional edge cases:

1. Multiple deposits from the same depositor
2. Deposit with multiple coins
3. Deposit with zero amount

Would you like me to provide implementations for these additional test cases?

precompiles/gov/types.go (2)

137-139: Improve error message specificity

The error message should provide more context about why the deposit is invalid.

 if len(amount) == 0 {
-    return nil, common.Address{}, fmt.Errorf(ErrInvalidDeposit)
+    return nil, common.Address{}, fmt.Errorf("%s: empty amount", ErrInvalidDeposit)
 }

🧰 Tools

🪛 GitHub Check: Run golangci-lint

[failure] 138-138:
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)

---

143-145: Improve error message for invalid deposit amount

The error message should specify which coin has an invalid amount.

 if c.Amount.Sign() <= 0 {
-    return nil, common.Address{}, fmt.Errorf(ErrInvalidDeposit)
+    return nil, common.Address{}, fmt.Errorf("%s: non-positive amount for coin %s", ErrInvalidDeposit, c.Denom)
 }

🧰 Tools

🪛 GitHub Check: Run golangci-lint

[failure] 144-144:
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)

precompiles/gov/integration_test.go (1)

276-276: Consider using a more specific error message.

The error message "failed ETH tx" is too generic. Consider checking for a more specific error message that indicates the proposal doesn't exist.

-			depositCheck := defaultLogCheck.WithErrContains("failed ETH tx")
+			depositCheck := defaultLogCheck.WithErrContains("not found: key '999'")

Configuration used: CodeRabbit UI
Review profile: CHILL

return method.Outputs.Pack(
    true,
    uint64(resp.CanceledTime.Unix()), //nolint:gosec // G115
    resp.CanceledHeight,
)

#!/bin/bash
# Search for timestamp handling in tests
rg -A 5 "CanceledTime|Unix" 

#!/bin/bash
# Search for tests covering the balance scaling logic
rg -A 5 "ConvertAmountTo18DecimalsBigInt|SetBalanceChangeEntries" 

#!/bin/bash
# Search for test files that might cover the balance scaling logic
rg -t go -g "*_test.go" "ConvertAmountTo18DecimalsBigInt|SetBalanceChangeEntries" -A 5

# Also check the implementation of GetEVMCoinDecimals which is crucial for the conversion
ast-grep --pattern 'func GetEVMCoinDecimals() $_'

#!/bin/bash
# Check the Decimals type and conversion factor implementation
rg -A 10 "type Decimals struct" x/evm/types/

# Look for test files covering decimal conversions
rg -g "*_test.go" "TestConvert.*Decimals|TestGetEVMCoin" -A 10

#!/bin/bash
# Get the full content of the scaling tests
cat x/evm/types/scaling_test.go

# Check for any integration tests that might cover this functionality
rg -g "*_test.go" "TestIntegration.*Balance|TestPrecompile.*Balance" -A 10

 weightedOptions := make([]*v1.WeightedVoteOption, len(options))
 for i, option := range options {
+    if _, ok := v1.VoteOption_name[int32(option.Option)]; !ok {
+        return nil, common.Address{}, WeightedVoteOptions{}, fmt.Errorf("invalid vote option: %d", option.Option)
+    }
     weightedOptions[i] = &v1.WeightedVoteOption{
         Option: v1.VoteOption(option.Option),
         Weight: option.Weight,
     }
 }

precompiles/gov/tx_test.go (1)

372-373: Improve clarity of deposit amount verification

The comment about initial deposit (100) needs better documentation and the verification could be more explicit.

-				// 100 is the initial deposit
-				s.Require().Equal(math.NewInt(1e18).AddRaw(100).BigInt(), deposits[0].Amount[0].Amount.BigInt())
+				// Verify total deposit amount (initial proposal deposit + new deposit)
+				initialDeposit := int64(100) // Initial deposit required for proposal creation
+				newDeposit := math.NewInt(1e18)
+				expectedAmount := newDeposit.AddRaw(initialDeposit)
+				s.Require().Equal(expectedAmount.BigInt(), deposits[0].Amount[0].Amount.BigInt())

Configuration used: CodeRabbit UI
Review profile: CHILL

 			func() {
+				// Verify proposal exists before cancellation
+				found, err := s.network.App.GovKeeper.Proposals.Has(ctx, proposalID)
+				s.Require().NoError(err)
+				s.Require().True(found)
+
+				// Verify proposal is cancelled
 				found, err := s.network.App.GovKeeper.Proposals.Has(s.network.GetContext(), proposalID)
 				s.Require().NoError(err)
 				s.Require().False(found)
 			},

precompiles/gov/tx.go (3)

107-115: Enhance deposit validation

While basic validation exists, consider adding more comprehensive checks:

1. Validate that the amount is positive
2. Verify the denomination is correct for your chain
3. Add maximum deposit amount validation if applicable

 if len(msg.Amount) != 1 {
   return nil, errors.New(ErrInvalidDeposit)
 }
+if msg.Amount[0].IsZero() {
+    return nil, errors.New("deposit amount must be positive")
+}
+if msg.Amount[0].Denom != ExpectedDenom {
+    return nil, fmt.Errorf("invalid denomination: expected %s, got %s", ExpectedDenom, msg.Amount[0].Denom)
+}

---

134-143: Improve documentation of EVM state synchronization

The current comment could be more explicit about the technical details of the state synchronization process.

Consider expanding the comment to:

-// NOTE: This ensures that the changes in the bank keeper are correctly mirrored to the EVM stateDB
-// when calling the precompile from a smart contract
-// This prevents the stateDB from overwriting the changed balance in the bank keeper when committing the EVM state.
-// Need to scale the amount to 18 decimals for the EVM balance change entry
+// NOTE: When a precompile is called from a smart contract, we need to ensure that
+// balance changes made by the bank keeper are correctly reflected in the EVM state:
+// 1. The bank keeper modifies the actual balance
+// 2. We record a corresponding change in the EVM stateDB
+// 3. This prevents the final EVM state commit from overwriting the bank keeper's changes
+// 4. The amount must be scaled to 18 decimals to match EVM precision

---

178-182: Document timestamp handling

The Unix timestamp conversion needs clearer documentation about timezone handling and potential limitations.

 return method.Outputs.Pack(
     true,
-    uint64(resp.CanceledTime.Unix()), //nolint:gosec // G115
+    uint64(resp.CanceledTime.Unix()), // Convert to Unix timestamp (UTC) for EVM compatibility
     resp.CanceledHeight,
 )

precompiles/gov/types.go (1)

138-146: Enhance deposit amount validation

While the function validates that the amount is not empty and positive, consider adding these additional checks:

• Validate maximum deposit amount
• Ensure denomination is allowed

 if len(amount) == 0 {
   return nil, common.Address{}, errors.New(ErrInvalidDeposit)
 }

+// Define allowed denominations
+var allowedDenoms = map[string]bool{"stake": true, "evmos": true}
+
 coins := make([]sdk.Coin, len(amount))
 for i, c := range amount {
   if c.Amount.Sign() <= 0 {
     return nil, common.Address{}, errors.New(ErrInvalidDeposit)
   }
+  if !allowedDenoms[c.Denom] {
+    return nil, common.Address{}, fmt.Errorf("invalid denomination: %s", c.Denom)
+  }
+  // Add maximum deposit check if applicable
+  if c.Amount.BitLen() > 256 {
+    return nil, common.Address{}, fmt.Errorf("deposit amount too large")
+  }
   coins[i] = sdk.Coin{

precompiles/gov/integration_test.go (4)

225-225: Consider using a more specific error message check.

The error message "not found: key '999'" is implementation-specific. Consider using a more domain-specific error message that better describes the business logic failure.

---

213-257: Add more test cases for comprehensive coverage.

While the current test cases cover the basic scenarios, consider adding tests for:

1. Attempting to cancel an already executed proposal
2. Attempting to cancel a proposal after voting period ends
3. Attempting to cancel a proposal with insufficient gas

---

276-276: Consider using a more specific error message check.

Similar to the cancel proposal tests, the error message "not found: key '999'" is implementation-specific. Consider using a more domain-specific error message.

---

259-325: Add test cases for additional edge cases.

Consider adding tests for:

1. Depositing after the deposit period ends
2. Depositing with invalid denomination
3. Depositing with zero amount
4. Multiple deposits from the same depositor

Configuration used: CodeRabbit UI
Review profile: CHILL

precompiles/gov/types.go (2)

129-131: Improve error message for method inputs validation

The error message should be more specific about the expected number of inputs.

-		return nil, common.Address{}, fmt.Errorf(cmn.ErrInvalidMethodInputs)
+		return nil, common.Address{}, fmt.Errorf("%s: expected > 2 inputs, got %d", cmn.ErrInvalidMethodInputs, len(method.Inputs))

---

138-146: Enhance deposit amount validation

The current validation could be more specific about the requirements for valid deposits.

 	if len(amount) == 0 {
-		return nil, common.Address{}, errors.New(ErrInvalidDeposit)
+		return nil, common.Address{}, fmt.Errorf("%s: deposit amount cannot be empty", ErrInvalidDeposit)
 	}
 
 	coins := make([]sdk.Coin, len(amount))
 	for i, c := range amount {
 		if c.Amount.Sign() <= 0 {
-			return nil, common.Address{}, errors.New(ErrInvalidDeposit)
+			return nil, common.Address{}, fmt.Errorf("%s: deposit amount must be positive for coin %s", ErrInvalidDeposit, c.Denom)
 		}

Configuration used: CodeRabbit UI
Review profile: CHILL

 	proposer, ok := args[0].(common.Address)
 	if !ok || proposer == (common.Address{}) {
-		return nil, common.Address{}, errors.New(ErrInvalidProposer)
+		return nil, common.Address{}, fmt.Errorf(ErrInvalidProposer, proposer)
 	}

 	proposalID, ok := args[1].(uint64)
 	if !ok {
 		return nil, common.Address{}, fmt.Errorf(ErrInvalidProposalID, args[1])
 	}
+	if proposalID == 0 {
+		return nil, common.Address{}, fmt.Errorf("%s: proposal ID cannot be zero", ErrInvalidProposalID)
+	}

#!/bin/bash
# Description: Check for authorization checks in the precompile contract

# Search for authorization checks in the precompile implementation
rg -A 5 "RequireNotPayable|RequireOnlyFromEOA|RequireAuth" precompiles/gov/

#!/bin/bash
# Let's check the precompile contract implementation for these methods
ast-grep --pattern 'func (p $_) Deposit($$$) error {
  $$$
}'

# Also check for CancelProposal
ast-grep --pattern 'func (p $_) CancelProposal($$$) error {
  $$$
}'

# Check for any authorization-related functions or methods
rg -B 2 -A 5 "authorize|auth|require" precompiles/gov/

#!/bin/bash
# Let's check the actual implementation of these methods in the contract
rg -B 2 -A 10 "func \(p \*Precompile\) (Deposit|CancelProposal)" precompiles/gov/

# Also check for any transaction validation or authorization in the base precompile
rg -B 2 -A 10 "func \(p \*Precompile\) (CheckTransaction|ValidateBasic)" precompiles/gov/

# Check the common precompile package for authorization mechanisms
rg -B 2 -A 10 "func.*Require(Auth|NotPayable|OnlyFromEOA)" precompiles/common/

#!/bin/bash
# Let's check the transaction methods in the contract file
rg -B 2 -A 10 "func.*Deposit|func.*CancelProposal" precompiles/gov/tx.go

# Check the base contract implementation
cat precompiles/gov/gov.go

# Check if there are any other transaction methods that might show the pattern
rg -l "MsgServer" precompiles/gov/

#!/bin/bash
# Let's check the common precompile implementation for authorization checks
cat precompiles/common/precompile.go

# Check if there are any authorization checks in the base setup
rg -B 2 -A 10 "RunSetup" precompiles/common/

# Check for any authorization checks in the transaction execution
rg -B 2 -A 10 "HandleGasError|AddJournalEntries" precompiles/common/

CHANGELOG.md (1)

53-54: Consider breaking long line into multiple lines for better readability.

The changelog entry exceeds the recommended line length. Consider reformatting it as:

-- (precompiles) [#3004](https://github.com/evmos/evmos/pull/3004) Add `Deposit` and `CancelProposal` tx to gov precompile.
++ (precompiles) [#3004](https://github.com/evmos/evmos/pull/3004) Add `Deposit` and 
++   `CancelProposal` tx to gov precompile.

🧰 Tools

🪛 Markdownlint (0.35.0)

54-54: Expected: 120; Actual: 137
Line length

(MD013, line-length)

Configuration used: CodeRabbit UI
Review profile: CHILL

precompiles/gov/tx.go (3)

112-115: Enhance deposit amount validation

While the length check is good, consider adding more comprehensive validation:

• Verify amount is positive
• Validate denomination if specific tokens are required

 if len(msg.Amount) != 1 {
     return nil, errors.New(ErrInvalidDeposit)
 }
+if msg.Amount[0].IsZero() {
+    return nil, errors.New("deposit amount must be positive")
+}

---

134-143: Consider documenting the scaling factor

The balance scaling logic is correct, but it would be helpful to document:

1. Why 18 decimals is used as the target
2. What happens if the original token has more than 18 decimals

---

178-182: Add comment explaining the return values

The method returns multiple values (success, canceledTime, canceledHeight) but their purpose isn't immediately clear. Consider adding a comment explaining what these values represent and how they should be used.

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Check if there are any time zone conversions or assumptions in the codebase
rg -A 2 "Unix\(\)"