Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "fee units" from API #3164

Open
mDuo13 opened this issue Nov 27, 2019 · 4 comments
Open

Remove "fee units" from API #3164

mDuo13 opened this issue Nov 27, 2019 · 4 comments

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Nov 27, 2019

Fee units are used internally to calculate fees with higher precision than integer drops, but are exposed directly in the ledger subscription stream (possibly other places, though I don't see any immediately). These fields are more confusing than helpful; and in the ledger stream at least the exact XRP values (which are actually useful) are reported anyway.

Fee units should be an "internal only" property that is not exposed in the API.

Note, "fee units" are different from "fee levels". I'm willing to entertain the argument that "fee levels" should also be removed because they are confusing, but "fee levels" do have some use to API users, whereas "fee units" don't. Fee levels show up in the queuing endpoints, and compare the relative costs of transactions with different minimum fees. That's important for knowing which transactions will be queued or evicted from the queue first. For example, an EscrowFinish transaction containing a 32-byte crypto-condition preimage paying the minimum of 350 drops is ranked lower than a basic AccountSet transaction paying a "double" cost of 20 drops.

@mDuo13
Copy link
Collaborator Author

mDuo13 commented Nov 19, 2020

Using this commit as a template, I think the fix for this is to wrap both of the following fee_ref lines from NetworkOPs in a condition kind of like this:

if (context.apiVersion == 1)

In subLedger(): https://github.com/ripple/rippled/blob/0b4e34b03b72196510998375fd28622c8b63aaa7/src/ripple/app/misc/NetworkOPs.cpp#L2887

In pubLedger():
https://github.com/ripple/rippled/blob/0b4e34b03b72196510998375fd28622c8b63aaa7/src/ripple/app/misc/NetworkOPs.cpp#L3287

I started to make the commit myself but realized I don't know the appropriate way to get the right context into the subscription info, especially since multiple clients could be subscribed using different API versions.

@mvadari
Copy link
Collaborator

mvadari commented Nov 1, 2023

Is this resolved by XRPFees?

@ximinez
Copy link
Collaborator

ximinez commented Nov 2, 2023

Is this resolved by XRPFees?

I think so.

@intelliot
Copy link
Collaborator

We can keep this issue open until XRPFees successfully activates (and @mDuo13 confirms that it fully resolves this issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants