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

Improve checking of the path lengths during Payments #4519

Closed
wants to merge 6 commits into from

Conversation

a-noni-mousse
Copy link
Contributor

High Level Overview of Change

The code that does the check of the payment path lengths was sometimes executed but without an effect so change it to only work when it matters and to not make not necessary copies of the path vectors.

Context of Change

The code checks size of payment path lengths but it is only pertinente when executing again open view and doing the work when not open.
This is only start I think there are also other issue with the paths codes that need to be fixed.

Type of Change

  • Refactor (non-breaking change that only restructures code)

Test Plan

Do the unit tests.

Copy link
Contributor

@nbougalis nbougalis left a comment

Choose a reason for hiding this comment

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

One bug needs to be addressed; once it is, this gets a 👍 from me. Nice find.

src/ripple/app/tx/impl/Payment.cpp Outdated Show resolved Hide resolved
@intelliot intelliot added this to the 1.12 milestone May 24, 2023
@intelliot intelliot modified the milestones: 1.12, 1.13 Jul 12, 2023
@intelliot intelliot requested a review from ChronusZ July 12, 2023 02:55
Copy link
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@intelliot intelliot added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Aug 14, 2023
@intelliot
Copy link
Collaborator

Suggested commit message:

refactor: improve checking of path lengths (#4519)

Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

@a-noni-mousse - please confirm (1) that this PR is ready to merge, in your opinion, and (2) the above commit message looks good (for the squashed commit when this PR is merged). Alternatively, you can suggest a different commit message.

@a-noni-mousse
Copy link
Contributor Author

yes the message is okey and this is ready for the merge

@intelliot intelliot modified the milestones: 1.13, 1.12 Aug 16, 2023
@intelliot
Copy link
Collaborator

@manojsdoshi to consider for the next beta - please squash and use the commit message above

manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 18, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
This was referenced Aug 18, 2023
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 19, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 19, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 19, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
@manojsdoshi manojsdoshi mentioned this pull request Aug 19, 2023
ximinez pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 19, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
manojsdoshi pushed a commit to manojsdoshi/rippled that referenced this pull request Aug 21, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
Improve the checking of the path lengths during Payments. Previously,
the code that did the check of the payment path lengths was sometimes
executed, but without any effect. This changes it to only check when it
matters, and to not make unnecessary copies of the path vectors.

Signed-off-by: Manoj Doshi <[email protected]>
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
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants