bitcoinjs-lib icon indicating copy to clipboard operation
bitcoinjs-lib copied to clipboard

signet support

Open nicolasburtey opened this issue 2 years ago • 4 comments

signet support is missing from the library

(this is generating typescript error here: https://github.com/GaloyMoney/galoy/blob/main/src/domain/bitcoin/onchain/tx-decoder.ts @openoms @dolcalmi)

nicolasburtey avatar Jun 26 '22 20:06 nicolasburtey

Adding signet to this library will only temporarily fix the issue of how networks are handled by galoy.

  1. The list of networks depends directly on the version of bitcoinjs-lib they are using, and yet they define the list explicitly, separately in their library.
  2. They have a list of address regexes which is based off of known address prefix results which does not depend at all on bitcoinjs-lib.
  3. They define an implicit alias of mainnet -> bitcoin at the point of usage (not at the point of definition of this list)

At each point where BtcNetwork comes into play, it has varying levels of depending on bitcoinjs-lib, which will cause problems.

It would be better to gather everything into one place in their codebase, import the existing network objects from bitcoinjs-lib, add whichever networks are missing + create any aliases needed.

Because they did not do this, and the code was written with the implicit misunderstanding that "bitcoinjs-lib already has signet as a default network object" which causes the problem.


That being said, I am not opposed to adding signet as a named network here. It's basically like another testnet, so having it as a named network should be fine.

Either way you will need galoy to update to a new version, so why not fix their network usage on a more fundamental level instead? ie. Remove assumptions from the code instead of making the library mold to the assumptions?

To make my concerns easier to understand, I will pose this rhetorical question: "Every time galoy adds some new network (signet2 or signet3 or signetTheEmpireStrikesBack) are we going to be required to add that network as well?"

The fact that I have to ask that question points to there being a fundamental flaw in the way galoy is handling the network object usage, and I think the issue should be fixed there first.


Again, I am not against adding signet, though.

junderw avatar Jun 26 '22 23:06 junderw

we could indeed have a proper mapping from our domain object to bitcoinjs-lib, but signet is part of the default network in bitcoin-core so I think that would make sense to have it here also?

nicolasburtey avatar Jun 27 '22 08:06 nicolasburtey

PR to have proper mapping on the galoy side: https://github.com/GaloyMoney/galoy/pull/1384/commits/b474369dff8c0f2d08ee947c0f5a98669765aab4

I guess we can run with this for now

nicolasburtey avatar Jun 27 '22 09:06 nicolasburtey

I used bitcoinjs-lib with signet with no problem. I just setting the network to "testnet", then bitcoinjs-lib can generate address and transaction using on signet. The signet address seems like same as testnet address.

h76oeI6pMxU9g4p8aCpc6Q avatar Jul 24 '22 21:07 h76oeI6pMxU9g4p8aCpc6Q

I agree that signet and testnet are almost exactly the same at the application level.

jasonandjay avatar May 01 '24 13:05 jasonandjay