fix: Rework revert_reason

[k1rill-fedoseev]

Motivation

Multiple problems in the current logic handling revert_reason have been discovered:

1. transactions.revert_reason column type is text, although it should be bytea, this bloats the storage unnecessary, as hex values are stored as text.
2. transactions.revert_reason contain irrelevant error messages, that should not be stored in the database. Here are the results of the SELECT revert_reason, count(*) FROM transactions WHERE status = 0 AND revert_reason IS NOT NULL GROUP BY revert_reason query on Goerli: revert_reasons_stat.csv. Values like these are clearly not valid revert reasons and were saved by mistake:
   • err: insufficient funds for gas * price + value: ...
   • execution reverted
   • insufficient funds for gas * price + value: ...
   • we can't execute this request
3. Using eth_call for obtaining revert reasons is not 100% robust, as it can't simulate the call in the middle of the block using the intermediary state. trace_* methods should be used instead. This is also likely the reason how messages like insufficient funds for gas * price + value: ... appeared in the revert_reason column.
4. Standard solidity error ABI (Error(string) and Panic(uint256)) decoding does not properly work:
   • https://eth-goerli.blockscout.com/tx/0xbc0b5f96b12994c1ff8158fb7c7a8edce5adf30bd0056f6503344611cd20b570
   • https://eth-goerli.blockscout.com/tx/0x033672e335f3ee76deb91a78edafa50b7a4306e41489f554d40a0b4a2fd32ff4
5. Obtained internal transactions are not used to fill up the revert_reason fields in the database, due to call_has_error_or_result constraint. Filling revert_reasons in internal transactions will potentially reduce number of on-demand rpc calls to trace_* or eth_call methods used for obtaining revert reasons.

Changelog

Attempt to fix problems 3&4 from the above list. 1&2&5 might be fixed as well if deemed feasible.

Checklist for your Pull Request (PR)

• I added an entry to CHANGELOG.md with this PR
• If I added new functionality, I added tests covering it.
• If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
• I checked whether I should update the docs and did so by submitting a PR to https://github.com/blockscout/docs
• If I added/changed/removed ENV var, I submitted a PR to https://github.com/blockscout/docs to update the list of env vars at https://github.com/blockscout/docs/blob/master/for-developers/information-and-settings/env-variables.md and I updated the version to master in the Version column. Changes will be reflected in this table: https://docs.blockscout.com/for-developers/information-and-settings/env-variables.
• If I add new indices into DB, I checked, that they are not redundant with PGHero or other tools

[k1rill-fedoseev]

Not sure if we already have any specific UI views for these standard solidity errors, or we can render them just in the same way as custom ones.

For panics, the following error code description should be displayed somewhere I guess - https://docs.soliditylang.org/en/v0.8.23/control-structures.html#panic-via-assert-and-error-via-require

[nikitosing]

Let's save a first trace here, as done in

blockscout/apps/indexer/lib/indexer/fetcher/first_trace_on_demand.ex

Line 37 in 8780fdf

InternalTransactions.run_insert_only(first_trace_params, %{

[k1rill-fedoseev]

I didn't put db insert here intentionally to not slow down API view by an extra db operation.
I also not a fan of current separated logic of saving first trace and remaining traces in different place. It needs some refactoring and internal tx fetcher should not drop the first trace if it's useful.

[nikitosing]

I totally agree with you. But what I see now, is that we added one more quite heavy RPC call and its result is not reused, and then it will most probably be duplicated. I don't think that db insertion will be slower than RPC call.
One more question. Shouldn't we check if first trace is already fetched and stored in DB by first_trace_on_demand?

[k1rill-fedoseev]

It's reused, since we save it in the revert_reason column, we won't re-fetch traces if the revert_reason is already non-empty.

[nikitosing]

I meant that Indexer.Fetcher.FirstTraceOnDemand will request first trace again

[k1rill-fedoseev]

Right, but Indexer.Fetcher.FirstTraceOnDemand is only triggered for raw-trace API endpoints, which are not called very often I guess.

If my understanding is correct, then with proper future refactoring in internal tx import logic Indexer.Fetcher.FirstTraceOnDemand and all of its associated function won't be needed at all. This code will also work as a fallback, since internal tx import will contain the main logic extracting revert reasons.

[nikitosing]

Yes, you are right but...