Skip to content

Look up RK instead of looping#1706

Merged
DanGould merged 1 commit into
payjoin:masterfrom
benalleng:lookup-rk
Jul 6, 2026
Merged

Look up RK instead of looping#1706
DanGould merged 1 commit into
payjoin:masterfrom
benalleng:lookup-rk

Conversation

@benalleng

@benalleng benalleng commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Follow up from #995. We can replace a loop over all active session ids with a WHERE clause.

Supersedes #1013

since this helper function returns a Result<Option<SessionId>> I opted to move some match error handing within the function itself instead of matching at the send_payjoin level

Additionally adds some enhacements suggested from the original PR like adding "completed_at IS NULL"

Pull Request Checklist

Please confirm the following before requesting review:

@benalleng benalleng requested a review from DanGould July 2, 2026 19:54
@benalleng benalleng added this to the payjoin-cli-1.0 milestone Jul 2, 2026
@coveralls

coveralls commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 28665120300

Coverage decreased (-0.04%) to 85.899%

Details

  • Coverage decreased (-0.04%) from the base build.
  • Patch coverage: 21 of 21 lines across 2 files are fully covered (100%).
  • 7 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

7 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
payjoin-cli/src/db/v2.rs 7 82.9%

Coverage Stats

Coverage Status
Relevant Lines: 15538
Covered Lines: 13347
Line Coverage: 85.9%
Coverage Strength: 350.88 hits per line

💛 - Coveralls

@benalleng

Copy link
Copy Markdown
Collaborator Author

@DanGould I think this PR still has utility even after our upgrade to uuid v7 simply for the sake of code simplification on the sender side.

DanGould
DanGould previously approved these changes Jul 3, 2026

@DanGould DanGould left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only nits otherwise lgtm ACK 8b7486a

Comment thread payjoin-cli/src/db/v2.rs Outdated
Comment thread payjoin-cli/src/db/v2.rs Outdated
Comment thread payjoin-cli/src/db/v2.rs Outdated
Follow up from payjoin#995.
We can replace a loop over all active session ids with a WHERE clause.

@xstoicunicornx xstoicunicornx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK 2490ef2

Reviewed changes and tested locally. This is a much more straightforward implementation that avoids unnecessary db calls made by the now unneeded loop.

@DanGould DanGould merged commit b020652 into payjoin:master Jul 6, 2026
13 checks passed
@DanGould DanGould mentioned this pull request Jul 6, 2026
2 tasks
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.

5 participants