[analyzer] return default message if no revert reason specified

[Andrew7234]

Per discussion on slack, we want to return a more human-friendly default error message if no transaction revert reason is provided.

Note:

• This PR only handles the case where the error message is reverted: . It does not handle the case where the error message is entirely empty, since this should never occur (staging mainnet has no instances of a failed tx with a rawErrMessage of "")

Testing:

• Unfortunately our e2e_regression test ranges don't include a suitable failed tx with an empty revert message. Tested manually on the originally reported transaction:

~/oasis/oasis-block-indexer(andrew7234/default-runtime-tx-err*) » curl "http://localhost:8008/v1/sapphire/transactions/872d95e14bd497f35209539341958346a4e20ddf66d1ef2f282251294aa5d3d0" | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2249    0  2249    0     0   107k      0 --:--:-- --:--:-- --:--:--  244k
{
  "is_total_count_clipped": false,
  "total_count": 1,
  "transactions": [
    {
      "amount": "0",
      "body": {
        "address": "llRjnKXeSEg9muiB+exn7IR/G3o=",
        "data": "OALN1QAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOAAAAAAAAAAAAAAAACx/yhbXkLNKgq/Z+RVLPWmmG7bpAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY2DMN3FAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAElByb29mLW9mLUF1dGhvcml0eQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAC5RbWNvMUI4QWFHVEttSnJrVjNYdEp6c3FWRmFKR3B6UjhhU056RnJ4TTNjZDhzAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAALH/KFteQs0qCr9n5FUs9aaYbtukAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAnt9AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAdkYW9zaWduAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACe30AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQWwftSr0/smPCfeMrrBKn85WcjfkOPC3e+9HVm6wUnY6SgZ0fyq9gfBtfb5qL61LIeq7lH3lqj6k5Tu24NFaPzQcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAuUW1aNm41Nkg5THhQcnVDUWRlRVJVazFpdVZvNzRrRWFuZFpxdnk2dGp1cFNQMwAAAAAAAAAAAAAAAAAAAAAAAA==",
        "value": ""
      },
      "charged_fee": "30870585000000000",
      "error": {
        "code": 8,
        "message": "reverted without a message",
        "module": "evm"
      },
      "eth_hash": "872d95e14bd497f35209539341958346a4e20ddf66d1ef2f282251294aa5d3d0",
      "fee": "10100250000000000000",
      "gas_limit": 10050000,
      "gas_used": 30717,
      "hash": "7b8f21f8f233412855758ae5a8a5f5032649b0dffa578a69b4516bc762e82ccb",
      "index": 0,
      "method": "evm.Call",
      "nonce_0": 10,
      "round": 5029734,
      "sender_0": "oasis1qqmkkw69a56lqsrqqhvetknlup3gcl6885g8lqjm",
      "sender_0_eth": "0x1e59ce931B4CFea3fe4B875411e280e173cB7A9C",
      "size": 1197,
      "success": false,
      "timestamp": "2024-02-23T05:46:50-05:00",
      "to": "oasis1qqd94y2hqwl48t4sh7ck2f72y8g06f42fvz82lmp",
      "to_eth": "0x9654639cA5De48483D9Ae881f9ec67eC847f1b7A"
    }
  ]
}

[mitjat]

Thank you, looks good!

Let's follow our belated New Year's resolution though: unit tests.
They don't need to be big, but we should get into the habit more. You can use white-box tests: For example, evm_abi_backfill_test.go will start with package evmabibackfill and can exercise the private parseTxErr directly.
We have a few ABIs lying around already if you need them for testing, e.g. the erc20 one.

I'm surprised there are no regression test changes. Actually, of the last 10_000_000 runtime txs, none reverted with "reverted: " 🤔 . But there are some interesting ones (only those rows preserved below)

select *, count(*) cnt from (select error_message from chain.runtime_transactions limit 10000000) x where error_message like 'reverted%' group by 1 order by cnt desc;
                                    error_message                                    |  cnt  
-------------------------------------------------------------------------------------+-------
 reverted: unknown                                                                   | 99353
 reverted: no revert reason                                                          |   725
 reverted: -                                                                         |    62

"reverted: unknown" is also the most common of all revert reasons. Maybe the "unknown" is an artifact of our own parsing? Please check what the unit tests return if your code diff is not in place, I'm curious. Hm or maybe it's from the olden runtimes that tried to decode the revert reason themselves, always assuming the ABI is Error(string). When I look at modern txs, I see no instances of that revert reason.

My guess is that the other two "non-reasons" in the table above are manual strings by smart contract authors.

[mitjat]

Oh haha I see we do have a unit test for extract.go already, and it's failing in CI because it wants a nil output :)

[Andrew7234]

Let's follow our belated New Year's resolution though: unit tests.

Good call; I'll add unit tests to the abi analyzer!

"reverted: unknown" is also the most common of all revert reasons... maybe it's from the olden runtimes

Looks like the reverted: unknown was indeed from the cobalt days (damask started at 1003298). We could also insert the default error message when we see this case, but given that those txs were from so long ago I'm leaning against it. This also avoids another special-case we'd have to think about going forward.

oasisindexer=> select runtime, round, error_message_raw from chain.runtime_transactions where error_message='reverted: unknown' order by round desc limit 100;
 runtime |  round  | error_message_raw
---------+---------+-------------------
 emerald | 1003293 | reverted: unknown

[mitjat]

Thank you for the added tests! I was trying to request just a test for what this PR fixes (= parsing of reverted: ), but of course this is even nicer :). I see the reverted: test is still TODO but that should be easy now that you have all the infrastructure set up in the new file.

No good deed goes unpuninshed, so with more tests, I have more feedback/tips. Nothing major though, and thanks again!