Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pivx wallet pivx: 3.2.0 -> 3.4.0 #67412

Merged
merged 4 commits into from Oct 12, 2019
Merged

Pivx wallet pivx: 3.2.0 -> 3.4.0 #67412

merged 4 commits into from Oct 12, 2019

Conversation

wucke13
Copy link
Contributor

@wucke13 wucke13 commented Aug 24, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@mmahut
Copy link
Member

mmahut commented Aug 25, 2019

This is marked as broken on master, please see 2542928. If you update supports this, please rebase and drop the broken flag.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 26, 2019

Interesting. I was able to build and run the application from this patched derivation. I will look into this soon.

EDIT 1: I haven't rebased to upstream before submitting this PR, and indeed I'm running with openssl 1.0.2.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 26, 2019

I fixed this by manually speciying openssl_1_0_2 as dep. Why didn't you do that in the first place in favor of marking the package as broken? @globin @mmahut
At least building is no possible, on master with the most recent version of pivx.

@globin
Copy link
Member

globin commented Aug 26, 2019

I don't trust upstream who haven't updated from an openssl version that has been EOL for years and includes a number of security issues. That is quite an alarming sign of negligence on their part.

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

@globin has a good point, that the trust in the product is questionable in case they are using EOL libraries, even more if it's a cryptocurrency, however I agree with @wucke13 that we should leave it up to the maintainer to fix any problems with the package and only when we fail to get in touch with the maintainer we should mark the package as broken.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 27, 2019

Thank you for the fast responses! Indeed, it depends on openssl-1.0.x, neither openssl-1.1.x nor any libressl will work. While there was some discussion about getting totally rid of openssl as a dependency, I wasn't able to quickly find proof of someone actually working on it, so openssl-1.0.2 is going to stay a dep for some more time, most likely.
From that standpoint, I'm considering this PR ready for merge, as it won't become any better in the near future. Do you have any other feedback left?

PS: I guess hope that pivx mostly relies on openssl code for their consensus algorithm, and not for actual SSL tunneling. In that case using an outdated version might not be such a big problem?

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

Hard to say how they are using it, but I would maybe open a github issue with them with the request to update, at least in the future? One last rebuild before merging.

@GrahamcOfBorg build altcoints.pivx

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 27, 2019

@mmahut You had a typo, one t too much :)

After doing a quick research, I have to oppose to @globin 's claim of

[an] openssl version that has been EOL for years

According to https://mta.openssl.org/pipermail/openssl-announce/2019-May/000151.html openssl-1.0.2 is still maintained, with having its most recent release just four months back in the past. To me this looks like a 'not so well though-through' basis to convince any pivx maintainer of this being an actual issue.

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

@GrahamcOfBorg build altcoins.pivx

@wucke13 but they claim 1.0.2 is not supported, only 1.0.1 is.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 30, 2019

Where do they claim that?
https://github.com/PIVX-Project/PIVX/blob/master/doc/build-unix.md only mentiones openssl-1.0
Anyway, why didn't the build succeed? Shall I rebase on master once more?

@mmahut
Copy link
Member

mmahut commented Aug 30, 2019

Looks like they have stopped using that check yesteday as per PIVX-Project/PIVX#991.

Please rebase as there is a conflict, given the file has been moved from altcoins to blockchains.

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 30, 2019

Ok. The package is still not evaluating with openssl, so I'm staying with openssl_1_0_2 so far. I rebased on master and added a commit for pivx 3.4.0, hopefully this will work. I will edit this post, once compilation succeeded.

@mmahut
Copy link
Member

mmahut commented Aug 31, 2019

@GrahamcOfBorg build pivx

@wucke13 wucke13 changed the title Pivx wallet pivx: 3.2.0 -> 3.3.0 Pivx wallet pivx: 3.2.0 -> 3.4.0 Sep 26, 2019
Furthermore, this fixes 2542928
Added `test_pivx` as test ran by nixpkgs
@wucke13
Copy link
Contributor Author

wucke13 commented Sep 26, 2019

@mmahut
I'm done I guesss. I added a test, added wrapQtAppsHook to solve the qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in "" issue. Should build both pivx and pivxd just fine.

@Lassulus
Copy link
Member

tested with nix-review, started pivx-qt and clicked around a little bit, everything looked fine

Thanks to @Lassulus for fixing this typo!

Co-Authored-By: Lassulus <github@lassul.us>
@mmahut
Copy link
Member

mmahut commented Oct 11, 2019

@GrahamcOfBorg eval

@Lassulus Lassulus merged commit e9cd8a2 into NixOS:master Oct 12, 2019
@wucke13
Copy link
Contributor Author

wucke13 commented Oct 12, 2019

@Lassulus Thank you! Can we backport this to 19.09 too?

@Lassulus
Copy link
Member

Sure, can you open a backport PR?

@wucke13 wucke13 mentioned this pull request Oct 12, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants