Add revert reason to exceptions

[lyotam]

What does this PR do?

Adds the revert reason for contract calls and transactions which were reverted by the EVM to the respective exception thrown, using the already implemented ethCall.getRevertReason method.
For transactions, because it requires an additional EthCall to retrieve it, I added an optional sendTransactionWithRevertReason method to Contract that will retrieve it only if the txn reverts.

Where should the reviewer start?

TransactionManager.java (for calls)
Contract.java (for transactions)

Why is it needed?

Currently not only that the reverted calls & txns are missing the revert reason string when exception is thrown, they also don't provide the required txn information (hash for example) to retrieve it using additional call.
This adjusts the error flow for calls & txns while including the reverted string and TransactionReceipt in the exception.

[iikirilov]

I am not a fan of this approach - is revert reason only available for ethCall?

[lyotam]

I am not a fan of this approach - is revert reason only available for ethCall?

Unfortunately yes at this time. As far as I'm aware, the only way to retrieve the revert reason for a transaction is to execute an ethCall with said tx data. See for reference:

JS & Go similar implementation:
https://gist.github.com/gluk64/fdea559472d957f1138ed93bcbc6f78a
https://gist.github.com/msigwart/d3e374a64c8718f8ac5ec04b5093597f

https://ethereum.stackexchange.com/questions/48383/how-to-receive-revert-reason-for-past-transactions?rq=1

Original solidity merged PR that added the feature to v0.4.22: ethereum/solidity#3364

[iikirilov]

related to #1024 - seems like besu does not need the second call to get the revert reason

[iikirilov]

@lyotam Could you provide an example response of something that reverts please

[lyotam]

@lyotam Could you provide an example response of something that reverts please

Sure, as I added to ContractTest.java, Here is an example result for EthCall result that reverted:
https://github.com/web3j/web3j/pull/1026/files#diff-3809d1fdfbbb293f7594287e9771da95R247

However for transaction that reverts, there is no revert reason data available, for example:
TransactionReceipt
{
transactionHash='0xa130cab4317ba267e664b99f573f434c15d8fd9c579b14d067899b089ba3cd38',
transactionIndex='0x0',
blockHash='0xbdbff8a367f29f2628cbef2bc75654508c6d831c3a4f6fe0c2430d1f2322ecf9',
blockNumber='0xa9',
cumulativeGasUsed='0x0',
gasUsed='0x0',
contractAddress='null',
root='null',
status='0x0',
from='0xb3bdfe99c0fbb7b469dc4b031e08313e73e77ad7', to='0x830e22f5dfa3d660975d70d38038267ec470d00a',
logs=[], logsBloom='0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'
}

[iikirilov]

I see - so we can put the revertReason in the transaction receipt?

[lyotam]

I see - so we can put the revertReason in the transaction receipt?

We can add a field called revertReason to TransactionReceipt but I don't see how we can populate it unless we force an ethCall for each reverted transaction, an approach that I chose to avoid here but is possible of course.

Regarding Besu, it seems that they implemented an optional mechanism to produce the revertReason which needs to be enabled when starting the client which may use significant amount of memory: https://besu.hyperledger.org/en/stable/HowTo/Send-Transactions/Revert-Reason/

Another possibility is to add a flag, similar to Besu's approach , to the Contract sendTransaction method which if enabled will call ethCall and populate the revertReason field. This way will have a unified approach between Besu and other clients and also implement Besu's revertReason in the process.
What do you think?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[iikirilov]

Another possibility is to add a flag, similar to Besu's approach , to the Contract sendTransaction method which if enabled will call ethCall and populate the revertReason field. This way will have a unified approach between Besu and other clients and also implement Besu's revertReason in the process.
What do you think?

I like this approach - I think the flag should be true by default in web3j - i.e. by default if the transaction fails we will do an ethCall to get the revert reason.

[lyotam]

Another possibility is to add a flag, similar to Besu's approach , to the Contract sendTransaction method which if enabled will call ethCall and populate the revertReason field. This way will have a unified approach between Besu and other clients and also implement Besu's revertReason in the process.
What do you think?

I like this approach - I think the flag should be true by default in web3j - i.e. by default if the transaction fails we will do an ethCall to get the revert reason.

Added the changes as discussed. Also confirmed it is working with Besu & Quorum Transaction Managers (For Besu it will extract the reason when #1132 is fixed).

Please review.

[iikirilov]

LGTM