Bug 215059 - [PATCH] for net-p2p/bitcoin and net-p2p/bitcoin-daemon to fix startup and create user/group
Summary: [PATCH] for net-p2p/bitcoin and net-p2p/bitcoin-daemon to fix startup and cre...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Li-Wen Hsu
URL:
Keywords: patch
Depends on: 222167
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-05 04:04 UTC by Christopher Hall
Modified: 2017-09-09 11:14 UTC (History)
3 users (show)

See Also:
robbak: maintainer-feedback+
koobs: merge-quarterly?


Attachments
combined bitcoin / bitcoin-daemon patches (2.17 KB, patch)
2016-12-05 04:04 UTC, Christopher Hall
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Hall 2016-12-05 04:04:44 UTC
Created attachment 177675 [details]
combined bitcoin / bitcoin-daemon patches

Patch file for:

a) net-p2p/bitcoin to fix path error in startup script files/bitcoin.in
   also removes reliance on unnecessary cli script.

b) net-p2p/bitcoin-daemon to create the user/group for the daemon to use
   also install a sample configuration file.

With these two patches a "pkg install bitcoin-daemon" followed by "service start bitcoind" will be sufficient to run as a non-privileged user and automatically connect to bitcoin live network; using /var/db/bitcoin to store blockchain data.
Comment 1 robbak 2016-12-10 05:54:56 UTC
This looks fine. Nice addition to the scripts. I also agree that just a normal kill of the daemon is fine.
Comment 2 Li-Wen Hsu freebsd_committer 2017-07-16 22:55:02 UTC
I am a bit curious here:

1) Why change data directory to /var/db/bitcoin, is this used as default location in the application?

2) Why directly use `kill ${pid}` but not original stop command in cli?
Comment 3 robbak 2017-07-16 23:34:01 UTC
(In reply to Li-Wen Hsu from comment #2)
1) Why change data directory to /var/db/bitcoin, is this used as default
location in the application?

Because /var/db is the canonical location for data like this. The default location is inside the user's home directory, which, for a system user like this, should not exist.

2) Why directly use `kill ${pid}` but not original stop command in cli?

Because the -cli commands are installed by the separate Bitcoin-cli port, and are not strictly necessary to run the daemon.
Comment 4 Li-Wen Hsu freebsd_committer 2017-07-17 00:27:49 UTC
(In reply to robbak from comment #3)
>> 1) Why change data directory to /var/db/bitcoin, is this used as default
>> location in the application?

> Because /var/db is the canonical location for data like this. The default location is inside the user's home directory, which, for a system user like this, should not exist.

But the patch only rename the directory from /var/db/bitcoind to /var/db/bitcoin, is this a cosmetic change?  (That also works, but there are lots of "bitcoin" and "bitcoind" in this file. Do we have a convention about how to use them?)

>> 2) Why directly use `kill ${pid}` but not original stop command in cli?

> Because the -cli commands are installed by the separate Bitcoin-cli port, and are not strictly necessary to run the daemon.

That makes sense.
Comment 5 Christopher Hall 2017-07-17 02:05:33 UTC
About bitcoin/bitcoind the default configuration is bitcoin.conf so I was thinking that maybe user/group/datadir should be bitcoin and only the daemon name is bitcoind; does that seem reasonable?
Comment 6 Li-Wen Hsu freebsd_committer 2017-07-17 12:20:31 UTC
(In reply to Christopher Hall from comment #5)
> About bitcoin/bitcoind the default configuration is bitcoin.conf so I was thinking that maybe user/group/datadir should be bitcoin and only the daemon name is bitcoind; does that seem reasonable?

I have no preference between them, but please note that if we change this directory, the users who upgrade from old version have original data in /var/db/bitcoind.  These data will not be found by the new package.  If we are going to do this, we need to have an UPDATING entry to notify users to move data or specify bitcoind_data_dir by themselves.

Also, this modification needs bump PORTREVISION.
Comment 7 robbak 2017-07-17 12:43:21 UTC
(In reply to Li-Wen Hsu from comment #6)
OK, agree on both points.

While /var/db/bitcoin is the best place for it (as the location still makes sense if used with bitcon-qt), if the existing script uses /var/db/bitcoind, then it makes sense to keep it.

Also ACK on the portrevision bump.
Comment 8 Li-Wen Hsu freebsd_committer 2017-07-17 13:16:26 UTC
(In reply to robbak from comment #7)

If we believe that /var/db/bitcoin is a better location, just writing an UPDATING entry to notify user is okay.  Or we can modify the startup script to provide backward compatibility.

I am happy to commit your new patch :-)
Comment 9 robbak 2017-07-17 21:46:20 UTC
(In reply to Li-Wen Hsu from comment #8)
Then go for it. This is been sitting in the queue for too long.
Comment 10 commit-hook freebsd_committer 2017-08-01 16:46:16 UTC
A commit references this bug:

Author: lwhsu
Date: Tue Aug  1 16:45:33 UTC 2017
New revision: 447030
URL: https://svnweb.freebsd.org/changeset/ports/447030

Log:
  - net-p2p/bitcoin: fix path error in startup script files/bitcoin.in also
    removes reliance on unnecessary cli script.

  - net-p2p/bitcoin-daemon: create the user/group for the daemon to use also
    install a sample configuration file.

  With these two patches a "pkg install bitcoin-daemon" followed by "service
  start bitcoind" will be sufficient to run as a non-privileged user and
  automatically connect to bitcoin live network; using /var/db/bitcoin to store
  blockchain data.

  PR:		215059
  Submitted by:	Christopher Hall <hsw@bitmark.com>
  Approved by:	<robbak@robbak.com> (maintainer)

Changes:
  head/UPDATING
  head/net-p2p/bitcoin/Makefile
  head/net-p2p/bitcoin/files/bitcoind.in
  head/net-p2p/bitcoin-daemon/Makefile