Skip to content

fix(PaymentSession): find minSendAmount independent of rate#1048

Merged
sidvishnoi merged 1 commit into
mainfrom
minSendAmount
May 2, 2025
Merged

fix(PaymentSession): find minSendAmount independent of rate#1048
sidvishnoi merged 1 commit into
mainfrom
minSendAmount

Conversation

@sidvishnoi
Copy link
Copy Markdown
Member

@sidvishnoi sidvishnoi commented May 2, 2025

Context

Part of #976
Extracted from #1046

Changes proposed in this pull request

  • When a new link tag is added, we want to keep using older minSendAmount, even if the adjustAmount may change due to splits. adjustAmount resulting rate is >= minSendAmount, so it makes sense to find minSendAmount first, and not find it ever again.

Known existing issues (discovered while working this out):

  • Currency fluctuations can change this minSendAmount in future. In that case, we'd need to probe/findMinSendAmount again.
  • Exponential probing is likely not the best approach. (Raw thoughts follow) I wonder if we can use currency conversion and then binary search or two-way search some values (specially relevant when sending from e.g. MXN to USD: with exp probing, we go 1, 2, 4, 8... with some two way search, we can go like: 21 (ceil exchange rate), 22, 19)
  • Can we only pay in multiples of minSendAmount? Need to check how OP/Rafiki behaves in this scenario. For example, when sending ZAR to USD (20 units vs 1), minSendAmount can be 20. Rate of pay can say we've to pay 30, but we can pay either 20 or 40 (1 or 2 units only). OP allows this, but is this optimum distribution?

@sidvishnoi sidvishnoi requested a review from DarianM May 2, 2025 10:14
@github-actions github-actions Bot added the area: background Improvements or additions to extension background script label May 2, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 2, 2025

Extension builds preview

Name Link
Latest commit 2c57f4f
Latest job logs Run #14793283803
Chrome (990.1KB)Download
Firefox (990.13KB)Download

#minSendAmountPromise: ReturnType<typeof this._findMinSendAmount>;

findMinSendAmount(): Promise<void> {
this.#minSendAmountPromise ??= this._findMinSendAmount();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This ensures we call _findMinSendAmount() only once per PaymentSession instance.

Copy link
Copy Markdown
Member

@DarianM DarianM left a comment

Choose a reason for hiding this comment

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

test-e2e

@sidvishnoi
Copy link
Copy Markdown
Member Author

Aside: I really need to figure out page.evaluate: Execution context was destroyed errors in E2E!

@sidvishnoi sidvishnoi merged commit c1c62f9 into main May 2, 2025
17 of 19 checks passed
@sidvishnoi sidvishnoi deleted the minSendAmount branch May 2, 2025 11:18
@sidvishnoi sidvishnoi linked an issue May 2, 2025 that may be closed by this pull request
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: background Improvements or additions to extension background script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Payment streams overhaul

2 participants