Skip to content

Commit

Permalink
Merge pull request #650 from oasisprotocol/andrew7234/default-runtime…
Browse files Browse the repository at this point in the history
…-tx-err

[analyzer] return default message if no revert reason specified
  • Loading branch information
Andrew7234 authored Mar 8, 2024
2 parents a619fde + 436e867 commit 737021c
Show file tree
Hide file tree
Showing 5 changed files with 262 additions and 15 deletions.
34 changes: 23 additions & 11 deletions analyzer/evmabibackfill/evm_abi_backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ func (p *processor) parseEvent(ev *abiEncodedEvent, contractAbi abi.ABI) (*strin
}
eventArgs, err := marshalArgs(abiEvent.Inputs, abiEventArgs)
if err != nil {
p.logger.Warn("error processing event args using abi", "err", err)
return nil, nil, nil, fmt.Errorf("error processing event args using abi: %w", err)
}

Expand All @@ -186,9 +185,10 @@ func (p *processor) parseTxCall(tx *abiEncodedTx, contractAbi abi.ABI) (*string,
return &method.RawName, txArgs, nil
}

// Attempts to parse the transaction revert reason into the error name and args
// as defined by the abi of the contract that was called.
func (p *processor) parseTxErr(tx *abiEncodedTx, contractAbi abi.ABI) (*string, []*abiEncodedArg) {
// Attempts to parse the transaction revert reason into the error name (or plaintext message;
// see chain.runtime_transactions.error_message in DB) and args as defined by the abi of the
// contract that was called.
func (p *processor) parseTxErr(tx *abiEncodedTx, contractAbi abi.ABI) (*string, []*abiEncodedArg, error) {
var abiErrMsg string
var abiErr *abi.Error
var abiErrArgs []interface{}
Expand All @@ -200,22 +200,30 @@ func (p *processor) parseTxErr(tx *abiEncodedTx, contractAbi abi.ABI) (*string,
// as "reverted: Ownable: caller is not the owner". In this case, we do
// not parse the error with the abi.
p.logger.Info("encountered likely old-style reverted transaction", "revert reason", tx.TxRevertReason, "tx hash", tx.TxHash, "err", err)
return nil, nil
return nil, nil, nil
}
// If no revert reason was provided, we return nil here to skip the update
// and preserve the existing error_message, which should be the default
// tx error message defined in runtime.DefaultTxRevertErrMsg.
//
// Note: Although we could return the default error message here, conceptually
// the abi analyzer should only update fields that were parsed using the abi.
if len(txrr) == 0 {
p.logger.Info("no revert reason provided, skipping update")
return nil, nil, nil
}
abiErr, abiErrArgs, err = abiparse.ParseError(txrr, &contractAbi)
if err != nil || abiErr == nil {
p.logger.Warn("error processing tx error using abi", "contract address", "err", err)
return nil, nil
return nil, nil, fmt.Errorf("error processing error using abi: %w", err)
}
abiErrMsg = runtime.TxRevertErrPrefix + abiErr.Name + prettyPrintArgs(abiErrArgs)
errArgs, err = marshalArgs(abiErr.Inputs, abiErrArgs)
if err != nil {
p.logger.Warn("error processing tx error args", "err", err)
return nil, nil
return nil, nil, fmt.Errorf("error marshalling tx err args: %w", err)
}
}

return &abiErrMsg, errArgs
return &abiErrMsg, errArgs, nil
}

