fix(relayer): return revert error instead of nil

[bufrr]

Fixed condition that checks receipt status in function, ensuring revert in case of unsuccessful transactions

[cyberhorsey]

We are not returning nil, we are returning err.

[bufrr]

We are not returning nil, we are returning err.

err will definitely be nil in this scenario, please review again @cyberhorsey

[cyberhorsey]

We are not returning nil, we are returning err.

err will definitely be nil in this scenario, please review again @cyberhorsey

Ah, you're correct. However, we should try and get the actual revert reason here, and not return a generic error, as the proper fix. And then slog.Error the revert reason, if it was able to be decoded.

[bufrr]

We are not returning nil, we are returning err.

err will definitely be nil in this scenario, please review again @cyberhorsey

Ah, you're correct. However, we should try and get the actual revert reason here, and not return a generic error, as the proper fix. And then slog.Error the revert reason, if it was able to be decoded.

Sure, let me update the fix

[cyberhorsey]

After looking at this code, this branch can not be hit. Because in sendProcessMessageCall, we have this:

    if receipt.Status != types.ReceiptStatusSuccessful {
        relayer.MessageSentEventsProcessedReverted.Inc()

        return nil, errTxReverted
    }

and thus:

    receipt, err := p.sendProcessMessageCall(ctx, msgBody.Event, encodedSignalProof)
    if err != nil {
        return false, msgBody.TimesRetried, err
    }

Will capture the scneario where the receipt status is not successful.

So, the above scenario is actually a duplicate status check left from a refactor, and if any improvement is to be made here, it would be to remove the top-level check, and replace it with decoding a revert reason and logging it.