Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fee params arg to createInvoice #666

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Conversation

ok300
Copy link
Collaborator

@ok300 ok300 commented Oct 9, 2023

Add the cheapest known feerate as argument to createInvoice.

Fixes #643

@ok300 ok300 requested a review from ubbabeck October 9, 2023 15:37
@@ -198,9 +198,13 @@ class CreateInvoicePageState extends State<CreateInvoicePage> {
final accountBloc = context.read<AccountBloc>();
final currencyBloc = context.read<CurrencyBloc>();

final lspInfo = context.read<LSPBloc>().state?.lspInfo;
final cheapestFeeParams = lspInfo?.openingFeeParamsList.values.first;
Copy link
Member

Choose a reason for hiding this comment

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

This seems to work but I think it is better to have one stream of the LSP that wraps the build method and pass the fee params explicitly to this _createInvoice function.
This fee params also should be passed to the ReceivableBTCBox.
This way we are sure that the same fee params that are displayed to the user are sent to the create invoice and it is trivial to understand that from the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

we can use context.watch which listens to changes to ().state. Though we need to define it earlier in the build method so that it rebuilds if there are any changes.

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

Copy link
Contributor

@ubbabeck ubbabeck left a comment

Choose a reason for hiding this comment

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

tACK

looks good to me

@ok300 ok300 merged commit e398efa into main Oct 10, 2023
1 check failed
@ok300 ok300 deleted the ok300-add-feerate-arg-create-invoice branch October 10, 2023 13:54
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.

Difference in the fee for opening a channel
3 participants