fix "account_nfts" with unassociated marker returning issue

[yinyiqian1]

fix #4314

High Level Overview of Change

Context of Change

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Performance (increase or change in throughput and/or latency)
• Tests (you added tests for code that already exists, or your new feature included in this PR)
• Documentation update
• Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
• Release

API Impact

• Public API: New feature (new methods and/or new fields)
• Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• Peer protocol change (must be backward compatible or bump the peer protocol version)

[shawnxie999]

Good job! left some comments.

also I'm wondering if this should be part of a new API version instead? @intelliot do you know if such API bug changes should introduce a new version or can we just change directly? (edit: nevermind i don't think it needs it since it's not breaking change)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.3%. Comparing base (ef02893) to head (7cfe5b2).

❗ Current head 7cfe5b2 differs from pull request most recent head bf284e7

Please upload reports for the commit bf284e7 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5045     +/-   ##
=========================================
- Coverage     71.3%   71.3%   -0.0%     
=========================================
  Files          796     796             
  Lines        66987   66978      -9     
  Branches     10892   10870     -22     
=========================================
- Hits         47793   47774     -19     
- Misses       19194   19204     +10     

Files	Coverage Δ
src/ripple/rpc/handlers/AccountObjects.cpp	92.9% <100.0%> (ø)

... and 1591 files with indirect coverage changes

[intelliot]

I'm wondering if this should be part of a new API version instead? @intelliot do you know if such API bug changes should introduce a new version or can we just change directly? (edit: nevermind i don't think it needs it since it's not breaking change)

Right - I see this as a bug fix. However, if we know with certainty that there are API consumers who will be broken by this change, then it would also be ok to put it on api_version 3. But currently, I don't think that is needed.

[scottschurr]

Thanks for this pull request. This looks like a good change. I left a number of comments for you to consider. Most of the comments are optional, but I think they are good guidance. However the implementation of the createFakeNFTMarker lambda really needs to get fixed.

In case it's useful, I piled all of the changes I'm suggesting into the top-most commit here: https://github.com/scottschurr/rippled/commits/yinyiqian1-br_fix_nft_page/ You can look at the commit to see what I intended with my suggestions. You are also welcome to cherry-pick the commit if you decide you want to adopt all of the suggested changes.

I hope that helps. Feel free to ask further questions about any of the comments that I left. And thanks again for the pull request.

[shawnxie999]

LGTM

[scottschurr]

👍 Looks great! Thanks for the submission.