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

ignore_default option on account_lines #3980

Closed
wants to merge 1 commit into from

Conversation

RichardAH
Copy link
Collaborator

High Level Overview of Change

Add ignore_default flag to account_lines. This flag suppresses the output of incoming trustlines in default state.

Context of Change

Users of XUMM often have unwanted incoming trustlines in default state, these are not useful in the vast majority of cases. Ability to suppress these in account_lines saves bandwidth and resources.

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)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

@WietseWind
Copy link
Member

WietseWind commented Nov 15, 2021

@nbougalis FYI.

This would help reduce the data fetched for all XUMM users significantly on "mixed accounts" (issuing and holding) like we see a lot of these days.

@nbougalis nbougalis self-requested a review November 17, 2021 03:45
@nbougalis nbougalis added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Nov 17, 2021
@nbougalis
Copy link
Contributor

@manojsdoshi I'm proposing this for inclusion into 1.8.0-rc3, since it helps reduce the load for Xumm (and the server).

@WietseWind
Copy link
Member

@manojsdoshi @nbougalis What's the status on this?

This was referenced Dec 15, 2021
Comment on lines +210 to +232
bool ignore = false;
if (visitData.ignoreDefault)
{
if (sleCur->getFieldAmount(sfLowLimit).getIssuer() ==
visitData.accountID)
ignore =
!(sleCur->getFieldU32(sfFlags) & lsfLowReserve);
else
ignore = !(
sleCur->getFieldU32(sfFlags) & lsfHighReserve);
}

auto const line =
RippleState::makeItem(visitData.accountID, sleCur);
if (line != nullptr &&
(!visitData.hasPeer ||
visitData.raPeerAccount == line->getAccountIDPeer()))
{
visitData.items.emplace_back(line);
if (!ignore)
visitData.items.emplace_back(line);

visitData.lastFound = line;
visitData.foundCount++;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be somewhat more readable if ignore was include. So it would look something like:

bool include = true;
if (visitData.ignoreDefault)
{
    if (sleCur->getFieldAmount(sfLowLimit).getIssuer() ==
       visitData.accountID)
       include = sleCur->getFieldU32(sfFlags) & lsfLowReserve;
    else
        ignore = sleCur->getFieldU32(sfFlags) & lsfHighReserve;
}
[...]
{
    if (include)
        visitData.items.emplace_back(line);
[...]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants