fix: audit (#80)

* fix cli & error handling * add error context * remove print * cleanup * fix to return proper error code * fix to allow empty from address at gas estimate
initia-labs · Oct 23, 2024 · c76e767 · c76e767
1 parent 4cb7901
commit c76e767
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 44 deletions.
diff --git a/jsonrpc/backend/gas.go b/jsonrpc/backend/gas.go
@@ -21,7 +21,7 @@ func (b *JSONRPCBackend) EstimateGas(args rpctypes.TransactionArgs, blockNrOrHas
 		return hexutil.Uint64(0), errors.New("state overrides are not supported")
 	}
 
-	if args.Nonce == nil {
+	if args.Nonce == nil && args.From != nil {
 		nonce, err := b.GetTransactionCount(*args.From, rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber))
 		if err != nil {
 			return hexutil.Uint64(0), err

diff --git a/jsonrpc/types/transaction_args.go b/jsonrpc/types/transaction_args.go
@@ -50,6 +50,9 @@ func (args *TransactionArgs) CallDefaults() {
 	if args.Value == nil {
 		args.Value = new(hexutil.Big)
 	}
+	if args.From == nil {
+		args.From = new(common.Address)
+	}
 }
 
 // String returns the struct in a string format.

diff --git a/x/evm/client/cli/tx.go b/x/evm/client/cli/tx.go
@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"encoding/hex"
 	"fmt"
 	"math/big"
 	"os"
@@ -64,32 +65,14 @@ $ %s tx evm create ERC20.bin --input 0x1234 --value 100 --from mykey
 				return err
 			}
 
-			contractBz, err := os.ReadFile(args[0])
+			codeBz, val, err := prepareContractCreation(cmd, args[0])
 			if err != nil {
-				return errors.Wrap(err, "failed to read contract file")
-			}
-
-			input, err := cmd.Flags().GetString(FlagInput)
-			if err != nil {
-				return errors.Wrap(err, "failed to get input")
-			}
-			inputBz, err := hexutil.Decode(input)
-			if err != nil {
-				return errors.Wrap(err, "failed to decode input")
-			}
-
-			value, err := cmd.Flags().GetString(FlagValue)
-			if err != nil {
-				return errors.Wrap(err, "failed to get value")
-			}
-			val, ok := new(big.Int).SetString(value, 10)
-			if !ok {
-				return fmt.Errorf("invalid value: %s", value)
+				return err
 			}
 
 			msg := &types.MsgCreate{
 				Sender: sender,
-				Code:   hexutil.Encode(append(contractBz, inputBz...)),
+				Code:   hexutil.Encode(codeBz),
 				Value:  math.NewIntFromBigInt(val),
 			}
 
@@ -105,7 +88,7 @@ $ %s tx evm create ERC20.bin --input 0x1234 --value 100 --from mykey
 
 func Create2Cmd(ac address.Codec) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "create2 [salt] [bin file]  --input [input-hex-string] --value [value]",
+		Use:   "create2 [salt] [bin file] --input [input-hex-string] --value [value]",
 		Short: "Deploy evm contracts with CREATE2 opcode",
 		Long: strings.TrimSpace(
 			fmt.Sprintf(`
@@ -133,33 +116,16 @@ $ %s tx evm create2 100 ERC20.bin --input 0x1234 --value 100 --from mykey
 			if err != nil {
 				return errors.Wrap(err, "failed to parse salt")
 			}
-			contractBz, err := os.ReadFile(args[1])
-			if err != nil {
-				return errors.Wrap(err, "failed to read contract file")
-			}
 
-			input, err := cmd.Flags().GetString(FlagInput)
-			if err != nil {
-				return errors.Wrap(err, "failed to get input")
-			}
-			inputBz, err := hexutil.Decode(input)
-			if err != nil {
-				return errors.Wrap(err, "failed to decode input")
-			}
-
-			value, err := cmd.Flags().GetString(FlagValue)
+			codeBz, val, err := prepareContractCreation(cmd, args[1])
 			if err != nil {
-				return errors.Wrap(err, "failed to get value")
-			}
-			val, ok := new(big.Int).SetString(value, 10)
-			if !ok {
-				return fmt.Errorf("invalid value: %s", value)
+				return err
 			}
 
 			msg := &types.MsgCreate2{
 				Sender: sender,
 				Salt:   salt,
-				Code:   hexutil.Encode(append(contractBz, inputBz...)),
+				Code:   hexutil.Encode(codeBz),
 				Value:  math.NewIntFromBigInt(val),
 			}
 
@@ -233,3 +199,44 @@ $ %s tx evm call 0x1 0x123456 --from mykey
 	cmd.Flags().String(FlagValue, "0", "value")
 	return cmd
 }
+
+func readContractBinFile(binFile string) ([]byte, error) {
+	contractBz, err := os.ReadFile(binFile)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to read contract file")
+	}
+
+	contractBz, err = hex.DecodeString(string(contractBz))
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to read contract file: expect hex string")
+	}
+
+	return contractBz, nil
+}
+
+func prepareContractCreation(cmd *cobra.Command, contractFile string) ([]byte, *big.Int, error) {
+	contractBz, err := readContractBinFile(contractFile)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	input, err := cmd.Flags().GetString(FlagInput)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to get input")
+	}
+	inputBz, err := hexutil.Decode(input)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to decode input")
+	}
+
+	value, err := cmd.Flags().GetString(FlagValue)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to get value")
+	}
+	val, ok := new(big.Int).SetString(value, 10)
+	if !ok {
+		return nil, nil, fmt.Errorf("invalid value: %s", value)
+	}
+
+	return append(contractBz, inputBz...), val, nil
+}
diff --git a/x/evm/keeper/context.go b/x/evm/keeper/context.go
@@ -261,6 +261,11 @@ func (k Keeper) EVMCallWithTracer(ctx context.Context, caller common.Address, co
 		value,
 	)
 
+	// evm sometimes return 0 gasRemaining, but it's not an out of gas error.
+	if gasRemaining == 0 && err != nil && err != vm.ErrOutOfGas {
+		return nil, nil, types.ErrEVMCallFailed.Wrap(err.Error())
+	}
+
 	// London enforced
 	gasUsed := types.CalGasUsed(gasBalance, gasRemaining, evm.StateDB.GetRefund())
 	sdkCtx.GasMeter().ConsumeGas(gasUsed, "EVM gas consumption")
@@ -358,6 +363,11 @@ func (k Keeper) EVMCreateWithTracer(ctx context.Context, caller common.Address,
 		)
 	}
 
+	// evm sometimes return 0 gasRemaining, but it's not an out of gas error.
+	if gasRemaining == 0 && err != nil && err != vm.ErrOutOfGas {
+		return nil, common.Address{}, nil, types.ErrEVMCreateFailed.Wrap(err.Error())
+	}
+
 	// London enforced
 	gasUsed := types.CalGasUsed(gasBalance, gasRemaining, evm.StateDB.GetRefund())
 	sdkCtx.GasMeter().ConsumeGas(gasUsed, "EVM gas consumption")

diff --git a/x/evm/precompiles/cosmos/contract.go b/x/evm/precompiles/cosmos/contract.go
@@ -139,7 +139,7 @@ func (e CosmosPrecompile) ExtendedRun(caller vm.ContractRef, input []byte, suppl
 			return nil, ctx.GasMeter().GasConsumedToLimit(), types.ErrPrecompileFailed.Wrap(err.Error())
 		}
 	case METHOD_IS_MODULE_ADDRESS:
-		ctx.GasMeter().ConsumeGas(IS_MODULE_ADDRESS_GAS, "is_blocked_address")
+		ctx.GasMeter().ConsumeGas(IS_MODULE_ADDRESS_GAS, "is_module_address")
 
 		var isModuleAddressArguments IsModuleAddressArguments
 		if err := method.Inputs.Copy(&isModuleAddressArguments, args); err != nil {