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

[TRIVIAL] Add unit test for redundant payment #4860

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

Bronek
Copy link
Collaborator

@Bronek Bronek commented Jan 2, 2024

High Level Overview of Change

The preflight check to enforce that payee and payer must not be the same account have been in rippled at least 9 years, however we do not have unit test for this case. Add it.

Context of Change

Apparently other crypto ledgers are attacked by creating large number of transactions with the same payee and payer. We are immune to this type of attack, but it is not obvious. Let's make it clear and also improve unit test coverage.

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 update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

@Bronek Bronek force-pushed the feature/add_test_redunant_payment branch from c40c91e to bec1e8f Compare January 23, 2024 11:25
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fad9d63) 61.48% compared to head (bec1e8f) 61.49%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4860   +/-   ##
========================================
  Coverage    61.48%   61.49%           
========================================
  Files          797      797           
  Lines        70121    70122    +1     
  Branches     36238    36238           
========================================
+ Hits         43114    43119    +5     
- Misses       19763    19764    +1     
+ Partials      7244     7239    -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@Bronek
Copy link
Collaborator Author

Bronek commented Jan 26, 2024

@intelliot this is ready to merge

@intelliot intelliot merged commit 90d463b into XRPLF:develop Jan 31, 2024
17 checks passed
@gregtatcam gregtatcam mentioned this pull request Feb 7, 2024
1 task
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
If the payee and payer are the same account, then the transaction fails
in preflight with temREDUNDANT.
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 this pull request may close these issues.

4 participants