Check tx status more thoroughly
THIS HAS BECOME JUST AN OPTIMIZATION CARD (explained in the comments at the bottom)
Originally I thought we would replace eth_getTransactionReceipt for eth_getTransactionByHash, but it turned out we don't want to. We'd rather have both methods and combine the knowledge that they provide.
The benefit of having the new one is mainly that it can reflect on if a transaction has been lost through the process of its processing. There are some clues this is possible (the AI's insights) and we actually experienced some weird stuff during a QA testing where the Nodes were examined in their databases and we found some left pending payable entries which we intuitively went to check up by the blockchain explorer, however, it didn't find any transactions like that at all. It happened a long time after they were supposed to be submitted, so it cannot be because of the data of the Blockchain Node not up to date yet. Therefore, we probably should pay some attention to this phenomenon too. Even though, as stated in the comment below, we are not hinged on this and we can settle for considering these vanished transaction (while not actually knowing this has been the case) just some general failed transaction, because if nothing else, we do know that they did not succeed in the time interval that we provide, and so it shares the spot with real transactions just too long pending, also considered a failure. Yeah, actually the vanished ones will be evaluated exactly like those (inaccurate but good enough at the moment).
Consider implementing the new method so that it's called after the call of eth_getTransactionReceipt and only for transactions that are still pending based on the status we read from the first call. That means we don't need to make this second call for mined transactions (completed, successful,...) and obvious failures, reported through the rich data we get back upon the first call. (Actually, we are able to process only the field with status that evaluates to a failure if it is 0. @utkarshg6 believes, though, we could mine even more from the data, maybe by looking into the event logs bundled with it, or the topics. It would be quite great if we could use it to determine even the failure reason one day.)
Hi @bertllll!
To recognise whether a transaction has failed or is successful then we'll need to call eth_getTransactionReceipt, as it can't be determined by eth_getTransactionByHash.
I'm unsure at the moment, whether we should definitely replace eth_getTransactionReceipt or not?
As long as we are going to retry the transaction after a certain duration, it doesn't matter that much.
I think the eth call we are using is the better choice.
During GH-744 Syther and I introduced this status, which is working algood. I'm confident that's all we need.
pub enum TxStatus {
Failed,
Pending,
Succeeded(TransactionBlock),
}
That's just something I discovered.
Okay, we had a discussion and agreed that the eth_getTransactionReceipt should stay in our arsenal and we should only consider adding the other method too. The way we understand it, this method allows to answer if the transaction is being even noticeable or registered somehow, which probably cannot be achieved with the former method. We believe that we can leave the code without this extra recognition and just consider a failure (of an unclear reason) because it means that we will pass the time for transaction to complete and it will be commanded a retry anyway.