use mempoolInstance across the app. #684

ubbabeck · 2023-10-18T09:34:34Z

This enables the user to use their preferred ~~blockexplorer~~ mempool instance across the app. If it is not set we default to mempool.space.

Added tests to verify that the instance supports mempool space's rest api too.

ubbabeck · 2023-10-18T09:44:12Z

How to test

Go to network page (accessible from side menu)
Change mempool explorer to another Mempool space instance
press Save
Verify that the app uses the new Mempool instace. by checking ongoing swaps or closed channels.

roeierez · 2023-10-18T09:57:08Z

lib/routes/home/widgets/payments_list/dialog/closed_channel_payment_details.dart

@@ -36,7 +36,7 @@ class ClosedChannelPaymentDetailsWidget extends StatelessWidget {
                if (paymentMinutiae.paymentType == sdk.PaymentType.ClosedChannel &&
                    paymentMinutiae.closingTxid != null) ...[
                  TxWidget(
-                    txURL: "$blockExplorer/tx/${paymentMinutiae.closingTxid!}",
+                    txURL: "$blockexplorer/tx/${paymentMinutiae.closingTxid!}",


I do understand the attempt to use the ServiceInjector for the block explorer url but the format is specific to mempoolspace so perhaps we can abstract a block explorer service that format the final url?
Maybe we can set a service with one method "formatTransactionUrl(txID)" ?

Thanks, I was not sure where to put it. I created a new helper class MempoolHelper, feel free to suggest another name and two helper functions. I ended up creating two, one getter for the mempoolexplorer and one for the formater as you suggested. I landed on two since we often wish to display more than one transaction and do not need unnecessary future handling. Let me know what you think.

I would use one service (resolved by the ServiceInjector) called ChainExplorer. On this service we can add methods such as:
transactionUrl(string txID)

ademar111190

I saw you mention Blockstream and mempool (of course), but other than that; do you know what explorers are being supported by these changes?

lib/routes/home/widgets/payments_list/dialog/closed_channel_payment_details.dart

lib/bloc/network/network_settings_bloc.dart

ademar111190 · 2023-10-19T12:11:02Z

lib/routes/withdraw/reverse_swap/in_progress/reverse_swap_in_progress.dart

@@ -46,19 +46,20 @@ class _TxLink extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final text = context.texts();
+    final mempoolHelper = MempoolHelper();


Better create it outside build method

If we get it from the context.read<NetworkSettingsBloc>() we will have to get it here.

it is not a problem because we are getting an existing instance not creating a new instance

Note sure how this might cause a problem, could you expand on hos this might be an issue.

ubbabeck · 2023-10-19T13:24:37Z

I saw you mention Blockstream and mempool (of course), but other than that; do you know what explorers are being supported by these changes?

It is mostly so that people can use their own mempool.space instances for better privacy. Blocksteam does not have the mempool.space rest api so we cannot use them.

ademar111190 · 2023-10-19T13:59:26Z

I saw you mention Blockstream and mempool (of course), but other than that; do you know what explorers are being supported by these changes?

It is mostly so that people can use their own mempool.space instances for better privacy. Blocksteam does not have the mempool.space rest api so we cannot use them.

We support custom mempool instances already; if you are not adding support for other providers (such as blockstream.info) what is this PR for?

ubbabeck · 2023-10-19T14:19:43Z

We support custom mempool instances already; if you are not adding support for other providers (such as blockstream.info) what is this PR for?

We do not use it across the app for tx lookups as we default to mempool.space. We do not properly check if the mempool instance works before setting it. This is what I am trying address.

roeierez · 2023-10-19T14:38:04Z

lib/routes/home/widgets/payments_list/dialog/closed_channel_payment_details.dart

-    return FutureBuilder<Config>(
-      future: Config.instance(),
-      builder: (BuildContext context, AsyncSnapshot<Config> snapshot) {
+    final mempoolHelper = MempoolHelper();


I think it is better to resolve the MempoolHelper via the ServiceInjector (where all our services defined).
Also we can rename it to ChainExplorer or similar. The whole idea of abstracting the URL formatting is not to imply on a specific implementation (mempool space).

Do we still want to explose the mempoolInstance to the serviceInjetor after we decided to change the scope of the pr from abstracting a chainexplorer to letting users use their own mempool instance?

lib/bloc/network/network_settings_bloc.dart

lib/utils/mempool_helper.dart

lib/routes/home/widgets/payments_list/dialog/closed_channel_payment_details.dart

lib/bloc/network/network_settings_bloc.dart

lib/utils/blockchain_explorer_utils.dart

update tests rename to mempoolInstance get mempool instance from NetworkSettingsBloc create blockchain explorer utils

ubbabeck requested review from ademar111190, erdemyerebasmaz and roeierez October 18, 2023 09:34

ubbabeck self-assigned this Oct 18, 2023

ubbabeck added this to the v0.2.4 alpha(sdk) milestone Oct 18, 2023

ubbabeck force-pushed the ubbabeck-blockexplorer branch from 3ede527 to 1257413 Compare October 18, 2023 09:37

ubbabeck added the QA-testing QA testing needed label Oct 18, 2023

ubbabeck force-pushed the ubbabeck-blockexplorer branch from 1257413 to 00fda81 Compare October 18, 2023 09:46

roeierez requested changes Oct 18, 2023

View reviewed changes

ubbabeck requested a review from roeierez October 18, 2023 12:12

ubbabeck force-pushed the ubbabeck-blockexplorer branch from 3f0521f to 501c3ad Compare October 18, 2023 12:20

ubbabeck changed the title ~~get blockexplorer from ServiceInjector~~ get blockexplorer Oct 18, 2023

ubbabeck force-pushed the ubbabeck-blockexplorer branch 2 times, most recently from 967b219 to 573a5e7 Compare October 19, 2023 11:56

ademar111190 requested changes Oct 19, 2023

View reviewed changes

roeierez requested changes Oct 19, 2023

View reviewed changes

ubbabeck changed the title ~~get blockexplorer~~ use mempoolInstance across the app. Oct 19, 2023

ubbabeck requested a review from ademar111190 October 19, 2023 14:54

ademar111190 requested changes Oct 19, 2023

View reviewed changes

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

ademar111190 requested changes Oct 19, 2023

View reviewed changes

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

lib/utils/mempool_helper.dart Outdated Show resolved Hide resolved

ubbabeck requested review from ademar111190 and roeierez October 20, 2023 07:28

ubbabeck commented Oct 20, 2023

View reviewed changes

lib/routes/home/widgets/payments_list/dialog/closed_channel_payment_details.dart Show resolved Hide resolved

ademar111190 approved these changes Oct 20, 2023

View reviewed changes

erdemyerebasmaz reviewed Oct 24, 2023

View reviewed changes

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

lib/bloc/network/network_settings_bloc.dart Outdated Show resolved Hide resolved

add blockexplorer to ServiceInjector

f7fafde

create mempool helper

4933430

ubbabeck force-pushed the ubbabeck-blockexplorer branch from 4ea84da to 61014f3 Compare October 25, 2023 07:44

ubbabeck requested a review from erdemyerebasmaz October 25, 2023 07:46

erdemyerebasmaz reviewed Oct 25, 2023

View reviewed changes

lib/utils/blockchain_explorer_utils.dart Outdated Show resolved Hide resolved

use mempool instance accross app

0a19912

update tests rename to mempoolInstance get mempool instance from NetworkSettingsBloc create blockchain explorer utils

ubbabeck force-pushed the ubbabeck-blockexplorer branch from 61014f3 to 0a19912 Compare October 25, 2023 11:43

ubbabeck requested a review from erdemyerebasmaz October 25, 2023 11:50

ubbabeck merged commit f78963d into main Oct 26, 2023
1 check passed

erdemyerebasmaz deleted the ubbabeck-blockexplorer branch November 16, 2023 20:11

erdemyerebasmaz restored the ubbabeck-blockexplorer branch November 16, 2023 20:12

erdemyerebasmaz deleted the ubbabeck-blockexplorer branch November 16, 2023 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use mempoolInstance across the app. #684

use mempoolInstance across the app. #684

ubbabeck commented Oct 18, 2023 •

edited

Loading

ubbabeck commented Oct 18, 2023 •

edited

Loading

roeierez Oct 18, 2023

ubbabeck Oct 18, 2023

roeierez Oct 19, 2023

ademar111190 left a comment

ademar111190 Oct 19, 2023

ubbabeck Oct 20, 2023

ademar111190 Oct 20, 2023

ubbabeck Oct 20, 2023

ubbabeck commented Oct 19, 2023

ademar111190 commented Oct 19, 2023

ubbabeck commented Oct 19, 2023

roeierez Oct 19, 2023

ubbabeck Oct 20, 2023

use mempoolInstance across the app. #684

use mempoolInstance across the app. #684

Conversation

ubbabeck commented Oct 18, 2023 • edited Loading

ubbabeck commented Oct 18, 2023 • edited Loading

How to test

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ademar111190 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ubbabeck commented Oct 19, 2023

ademar111190 commented Oct 19, 2023

ubbabeck commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ubbabeck commented Oct 18, 2023 •

edited

Loading

ubbabeck commented Oct 18, 2023 •

edited

Loading