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

account_lines paginating when there is only one "line" (Version: 1.9.4) #4340

Closed
zgrguric opened this issue Nov 10, 2022 · 2 comments · Fixed by #4361
Closed

account_lines paginating when there is only one "line" (Version: 1.9.4) #4340

zgrguric opened this issue Nov 10, 2022 · 2 comments · Fixed by #4361

Comments

@zgrguric
Copy link

zgrguric commented Nov 10, 2022

Issue Description

account_lines command of account rBLadExFZKY7AqpTDxqSoWJgmgVZyA6wcX returns marker where there should be no markers.

Also with limit of 100 there is no "lines" returned, when querying 200 there is 1 item in "lines" returned.

Note: There are lot of NFTs in this account.

Steps to Reproduce

Use https://xrpl.org/websocket-api-tool.html#account_lines

{
  "id": 2,
  "command": "account_lines",
  "account": "rBLadExFZKY7AqpTDxqSoWJgmgVZyA6wcX",
  "ledger_index": "validated",
  "limit":100
}

This above returns marker and lines response is empty.

{
  "id": 2,
  "command": "account_lines",
  "account": "rBLadExFZKY7AqpTDxqSoWJgmgVZyA6wcX",
  "ledger_index": "validated",
  "limit":200
}

This above returns marker and lines response returns one (1) item in "lines".
Using marker returns "invalidParams" error.

Results are consistant for xrplcluster.com, s1.ripple.com and s2.ripple.com.

Expected Result

No marker, only one trustline returned with no limit defined or limit 1 minimum,

Actual Result

As you can see "marker" value filled and "lines" is empty.

{
  "id": 2,
  "result": {
    "account": "rBLadExFZKY7AqpTDxqSoWJgmgVZyA6wcX",
    "ledger_hash": "772BB5FB46D0E476E51D03AC8BF2F8992D0461910806FC71545AF63F4A47FE92",
    "ledger_index": 75654696,
    "limit": 100,
    "lines": [],
    "marker": "36E86F954365F2723351DB927416B40330D47E064FCE0365FCB6A09C5E5A80F1,4",
    "validated": true,
    "warnings": [
      {
        "id": 1004,
        "message": "This is a reporting server.  The default behavior of a reporting server is to only return validated data. If you are looking for not yet validated data, include \"ledger_index : current\" in your request, which will cause this server to forward the request to a p2p node. If the forward is successful the response will include \"forwarded\" : \"true\""
      }
    ]
  },
  "status": "success",
  "type": "response"
}

Supporting Files

Discovered by KegsRP https://twitter.com/KegsRp/status/1590593708130697216

@ximinez
Copy link
Collaborator

ximinez commented Dec 2, 2022

I've been able to reproduce this, and should have a fix PR soon.

ximinez added a commit to ximinez/rippled that referenced this issue Dec 2, 2022
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
ximinez added a commit to ximinez/rippled that referenced this issue Dec 2, 2022
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
@ximinez
Copy link
Collaborator

ximinez commented Dec 3, 2022

#4361 resolves the issue with the marker being invalid. It is expected that fewer than limit lines may be returned along with a marker, because of an optimization released in 1.8.2 to reduce load on rippled servers by counting all objects up to the limit instead of just the requested object type.

Please read the description of that PR for more details.

ximinez added a commit to ximinez/rippled that referenced this issue Dec 9, 2022
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
ximinez added a commit to ximinez/rippled that referenced this issue Dec 14, 2022
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
ximinez added a commit to ximinez/rippled that referenced this issue Dec 16, 2022
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
ximinez added a commit to ximinez/rippled that referenced this issue Jan 4, 2023
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
ximinez added a commit to ximinez/rippled that referenced this issue Jan 5, 2023
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
intelliot pushed a commit that referenced this issue Mar 20, 2023
There were situations where `marker`s  returned by `account_lines` did
not work on subsequent requests, returning "Invalid Parameters".

This was caused by the optimization implemented in "Enforce account RPC
limits by account objects traversed":

e289896?diff=unified&w=1

Previously, the ledger traversal would find up to `limit` account lines,
and if there were more, the marker would be derived from the key of the
next account line. After the change, ledger traversal would _consider_
up to `limit` account objects of any kind found in the account's
directory structure. If there were more, the marker would be derived
from the key of the next object, regardless of type.

With this optimization, it is expected that `account_lines` may return
fewer than `limit` account lines - even 0 - along with a marker
indicating that there are may be more available.

The problem is that this optimization did not update the
`RPC::isOwnedByAccount` helper function to handle those other object
types. Additionally, XLS-20 added `ltNFTOKEN_OFFER` ledger objects to
objects that have been added to the account's directory structure, but
did not update `RPC::isOwnedByAccount` to be able to handle those
objects. The `marker` provided in the example for #4354 includes the key
for an `ltNFTOKEN_OFFER`. When that `marker` is used on subsequent
calls, it is not recognized as valid, and so the request fails.

* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
* Fix #4340
* Fix #4354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants