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

bisq2: init at 2.1.2 #347160

Merged
merged 1 commit into from
Oct 31, 2024
Merged

bisq2: init at 2.1.2 #347160

merged 1 commit into from
Oct 31, 2024

Conversation

emmanuelrosa
Copy link
Contributor

This PR adds Bisq 2, a decentralized bitcoin exchange that allows anyone to buy and sell bitcoin in exchange for national currencies or other cryptocurrencies.

Bisq 2 is the successor to what is now known as Bisq 1, however Bisq 2 hasn't reached feature parity with Bisq 1. Nevertheless, this package will replace bisq-desktop since openjfx 11 is being dropped from Nixpkgs. See #347149

Verification

It is customary to verify the downloaded Bisq application by importing upstream's GPG key and verifying the signature of the downloaded package.

This Nix package includes build-time verification of the downloaded Debian package. This is to give users of this package a greater level of confirmation --compared to verification by the maintainer, (me)-- that the package has been signed by upstream. In short, if this package builds, then it's also verified.

QR code scanning

There's a bundled "webcam app" which is used for webcam access to scan QR codes. This app does not yet work on NixOS. I'll need to work on that in the future.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@emmanuelrosa
Copy link
Contributor Author

Damn it!

I already ran nixfmt. It's clean:

❯ nixfmt pkgs/by-name/bi/bisq/package.nix

nixpkgs on  bisq2

@emmanuelrosa emmanuelrosa mentioned this pull request Oct 7, 2024
13 tasks
@Shawn8901
Copy link
Contributor

Shawn8901 commented Oct 7, 2024

are you by chance running classic nixfmt instead of nixfmt-rfc-style?

When running nixfmt-classic its happy (as you say), running rfc-style, it returns

[shawn@pointalpha:~/dev/nixpkgs]$ nixfmt -c pkgs/by-name/bi/bisq/package.nix  
pkgs/by-name/bi/bisq/package.nix: not formatted

as the checking action

you might want to either use nix develop or nix shell to auto pick the right tool in the nixpkgs root dir to enable a dev shell with all needed tools (that includes nixfmt-rfc-style).

@emilazy
Copy link
Member

emilazy commented Oct 7, 2024

This currently uses JDK 22, which is in the process of being removed due to reaching the end‐of‐life of its upstream support. Do you know if Bisq 2 will work with JDK 21 (preferable as it’s an LTS release) or 23?

@emmanuelrosa
Copy link
Contributor Author

Thank you @Shawn8901, that was the issue.

@emilazy, I don't know if Bisq 2 will work on JDK 23. I don't see why it wouldn't. However, it will not work on JDK 21.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Oct 7, 2024
@emmanuelrosa emmanuelrosa changed the title bisq2: init @ 2.1.0 bisq2: init @ 2.1.2 Oct 23, 2024
@SuperSandro2000
Copy link
Member

The @ in the title should be replaced with at.

pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved

# Verify the upstream Debian package prior to extraction.
# See https://bisq.wiki/Downloading_and_installing#Verify_installer_file
# This ensures that a successful build of this Nix package requires the Debian
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea except that when the key expires it will break and be no longer reproducible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was planning out this package it was my intention to make it reproducible. However, I had not considered the impact of an expiring key.

I performed a test to see what happens when an expired key is used to verify a signature:

❯ gpg --list-keys
gpg: checking the trustdb
gpg: marginals needed: 3  completes needed: 1  trust model: pgp
gpg: depth: 0  valid:   1  signed:   0  trust: 0-, 0q, 0n, 0m, 0f, 1u
/home/emmanuel/projects/gpgtest/./gnupg/pubring.kbx
---------------------------------------------------
pub   ed25519 2024-10-28 [SC] [expired: 2024-10-29]
      8A9DE59F1481A17B052F4746C325905B2798233C
uid           [ expired] test <[email protected]>

❯ gpg --verify you-didnt-come-this-far.jpg.sig
gpg: assuming signed data in 'you-didnt-come-this-far.jpg'
gpg: Signature made Mon 28 Oct 2024 04:02:14 PM EDT
gpg:                using EDDSA key 8A9DE59F1481A17B052F4746C325905B2798233C
gpg: Good signature from "test <[email protected]>" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8A9D E59F 1481 A17B 052F  4746 C325 905B 2798 233C

~/projects/gpgtest
❯ echo $?
0

The test shows that an expired key does not affect verification.

Now that upstream started adding the keys to the Github release assets, I modified the package to use those. That should also help ensure the builds are reproducible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it factor in that the signing was done before the key expired?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, there's no way to ensure that a signature was created prior to the key expiring.

GPG does refuse to create a signature using an expired key, but certainly that refusal can be circumvented.

This reveals a weakness in the verification process since there's no way to tell if upstream's deb package was signed with an expired key or not. If I were to modify the package to fail upon detecting an expired key, then it would prevent old versions of the package from building.

Since I have the two public keys in my key ring, I can manually check the expiration dates when updating this package, as a form of mitigation.

pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/bisq/package.nix Outdated Show resolved Hide resolved
@emmanuelrosa emmanuelrosa changed the title bisq2: init @ 2.1.2 bisq2: init at 2.1.2 Oct 25, 2024
@emmanuelrosa emmanuelrosa force-pushed the bisq2 branch 3 times, most recently from d9163ca to 6dbe161 Compare October 28, 2024 19:52
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2068

@SuperSandro2000 SuperSandro2000 merged commit 5bc9884 into NixOS:master Oct 31, 2024
27 of 28 checks passed
@emilazy
Copy link
Member

emilazy commented Oct 31, 2024

staging-next is about to hit master with JDK 23, this probably shouldn't have been merged before that as it is going to break eval on the next merge back into staging-next.

@paparodeo
Copy link
Contributor

paparodeo commented Oct 31, 2024

breaks eval: https://gist.github.com/GrahamcOfBorg/27b0818059e4d2c7731cd6800abdd142

   error: undefined variable 'jdk22'

       at /ofborg/checkout/2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-2/pkgs/top-level/all-packages.nix:3295:15:

         3294|   bisq2 = callPackage ../by-name/bi/bisq/package.nix {
         3295|     openjdk = jdk22.override { enableJavaFX = true; };
             |               ^
         3296|   };

please revert or fix.

@paparodeo
Copy link
Contributor

pr to fix eval: #352550

@paparodeo paparodeo mentioned this pull request Oct 31, 2024
13 tasks
@emilazy
Copy link
Member

emilazy commented Oct 31, 2024

@emmanuelrosa, you can open another PR reverting the revert (or just re‐adding it) and redirecting the JDK to 23 if that works. Sorry for not being clearer in my original comment that this should have targeted staging-next to be able to use a JDK not pending removal.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Oct 31, 2024

I do not know the state of staging-next at any given time and if a compiler version is going to be removed there. When I merged it, it was recently still there.

Also from a contributor perspective I would find this very discouraging.

@emilazy
Copy link
Member

emilazy commented Oct 31, 2024

I mentioned it already in this thread, see #347160 (comment). I apologize for not being clearer about what should happen with this PR, but I think I conveyed adequately that JDK 22 is being removed. (It was already removed in staging at that point; I forget whether it had hit staging-next yet.)

@emmanuelrosa emmanuelrosa deleted the bisq2 branch October 31, 2024 16:44
@emmanuelrosa emmanuelrosa mentioned this pull request Nov 15, 2024
13 tasks
@emmanuelrosa
Copy link
Contributor Author

To be continued on #356043

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants