Skip to content

doc: added info to bitcoin.conf doc #22393

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Jul 15, 2021
Merged

doc: added info to bitcoin.conf doc #22393

maflcko merged 1 commit into from
Jul 15, 2021

Conversation

@bliotti
Copy link
Contributor

@bliotti bliotti commented Jul 3, 2021 •
edited
Loading

Should probably be explicitly stated to not make modifications to the conf file while daemon is running. ref #11586

For example, if rpc credentials are modified while bitcoind is running, bitcoin-cli stop is unable to stop bitcoind until the original credentials are restored in bitcoin.conf

@bliotti bliotti changed the title added info to bitcoin.conf doc doc: added info to bitcoin.conf doc Jul 3, 2021
@DrahtBot DrahtBot added the Docs label Jul 3, 2021
hebasto
hebasto reviewed Jul 3, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

@Zero-1729
Copy link
Contributor

Zero-1729 commented Jul 3, 2021

Concept ACK (esp with @hebasto's suggestion)

@sipa
Copy link
Member

sipa commented Jul 3, 2021

I'm not sure that's the advice we should give. There is no problem with modifying the config file while Bitcoin Core is running per se; it's just that changes won't take effect until you restart.

@Zero-1729
Copy link
Contributor

Zero-1729 commented Jul 3, 2021 •
edited
Loading

To @sipa's point, if you are going to add a warning it should probably be to notify users that the changes take effect only after restart. bitcoin-qt already warns the users about it when they modify the configuration file.

So I suppose maybe rewording it to something like:

Changes to the configuration file while `bitcoind` or `bitcoin-qt` is running only takes effect after reset.

@brunoerg
Copy link
Contributor

brunoerg commented Jul 4, 2021

To @sipa's point, if you are going to add a warning it should probably be to notify users that the changes take effect only after restart. bitcoin-qt already warns the users about it when they modify the configuration file.

So I suppose maybe rewording it to something like:

Changes to the configuration file while `bitcoind` or `bitcoin-qt` is running only takes effect after reset.

I think restarting instead of reset would be better.

@jonatack
Copy link
Member

jonatack commented Jul 4, 2021

and s/takes/take/

@Zero-1729
Copy link
Contributor

Zero-1729 commented Jul 5, 2021 •
edited
Loading

Thanks @brunoerg and @jonatack for the catch! It should have been restarting instead of reset. take also fits better, grammatically speaking.

So it should (have) read:

Changes to the configuration file while `bitcoind` or `bitcoin-qt` is running only take effect after restarting.

@bitcoin bitcoin deleted a comment from Meru852 Jul 5, 2021
Zero-1729
Zero-1729 approved these changes Jul 6, 2021
View reviewed changes
Copy link
Contributor

@Zero-1729 Zero-1729 left a comment

Choose a reason for hiding this comment

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

ACK fa84cae

theStack
theStack approved these changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

LGTM
ACK fa84cae 🗄️

@rajarshimaitra
Copy link
Contributor

rajarshimaitra commented Jul 8, 2021

ACK fa84cae

@maflcko maflcko merged commit 853ac47 into bitcoin:master Jul 15, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 23, 2021
Merge bitcoin#22393: doc: added info to bitcoin.conf doc
a2aa6e1 
fa84cae doc: added info to bitcoin.conf doc (Brian Liotti)

Pull request description:

  Should probably be explicitly stated to not make modifications to the conf file while daemon is running. ref bitcoin#11586

  For example, if rpc credentials are modified while bitcoind is running, `bitcoin-cli stop` is unable to stop bitcoind until the original credentials are restored in `bitcoin.conf`

ACKs for top commit:
  rajarshimaitra:
    ACK bitcoin@fa84cae
  Zero-1729:
    ACK fa84cae
  theStack:
    ACK fa84cae 🗄️

Tree-SHA512: f6ddffc25563c0b01e661b6abe43a7909938ad8eca38d6d0e2d4a2ce9fb850e51b54d950ef3118b74b6e340c64fe3f37205861720a2de2933db29782234869bb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 7, 2022
@PastaPastaPasta
Merge bitcoin#22393: doc: added info to bitcoin.conf doc
e206ffe 
fa84cae doc: added info to bitcoin.conf doc (Brian Liotti)

Pull request description:

  Should probably be explicitly stated to not make modifications to the conf file while daemon is running. ref bitcoin#11586

  For example, if rpc credentials are modified while bitcoind is running, `bitcoin-cli stop` is unable to stop bitcoind until the original credentials are restored in `bitcoin.conf`

ACKs for top commit:
  rajarshimaitra:
    ACK bitcoin@fa84cae
  Zero-1729:
    ACK fa84cae
  theStack:
    ACK fa84cae 🗄️

Tree-SHA512: f6ddffc25563c0b01e661b6abe43a7909938ad8eca38d6d0e2d4a2ce9fb850e51b54d950ef3118b74b6e340c64fe3f37205861720a2de2933db29782234869bb
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@apoelstra
Merge 853ac47 into merged_master (Bitcoin PR bitcoin/bitcoin#22393)
0bfb336
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto left review comments

+2 more reviewers

@theStack theStack theStack approved these changes

@Zero-1729 Zero-1729 Zero-1729 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@bliotti @Zero-1729 @sipa @brunoerg @jonatack @rajarshimaitra @theStack @hebasto @maflcko @DrahtBot