rippled icon indicating copy to clipboard operation
rippled copied to clipboard

account_objects: Disallow filtering by types that an account can't own.

Open mDuo13 opened this issue 1 year ago • 0 comments

Inspired by #4678, it would reduce confusion if the account_objects API method threw an error in cases where the user attempted to filter by a ledger entry type that accounts can't own.

(Note: Clio already does this. The documentation has always omitted mention of the types that accounts can't own.)

Summary

The account_objects method uses shared code to allow the user to specify a type of ledger entry and filter results to only the given type.

Some types, like the Amendments table, can't be owned by any account, so specifying that type in account_objects always returns an empty list. It might even be a paginated empty list, since the API implementation currently—as best I can tell—needs to walk the entire owner directory to confirm that no entry of the given type is present; it doesn't have a concept of which entries can or can't be present.

Source file: AccountObjects.cpp

Example request:

{
  "id": 1,
  "command": "account_objects",
  "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
  "ledger_index": "validated",
  "type": "fee",
  "limit": 10
}

Motivation

Users may be confused why they get no results if they expect something to appear that isn't there (this came up with AMM). An error more clearly explains the situation. Also, if they do, the server does extra work looking for something that doesn't exist.

Solution

The following types cannot ever appear in an owner directory, and should return an invalidParams error if the user specifies their short name in the type field of an account_objects request (short name in parens is what the user would specify for the command—see also #4393):

  • Amendments (amendments)
  • DirectoryNode (directory)
  • FeeSettings (fee)
  • LedgerHashes (hashes)
  • NegativeUNL (no short name—can't currently be filtered, but should be disallowed if a filter is added in the future)

Currently it's TBD whether or not an AMM entry gets added to the associated AMM-account's owner directory, but if not, it should be excluded also.

Filtering in the ledger and ledger_data methods should be unaffected. Those methods can return full ledger data, which does include these entry types, so it makes sense to allow filtering on them there.

Notes on API Compatibility

Because (a) the Clio API already works this way, and (b) the documentation only lists the types that can appear and would still be considered valid, I consider this a non-breaking change and think it does not need to be put on an API version switch.

mDuo13 avatar Aug 30 '23 18:08 mDuo13