func (p *processor) ProcessItem(ctx context.Context, batch *storage.QueryBatch, item *abiEncodedItem) error {
Expand Down Expand Up @@ -248,7 +256,11 @@ func (p *processor) ProcessItem(ctx context.Context, batch *storage.QueryBatch,
p.logger.Warn("error parsing tx with abi", "err", err, "contract_address", item.ContractAddr, "tx_hash", item.Tx.TxHash)
// Write to the DB regardless of error so we don't keep retrying the same item.
}
errMsg, errArgs := p.parseTxErr(item.Tx, contractAbi)
errMsg, errArgs, err := p.parseTxErr(item.Tx, contractAbi)
if err != nil {
p.logger.Warn("error parsing tx err with abi", "err", err, "contract_address", item.ContractAddr, "tx_hash", item.Tx.TxHash)
// Write to the DB regardless of error so we don't keep retrying the same item.
}
batch.Queue(
queries.RuntimeTransactionEvmParsedFieldsUpdate,
p.runtime,
Expand Down
221 changes: 221 additions & 0 deletions analyzer/evmabibackfill/evm_abi_backfill_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
package evmabibackfill

import (
"encoding/base64"
"encoding/json"
"testing"

ethCommon "github.com/ethereum/go-ethereum/common"

"github.com/oasisprotocol/oasis-sdk/client-sdk/go/modules/evm"
"github.com/stretchr/testify/require"

"github.com/oasisprotocol/nexus/analyzer/evmabi"
"github.com/oasisprotocol/nexus/common"
"github.com/oasisprotocol/nexus/log"
)

// TODO: Update WROSE source and add tests for more txs, mismatched txs, and custom errors

type mockParsedEvent struct {
Name *string
Args []*abiEncodedArg
Sig *ethCommon.Hash
}

type mockParsedTxCall struct {
Name *string
Args []*abiEncodedArg
}

func unmarshalEvmEvent(t *testing.T, body []byte) *abiEncodedEvent {
var ev evm.Event
err := json.Unmarshal(body, &ev)
require.Nil(t, err)
return &abiEncodedEvent{
Round: 0,
TxIndex: nil,
EventBody: ev,
}
}

func unmarshalEvmTx(t *testing.T, body string, error_message *string) *abiEncodedTx {
txData, err := base64.StdEncoding.DecodeString(body)
require.Nil(t, err)
return &abiEncodedTx{
TxHash: "", // not relevant for current unit tests
TxData: txData,
TxRevertReason: error_message,
}
}

func TestParseEvent(t *testing.T) {
abi := evmabi.WROSE
p := &processor{
runtime: common.RuntimeSapphire,
target: nil,
logger: log.NewDefaultLogger("testing"),
}
ev := unmarshalEvmEvent(t, []byte(`{"data": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADCopscuTr4A=", "topics": ["3fJSrRviyJtpwrBo/DeNqpUrp/FjxKEWKPVaTfUjs+8=", "AAAAAAAAAAAAAAAAw3+DQaxuSpRTgwK81NSc8IUtMMA=", "AAAAAAAAAAAAAAAAWZiaj7/4vj0d6v/ytGtmtjrLeX4="], "address": "i8KwMLKZlk7vteHgs2mRNS5W0tM="}`))
expected := mockParsedEvent{
Name: common.Ptr("Transfer"),
Sig: common.Ptr(ethCommon.HexToHash("0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef")),
Args: []*abiEncodedArg{
{
Name: "src",
EvmType: "address",
Value: ethCommon.HexToAddress("0xc37F8341Ac6e4a94538302bCd4d49Cf0852D30C0"),
},
{
Name: "dst",
EvmType: "address",
Value: ethCommon.HexToAddress("0x59989A8fBff8be3d1deAFFF2b46B66B63ACB797e"),
},
{
Name: "wad",
EvmType: "uint256",
Value: "876558921078386560",
},
},
}
name, args, sig, err := p.parseEvent(ev, *abi)
require.Nil(t, err)
verifyEvent(t, &expected, name, args, sig)
}

func TestParseUnknownEvent(t *testing.T) {
abi := evmabi.WROSE
p := &processor{
runtime: common.RuntimeSapphire,
target: nil,
logger: log.NewDefaultLogger("testing"),
}
// Non-WROSE event; it doesn't fit the `abi` and should fail to parse.
rawEv := []byte(`{"data": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", "topics": ["8nnmofXjIMypETVnbZy25EyooIwLiDQrzbEUT2URtWg=", "AAAAAAAAAAAAAAAArk2RKpUohmFUB3UShFG6FIKuqKc=", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAI="], "address": "jZzJ7hGq+GWRPe7JOe6y3Hg4q3s="}`)
ev := unmarshalEvmEvent(t, rawEv)

name, args, sig, err := p.parseEvent(ev, *abi)
require.NotNil(t, err)
require.Nil(t, name)
require.Nil(t, args)
require.Nil(t, sig)
}

func TestParseTransactionCall(t *testing.T) {
abi := evmabi.WROSE
p := &processor{
runtime: common.RuntimeSapphire,
target: nil,
logger: log.NewDefaultLogger("testing"),
}
txs := []*abiEncodedTx{
unmarshalEvmTx(t, "Lhp9TQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAKcV46p6RigAA", nil),
unmarshalEvmTx(t, "qQWcuwAAAAAAAAAAAAAAAOY8J3oxo9tm0HXYxapOlEKOuxjxAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", nil),
unmarshalEvmTx(t, "0OMNsA==", nil),
}

expected := []mockParsedTxCall{
{
Name: common.Ptr("withdraw"),
Args: []*abiEncodedArg{
{
Name: "wad",
EvmType: "uint256",
Value: "770545888011536171008",
},
},
},
{
Name: common.Ptr("transfer"),
Args: []*abiEncodedArg{
{
Name: "dst",
EvmType: "address",
Value: ethCommon.HexToAddress("0xE63c277a31A3dB66D075D8C5aA4E94428Ebb18F1"),
},
{
Name: "wad",
EvmType: "uint256",
Value: "0",
},
},
},
{
Name: common.Ptr("deposit"),
Args: []*abiEncodedArg{},
},
}
for i, tx := range txs {
name, args, err := p.parseTxCall(tx, *abi)
require.Nil(t, err)
verifyTxCall(t, &expected[i], name, args)
}
}

func TestParseTxErrorPlaintext(t *testing.T) {
abi := evmabi.WROSE
p := &processor{
runtime: common.RuntimeSapphire,
target: nil,
logger: log.NewDefaultLogger("testing"),
}
errMsg := "reverted: plaintext error message"
emptyErrMsg := "reverted: "
txs := []*abiEncodedTx{
unmarshalEvmTx(t, "", &errMsg),
unmarshalEvmTx(t, "", &emptyErrMsg),
}

for _, tx := range txs {
parsedMsg, args, err := p.parseTxErr(tx, *abi)
require.Nil(t, err)
require.Nil(t, parsedMsg)
require.Nil(t, args)
}
}

func verifyEvent(t *testing.T, expected *mockParsedEvent, name *string, args []*abiEncodedArg, sig *ethCommon.Hash) {
if expected.Name != nil {
require.NotNil(t, name)
require.Equal(t, *expected.Name, *name)
} else {
require.Nil(t, name)
}
if expected.Args != nil {
require.NotNil(t, args)
require.Equal(t, len(expected.Args), len(args))
for i, ea := range expected.Args {
require.Equal(t, ea.Name, args[i].Name)
require.Equal(t, ea.EvmType, args[i].EvmType)
require.Equal(t, ea.Value, args[i].Value)
}
} else {
require.Nil(t, args)
}
if expected.Sig != nil {
require.NotNil(t, sig)
require.Equal(t, *expected.Sig, *sig)
} else {
require.Nil(t, sig)
}
}

func verifyTxCall(t *testing.T, expected *mockParsedTxCall, name *string, args []*abiEncodedArg) {
if expected.Name != nil {
require.NotNil(t, name)
require.Equal(t, *expected.Name, *name)
} else {
require.Nil(t, name)
}
if expected.Args != nil {
require.NotNil(t, args)
require.Equal(t, len(expected.Args), len(args))
for i, ea := range expected.Args {
require.Equal(t, ea.Name, args[i].Name)
require.Equal(t, ea.EvmType, args[i].EvmType)
require.Equal(t, ea.Value, args[i].Value)
}
} else {
require.Nil(t, args)
}
}
3 changes: 3 additions & 0 deletions analyzer/runtime/abiparse/abiparse.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ func ParseEvent(topics [][]byte, data []byte, contractABI *abi.ABI) (*abi.Event,

for i, in := range event.Inputs {
if in.Indexed {
if nextTopicIndex == len(topicsEC) {
return nil, nil, fmt.Errorf("number of indexed inputs exceeds number of event topics")
}
switch in.Type.T {
case abi.StringTy, abi.SliceTy, abi.ArrayTy, abi.TupleTy, abi.BytesTy:
// https://docs.soliditylang.org/en/latest/abi-spec.html
Expand Down
12 changes: 11 additions & 1 deletion analyzer/runtime/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ import (
"github.com/oasisprotocol/nexus/storage/oasis/nodeapi"
)

const TxRevertErrPrefix = "reverted: "
const (
TxRevertErrPrefix = "reverted: "
DefaultTxRevertErrMsg = "reverted without a message"
)

type BlockTransactionSignerData struct {
Index int
Expand Down Expand Up @@ -678,6 +681,9 @@ func extractTxError(fcr sdkTypes.FailedCallResult) TxError {
// Otherwise, we optimistically try to decode the error as the prevailing
// Error(string) type and return the error message if successful.
//
// If no revert reason was provided, e.g. the raw error message is
// "reverted: ", we return a fixed default error message instead.
//
// Note that the error type can be any data type specified in the abi.
// We may not have the abi available now, so in those cases the
// abi analyzer will extract the error message once the contract
Expand All @@ -702,6 +708,10 @@ func tryParseErrorMessage(errorModule string, errorCode uint32, msg string) *str
sanitizedMsg := storage.SanitizeString(msg)
return &sanitizedMsg
}
// Return a default error message if no revert reason was provided.
if len(abiEncodedErr) == 0 {
return common.Ptr(DefaultTxRevertErrMsg)
}
// Try to abi decode as Error(string).
stringAbi, _ := abi.NewType("string", "", nil)
errAbi := abi.NewError("Error", abi.Arguments{{Type: stringAbi}})
Expand Down
7 changes: 4 additions & 3 deletions analyzer/runtime/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ func TestUnknownError(t *testing.T) {
}

func TestEmptyError(t *testing.T) {
input := &mockError{module: "evm", code: 8, message: "reverted: "}
msg := tryParseErrorMessage(input.module, input.code, input.message)
require.Nil(t, msg, "tryParseErrorMessage extracted a message when it should not have")
emptyErr := mockError{module: "evm", code: 8, message: "reverted: "}
msg := tryParseErrorMessage(emptyErr.module, emptyErr.code, emptyErr.message)
require.NotNil(t, msg, "failed to extract default error message")
require.Equal(t, DefaultTxRevertErrMsg, *msg, "tryParseErrorMessage extracted a message that differed from the expected default error message")
}

func TestExtractSuccessfulEncryptedTx(t *testing.T) {
Expand Down

0 comments on commit 737021c

Please sign in to comment.