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

Request: Parse flags in account_info method #2457

Closed
mDuo13 opened this issue Mar 24, 2018 · 6 comments · Fixed by #4459
Closed

Request: Parse flags in account_info method #2457

mDuo13 opened this issue Mar 24, 2018 · 6 comments · Fixed by #4459
Labels
Added to API Changelog API changes have been documented in API-CHANGELOG.md API Change Feature Request Used to indicate requests to add new features Reviewed

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Mar 24, 2018

One fairly simple bit of functionality that's more complicated for users than it needs to be is parsing account flags. We can make things friendlier for users by modifying the account_info command to break the flags up into separate booleans in the response.

Performing bitwise operations on the Flags field is not inherently difficult, but it is an extra step that's easy to get wrong, because there are three kinds of account flags and it's easy to mix them up. The three types of flags are account-set flags (asf*), AccountSet transaction flags (tf*), and AccountRoot ledger-state flags (lsf*). For purposes of account_info, we only need to care about the ledger-state flags.

I propose adding an account_flags object to the top level of the account_info response, alongside account_data, signer_lists, etc. This object would be a map flags enabled for the account. I recommend using the names of the fields from RippleAPI so we can be consistent across interfaces. (The lsf* names are currently used in the code and documentation but never exposed in the rippled APIs.)

The existing convention of the rippled APIs would dictate that the account_flags object in the response should contain only flags for which the account's value is not in the default state. Since all flags default to false, all fields which appear in the response would have the value true. I disagree with this design and believe that the response should always contain fields for all flags, whether their values are true or false. (That makes it easier to write simple code to check a flag's value, and more closely matches the design of RippleAPI.)

RippleAPI Name Code Name Description
defaultRipple lsfDefaultRipple If true, new trust lines to this address have rippling enabled by default on this account's side.
depositAuth lsfDepositAuth If true, the account can't be the destination of payments, etc.
disableMasterKey lsfDisableMaster If true, the account can't send transactions signed with its master key pair.
disallowIncomingXRP lsfDisallowXRP If true, client applications should reject or prompt before sending XRP to this account.
globalFreeze lsfGlobalFreeze If true, all currencies issued by this account are frozen.
noFreeze lsfNoFreeze If true, this account has given up the power to selectively freeze issued currencies.
passwordSpent lsfPasswordSpent If true, this account has used its free key reset.
requireAuthorization lsfRequireAuth If true, trust lines to this account require approval before they can hold currency issued by this account.
requireDestinationTag lsfRequireDestTag If true, transactions that send money to this account must specify a destination tag.
@sublimator
Copy link
Contributor

sublimator commented Mar 24, 2018 via email

@carlhua carlhua added Good First Issue Great issue for a new contributor Low Priority Reviewed labels Sep 16, 2020
@intelliot intelliot added the Feature Request Used to indicate requests to add new features label Nov 28, 2022
@intelliot
Copy link
Collaborator

Although RippleAPI is now obsolete, I think the RippleAPI names for these flags are still good. Of course, if we know of a better name for one (or more) of these fields, we should feel free to propose it as well.

@intelliot
Copy link
Collaborator

@godexsoft @officialfrancismendoza What are your thoughts on addressing this in Clio?

@godexsoft
Copy link
Collaborator

@intelliot would not be a high priority for clio but certainly can add to our backlog if @mDuo13 confirms this is still desired (it's been ~5 years).

@ximinez
Copy link
Collaborator

ximinez commented Feb 7, 2023

I'm not @mDuo13 , but I still think this would be a good idea. It's amazing how time flies when there are other priorities.

@github-project-automation github-project-automation bot moved this to 🆕 New in Core Ledger Feb 8, 2023
@intelliot intelliot moved this from 🆕 New to 📋 Backlog in Core Ledger Feb 8, 2023
@justinr1234
Copy link
Collaborator

justinr1234 commented Feb 8, 2023

I completely agree with the premise of this issue and the proposed implementation. Needing to do bitwise operations when making JSON API requests is definitely not a nice time.

While you could argue that "the client should deal with it", overall I think it makes a lot of sense to tighten up the API returned by clio/rippled to be a little more in line with the modern web. Providing nice/elegant interfaces that make interop with various tools easier improves developer happiness and experience which IMO is a high priority.

drlongle added a commit to drlongle/rippled that referenced this issue Mar 13, 2023
@intelliot intelliot moved this from 📋 Backlog to 👀 Needs review in Core Ledger Mar 13, 2023
@intelliot intelliot removed Good First Issue Great issue for a new contributor Request for Comments labels Mar 13, 2023
intelliot pushed a commit that referenced this issue Mar 30, 2023
Previously, the object `account_data` in the `account_info` response
contained a single field `Flags` that contains flags of an account. API
consumers must perform bitwise operations on this field to retrieve the
account flags.

This change adds a new object, `account_flags`, at the top level of the
`account_info` response `result`. The object contains relevant flags of
the account. This makes it easier to write simple code to check a flag's
value.

The flags included may depend on the amendments that are enabled.

Fix #2457.
@intelliot intelliot moved this from 👀 Needs review to ✅ Merged in Core Ledger Apr 10, 2023
@manojsdoshi manojsdoshi moved this from ✅ Merged to Testing in Core Ledger Apr 21, 2023
@manojsdoshi manojsdoshi moved this from Testing to Ready for the release branch in Core Ledger Jun 5, 2023
@intelliot intelliot moved this from Ready for the release branch to 🚢 Released in 1.11 in Core Ledger Jun 21, 2023
@intelliot intelliot added the Added to API Changelog API changes have been documented in API-CHANGELOG.md label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added to API Changelog API changes have been documented in API-CHANGELOG.md API Change Feature Request Used to indicate requests to add new features Reviewed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants