Skip to content

Commit

Permalink
fix mining reverted txs (#1712)
Browse files Browse the repository at this point in the history 
* add e2e tests to validate reverted txs

* fix unsigned tx error handling and test

* fix test ip

* finish test

* finish test

* fix reverted errors when calling eth_call

* commenting test reverting on constructor

* fix jsonrpc tests error handling

* fix state unit tests prover url

* fix state revert tests

---------

Co-authored-by: ToniRamirezM <toni@iden3.com>
  • Loading branch information
@tclemos @ToniRamirezM
tclemos and ToniRamirezM authored Mar 2, 2023
1 parent c584e5d commit ab52541
Show file tree
Hide file tree
Showing 9 changed files with 270 additions and 38 deletions.
14 changes: 10 additions & 4 deletions jsonrpc/codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ type Response struct {

// ErrorObject is a jsonrpc error
type ErrorObject struct {
Code int `json:"code"`
Message string `json:"message"`
Data interface{} `json:"data,omitempty"`
Code int `json:"code"`
Message string `json:"message"`
Data *argBytes `json:"data,omitempty"`
}

// NewResponse returns Success/Error response object
Expand All @@ -62,7 +62,13 @@ func NewResponse(req Request, reply []byte, err rpcError) Response {

var errorObj *ErrorObject
if err != nil {
errorObj = &ErrorObject{err.ErrorCode(), err.Error(), nil}
errorObj = &ErrorObject{
Code: err.ErrorCode(),
Message: err.Error(),
}
if err.ErrorData() != nil {
errorObj.Data = argBytesPtr(*err.ErrorData())
}
}

return Response{
Expand Down
6 changes: 4 additions & 2 deletions jsonrpc/endpoints_eth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ func (e *EthEndpoints) Call(arg *txnArgs, number *BlockNumber) (interface{}, rpc
blockNumberToProcessTx = &blockNumber
}

result := e.state.ProcessUnsignedTransaction(ctx, tx, sender, blockNumberToProcessTx, true, dbTx)
result := e.state.ProcessUnsignedTransaction(ctx, tx, sender, blockNumberToProcessTx, false, dbTx)
if result.Failed() {
return rpcErrorResponse(defaultErrorCode, result.Err.Error(), nil)
data := make([]byte, len(result.ReturnValue))
copy(data, result.ReturnValue)
return rpcErrorResponseWithData(revertedErrorCode, result.Err.Error(), &data, nil)
}

return argBytesPtr(result.ReturnValue), nil
Expand Down
43 changes: 22 additions & 21 deletions jsonrpc/endpoints_eth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethereum/go-ethereum/trie"
"github.com/gorilla/websocket"
"github.com/jackc/pgx/v4"
Expand Down Expand Up @@ -94,7 +95,7 @@ func TestBlockNumber(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -151,7 +152,7 @@ func TestCall(t *testing.T) {
})
m.State.On("GetNonce", context.Background(), testCase.from, blockNumber, m.DbTx).Return(nonce, nil).Once()
var nilBlockNumber *uint64
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, true, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
},
},
{
Expand Down Expand Up @@ -179,7 +180,7 @@ func TestCall(t *testing.T) {
return hasTx && gasMatch && toMatch && gasPriceMatch && valueMatch && dataMatch
})
var nilBlockNumber *uint64
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, true, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
},
},
{
Expand Down Expand Up @@ -208,7 +209,7 @@ func TestCall(t *testing.T) {
return hasTx && gasMatch && toMatch && gasPriceMatch && valueMatch && dataMatch
})
var nilBlockNumber *uint64
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, true, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
},
},
{
Expand Down Expand Up @@ -265,7 +266,7 @@ func TestCall(t *testing.T) {
value: big.NewInt(2),
data: []byte("data"),
expectedResult: nil,
expectedError: newRPCError(defaultErrorCode, "failed to process unsigned transaction"),
expectedError: newRPCError(revertedErrorCode, "failed to process unsigned transaction"),
setupMocks: func(c Config, m *mocks, testCase *testCase) {
blockNumber := uint64(1)
nonce := uint64(7)
Expand All @@ -284,7 +285,7 @@ func TestCall(t *testing.T) {
})
m.State.On("GetNonce", context.Background(), testCase.from, blockNumber, m.DbTx).Return(nonce, nil).Once()
var nilBlockNumber *uint64
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, true, m.DbTx).Return(&runtime.ExecutionResult{Err: errors.New("failed to process unsigned transaction")}).Once()
m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{Err: errors.New("failed to process unsigned transaction")}).Once()
},
},
}
Expand All @@ -299,7 +300,7 @@ func TestCall(t *testing.T) {
assert.Equal(t, testCase.expectedResult, result)
if err != nil || testCase.expectedError != nil {
if expectedErr, ok := testCase.expectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -604,7 +605,7 @@ func TestGetBalance(t *testing.T) {
balance, err := c.BalanceAt(context.Background(), tc.addr, tc.blockNumber)
assert.Equal(t, tc.expectedBalance, balance.Uint64())
if err != nil || tc.expectedError != nil {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, tc.expectedError.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, tc.expectedError.Error(), rpcErr.Error())
}
Expand Down Expand Up @@ -715,7 +716,7 @@ func TestGetL2BlockByHash(t *testing.T) {

if err != nil || tc.ExpectedError != nil {
if expectedErr, ok := tc.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -949,7 +950,7 @@ func TestGetL2BlockByNumber(t *testing.T) {

if err != nil || tc.ExpectedError != nil {
if expectedErr, ok := tc.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -1162,7 +1163,7 @@ func TestGetCode(t *testing.T) {

if err != nil || tc.ExpectedError != nil {
if expectedErr, ok := tc.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -1300,7 +1301,7 @@ func TestGetStorageAt(t *testing.T) {

if err != nil || tc.ExpectedError != nil {
if expectedErr, ok := tc.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -1419,7 +1420,7 @@ func TestSyncing(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -1598,7 +1599,7 @@ func TestGetTransactionL2onByBlockHashAndIndex(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2051,7 +2052,7 @@ func TestGetTransactionByHash(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2131,7 +2132,7 @@ func TestGetBlockTransactionCountByHash(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2667,7 +2668,7 @@ func TestGetTransactionReceipt(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2735,7 +2736,7 @@ func TestSendRawTransactionViaGeth(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2900,7 +2901,7 @@ func TestSendRawTransactionViaGethForNonSequencerNode(t *testing.T) {

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -2937,7 +2938,7 @@ func TestSendRawTransactionViaGethForNonSequencerNodeFailsToRelayTxToSequencerNo

if err != nil || testCase.ExpectedError != nil {
if expectedErr, ok := testCase.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down Expand Up @@ -3402,7 +3403,7 @@ func TestGetLogs(t *testing.T) {

if err != nil || tc.ExpectedError != nil {
if expectedErr, ok := tc.ExpectedError.(*RPCError); ok {
rpcErr := err.(rpcError)
rpcErr := err.(rpc.Error)
assert.Equal(t, expectedErr.ErrorCode(), rpcErr.ErrorCode())
assert.Equal(t, expectedErr.Error(), rpcErr.Error())
} else {
Expand Down
14 changes: 13 additions & 1 deletion jsonrpc/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import "fmt"

const (
defaultErrorCode = -32000
revertedErrorCode = 3
invalidRequestErrorCode = -32600
notFoundErrorCode = -32601
invalidParamsErrorCode = -32602
Expand All @@ -13,22 +14,28 @@ const (
type rpcError interface {
Error() string
ErrorCode() int
ErrorData() *[]byte
}

// RPCError represents an RPC error.
type RPCError struct {
err string
code int
data *[]byte
}

func newRPCError(code int, err string, args ...interface{}) *RPCError {
return newRPCErrorWithData(code, err, nil, args...)
}

func newRPCErrorWithData(code int, err string, data *[]byte, args ...interface{}) *RPCError {
var errMessage string
if len(args) > 0 {
errMessage = fmt.Sprintf(err, args...)
} else {
errMessage = err
}
return &RPCError{code: code, err: errMessage}
return &RPCError{code: code, err: errMessage, data: data}
}

// Error returns the error message.
Expand All @@ -40,3 +47,8 @@ func (e *RPCError) Error() string {
func (e *RPCError) ErrorCode() int {
return e.code
}

// ErrorData returns the error data.
func (e *RPCError) ErrorData() *[]byte {
return e.data
}
12 changes: 8 additions & 4 deletions jsonrpc/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,15 @@ func handleError(w http.ResponseWriter, err error) {
}
}

func rpcErrorResponse(code int, errorMessage string, err error) (interface{}, rpcError) {
func rpcErrorResponse(code int, message string, err error) (interface{}, rpcError) {
return rpcErrorResponseWithData(code, message, nil, err)
}

func rpcErrorResponseWithData(code int, message string, data *[]byte, err error) (interface{}, rpcError) {
if err != nil {
log.Errorf("%v:%v", errorMessage, err.Error())
log.Errorf("%v:%v", message, err.Error())
} else {
log.Error(errorMessage)
log.Error(message)
}
return nil, newRPCError(code, errorMessage)
return nil, newRPCErrorWithData(code, message, data)
}
3 changes: 2 additions & 1 deletion sequencer/finalizer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/0xPolygonHermez/zkevm-node/pool"
"github.com/0xPolygonHermez/zkevm-node/sequencer/metrics"
"github.com/0xPolygonHermez/zkevm-node/state"
"github.com/0xPolygonHermez/zkevm-node/state/runtime"
"github.com/0xPolygonHermez/zkevm-node/state/runtime/executor"
"github.com/ethereum/go-ethereum/common"
"github.com/jackc/pgx/v4"
Expand Down Expand Up @@ -385,7 +386,7 @@ func (f *finalizer) processTransaction(ctx context.Context, tx *TxTracker) error
func (f *finalizer) handleSuccessfulTxProcessResp(ctx context.Context, tx *TxTracker, result *state.ProcessBatchResponse) error {
if len(result.Responses) > 0 {
// Handle Transaction Error
if result.Responses[0].RomError != nil {
if result.Responses[0].RomError != nil && !errors.Is(result.Responses[0].RomError, runtime.ErrExecutionReverted) {
f.handleTransactionError(ctx, result, tx)
return result.Responses[0].RomError
}
Expand Down
9 changes: 6 additions & 3 deletions state/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1189,14 +1189,17 @@ func (s *State) ProcessUnsignedTransaction(ctx context.Context, tx *types.Transa
if err != nil {
result.Err = err
}

if response.Responses[0] != nil {
r := response.Responses[0]
result.ReturnValue = r.ReturnValue
result.GasLeft = r.GasLeft
result.GasUsed = r.GasUsed
result.CreateAddress = r.CreateAddress
result.StateRoot = r.StateRoot.Bytes()

if result.Err == nil {
result.Err = r.RomError
}
}

return result
Expand Down Expand Up @@ -1274,9 +1277,9 @@ func (s *State) internalProcessUnsignedTransaction(ctx context.Context, tx *type
if processBatchResponse.Responses[0].Error != pb.RomError(executor.ROM_ERROR_NO_ERROR) {
err := executor.RomErr(processBatchResponse.Responses[0].Error)
if isEVMRevertError(err) {
return nil, constructErrorFromRevert(err, processBatchResponse.Responses[0].ReturnValue)
return response, constructErrorFromRevert(err, processBatchResponse.Responses[0].ReturnValue)
} else {
return nil, err
return response, err
}
}

Expand Down
Loading

0 comments on commit ab52541

Please sign in to comment.