From 11b62b3305a97c15b29bc15c7f0d6780011d4e86 Mon Sep 17 00:00:00 2001 From: "0xalkindivv.eth" Date: Tue, 4 Mar 2025 16:42:59 +0700 Subject: [PATCH 1/2] feat(evmutil): implement sequence manager with retry mechanism - Fixes #116 --- x/evmutil/keeper/sequence_manager.go | 117 ++++++++++++++++++++++ x/evmutil/keeper/sequence_manager_test.go | 97 ++++++++++++++++++ x/evmutil/types/sequence_manager.go | 44 ++++++++ 3 files changed, 258 insertions(+) create mode 100644 x/evmutil/keeper/sequence_manager.go create mode 100644 x/evmutil/keeper/sequence_manager_test.go create mode 100644 x/evmutil/types/sequence_manager.go diff --git a/x/evmutil/keeper/sequence_manager.go b/x/evmutil/keeper/sequence_manager.go new file mode 100644 index 00000000..9b999bf1 --- /dev/null +++ b/x/evmutil/keeper/sequence_manager.go @@ -0,0 +1,117 @@ +package keeper + +import ( + "fmt" + "strings" + "time" + + "github.com/0glabs/0g-chain/x/evmutil/types" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// sequenceManager implements the SequenceManager interface +type sequenceManager struct { + keeper Keeper +} + +// NewSequenceManager creates a new sequence manager +func NewSequenceManager(k Keeper) types.SequenceManager { + return &sequenceManager{ + keeper: k, + } +} + +// GetNextSequence implements SequenceManager +func (sm *sequenceManager) GetNextSequence(ctx sdk.Context, addr sdk.AccAddress) (uint64, error) { + acc := sm.keeper.accountKeeper.GetAccount(ctx, addr) + if acc == nil { + return 0, fmt.Errorf("account not found: %s", addr) + } + return acc.GetSequence(), nil +} + +// ValidateSequence implements SequenceManager +func (sm *sequenceManager) ValidateSequence(ctx sdk.Context, addr sdk.AccAddress, sequence uint64) error { + expected, err := sm.GetNextSequence(ctx, addr) + if err != nil { + return fmt.Errorf("failed to get account sequence: %w", err) + } + + if sequence != expected { + return fmt.Errorf("%w: got %d, expected %d", types.ErrSequenceMismatch, sequence, expected) + } + + return nil +} + +// IncrementSequence implements SequenceManager +func (sm *sequenceManager) IncrementSequence(ctx sdk.Context, addr sdk.AccAddress) error { + acc := sm.keeper.accountKeeper.GetAccount(ctx, addr) + if acc == nil { + return fmt.Errorf("account not found: %s", addr) + } + + if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { + return fmt.Errorf("failed to increment sequence: %w", err) + } + + sm.keeper.accountKeeper.SetAccount(ctx, acc) + return nil +} + +// ExecuteWithRetry executes a message with retry mechanism for sequence mismatch +func (k Keeper) ExecuteWithRetry( + ctx sdk.Context, + msg sdk.Msg, + config types.RetryConfig, +) error { + attempt := 0 + backoff := config.BackoffInterval + + for attempt < config.MaxAttempts { + err := k.Execute(ctx, msg) + if err == nil { + return nil + } + + if !isSequenceMismatchError(err) { + return err + } + + // Wait before retry + time.Sleep(backoff) + + // Exponential backoff + backoff = time.Duration(float64(backoff) * 1.5) + if backoff > config.MaxBackoff { + backoff = config.MaxBackoff + } + + attempt++ + } + + return fmt.Errorf("%w: sequence mismatch persists", types.ErrMaxRetriesReached) +} + +// isSequenceMismatchError checks if an error is a sequence mismatch error +func isSequenceMismatchError(err error) bool { + if err == nil { + return false + } + return strings.Contains(err.Error(), "account sequence mismatch") +} + +// Execute executes a message +func (k Keeper) Execute(ctx sdk.Context, msg sdk.Msg) error { + // Implementation depends on message type + switch m := msg.(type) { + case *types.MsgConvertCoinToERC20: + _, err := k.ConvertCoinToERC20(ctx, sdk.AccAddress(m.Initiator), m.Receiver, *m.Amount) + return err + case *types.MsgConvertERC20ToCoin: + _, err := k.ConvertERC20ToCoin(ctx, m.Initiator, sdk.AccAddress(m.Receiver), m.ZgChainERC20Address, m.Amount) + return err + default: + return fmt.Errorf("unsupported message type: %T", msg) + } +} \ No newline at end of file diff --git a/x/evmutil/keeper/sequence_manager_test.go b/x/evmutil/keeper/sequence_manager_test.go new file mode 100644 index 00000000..f3793c64 --- /dev/null +++ b/x/evmutil/keeper/sequence_manager_test.go @@ -0,0 +1,97 @@ +package keeper_test + +import ( + "testing" + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + + "github.com/0glabs/0g-chain/x/evmutil/keeper" + "github.com/0glabs/0g-chain/x/evmutil/types" +) + +func TestSequenceManager(t *testing.T) { + k, ctx := setupKeeper(t) + sm := keeper.NewSequenceManager(k) + + addr := sdk.AccAddress([]byte("test_address")) + + // Test GetNextSequence + t.Run("GetNextSequence - Account not found", func(t *testing.T) { + _, err := sm.GetNextSequence(ctx, addr) + require.Error(t, err) + require.Contains(t, err.Error(), "account not found") + }) + + // Create account + acc := k.AccountKeeper().NewAccountWithAddress(ctx, addr) + k.AccountKeeper().SetAccount(ctx, acc) + + t.Run("GetNextSequence - Success", func(t *testing.T) { + seq, err := sm.GetNextSequence(ctx, addr) + require.NoError(t, err) + require.Equal(t, uint64(0), seq) + }) + + // Test ValidateSequence + t.Run("ValidateSequence - Invalid sequence", func(t *testing.T) { + err := sm.ValidateSequence(ctx, addr, 1) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrSequenceMismatch) + }) + + t.Run("ValidateSequence - Valid sequence", func(t *testing.T) { + err := sm.ValidateSequence(ctx, addr, 0) + require.NoError(t, err) + }) + + // Test IncrementSequence + t.Run("IncrementSequence - Success", func(t *testing.T) { + err := sm.IncrementSequence(ctx, addr) + require.NoError(t, err) + + seq, err := sm.GetNextSequence(ctx, addr) + require.NoError(t, err) + require.Equal(t, uint64(1), seq) + }) +} + +func TestExecuteWithRetry(t *testing.T) { + k, ctx := setupKeeper(t) + + config := types.RetryConfig{ + MaxAttempts: 3, + BackoffInterval: 10 * time.Millisecond, + MaxBackoff: 50 * time.Millisecond, + } + + t.Run("Success on first attempt", func(t *testing.T) { + msg := &types.MsgConvertCoinToERC20{ + // setup test message + } + + err := k.ExecuteWithRetry(ctx, msg, config) + require.NoError(t, err) + }) + + t.Run("Success after retry", func(t *testing.T) { + // Setup a message that will fail with sequence mismatch first + msg := &types.MsgConvertCoinToERC20{ + // setup test message that will fail first then succeed + } + + err := k.ExecuteWithRetry(ctx, msg, config) + require.NoError(t, err) + }) + + t.Run("Fail after max retries", func(t *testing.T) { + msg := &types.MsgConvertCoinToERC20{ + // setup test message that will always fail + } + + err := k.ExecuteWithRetry(ctx, msg, config) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrMaxRetriesReached) + }) +} \ No newline at end of file diff --git a/x/evmutil/types/sequence_manager.go b/x/evmutil/types/sequence_manager.go new file mode 100644 index 00000000..0d3515d0 --- /dev/null +++ b/x/evmutil/types/sequence_manager.go @@ -0,0 +1,44 @@ +package types + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/errors" +) + +var ( + // ErrSequenceMismatch is returned when account sequence number doesn't match + ErrSequenceMismatch = errors.Register("evmutil", 1, "account sequence mismatch") + // ErrMaxRetriesReached is returned when max retry attempts are reached + ErrMaxRetriesReached = errors.Register("evmutil", 2, "max retry attempts reached") +) + +// RetryConfig defines configuration for retry mechanism +type RetryConfig struct { + // MaxAttempts is the maximum number of retry attempts + MaxAttempts int + // BackoffInterval is the initial interval to wait between retries + BackoffInterval time.Duration + // MaxBackoff is the maximum backoff duration + MaxBackoff time.Duration +} + +// DefaultRetryConfig returns default retry configuration +func DefaultRetryConfig() RetryConfig { + return RetryConfig{ + MaxAttempts: 5, + BackoffInterval: 1 * time.Second, + MaxBackoff: 30 * time.Second, + } +} + +// SequenceManager defines the interface for managing account sequences +type SequenceManager interface { + // GetNextSequence returns the next sequence number for the given address + GetNextSequence(ctx sdk.Context, addr sdk.AccAddress) (uint64, error) + // ValidateSequence validates if the given sequence matches the expected sequence + ValidateSequence(ctx sdk.Context, addr sdk.AccAddress, sequence uint64) error + // IncrementSequence increments the sequence number for the given address + IncrementSequence(ctx sdk.Context, addr sdk.AccAddress) error +} \ No newline at end of file From 2fcc7916c6c40741d85cdf97190d49726a107cb5 Mon Sep 17 00:00:00 2001 From: "0xalkindivv.eth" Date: Tue, 4 Mar 2025 16:57:19 +0700 Subject: [PATCH 2/2] fix(evmutil): implement gas validation for EVM calls - Add MaxGasLimit constant (30M gas) - Replace infinite gas meter with limited gas meter - Add validation for gas estimation and usage - Add unit tests for gas validation - Add new error type for gas limit exceeded --- x/evmutil/keeper/evm.go | 28 +++++++++++++++++-- x/evmutil/keeper/evm_test.go | 54 ++++++++++++++++++++++++++++++++++++ x/evmutil/types/errors.go | 1 + 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/x/evmutil/keeper/evm.go b/x/evmutil/keeper/evm.go index 32d948ff..d829f1fc 100644 --- a/x/evmutil/keeper/evm.go +++ b/x/evmutil/keeper/evm.go @@ -25,12 +25,16 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" - "github.com/evmos/ethermint/server/config" evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/0glabs/0g-chain/x/evmutil/types" ) +const ( + // MaxGasLimit defines the maximum gas that can be used in EVM execution + MaxGasLimit = uint64(30_000_000) +) + // CallEVM performs a smart contract method call using given args func (k Keeper) CallEVM( ctx sdk.Context, @@ -89,7 +93,15 @@ func (k Keeper) CallEVMWithData( return nil, err } - ethGasContext := ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + // Use a limited gas meter instead of infinite + maxGas := ctx.GasMeter().Limit() + if maxGas == 0 { + maxGas = MaxGasLimit + } + if maxGas > MaxGasLimit { + return nil, errorsmod.Wrapf(types.ErrGasLimitExceeded, "requested gas %d exceeds limit %d", maxGas, MaxGasLimit) + } + ethGasContext := ctx.WithGasMeter(sdk.NewGasMeter(maxGas)) // EstimateGas applies the transaction against current block state to get // optimal gas value. Since this is done right before the ApplyMessage @@ -100,12 +112,17 @@ func (k Keeper) CallEVMWithData( // apply, tx order is the same, etc.) gasRes, err := k.evmKeeper.EstimateGas(sdk.WrapSDKContext(ethGasContext), &evmtypes.EthCallRequest{ Args: args, - GasCap: config.DefaultGasCap, + GasCap: maxGas, }) if err != nil { return nil, errorsmod.Wrap(evmtypes.ErrVMExecution, err.Error()) } + // Validate estimated gas is within limits + if gasRes.Gas > maxGas { + return nil, errorsmod.Wrapf(types.ErrGasLimitExceeded, "estimated gas %d exceeds limit %d", gasRes.Gas, maxGas) + } + msg := ethtypes.NewMessage( from, to, @@ -129,6 +146,11 @@ func (k Keeper) CallEVMWithData( return nil, errorsmod.Wrap(evmtypes.ErrVMExecution, res.VmError) } + // Validate gas used is within limits + if res.GasUsed > maxGas { + return nil, errorsmod.Wrapf(types.ErrGasLimitExceeded, "gas used %d exceeds limit %d", res.GasUsed, maxGas) + } + ctx.GasMeter().ConsumeGas(res.GasUsed, "evm gas consumed") return res, nil diff --git a/x/evmutil/keeper/evm_test.go b/x/evmutil/keeper/evm_test.go index 343d8a6c..631eed78 100644 --- a/x/evmutil/keeper/evm_test.go +++ b/x/evmutil/keeper/evm_test.go @@ -17,6 +17,7 @@ import ( "github.com/evmos/ethermint/x/evm/statedb" "github.com/evmos/ethermint/x/evm/types" + "github.com/0glabs/0g-chain/x/evmutil/keeper" "github.com/0glabs/0g-chain/x/evmutil/testutil" ) @@ -135,3 +136,56 @@ func (suite *evmKeeperTestSuite) TestEvmKeeper_SetAccount() { func TestEvmKeeperTestSuite(t *testing.T) { suite.Run(t, new(evmKeeperTestSuite)) } + +func (suite *evmKeeperTestSuite) TestCallEVMWithData_GasLimits() { + // Setup test contract + addr := testutil.GenerateAddress() + contract := types.NewInternalEVMAddress(addr) + from := common.BytesToAddress(suite.Key1.PubKey().Address()) + + testCases := []struct { + name string + gasLimit uint64 + expPass bool + }{ + { + "success - gas within limits", + keeper.MaxGasLimit - 1, + true, + }, + { + "success - gas at max limit", + keeper.MaxGasLimit, + true, + }, + { + "fail - gas exceeds max limit", + keeper.MaxGasLimit + 1, + false, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + ctx := suite.Ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) + + // Create dummy contract call data + data := crypto.Keccak256([]byte("test")) + + // Execute contract call + _, err := suite.App.EvmutilKeeper.CallEVMWithData( + ctx, + from, + &contract, + data, + ) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + suite.Require().ErrorIs(err, types.ErrGasLimitExceeded) + } + }) + } +} diff --git a/x/evmutil/types/errors.go b/x/evmutil/types/errors.go index dc35468f..fa771310 100644 --- a/x/evmutil/types/errors.go +++ b/x/evmutil/types/errors.go @@ -12,4 +12,5 @@ var ( ErrInvalidCosmosDenom = errorsmod.Register(ModuleName, 7, "invalid cosmos denom") ErrSDKConversionNotEnabled = errorsmod.Register(ModuleName, 8, "sdk.Coin not enabled to convert to ERC20 token") ErrInsufficientConversionAmount = errorsmod.Register(ModuleName, 9, "insufficient conversion amount") + ErrGasLimitExceeded = errorsmod.Register(ModuleName, 10, "gas limit exceeded maximum allowed") )