tx command fails to return transactions that can't be submitted

[mDuo13]

(Original issue: RIPD-1098)

The tx method cannot fetch pseudo-transactions, nor others transactions that aren't currently considered "valid" for submission, even if those transactions were incorporated into a validated ledger under rules that applied at the time. Doing so returns a misleading "not found" error. Example of a tx websocket command that will always fail (use a full-history server to confirm):

{
  "command": "tx",
  "transaction": "5B1F1E8E791A9C243DD728680F108FEF1F28F21BA3B202B8F66E7833CA71D3C3"
}

The transaction_entry method can fetch such transactions, provided you know what ledger contains them. Example of a transaction_entry websocket command that succeeds at fetching the same:

{
  "id": 2,
  "command": "transaction_entry",
  "tx_hash": "5B1F1E8E791A9C243DD728680F108FEF1F28F21BA3B202B8F66E7833CA71D3C3",
  "ledger_index": 21225473
}

Per @JoelKatz:

This occurs because the "tx" command asks the TransactionMaster to "fetch" the transaction, which calls Transaction::load. This function calls "checkValidity" to assure the returned transaction is valid. But this makes no sense. You should be able to retrieve transactions that are illegal under current rules. And pseudo-transactions are always illegal to submit, which is what checkValidity checks.

For historical retrieval, the only check should be that we can deserialize the binary transaction data, and we should return an error other than "not found" in that case (and we can include the ledger sequence in the RPC, which would be helpful).

So, the changes to make would be:

• Remove the validity check from the tx command. Return it if we can deserialize it.
• If we can't deserialize the transaction, return a different error. I suggest a new error code, txnDeserializingError. Also, this should be logged at the warning level, because rippled should be able to deserialize any transaction in valid history (even if that transaction is no longer valid under current rules).

[cjcobb23]

@mDuo13 This has been resolved here: https://github.com/ripple/rippled/commit/11cf27e00698dbfc099b23463927d1dac829ed19 Can you close this issue?

[mDuo13]

I'll test this out on a 1.5.0 beta server soon and close it when I confirm it works as expected.

[carlhua]

Closing this issue - this issue is fixed in https://github.com/ripple/rippled/pull/3167/commits/11cf27e00698dbfc099b23463927d1dac829ed19 and should have been closed by https://github.com/ripple/rippled/pull/3167/

[mDuo13]

This problem seems to have come back—but it only applies to Reporting Mode. (To repro: take the example requests from the OP and test them on s1/s2 vs xrplcluster.)

[vlntb]

If Reporting Mode is obsolete, is this ticket still relevant @mDuo13 ?

[mDuo13]

This exact problem actually came up in a Slack conversation just today, so it's still relevant, and should be changed in rippled.

It's confusing and misleading, potentially raising alarms, in cases where a historically-validated transaction (that can't be deserialized anymore) is reported as "not found", especially when that's a discrepancy with how Clio reports the transaction.

[ximinez]

I think one part of the problem is in STVar, specifically there is one ctor that takes a depth field, and checks

    if (depth > 10)
        Throw<std::runtime_error>("Maximum nesting depth of STVar exceeded");

That's probably a good starting point.