Port code from NBitcoin and create nuget package #1
Conversation
Why is this in the PR description? In current master branch there's no code at all, so "removed code that is not used in GWallet" is extremely confusing. Please be mindful when writing PR descriptions. |
If I explain to you why you have to remove the line "removed code that is not used in GWallet" it is so that you can take the initiative yourself to double-check again all the lines in the description to see if they need to remove as well, otherwise I would just tell you to remove without giving you any reasoning @webwarrior-ws . Please Taras be more proactive. |
|
Last change to be done here before I merge: pull first (to get my commit) with --rebase, and after that squash the commits that change FSharp.Core version into 1, because there's no point to split them (given that the oldest one didn't achieve what you wanted). |
|
Squashed version commits, but didn't use push1by1 to not trigger creation of several package versions. |
The way to avoid that problem in the future is to never propose a PR from a branch called "master"; the branch name should always reflect what you're working on (and this way pushing to it doesn't push to nuget). |
|
@webwarrior-ws actually how did you check geewallet running against this, just checking balances? don't tell me you haven't tested a transaction and still think this should be working? |
I ran unit tets, integration test, ran the console app. I didn't perform any transactions. |
Well, wallets are for performing transactions. I don't know how would you think only testing read-only operations is enough. If transactions don't work we're essentially freezing the funds of our users. |
|
Just tested sending some LTC. It worked. |
|
Ok, now can you please push 1by1, but to avoid it publishing many nuget pkgs, just move the commit that adds nugetPush to be the last one. (And now that you're going to change order of commits, you could also make the one that adds CI the first one.) |
Added a test project and ported tests from NBitcoin that use Litecoin class. Added CI workflow that compiles solution and runs tests. Co-authored-by: Parham <parhaamsaremi@gmail.com>
Ported Litecoin class and all code needed for it from NBitcoin repository.
Removed code that is not used in GWallet.
Removed use of mutable variables; minor code style changes.
Split Litecoin.ReadWrite method into Read and Write. Replaced how byref parameters are passed: use & instead of ref because methods that take them may mutate them. Tests were not affected by this change, so maybe coverage is not sufficient. Co-authored-by: Parham <parhaamsaremi@gmail.com>
Added properties related to generating nuget package to Litecoin.fsproj file.
Downgrade FSharp.Core to version 4.7.0 so that it doesn't create version confilcts in GWallet.
Removed DNSSeeds from builder since GWallet doesn't use them.
Added package job to CI that should create and publish a nuget package if NUGET_API_KEY secret is present and branch is master.
After that, you can start looking into Milestone 2. For that, I recommend using Litecoin-Core (with regtest or testnet) to create test transactions, that should later try to parse with NLitecoin. The goal is to have geewallet be able to send MimbleWimble-style transactions in Litecoin (we will investigate later about the UX of how to expose the feature to the user, but for now just make it work). |
|
BTW the FSharpCore commit contains a uint->uint32 change, is that intentional? |
Yes, older F# doesn't have |
Very interesting. Next time please clarify it in commit message because it's not immediately obvious. BTW one of the commits has red CI, you might want to squash it with next? |
You told me to "make the one that adds CI the first one", and I did so. That commit also introduces tests, so it fails, because there is no code to test yet. |
|
Fair enough but following what happened at the conventions PR, it should contain the term "failing test" ;-) Merging. |
Ported Litecoin class and all code needed for it from NBitcoin repository.
Added a test project and ported tests from NBitcoin that use Litecoin class.
Added CI workflow that compiles solution and runs tests, generates nuget package and uploads it to nuget.org.