expose reverse swap fees #1127
Open
expose reverse swap fees #1127
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `send_onchain` function was deprecated since SDK version 0.3.6. It has been replaced with `pay_onchain`. Removed deprecated functions: - `send_onchain`, has replacement `pay_onchain` - `max_reverse_swap_amount` has replacement `onchain_payment_limits` - `on_progress_reverse_swaps` has replacement `in_progress_onchain_payments`
roeierez
approved these changes
Nov 25, 2024
| invoice: response.invoice, | ||
| preimage: reverse_swap_keys.preimage, | ||
| private_key: reverse_swap_keys.priv_key, | ||
| timeout_block_height: response.timeout_block_height, | ||
| id: response.id, | ||
| onchain_amount_sat: response.onchain_amount, | ||
| sat_per_vbyte, | ||
| receive_amount_sat, | ||
| sat_per_vbyte: None, |
There was a problem hiding this comment.
Can we remove now the sat_per_vbyte from the struct?
ok300
reviewed
Nov 27, 2024
| @@ -602,7 +602,7 @@ dictionary PrepareOnchainPaymentResponse { | |||
| f64 fees_percentage; | |||
| u64 fees_lockup; | |||
| u64 fees_claim; | |||
|
|
|||
| u64 fees_service; | |||
There was a problem hiding this comment.
fees_percentage above and the new fees_service refer to the same thing. Wouldn't one have to go, to avoid confusing the user?
| u64 fees_lockup; | ||
| u64 fees_claim; | ||
| u64 fees_service; | ||
| u64 total_fees; |
There was a problem hiding this comment.
To keep with the other 3 new fields, maybe fees_total is better?
Comment on lines
+646
to
+649
| ALTER TABLE reverse_swaps ADD column fees_lockup INTEGER DEFAULT 0; | ||
| ALTER TABLE reverse_swaps ADD column fees_claim INTEGER DEFAULT 0; | ||
| ALTER TABLE reverse_swaps ADD column fees_service INTEGER DEFAULT 0; | ||
| ALTER TABLE reverse_swaps ADD column total_fees INTEGER DEFAULT 0; |
There was a problem hiding this comment.
Since the udl doesn't treat them as optional, maybe INTEGER NOT NULL DEFAULT 0 is more appropriate?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Expose the lockup/claim/service fees from reverse swaps.
Historical fees are currently not backfilled. So this currently only works for new swaps.
The fees are synced with the remote.
If the fees cannot be found, they're set to 0.
Fixes #1111
Builds on top of #1125