horizon reports incorrect fee charges for fail fast transactions

[tomerweller]

When submitting erroneous transactions, which stellar-core fails instantaneously (without being submitted for consensus), the resulting XDR (TransactionResult) is shown and has an incorrect feeCharged value. It shows the value of fees offered, although they haven't been charged.

This is confirmed to happen with txBAD_AUTH, txBAD_AUTH_EXTRA, txTOO_EARLY and txINSUFFICIENT_FEE.

It does not happen with txBAD_SEQ and txNO_ACCOUNT. Probably because these have special handling in horizon.

According to @MonsieurNicolas, while these TransactionResult XDRs are coming from stellar-core, they should be presented differently to the clients, as the transactions were never submitted for consensus.

[MonsieurNicolas]

Looking at the different results that Horizon returns...

compare a "fast fail transaction":

{
  "type": "https://stellar.org/horizon-errors/transaction_failed",
  "title": "Transaction Failed",
  "status": 400,
  "detail": "The transaction failed when submitted to the stellar network. The `extras.result_codes` field on this response contains further details.  Descriptions of each code can be found at: https://www.stellar.org/developers/learn/concepts/list-of-operations.html",
  "extras": {
    "envelope_xdr": "AAAAAP4owAbexNelaqr+1bdrDCin1dvMVa4R2NLWuvFYJHYDAAAAZACFZP0AAAACAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAA/ijABt7E16Vqqv7Vt2sMKKfV28xVrhHY0ta68VgkdgMAAAAAAAAAAEgfIoAAAAAAAAAAAlgkdgMAAABAYLPUBFtSxFn4K8iLdIVS9smneYt0s+iPVvyoGNhnCvOyXBZ2pb8xaSpK5Ly8wk242oLMsRwj1kReU9JHyW/DAxRoo20AAABAwEeuMC/sMErk1okvlx2jbn/JoYjR2syKdyWp7kp1reF/Q4J06cpJD+VuTTt2hH6xHHP04Zeauwc86CNDJXKDCQ==",
    "result_codes": {
      "transaction": "tx_bad_auth_extra"
    },
    "result_xdr": "AAAAAAAAAGT////2AAAAAA=="
  }
}

with a "transaction failed during consensus":

{
  "type": "https://stellar.org/horizon-errors/transaction_failed",
  "title": "Transaction Failed",
  "status": 400,
  "detail": "The transaction failed when submitted to the stellar network. The `extras.result_codes` field on this response contains further details.  Descriptions of each code can be found at: https://www.stellar.org/developers/learn/concepts/list-of-operations.html",
  "extras": {
    "envelope_xdr": "AAAAAP4owAbexNelaqr+1bdrDCin1dvMVa4R2NLWuvFYJHYDAAAAZACFZP0AAAADAAAAAAAAAAAAAAABAAAAAAAAAAgAAAAA4d85QWXBamLF7ytpq6s/T6XJ6BEEwxqektkXCBRoo20AAAAAAAAAAVgkdgMAAABA8AEqVQ+aaopkdOj4ML+ppVDkux/24MOvOx9+cufSfmlycszZhktRB/fARS+zIII6Wvqv6FjXxRuuRd0og/FQBg==",
    "result_codes": {
      "transaction": "tx_failed",
      "operations": [
        "op_no_account"
      ]
    },
    "result_xdr": "AAAAAAAAAGT/////AAAAAQAAAAAAAAAI/////gAAAAA="
  }
}

with a "transaction succeeding during consensus":

{
  "_links": {
    "transaction": {
      "href": "https://horizon-testnet.stellar.org/transactions/8023206957eaef27e8c632356ef073abec850d2b2297a889bfae1148517fd001"
    }
  },
  "hash": "8023206957eaef27e8c632356ef073abec850d2b2297a889bfae1148517fd001",
  "ledger": 8742157,
  "envelope_xdr": "AAAAAP4owAbexNelaqr+1bdrDCin1dvMVa4R2NLWuvFYJHYDAAAAZACFZP0AAAABAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAA/ijABt7E16Vqqv7Vt2sMKKfV28xVrhHY0ta68VgkdgMAAAAAAAAAAACYloAAAAAAAAAAAVgkdgMAAABA/5xdjP2LOrDYdCTTuaLztgdA7XF/FxWuZ7+rTLO0zv/E1KtBJJxHxgNSgNjEpX+BPE14kpJCJVH184fj51j+DQ==",
  "result_xdr": "AAAAAAAAAGQAAAAAAAAAAQAAAAAAAAABAAAAAAAAAAA=",
  "result_meta_xdr": "AAAAAAAAAAEAAAAA"
}

I would expect the "transaction failed during consensus" to look pretty much like the last one (maybe keeping the result nested as to avoid the accidental consumption of "result_xdr"). In particular, it should have the ledger information for when it was rejected.
Also, the error message in that case is wrong: it failed during consensus, not when the transaction was submitted.

If we had this information in the result, then distinguishing a "fast fail" from a "real" failure (that most people don't care about), would be trivial.

Maybe something like this (which would be aligned with solving #309 ):

{
  "type": "https://stellar.org/horizon-errors/transaction_failed",
  "title": "Transaction Failed",
  "status": 400,
    "_links": {
    "transaction": {
      "href": "https://horizon-testnet.stellar.org/transactions/8023206957eaef27e8c632356ef073abec850d2b2297a889bfae1148517fd001"
    }
  },
  "hash": "8023206957eaef27e8c632356ef073abec850d2b2297a889bfae1148517fd001",
  "ledger": 8742157,
  "detail": "The transaction failed when processed by the stellar network. The `extras.result_codes` field on this response contains further details.  Descriptions of each code can be found at: https://www.stellar.org/developers/learn/concepts/list-of-operations.html",
  "extras": {
    "envelope_xdr": "AAAAAP4owAbexNelaqr+1bdrDCin1dvMVa4R2NLWuvFYJHYDAAAAZACFZP0AAAADAAAAAAAAAAAAAAABAAAAAAAAAAgAAAAA4d85QWXBamLF7ytpq6s/T6XJ6BEEwxqektkXCBRoo20AAAAAAAAAAVgkdgMAAABA8AEqVQ+aaopkdOj4ML+ppVDkux/24MOvOx9+cufSfmlycszZhktRB/fARS+zIII6Wvqv6FjXxRuuRd0og/FQBg==",
    "result_codes": {
      "transaction": "tx_failed",
      "operations": [
        "op_no_account"
      ]
    },
    "result_xdr": "AAAAAAAAAGT/////AAAAAQAAAAAAAAAI/////gAAAAA=",
    "result_meta_xdr": "AAAAAAAAAAEAAAAA"
  },
}

[nullstyle]

I'm just going to leave this right here with regards to error response changes for horizon: https://tools.ietf.org/html/rfc7807

[nullstyle]

Your proposal makes sense to me @MonsieurNicolas, with a couple of notes:

1. hash and ledger will need to be pushed down into the extras collection. While I think it makes sense for us to expose this information to differentiate transactions that fail during consensus, we still fundamentally needs to render this output as a Problem according to the existing horizon conventions.

2. We should just go ahead an reify the "failure phase" of a transaction submission into a property of the response as well. I don't want programmers checking to see if a json response defines a ledger property is defined to switch on this concept. I'd rather see downstream coders write code that looks like if (error.extras.failure_phase === "consensus") { ... } or something of similar complexity.

[2opremio]

@bartekn do you know if this is still the case?