XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.48k stars 1.44k forks source link

fix "account_nfts" with unassociated marker returning issue #5045

Open yinyiqian1 opened 2 weeks ago

yinyiqian1 commented 2 weeks ago

fix #4314

High Level Overview of Change

Context of Change

Type of Change

API Impact

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

:exclamation: 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 [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/5045/graphs/tree.svg?width=650&height=150&src=pr&token=i2RPGI5xGF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)](https://app.codecov.io/gh/XRPLF/rippled/pull/5045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) ```diff @@ 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](https://app.codecov.io/gh/XRPLF/rippled/pull/5045?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) | Coverage Δ | | |---|---|---| | [src/ripple/rpc/handlers/AccountObjects.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5045?src=pr&el=tree&filepath=src%2Fripple%2Frpc%2Fhandlers%2FAccountObjects.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9ycGMvaGFuZGxlcnMvQWNjb3VudE9iamVjdHMuY3Bw) | `92.9% <100.0%> (ø)` | | ... and [1591 files with indirect coverage changes](https://app.codecov.io/gh/XRPLF/rippled/pull/5045/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/5045/graphs/tree.svg?width=650&height=150&src=pr&token=i2RPGI5xGF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)](https://app.codecov.io/gh/XRPLF/rippled/pull/5045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)
intelliot commented 1 week ago

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.