Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(precompile): do not execute write functions on staticcall #3001

Merged
merged 2 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions precompiles/bank/bank.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
8 changes: 4 additions & 4 deletions precompiles/bank/method_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
20 changes: 19 additions & 1 deletion precompiles/staking/staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@

// 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
Expand Down Expand Up @@ -415,6 +415,12 @@
}
return res, nil
case StakeMethodName:
if readOnly {
return nil, ptypes.ErrWriteMethod{
Method: method.Name,

Check warning on line 420 in precompiles/staking/staking.go

View check run for this annotation

Codecov / codecov/patch

precompiles/staking/staking.go#L419-L420

Added lines #L419 - L420 were not covered by tests
}
}

var res []byte
execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error {
res, err = c.Stake(ctx, evm, contract, method, args)
Expand All @@ -425,6 +431,12 @@
}
return res, nil
case UnstakeMethodName:
if readOnly {
return nil, ptypes.ErrWriteMethod{
Method: method.Name,

Check warning on line 436 in precompiles/staking/staking.go

View check run for this annotation

Codecov / codecov/patch

precompiles/staking/staking.go#L435-L436

Added lines #L435 - L436 were not covered by tests
}
}

var res []byte
execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error {
res, err = c.Unstake(ctx, evm, contract, method, args)
Expand All @@ -435,6 +447,12 @@
}
return res, nil
case MoveStakeMethodName:
if readOnly {
return nil, ptypes.ErrWriteMethod{
Method: method.Name,

Check warning on line 452 in precompiles/staking/staking.go

View check run for this annotation

Codecov / codecov/patch

precompiles/staking/staking.go#L451-L452

Added lines #L451 - L452 were not covered by tests
}
}

var res []byte
execErr := stateDB.ExecuteNativeAction(contract.Address(), nil, func(ctx sdk.Context) error {
res, err = c.MoveStake(ctx, evm, contract, method, args)
Expand Down
8 changes: 8 additions & 0 deletions precompiles/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 19 additions & 27 deletions precompiles/types/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -116,8 +100,16 @@ 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)
}

func Test_ErrWriteMethod(t *testing.T) {
e := ErrWriteMethod{
Method: "foo",
}
got := e.Error()
expect := "method not allowed in read-only mode: foo"
require.Equal(t, expect, got)
require.ErrorIs(t, ErrWriteMethod{"foo"}, e)
}
Loading