feat: bitcoin regtest network option #4270

Polybius93 · 2023-09-26T13:30:41Z

Try out this version of Leather — download extension builds.

We have added a Bitcoin regtest option to the 'Add Network' function. This new feature allows you to add Bitcoin network alongside the Stacks networks.

This feature allows the user to add and use Bitcoin regtest networks, which can be much faster Testnet.

Currently when you add a Bitcoin network, the associated Stacks network will be Stacks Testnet.

fbwoolf · 2023-09-26T14:54:32Z

src/app/features/add-network/add-network.tsx

+        <Stack justifyContent={'center'}>
+          <LeatherButton
+            onClick={() => setBlockchain('stacks')}
+            backgroundColor={


Can you post a screenshot of these UI changes? There are variants of the LeatherButton to use here.

Sure, here you go:

These would better be tabs than buttons. You can reuse the tabs from the homepage

Although, wouldn't it be better to have this all on the same page, seeing as you need them all?

So these buttons won't take you to another page, it just changes the blockchain variable. So if you click Bitcoin, you'll add a Bitcoin network with this form.

fbwoolf · 2023-09-26T14:55:55Z

src/app/features/add-network/add-network.tsx

@@ -165,6 +233,7 @@ export function AddNetwork() {
                placeholder="Key"
                value={values.key}
                width="100%"
+                isDisabled={blockchain === 'bitcoin' ? true : false}


I don't think you need the ternary here? The boolean itself will be true/false?

kyranjamie · 2023-09-27T12:01:00Z

@fabric-8 @mica000 are you able to provide to general design guidance for this feature we'd like to get into production?

The gist here is that @Polybius93, and many other developers, need to be able to target different network types, such as regtest. Our current UI doesn't support this, and a solution would valuable for many devs. As this is a developer-only feature, we don't need to finesse this particular part of the app, but think some feedback would still go a long way.

@Polybius93 for the designer to best help, can you please describe the problem this solves, and what the minimum requirements to fix the problem. I find the buttons added here a little confusing, plus I also don't understand why you don't need to fill in all the fields. Wouldn't a network type select here work too?

fabric-8 · 2023-09-28T08:20:23Z

A segmented control would be pretty good here! (figma mockup)

While doing a quick mockup I also noticed that we aren't "containerizing" the contents in fullpage view. Do I remember correctly that this is something @pete-watters is looking into as part of the broader brand refinement work? 😅

Polybius93 · 2023-09-28T08:43:37Z

@kyranjamie @fabric-8 @mica000

Hello!

In the current dev branch, the "add network" feature only allows users to add a new Stacks network. Afterwards, it associates a Bitcoin network constant with it (either Mainnet or Testnet). However, the network slice and its PersistedNetworkConfiguration are not prepared to handle a custom Bitcoin network.

This PR addresses the issue by enabling users to choose whether they want to add a new Stacks or Bitcoin network. If the user selects a Bitcoin network, the key field is disabled and automatically set to "bitcoin-regtest" for differentiation in the network converting logic.

The logic for adding Stacks networks remains unchanged, as it will still select a corresponding Bitcoin network. Additionally, when adding a Bitcoin network, the Stacks Testnet network is automatically made available alongside it.

Ideally, users should be able to set both Stacks and Bitcoin networks at the same time. However, I have not yet found an easy solution for this. There may be possible ways to modify the objects and functions involved to handle both scenarios effectively.

fabric-8 · 2023-10-09T09:53:11Z

Thanks for the summary @Polybius93! Here's a design proposal (Figma), WDYT?

Polybius93 · 2023-10-09T13:25:13Z

Thanks for the summary @Polybius93! Here's a design proposal (Figma), WDYT?

Hello! It looks really great, but before I proceed with making the changes to align with your design, I would prefer to wait for input from other developers, as the UI might undergo changes afterward (if any changes to the logic are required).
What do you think @kyranjamie ?

Thank you!

fabric-8 · 2023-10-10T12:45:56Z

@Polybius93 Discussed this with @kyranjamie yesterday based upon which the design proposal has been updated.

Link to Design on Figma

We simplified things as much as possible, effectively merging both "tabs" into one unified view as that represents things more accurately.

At it's core, we porpose a dropdown ("Bitcoin API") where any selection will populate the text inputs below with defaults we assume would be useful.

But since those are just text inputs, the end user can obv. change this to whatever they need.

Propose selection / applied defaults:

Mainnet
BTC Network: blockstream.info/api
STX Network: https://api.hiro.so
Testnet
BTC Network: https://blockstream.info/testnet/api
STX Network: https://api.testnet.hiro.so
Signet
BTC Network: https://mempool.space/signet/api
STX Network: https://api.testnet.hiro.so
Mainnet
BTC Network: https://blockstream.info/testnet/api
STX Network: https://api.testnet.hiro.so

Happy to hear any thoughts or questions you might have on this and thanks for your support!

kyranjamie · 2023-10-12T10:52:22Z

src/shared/utils.ts

+type NetworkMap<T> = Record<BitcoinNetworkModes, T>;
+export function whenNetwork(mode: BitcoinNetworkModes) {
+  return <T extends NetworkMap<unknown>>(networkMap: T) =>
+    networkMap[mode] as T[BitcoinNetworkModes];


Can you revert this change, sometimes we need the 2 network map. The function you're looking for exists https://github.com/leather-wallet/extension/blob/b926b27c50269d9f678e82a3aea0e1cdcc695375/src/app/common/utils.ts#L267-L269

kyranjamie · 2023-10-12T10:54:03Z

src/shared/constants.ts

+      stacksChainId: ChainID.Mainnet,
+      stacksUrl: HIRO_API_BASE_URL_MAINNET,


Is it necessary to change these names?

It was cleaner for me to distinguish it from the bitcoin properties, but I can revert this if you think it is unnecessary.

kyranjamie · 2023-10-12T10:55:09Z

src/app/store/accounts/blockchain/bitcoin/bitcoin-signer.ts

@@ -105,6 +105,7 @@ interface CreateSignersForAllNetworkTypesArgs {
  paymentFn: (keychain: HDKey, network: BitcoinNetworkModes) => unknown;
  mainnetKeychainFn: (accountIndex: number) => BitcoinAccount | undefined;
  testnetKeychainFn: (accountIndex: number) => BitcoinAccount | undefined;
+  regtestKeychainFn: (accountIndex: number) => BitcoinAccount | undefined;


Don't understand these changes and don't think they're needed? Afaik, regtest uses the same derivation path, and thus keychain as testnet?

Yeah, tried without this, it works just fine, so I'll revert this. At first, even though I used regtest, testnet addresses were shown, thought maybe this is one of the reasons, but turns out, it's not. Thank you!

fabric-8 · 2023-10-16T11:51:05Z

👋 Feel free to ping me once you feel the implementation is ready for a design review — or if there are any design-questions.

Polybius93 · 2023-10-16T13:42:15Z

👋 Feel free to ping me once you feel the implementation is ready for a design review — or if there are any design-questions.

@fabric-8 Hey! Finished the code and all the checks are passing now, so the PR is ready to be reviewed! Thank you!

fabric-8 · 2023-10-17T07:24:11Z

All good from my side for now, we'll probably do some design updates here in the aftermath as part of a broader design/brand-alignment initiative, but that shouldn't block the merge here!

Maybe just one minor thing: In fullscreen/tab view, the form contents are very narrow - any chance we can fix this? I think usually the max-width here should sit at around 400px, not quite sure why that doesn't work here. The width worked correctly in the previous add network view

Polybius93 · 2023-10-17T08:19:34Z

All good from my side for now, we'll probably do some design updates here in the aftermath as part of a broader design/brand-alignment initiative, but that shouldn't block the merge here!

Maybe just one minor thing: In fullscreen/tab view, the form contents are very narrow - any chance we can fix this? I think usually the max-width here should sit at around 400px, not quite sure why that doesn't work here. The width worked correctly in the previous add network view

Thank you @fabric-8 ! Yeah, now that you have wrote this, I noticed that there is an extra container in the layout, that's why it was narrow, I've fixed and pushed it up.

fabric-8

👍

markmhendrickson · 2023-10-23T14:57:43Z

@kyranjamie to do final code review

kyranjamie

Sorry @Polybius93, reviewing this I discovered that there's a breaking change for users with existing networks saved.

Because we changed the name of the networks from stacks.url --> stacks.stacksUrl and bitcoin.url to bitcoin.bitcoinUrl, users with existing networks persisted will try and read bitcoinUrl which will be undefined.

Is it possible to leave the network schema as it was before?

Polybius93 · 2023-10-31T09:42:27Z

Sorry @Polybius93, reviewing this I discovered that there's a breaking change for users with existing networks saved.

Because we changed the name of the networks from stacks.url --> stacks.stacksUrl and bitcoin.url to bitcoin.bitcoinUrl, users with existing networks persisted will try and read bitcoinUrl which will be undefined.

Is it possible to leave the network schema as it was before?

Sure, I'll make it work in the afternoon! Thank you!

markmhendrickson · 2023-11-13T11:32:50Z

@Polybius93 any updates here? It'd be great to get this merged in soon

Polybius93 · 2023-11-13T11:52:24Z

@Polybius93 any updates here? It'd be great to get this merged in soon

We were having some issues migrating the existing networks into this new structure, @kyranjamie is helping me out in in this issue currently.

…k too feat: modified add network tests fix: fixed types and removed unused exports

markmhendrickson added Enhancement 💡 enhancement-p2 Critical functionality needed by few users, with no clear alternatives area:networks area:dlcs labels Sep 26, 2023

Polybius93 requested review from kyranjamie and fbwoolf September 26, 2023 13:41

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch 2 times, most recently from 500282b to 842f90d Compare September 26, 2023 13:54

fbwoolf reviewed Sep 26, 2023

View reviewed changes

markmhendrickson assigned mica000 and unassigned mica000 Oct 2, 2023

markmhendrickson assigned fabric-8 and Polybius93 Oct 9, 2023

kyranjamie reviewed Oct 12, 2023

View reviewed changes

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch from 259b493 to d726882 Compare October 12, 2023 14:45

Polybius93 mentioned this pull request Oct 16, 2023

feat/bitcoin regtest network option updated #4360

Closed

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch from dedeb5e to cc27cba Compare October 16, 2023 13:36

markmhendrickson requested a review from fabric-8 October 16, 2023 14:35

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch from cc27cba to 86f6c54 Compare October 17, 2023 08:17

Polybius93 requested a review from kyranjamie October 17, 2023 14:03

fabric-8 approved these changes Oct 18, 2023

View reviewed changes

kyranjamie force-pushed the feat/bitcoin-regtest-network-option branch from 86f6c54 to 9c67d10 Compare October 31, 2023 09:35

kyranjamie requested changes Oct 31, 2023

View reviewed changes

markmhendrickson unassigned fabric-8 Nov 13, 2023

sosaucily mentioned this pull request Nov 13, 2023

Enable Bitcoin Contracts for Devnet/sBTC Devenv #4498

Open

kyranjamie force-pushed the feat/bitcoin-regtest-network-option branch from 9c67d10 to 8c18929 Compare November 15, 2023 09:59

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch from 8c5e0f1 to 637667b Compare November 15, 2023 11:42

Polybius93 and others added 3 commits November 21, 2023 12:23

feat: extended add-network page with the option to add bitcoin networ…

cab568f

…k too feat: modified add network tests fix: fixed types and removed unused exports

chore: remove patch file

9eca7d9

fix: fixed previously saved network loading

a81dace

Polybius93 force-pushed the feat/bitcoin-regtest-network-option branch from 704584c to a81dace Compare November 21, 2023 11:25

kyranjamie approved these changes Nov 21, 2023

View reviewed changes

Polybius93 added this pull request to the merge queue Nov 21, 2023

Merged via the queue into dev with commit c8724b4 Nov 21, 2023
26 checks passed

Polybius93 deleted the feat/bitcoin-regtest-network-option branch November 21, 2023 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: bitcoin regtest network option #4270

feat: bitcoin regtest network option #4270

Polybius93 commented Sep 26, 2023 •

edited by github-actions bot

Loading

fbwoolf Sep 26, 2023

Polybius93 Sep 26, 2023

kyranjamie Sep 26, 2023

kyranjamie Sep 26, 2023

Polybius93 Sep 26, 2023

fbwoolf Sep 26, 2023

kyranjamie commented Sep 27, 2023

fabric-8 commented Sep 28, 2023

Polybius93 commented Sep 28, 2023

fabric-8 commented Oct 9, 2023 •

edited

Loading

Polybius93 commented Oct 9, 2023

fabric-8 commented Oct 10, 2023

kyranjamie Oct 12, 2023

kyranjamie Oct 12, 2023

Polybius93 Oct 12, 2023

kyranjamie Oct 12, 2023

Polybius93 Oct 12, 2023

fabric-8 commented Oct 16, 2023

Polybius93 commented Oct 16, 2023

fabric-8 commented Oct 17, 2023

Polybius93 commented Oct 17, 2023

fabric-8 left a comment

markmhendrickson commented Oct 23, 2023

kyranjamie left a comment

Polybius93 commented Oct 31, 2023

markmhendrickson commented Nov 13, 2023

Polybius93 commented Nov 13, 2023

		stacksChainId: ChainID.Mainnet,
		stacksUrl: HIRO_API_BASE_URL_MAINNET,

feat: bitcoin regtest network option #4270

feat: bitcoin regtest network option #4270

Conversation

Polybius93 commented Sep 26, 2023 • edited by github-actions bot Loading

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

Choose a reason for hiding this comment

kyranjamie commented Sep 27, 2023

fabric-8 commented Sep 28, 2023

Polybius93 commented Sep 28, 2023

fabric-8 commented Oct 9, 2023 • edited Loading

Polybius93 commented Oct 9, 2023

fabric-8 commented Oct 10, 2023

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

fabric-8 commented Oct 16, 2023

Polybius93 commented Oct 16, 2023

fabric-8 commented Oct 17, 2023

Polybius93 commented Oct 17, 2023

fabric-8 left a comment

Choose a reason for hiding this comment

markmhendrickson commented Oct 23, 2023

kyranjamie left a comment

Choose a reason for hiding this comment

Polybius93 commented Oct 31, 2023

markmhendrickson commented Nov 13, 2023

Polybius93 commented Nov 13, 2023

Polybius93 commented Sep 26, 2023 •

edited by github-actions bot

Loading

fabric-8 commented Oct 9, 2023 •

edited

Loading