Skip to content

Stable balance improvements#739

Merged
dangeross merged 4 commits intomainfrom
savage-stable-balance-v2
Mar 27, 2026
Merged

Stable balance improvements#739
dangeross merged 4 commits intomainfrom
savage-stable-balance-v2

Conversation

@dangeross
Copy link
Copy Markdown
Collaborator

  • Runtime state switching - Stable balance activates/deactivates via user settings in real time without restart
  • Event middleware - Adds middleware that can mutate/react/suppress events. Used to:
    • React and inject a conversion status to a succeeded payment, before queuing a per-receive conversion
    • Suppress payment events for conversion child payments (the internal send/receive plumbing), so external listeners only see the parent payment events
  • Per-receive conversions - Each incoming payment is individually converted to the stable token on receipt, using deterministic transfer IDs for idempotency
  • Reserved removed - Auto-convert now converts the full sats balance above threshold instead of holding back reserved_sats (with token dust checks)
  • Dynamic min limit adjustment - Sends ToBitcoin automatically raise the amount to meet the min conversion limit when trying to send lower amounts, and convert the full token balance to avoid stranded token dust
  • Token dust prevention - Auto-convert skips if converting would leave a token balance below the ToBitcoin min limit

@dangeross dangeross force-pushed the savage-stable-balance-v2 branch 4 times, most recently from c0d197d to d3f1d5a Compare March 19, 2026 14:34
@dangeross dangeross marked this pull request as ready for review March 23, 2026 07:42
@dangeross dangeross force-pushed the savage-stable-balance-v2 branch from d3f1d5a to cb7ed48 Compare March 23, 2026 14:11
Copy link
Copy Markdown
Collaborator

@danielgranhao danielgranhao left a comment

Choose a reason for hiding this comment

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

Haven't finished reviewing but leaving here a few comments already :)

Copy link
Copy Markdown
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

ignore

Copy link
Copy Markdown
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

This is huge! I've done a first pass, but I'll definitely need to go through it again to fully wrap my head around it.


// Payment succeeded → check if it resolves a deferred conversion,
// then queue per-receive or auto-convert as needed
SdkEvent::PaymentSucceeded { mut payment } => {
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.

I think we have a behavior currently that PaymentSucceeded is emitted also for past payments during sync. Just making sure it is not an issue here.
Another thing is that I am not sure at all we should emit Payment events for past payments and I also want to make sure that when we change this behavior it will not affect this logic as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It only starts emitting PaymentSucceeded after the initial sync is complete

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.

Right. What about instance that has passed the initial sync and was restarted after a while? It willreceive these past events won't it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The initial sync is a watcher, its only triggered on the first sync after the instance is started. Payment events arn't emitted before that


// Check if a send-with-conversion is in progress on another instance
if let Some(client) = &self.signing_client {
match client.get_lock(PAYMENT_LOCK_NAME).await {
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.

Now that you came up with the deterministic transfer id for conversion I wonder if we can use idempotency to skip the distributed lock in this case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Its more of a belt-and-suspenders approach with redundant checking. We check:

  1. The lock
  2. The deterministic payment id is not is storage
  3. and, in case of lag, the idempotency is the catch all

Do you think its too much?

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.

I only think that if we can achieve the same result without the distributed lock then it is definitely better.
Do you see any issue if we count only on the idempotency? It seems that the first one to create the transfer wins and handle the conversion and the second one fails (probay need to handle it gracefully). Is that right or perhaps I am missing something?

@dangeross dangeross force-pushed the savage-stable-balance-v2 branch 2 times, most recently from 6aaae1e to 962f7c8 Compare March 24, 2026 14:24
@dangeross dangeross force-pushed the savage-stable-balance-v2 branch 2 times, most recently from 68d4559 to 1dc08b8 Compare March 25, 2026 08:09
@dangeross dangeross force-pushed the savage-stable-balance-v2 branch from 33e4d59 to 5af3e61 Compare March 26, 2026 22:09
@dangeross dangeross force-pushed the savage-stable-balance-v2 branch 2 times, most recently from aeb19db to a8447f8 Compare March 27, 2026 15:27
@dangeross dangeross force-pushed the savage-stable-balance-v2 branch from a8447f8 to efc6691 Compare March 27, 2026 16:18
Copy link
Copy Markdown
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

LGTM

@dangeross dangeross requested a review from danielgranhao March 27, 2026 16:42
Copy link
Copy Markdown
Collaborator

@danielgranhao danielgranhao left a comment

Choose a reason for hiding this comment

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

LGTM

@dangeross dangeross merged commit c8772a6 into main Mar 27, 2026
25 checks passed
@dangeross dangeross deleted the savage-stable-balance-v2 branch March 27, 2026 22:18
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.

3 participants