wallet: SendMoney does not report error correctly #14187

CommitTransaction returns true even if AcceptToMemoryPool fails, thus an error is not correctly thrown in send money rpc calls. Failure of RelayWalletTransaction isn't captured either.

I've closed the PR. Wasn't aware of this as a use case until I saw the wallet_basic unit test.

Would there be a case when catching the CommitTransaction error in the rpc calls would be useful?

I think you raise a fair point, though yes, it's not entirely predetermined what should be considered an error sending.

Temporary error conditions such as being unable to commit to the mempool (it might be accepted later) or relay (the wallet will try again later) are qualitatively different from a permanent failure which prevented the transaction from being created.

Ideally, these could be returned as part of a result tuple. However, the function returns a bare string, not a result tuple so that would require a breaking API change.

This is already a TODO in the code, no need to keep an issue open for each TODO.

        // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.