Skip to content

Conversation

@arminsabouri
Copy link
Collaborator

The reference implementation currently overrides the original max fee rate preference when resuming a session. This commit pins the max fee rate preference to the session by persisting it in the session context. When applying fees, if the applications’s fee rate preference has changed, the updated preference takes precedence.

Pull Request Checklist

Please confirm the following before requesting review:

--

Closes http://github.com/payjoin/rust-payjoin/issues/897
Changes were picked off #934

cc @Johnosezele

@coveralls
Copy link
Collaborator

coveralls commented Sep 2, 2025

Pull Request Test Coverage Report for Build 17436370745

Details

  • 30 of 30 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 85.946%

Totals Coverage Status
Change from base Build 17413958316: 0.04%
Covered Lines: 8201
Relevant Lines: 9542

💛 - Coveralls

@Johnosezele
Copy link
Contributor

Thanks @arminsabouri
cACK 4b468f8

@DanGould
Copy link
Contributor

DanGould commented Sep 2, 2025

"Co-authored-by" is the syntax you want in a commit message

@arminsabouri arminsabouri force-pushed the max-fee-rate-in-session-context branch from 4b468f8 to c1c79df Compare September 2, 2025 19:11
@arminsabouri
Copy link
Collaborator Author

Dart tests are problematic

00:00 +0 -1: loading test/test_payjoin_integration_test.dart [E]
  Failed to load "test/test_payjoin_integration_test.dart":
  lib/payjoin_ffi.dart:4902:13: Error: The argument type 'FeeRateExceptionErrorHandler' can't be assigned to the parameter type 'UniffiRustCallStatusErrorHandler?'.
   - 'FeeRateExceptionErrorHandler' is from 'lib/bitcoin.dart'.
   - 'UniffiRustCallStatusErrorHandler' is from 'lib/payjoin_ffi.dart'.
          )), feeRateExceptionErrorHandler);

Copy link
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 c1c79df.

I think the spooky uniffi-dart error in CI is due to the return type in with_max_fee_rate containing bitcoin_ffi::error::FeeRateError.

@arminsabouri arminsabouri force-pushed the max-fee-rate-in-session-context branch from c1c79df to 3f0c442 Compare September 3, 2025 14:11
The reference implementation currently overrides the original max fee
rate preference when resuming a session. This commit pins the max
fee rate preference to the session by persisting it in the session
context. When applying fees, if the applications’s fee rate
preference has changed, the updated preference takes precedence.

Co-authored-by: Johnosezele <johnosezele@gmail.com>
@arminsabouri arminsabouri force-pushed the max-fee-rate-in-session-context branch from 3f0c442 to e53b00b Compare September 3, 2025 14:16
@arminsabouri
Copy link
Collaborator Author

ACK c1c79df.

I think the spooky uniffi-dart error in CI is due to the return type in with_max_fee_rate containing bitcoin_ffi::error::FeeRateError.

Ended up wrapping bitcoin_ffi::error::FeeRateError in an internal error

Copy link
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, opened an issue here to track the underlying uniffi-dart issue.

@arminsabouri arminsabouri merged commit 069ad21 into payjoin:master Sep 3, 2025
14 checks passed
@arminsabouri arminsabouri deleted the max-fee-rate-in-session-context branch September 3, 2025 15:13
@arminsabouri arminsabouri restored the max-fee-rate-in-session-context branch September 3, 2025 15:14
@arminsabouri arminsabouri deleted the max-fee-rate-in-session-context branch September 3, 2025 15:31
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