Skip to content

Add amount to recv session context#920

Merged
spacebear21 merged 1 commit intopayjoin:masterfrom
arminsabouri:amount-create-session
Aug 12, 2025
Merged

Add amount to recv session context#920
spacebear21 merged 1 commit intopayjoin:masterfrom
arminsabouri:amount-create-session

Conversation

@arminsabouri
Copy link
Copy Markdown
Collaborator

@arminsabouri arminsabouri commented Aug 4, 2025

Currently application developers are having to apply the amount to the
bip21 uri outside of the API.

let mut pj_uri = session.pj_uri();
pj_uri.amount = Some(amount);

This is a convenience that allows amount to be already applied to the
bip21. It also allows us to capture this information in the session
history.

Related ticket: #895

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Aug 4, 2025

Pull Request Test Coverage Report for Build 16836032368

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 85.874%

Totals Coverage Status
Change from base Build 16811558027: 0.01%
Covered Lines: 7459
Relevant Lines: 8686

💛 - Coveralls

@arminsabouri arminsabouri force-pushed the amount-create-session branch from d71dbcc to 2de392c Compare August 8, 2025 16:54
@arminsabouri
Copy link
Copy Markdown
Collaborator Author

Unfortunately the dart tests are failing on my machine for a different reason

00:04 +6 -1: test/test_payjoin_integration_test.dart: Test integration Test integration v2 to v2 [E]
  UniFfi::rustPanic: Failed to execute docker command: Os { code: 10, kind: Uncategorized, message: "No child processes" }
  lib/payjoin_ffi.dart 6947:9                     checkCallStatus
  lib/payjoin_ffi.dart 6960:9                     rustCall
  lib/payjoin_ffi.dart 5689:40                    new TestServices.initialize
  test/test_payjoin_integration_test.dart 315:43  main.<fn>.<fn>

Currently application developers are having to apply the amount to the
bip21 uri outside of the API.
https://github.com/payjoin/rust-payjoin/blob/5ecfed745bd33f6d5706f497bdb7662a3a7c9177/payjoin-cli/src/app/v2/mod.rs#L112-L113
This is a convenience that allows amount to be already applied to the
bip21. It also allows us to capture this information in the session
history.
@arminsabouri arminsabouri force-pushed the amount-create-session branch from 2de392c to 4926384 Compare August 8, 2025 16:59
@arminsabouri arminsabouri changed the title WIP - Add amount to recv session context Add amount to recv session context Aug 8, 2025
@arminsabouri arminsabouri marked this pull request as ready for review August 8, 2025 17:00
@arminsabouri
Copy link
Copy Markdown
Collaborator Author

Per a recommendation I ran dart test ~ 10 times with the same result as above. I think I built the bindings correctly. Just ran ./scripts/generate_linux.sh. Could be missing something. Any ideas @benalleng ?

@benalleng
Copy link
Copy Markdown
Collaborator

benalleng commented Aug 11, 2025

Lol first try. Tested on 4926384

Building package executable... 
Built test:test.
00:05 +6: test/test_payjoin_integration_test.dart: Test integration Test integration v2 to v2                         
Database running on 127.0.0.1:32768
OHTTP relay binding to port [::]:45039
00:07 +7: All tests passed!             

Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

ACK 4926384

Thanks for ensuring the Dart tests pass! I'm hopeful the flakiness will be fixed soon.

@spacebear21 spacebear21 merged commit ffeabdd into payjoin:master Aug 12, 2025
16 checks passed
arminsabouri referenced this pull request Aug 13, 2025
Make max_fee_rate persistent across payjoin session resumptions
and simplify FFI layer architecture by eliminating duplicate fee
rate capping logic.

- Add max_fee_rate field to SessionContext with serde defaults for
    backward compatibility
- Remove session_max_fee_rate() accessor method (no longer needed)
- Simplify FFI layer to convert Option<u64> to Option<FeeRate> and
    pass directly to core apply_fee_range() method
- Eliminate code duplication between FFI and core layers
- Maintain all fee rate capping logic in core library only

Fixes #897
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