TRN-609 Improve transact error handling

[JCSanPedro]

Front-end developers often encountered a misleading error message 1010: Invalid Transaction: Transaction has a bad signature when working the xrpl pallet. This would usually be due to an unrelated problem, such as the sending account not having sufficient balance to pay for the extrinsic. Seeing the incorrect error message could be very confusing.

The above error message is correlated with the error InvalidTransaction::BadProof. The runtime would mistakingly return this error because the xrpl (and doughnut) pallet have additional extrinsic validation applied to their runtime calls, implemented via the SelfContainedCall trait that was returning the wrong value. The method validate_self_contained returns a type Option<TransactionValidityError>, where if the return value was None, the validate implementation for CheckedExtrinsics from evm frontier would default to InvalidTransaction::BadProof https://github.com/polkadot-evm/frontier/blob/master/primitives/self-contained/src/checked_extrinsic.rs#L93-L95.

This PR aims to ensure the correct InvalidTransaction variant is propagated when the runtime call is validated.

Release Notes

Error Messages:

Changed:

doughtnut: Invalid transactions should be correct variant instead of catch-all BadProof
xrpl: Invalid transactions should be correct variant instead of catch-all BadProof

[zees-dev]

Nice; LGTM 👍

[JasonTulp]

lgtm, nice work! Will definitely make it more clear to external devs. Although interesting that only one test needs changing in doughnuts, is this an indication that we need better test coverage?

[JCSanPedro]

Although interesting that only one test needs changing in doughnuts, is this an indication that we need better test coverage

Yes I think so. I found that for the Doughnut pallet tests, the tx would fail at check_self_contained before failing at validate_self_contained. Needs more exploration to figure out how to correctly write the tests so that it hits the code we expect it to.