bitcoinarmory: build correctly from source, using *our* libs

[michalrus]

Motivation for this change

Fixes #29956

To build it correctly, the messy upstream repo requires a patch of:

 22 files changed, 63 insertions(+), 68 deletions(-)

But a relatively simple one.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • Linux
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

/cc @elitak

One issue I’m seeing is this message sometimes on Armory’s stdout/err:

-ERROR - : (StringSockets.cpp:351) FcgiSocket::writeAndRead FcgiError: unexpected fcgi header version

… but it happens using both the newest libfcgi, and the old version that BitcoinArmory wants. Here’s the patch that uses the old version, in case anyone needs it:

diff --git a/pkgs/applications/misc/bitcoinarmory/default.nix b/pkgs/applications/misc/bitcoinarmory/default.nix
index 22974a3f57..82dd753272 100644
--- a/pkgs/applications/misc/bitcoinarmory/default.nix
+++ b/pkgs/applications/misc/bitcoinarmory/default.nix
@@ -6,6 +6,27 @@ let
 
   inherit (pythonPackages) mkPythonDerivation pyqt4 psutil twisted;
 
+  #
+  # BitcoinArmory uses an ancient version of libfcgi. With our current
+  # Nixpkgs’ one, the following assertion fails in runtime:
+  #
+  # -ERROR - : (StringSockets.cpp:351) FcgiSocket::writeAndRead FcgiError: unexpected fcgi header version
+  #
+  # Let’s build that ancient version, then:
+  #
+  fcgi1 = stdenv.mkDerivation rec {
+    name = "fcgi-${version}";
+    version = "1.21";
+    nativeBuildInputs = [ pkgconfig autoreconfHook ];
+    src = fetchFromGitHub {
+      owner = "goatpig";
+      repo = "libfcgi";
+      rev = "b00dc69199b34552144e1003a2001a24b13b6e56";
+      sha256 = "1d97k3hjrf3798ks358ycn1v8frykc6f96lf47253m1hcdbp2m3k";
+    };
+    postInstall = "ln -s . $out/include/fastcgi";
+  };
+
 in mkPythonDerivation rec {
 
   name = "bitcoinarmory-${version}";
@@ -48,7 +69,7 @@ in mkPythonDerivation rec {
 
   nativeBuildInputs = [ pkgconfig autoreconfHook ];
 
-  buildInputs = [ swig qt4 fcgi cryptopp lmdb ];
+  buildInputs = [ swig qt4 fcgi1 cryptopp lmdb ];
 
   propagatedBuildInputs = [ pyqt4 psutil twisted ];
 

[elitak]

I confirmed this fixes the build issues. Looks good, except that git log -p shows a lot of stray/trailing whitespace that could be cleaned and the version bump should be separated into its own commit.

Thanks a lot for this. Fixing obnoxious codebases like this one is a real chore, so I appreciate your effort.

[jb55]

That's quite the patch to maintain, any chance of convincing upstream to make some changes?

[michalrus]

@elitak, I left it out purposefully, not to bloat the patch even more, to keep it minimal. Thank you for the kind words! 😊

@jb55, I… d-don’t think so, given the specifics of that community, but if you want to try… 😞 *But*! Notice, that this patch is mostly changing include "something.h" to a correct include <cryptopp/something.h> — I wouldn’t expect those fragments to change very often. If you look into git-blame of upstream, they indeed don’t. 😊

[michalrus]

Also, I think we might need to apply this fcgi-1.21 patch above. :/ @elitak, does Armory work correctly for you w/ the newest fcgi? Hmm.

[elitak]

The basics work. I haven't seen that fcgi version mismatch error on the console. I'm not sure what triggers it. The detection of the satoshis/byte fee measurement still doesn't work, but I have no idea if that's to do with libfcgi or not.

[michalrus]

Related: goatpig/BitcoinArmory#324

[jb55]

maybe just throw that link next to the patches line as a note if it ever gets fixed

[michalrus]

goatpig/BitcoinArmory#324 (comment) 🤦‍♂️

I think I give up. @elitak, @jb55 feel free to take this over. *smh*

[jb55]

Thanks for your effort. If he has modified them locally maybe this patch might not be the best way to go about it. but that still raises the question as to why was it crashing originally...

[michalrus]

Uh-huh!

And also: if he modified general-purpose crypo libraries in place, then maybe this application should not be used for anything serious. 😃

[elitak]

More bad news: my build using this patch has issues syncing with bitcoind and checking new blocks, making it pretty much unusable for anything outside of offline transactions, probably. I think it's best we shelve this for now. I'll revisit this if I see a lot of upstream activity. Thanks again.

[Ekleog]

(triage) From what I read, no one is actually working on this PR… maybe close and open an issue instead, as it appears the direction of the original patch was problematic?

[jb55]

yeah this can be closed

[veprbl]

Unfortunately, this won't fly because: goatpig/BitcoinArmory#324 (comment)