Display fees charged in /fee_stats endpoint

[bartekn]

In #1358 we introduced new fields in Transaction resource:

• fee_charged - a fee actually paid for a transaction (txresult.FeeCharged)
• max_fee - a maximum fee source account is willing to pay (transaction.Fee)

The /fee_stats endpoint displays stats for fee bids (using max_fee value) instead of the fees that were actually paid for transactions. This can be confusing for users that can bid even higher fees.

Basically the main problem is that field names (accepted_fee suffix) are really confusing in a new context.

[MonsieurNicolas]

I think the whole purpose of the fee stats endpoint is to allow to outbid other people, so using the effective fee will break that property. The fee auction takes care of ensuring that people pay the least possible.

What actual problem are you trying to solve with those changes?

[bartekn]

I think there are multiple issues with the current state of this endpoint (after #1358):

• It can be confusing for users. The fields' names in /fee_stats have accepted_fee suffix. "Accepted" fee suggests that nothing below the value presented will be accepted.
• This can lead to constantly increasing fees as people will try to outbid imaginary bids (imaginary because max_fee is not what people really pay for a transaction).
  Consider that we have n users that need to decide on a fee they will use for their transactions. They are presented with /fee_stats endpoint and they think that it represents stats for fee_charged (but in reality it's for max_fee). Each of n users comes after the previous one submitted a transaction and bids something between p75 and max_bid*1.25. After a very short time the minimum fee_charged of txs in the ledger will start growing (as smaller max_fees will be quickly outbid by higher bids).
• This can make people think that Stellar is fees are not so small anymore (and will be higher if my reasoning above is correct).

Maybe we should do what you propose but in a smooth way? Like: First expose extra stats connected to max_fee, then deprecate fee_charged stats and finally remove fee_charged stats after weeks of educating people what these stats really mean.

[tomquisel]

I think returning max_fee values in /fee_stats makes sense as well. We want people to know what bids they need to out-compete to have their transaction be processed. max_fee-based stats gives actionable info to achieve that goal.

That said, I really like @bartekn's proposal of doing it a smooth way. In fact, I'd support returning both kinds of stats in the endpoint permanently. I think it's useful to know both (so you can have a sense for what you have to bid, and also know what you can expect to pay).

[leighmcculloch]

The term "Accepted fee" communicates to me that this was the fee that was actually paid, where-as I just discovered (#1857 (comment)) that it is actually the maximum fee defined in a transaction that was accepted into the network. Given that there are multiple incorrect interpretations there's very likely to be others making the wrong assumptions too.

+1 to changing the name of that concept or including both stats to clear up any ambiguity about what the fee represents.

[ire-and-curses]

This confused me again recently. I think it's not good in the current state and we need to take some action. I don't see an easy way to deprecate this gradually because the current name (accepted suffix) is so misleading for what is implemented. I'll also argue that gradual deprecation is less urgent now that actual fee paid is calculated dynamically on submission.

Proposal:

1. Change meaning of accepted so it means "the fee that was actually paid"
2. Rename accepted -> fee_charged
3. Add new stat max_fee that means "the maximum fee that transactors were willing to pay"

This then matches the naming in the Transaction endpoint. I think the consistency is pretty important (why did we choose "accepted" before?).

The only downside I see is the unfortunate naming of the new fields min_max_fee and mode_max_fee. But I think it will be understandable from the repeated context with min_fee_charged.

Proposed fee_stats endpoint would look like this:

{
  "last_ledger": "306006",
  "last_ledger_base_fee": "100",
  "ledger_capacity_usage": "0.35",
  "min_fee_charged": "100",
  "mode_fee_charged": "100",
  "p10_fee_charged": "100",
  "p20_fee_charged": "100",
  "p30_fee_charged": "100",
  "p40_fee_charged": "100",
  "p50_fee_charged": "100",
  "p60_fee_charged": "100",
  "p70_fee_charged": "100",
  "p80_fee_charged": "100",
  "p90_fee_charged": "100",
  "p95_fee_charged": "100",
  "p99_fee_charged": "100",
  "min_max_fee": "100",
  "mode_max_fee": "100",
  "p10_max_fee": "100",
  "p20_max_fee": "100",
  "p30_max_fee": "100",
  "p40_max_fee": "100",
  "p50_max_fee": "100",
  "p60_max_fee": "100",
  "p70_max_fee": "100",
  "p80_max_fee": "100",
  "p90_max_fee": "100",
  "p95_max_fee": "100",
  "p99_max_fee": "100",
}

[leighmcculloch]

I think we should improve this in a backwards compatible way.

I think we should only add new fields. While accepted_fee is easily misunderstood, it is less so when presented alongside charged_fee because there's enough tension that accepted can't mean the same as charged since there is a field named charged.

@@ -2,6 +2,19 @@
   "last_ledger": "26856530",
   "last_ledger_base_fee": "100",
   "ledger_capacity_usage": "0.09",
+  "min_charged_fee": "100",
+  "mode_charged_fee": "100",
+  "p10_charged_fee": "100",
+  "p20_charged_fee": "100",
+  "p30_charged_fee": "100",
+  "p40_charged_fee": "100",
+  "p50_charged_fee": "100",
+  "p60_charged_fee": "100",
+  "p70_charged_fee": "100",
+  "p80_charged_fee": "100",
+  "p90_charged_fee": "100",
+  "p95_charged_fee": "100",
+  "p99_charged_fee": "100",
   "min_accepted_fee": "100",
   "mode_accepted_fee": "100",
   "p10_accepted_fee": "100",

[abuiles]

@ire-and-curses @leighmcculloch @bartekn @tamirms I agree with both Eric and Leigh 💔

I think the suffix _fee_charged and _max_fee like Eric suggests makes more sense and aligns the response with what we do in /transactions, but, I also agree with Leigh that we should fix this without introducing breaking changes.

I'd like to propose a third option which would be to partially follow Eric's plan, introducing the the suffix _fee_charged and _max_fee, without removing _accepted_fee (which we can mark for deprecation in a later version)

Along with this, we'll have to update the docs to explain that _accepted_fee is an alias to _max_fee, and explain what _max_fee and _fee_charged means.

We have to update the documentation anyways, since the message conveyed today is that the value you are getting actually referrers to _fee_charged not _max_fee - https://www.stellar.org/developers/horizon/reference/endpoints/fee-stats.html

[leighmcculloch]

That sounds like a good idea to me.

Having two lots of these values at the top-level was going to be noisy, having three is going to be a lot I think. If we do this could we group the new fields with common sub-fields?

Example:

{
  "last_ledger": "306006",
  "last_ledger_base_fee": "100",
  "ledger_capacity_usage": "0.35",
  "fee_charged": {
    "min": "100",
    "mode": "100",
    "p10": "100",
    "p20": "100",
    "p30": "100",
    "p40": "100",
    "p50": "100",
    "p60": "100",
    "p70": "100",
    "p80": "100",
    "p90": "100",
    "p95": "100",
    "p99": "100",
  },
  "max_fee": {
    "min": "100",
    "mode": "100",
    "p10": "100",
    "p20": "100",
    "p30": "100",
    "p40": "100",
    "p50": "100",
    "p60": "100",
    "p70": "100",
    "p80": "100",
    "p90": "100",
    "p95": "100",
    "p99": "100",
  },
  "min_accepted_fee": "100",
  "mode_accepted_fee": "100",
  "p10_accepted_fee": "100",
  "p20_accepted_fee": "100",
  "p30_accepted_fee": "100",
  "p40_accepted_fee": "100",
  "p50_accepted_fee": "100",
  "p60_accepted_fee": "100",
  "p70_accepted_fee": "100",
  "p80_accepted_fee": "100",
  "p90_accepted_fee": "100",
  "p95_accepted_fee": "100",
  "p99_accepted_fee": "100",
}

[ire-and-curses]

works for me! Also I like the nesting suggestion @leighmcculloch

[abuiles]

@leighmcculloch I like that approach! The only cons is that it might require "extra" handling in some languages vs dot notation on JS, but I think that's a low-level detail and a problem which has been solved already. I'll move ahead with this solution.

[leighmcculloch]

it might require "extra" handling in some languages

@abuiles Given that our JSON API serves up complex responses containing _links and _embedded fields I don't think the nesting is going to introduce a new problem.

[bartekn]

Yep, @leighmcculloch's solution LGTM!

While we're at it I suggest to add p100/max. The reasoning behind it is that /fee_stats should allow you to outbid others if you want to. Optionally, we can remove p95 and p99. These two values seem redundant when p90 and p100 is there. Can create a new issue if it's controversial.

[abuiles]

@bartekn

Optionally, we can remove p95 and p99. These two values seem redundant when p90 and p100 is there. Can create a new issue if it's controversial.

Sounds good to me and since this is new data, we can use any format we want :)

[leighmcculloch]

+1 to adding max.

I think p95 and p99 are still going to be of interest, even if we add max.

If we add the max field can we call it max instead of p100 so it aligns with the existing min field. It's more easily understood by folks who haven't encountered percentiles.

[bartekn]

Fixed in #1964.