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

Fix serialization of FeeSettings object #3165

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

Fix serialization of FeeSettings object #3165

mDuo13 opened this issue Nov 27, 2019 · 4 comments
Assignees
Labels
API Change Tech Debt Non-urgent improvements
Milestone

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Nov 27, 2019

The FeeSettings ledger object type has some weird behavior when it's serialized to JSON (In ledger_entry or ledger_data responses, or in the metadata of a SetFee transaction):

  • BaseFee is serialized as hex despite being an XRP amount (UInt64)
  • ReferenceFeeUnits, ReserveBase, and ReserveIncrement are serialized as JSON numbers, not strings as XRP usually is. (This is because they are actually UInt32 types instead of the usual UInt64.)

It would be nice to change the binary format of the FeeSettings object to make it use the proper data type for XRP amounts in all its XRP fields, but that's harder to fix because it would require an amendment.

@mDuo13
Copy link
Collaborator Author

mDuo13 commented Nov 16, 2022

Note that with the given data structures the maximum theoretical values for fee settings are, shall we say, strange. The reserve settings, which would make sense to be much larger than the costs of sending a transaction, are more limited.

@intelliot intelliot added this to the 1.10.0 milestone Feb 18, 2023
@intelliot
Copy link
Collaborator

Related: #4425

@intelliot
Copy link
Collaborator

@mDuo13 can you confirm this issue has been fixed as of what's in develop now?

@intelliot
Copy link
Collaborator

mDuo13 noted that this is probably fine. If anyone sees any potential issues, please comment here or create a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change Tech Debt Non-urgent improvements
Projects
Archived in project
Development

No branches or pull requests

3 participants