Backend/Utxo: support native segwit #211

aarani · 2023-04-17T10:14:32Z

No description provided.

src/GWallet.Backend/UtxoCoin/UtxoCoinAccount.fs

src/GWallet.Backend/UtxoCoin/ElectrumClient.fs

src/GWallet.Backend/UtxoCoin/UtxoCoinAccount.fs

knocte · 2023-04-17T13:17:29Z

One last thing, can we do this in a soft-launch fashion? put a setting in Config.fs that is for now as =false, so geewallet behaves in same way as before. Then whenever we set it to =true, this new behaviour starts happening (so maybe we set it as true in lightning branch).

aarani · 2023-04-17T13:25:23Z

One last thing, can we do this in a soft-launch fashion? put a setting in Config.fs that is for now as =false, so geewallet behaves in same way as before. Then whenever we set it to =true, this new behaviour starts happening (so maybe we set it as true in lightning branch).

Done

src/GWallet.Backend/Config.fs

knocte · 2023-10-19T08:58:00Z

Title's scope should be Backend/Utxo:, not just Backend.

knocte · 2023-10-19T13:58:42Z

~~CI is red~~

knocte · 2023-10-20T06:45:50Z

@aarani after you finish with the --console task, can you please add a commit to this PR that does the following:

Make UseNativeSegwit default to true if you pass certain parameter to ./configure.sh (e.g. --native-segwit), which in turn would make msbuild/xbuild/dotnet build with DefineConstants+=NATIVE_SEGWIT .
Make CI have additional steps that build with this constant turned on, and publish the result in the beta channel (as opposed to the stable channel). See 37f9c20 for context.

knocte · 2023-10-20T06:47:02Z

Make CI have additional steps that build with this constant turned on...

Just to make sure you understand the above, take in account that I want the build to publish 2 snaps: one for the stable channel and one for the beta one.

scripts/configure.fsx

scripts/make.fsx

knocte · 2023-11-16T08:37:01Z

.github/workflows/CI.yml

@@ -576,6 +576,7 @@ jobs:
    - name: Upload snap package to Snap Store
      env:
        SNAPCRAFT_LOGIN: ${{ secrets.SNAPCRAFT_LOGIN }}
+        OVERRIDE_SNAP_VERSION: 0.7.2.1


@aarani harcoding a version number? are you serious????

scripts/snap_release.fsx

knocte · 2023-11-20T14:28:25Z

@Mersho what value is the 2nd commit adding in this PR? Why is it separated from the 3rd commit?

knocte · 2023-11-20T14:29:05Z

And why the scope of the last commit is completely wrong??????

Previously, native segwit was not widly supported so it was necessary to do segwit over P2SH, these days native segwit is supported by most wallets and with it's lower fee is the recommended choice. Lightning protocol is even dropping support for using P2SH shutdown scripts [1]. This commit adds support for native segwit (P2WPKH) while keeping the support for spending funds in users's old P2SH wallets. [1] lightning/bolts@8f2104e

This commit aims to create beta snap packages to allow testing experimental features like native segwit. This commit also updates Fsdk inside our scripts because the `GatherOrGetDefaultPrefix` function in the previous version did not allow additional arguments beside --prefix. A flag called --auto was also added for bypassing the "press any key" in bump.fsx. Co-authored-by: Mehrshad <[email protected]>

knocte · 2024-01-30T12:55:47Z

@Mersho FYI this PR was not properly tested, because I had to fix a couple of things after I merged it:

Next time you're working with a piece of code that only runs when CI is processing a git tag, please TEST A GIT TAG in your fork.

Thanks

This change also reduces a bit of primitive obsession (i.e. string vs BitcoinAddress) to make the code less "stringly-typed" lol. This should have been done in [1] but was an oversight on my part when reviewing. [1] #211

This fixes the following crash when trying to broadcast a signed transaction from your cold-storage device: ``` unknown origin account at GWallet.Backend.UtxoCoin.Account.GetSignedTransactionDetails[T](String rawTransaction, Currency currency) in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Backend/UtxoCoin/UtxoCoinAccount.fs:line 728 at GWallet.Backend.Account.GetSignedTransactionDetails[T](SignedTransaction`1 signedTransaction) in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Backend/Account.fs:line 793 at Program.BroadcastPayment() in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Frontend.Console/Program.fs:line 82 at Program.PerformOperation(UInt32 numActiveAccounts, UInt32 numHotAccounts) in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Frontend.Console/Program.fs:line 398 at Program.ProgramMainLoop[a]() in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Frontend.Console/Program.fs:line 481 at Program.NormalStartWithNoParameters() in /Users/knocte/Documents/Code/geewalletMASTERclean/src/GWallet.Frontend.Console/Program.fs:line 493 ``` This change also reduces a bit of primitive obsession (i.e. string vs BitcoinAddress) to make the code less "stringly-typed" lol. This should have been done in [1] but was an oversight on my part when reviewing. [1] #211

aarani commented Apr 17, 2023

View reviewed changes