From 715d7290e64da63dfb332c5b64e9ca28ee987e89 Mon Sep 17 00:00:00 2001 From: Francisco de Borja Aranda Castillejo Date: Fri, 11 Oct 2024 20:05:15 +0200 Subject: [PATCH 1/2] refactor(staking): do not execute write functions on staticcall --- precompiles/bank/bank.go | 4 ++-- precompiles/bank/method_test.go | 8 ++++---- precompiles/staking/staking.go | 20 +++++++++++++++++++- precompiles/types/errors.go | 8 ++++++++ precompiles/types/errors_test.go | 12 ++++++++++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/precompiles/bank/bank.go b/precompiles/bank/bank.go index 436364dfa6..51a559edd3 100644 --- a/precompiles/bank/bank.go +++ b/precompiles/bank/bank.go @@ -136,8 +136,8 @@ func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, readOnly bool) ([]byt // Deposit and Withdraw methods are both not allowed in read-only mode. case DepositMethodName, WithdrawMethodName: if readOnly { - return nil, ptypes.ErrUnexpected{ - Got: "method not allowed in read-only mode: " + method.Name, + return nil, ptypes.ErrWriteMethod{ + Method: method.Name, } } diff --git a/precompiles/bank/method_test.go b/precompiles/bank/method_test.go index 912c41b40c..107439c576 100644 --- a/precompiles/bank/method_test.go +++ b/precompiles/bank/method_test.go @@ -46,8 +46,8 @@ func Test_Methods(t *testing.T) { success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, true) require.ErrorIs( t, - ptypes.ErrUnexpected{ - Got: "method not allowed in read-only mode: deposit", + ptypes.ErrWriteMethod{ + Method: "deposit", }, err) require.Empty(t, success) @@ -73,8 +73,8 @@ func Test_Methods(t *testing.T) { success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, true) require.ErrorIs( t, - ptypes.ErrUnexpected{ - Got: "method not allowed in read-only mode: withdraw", + ptypes.ErrWriteMethod{ + Method: "withdraw", }, err) require.Empty(t, success) diff --git a/precompiles/staking/staking.go b/precompiles/staking/staking.go index 7ef467b039..f3a01a541e 100644 --- a/precompiles/staking/staking.go +++ b/precompiles/staking/staking.go @@ -380,7 +380,7 @@ func (c *Contract) MoveStake( // Run is the entrypoint of the precompiled contract, it switches over the input method, // and execute them accordingly. -func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, _ bool) ([]byte, error) { +func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, readOnly bool) ([]byte, error) { method, err := ABI.MethodById(contract.Input[:4]) if err != nil { return nil, err @@ -415,6 +415,12 @@ func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, _ bool) ([]byte, erro } return res, nil case StakeMethodName: + if readOnly { + return nil, ptypes.ErrWriteMethod{ + Method: method.Name, + } + } + var res []byte execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error { res, err = c.Stake(ctx, evm, contract, method, args) @@ -425,6 +431,12 @@ func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, _ bool) ([]byte, erro } return res, nil case UnstakeMethodName: + if readOnly { + return nil, ptypes.ErrWriteMethod{ + Method: method.Name, + } + } + var res []byte execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error { res, err = c.Unstake(ctx, evm, contract, method, args) @@ -435,6 +447,12 @@ func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, _ bool) ([]byte, erro } return res, nil case MoveStakeMethodName: + if readOnly { + return nil, ptypes.ErrWriteMethod{ + Method: method.Name, + } + } + var res []byte execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error { res, err = c.MoveStake(ctx, evm, contract, method, args) diff --git a/precompiles/types/errors.go b/precompiles/types/errors.go index 03e397fa14..a624ab27af 100644 --- a/precompiles/types/errors.go +++ b/precompiles/types/errors.go @@ -94,6 +94,14 @@ func (e ErrInvalidMethod) Error() string { return fmt.Sprintf("invalid method: %s", e.Method) } +type ErrWriteMethod struct { + Method string +} + +func (e ErrWriteMethod) Error() string { + return fmt.Sprintf("method not allowed in read-only mode: %s", e.Method) +} + type ErrUnexpected struct { When string Got string diff --git a/precompiles/types/errors_test.go b/precompiles/types/errors_test.go index c80647a08d..7ba9456d73 100644 --- a/precompiles/types/errors_test.go +++ b/precompiles/types/errors_test.go @@ -121,3 +121,15 @@ func Test_ErrInvalidToken(t *testing.T) { } require.ErrorIs(t, ErrInvalidToken{"foo", "bar"}, e) } + +func Test_ErrWriteMethod(t *testing.T) { + e := ErrWriteMethod{ + Method: "foo", + } + got := e.Error() + expect := "method not allowed in read-only mode: foo" + if got != expect { + t.Errorf("Expected %v, got %v", expect, got) + } + require.ErrorIs(t, ErrWriteMethod{"foo"}, e) +} \ No newline at end of file From 2105ca6de64eace20009894bd9d48ae1e02cee96 Mon Sep 17 00:00:00 2001 From: lumtis Date: Mon, 14 Oct 2024 14:23:53 +0200 Subject: [PATCH 2/2] use require --- precompiles/types/errors_test.go | 42 +++++++++----------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/precompiles/types/errors_test.go b/precompiles/types/errors_test.go index 7ba9456d73..d432aa901c 100644 --- a/precompiles/types/errors_test.go +++ b/precompiles/types/errors_test.go @@ -13,9 +13,7 @@ func Test_ErrInvalidAddr(t *testing.T) { } got := e.Error() expect := "invalid address foo, reason: bar" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidAddr{"foo", "bar"}, e) } @@ -26,9 +24,7 @@ func Test_ErrInvalidNumberOfArgs(t *testing.T) { } got := e.Error() expect := "invalid number of arguments; expected 2; got: 1" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidNumberOfArgs{1, 2}, e) } @@ -38,9 +34,7 @@ func Test_ErrInvalidArgument(t *testing.T) { } got := e.Error() expect := "invalid argument: foo" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidArgument{"foo"}, e) } @@ -50,9 +44,7 @@ func Test_ErrInvalidMethod(t *testing.T) { } got := e.Error() expect := "invalid method: foo" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidMethod{"foo"}, e) } @@ -65,9 +57,7 @@ func Test_ErrInvalidCoin(t *testing.T) { } got := e.Error() expect := "invalid coin: denom: foo, is negative: true, is nil: false, is empty: false" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidCoin{"foo", true, false, false}, e) } @@ -77,9 +67,7 @@ func Test_ErrInvalidAmount(t *testing.T) { } got := e.Error() expect := "invalid token amount: foo" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidAmount{"foo"}, e) } @@ -90,9 +78,7 @@ func Test_ErrUnexpected(t *testing.T) { } got := e.Error() expect := "unexpected error in foo: bar" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrUnexpected{"foo", "bar"}, e) } @@ -103,9 +89,7 @@ func Test_ErrInsufficientBalance(t *testing.T) { } got := e.Error() expect := "insufficient balance: requested foo, current bar" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInsufficientBalance{"foo", "bar"}, e) } @@ -116,9 +100,7 @@ func Test_ErrInvalidToken(t *testing.T) { } got := e.Error() expect := "invalid token foo: bar" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrInvalidToken{"foo", "bar"}, e) } @@ -128,8 +110,6 @@ func Test_ErrWriteMethod(t *testing.T) { } got := e.Error() expect := "method not allowed in read-only mode: foo" - if got != expect { - t.Errorf("Expected %v, got %v", expect, got) - } + require.Equal(t, expect, got) require.ErrorIs(t, ErrWriteMethod{"foo"}, e) -} \ No newline at end of file +}