Skip to content

init: Changing the rpcbind argument being ignored to a pop up warning #33813

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
Closed
Ataraxia009 wants to merge 1 commit into from
Closed

init: Changing the rpcbind argument being ignored to a pop up warning #33813

Ataraxia009 wants to merge 1 commit into from

Conversation

@Ataraxia009
Copy link

@Ataraxia009 Ataraxia009 commented Nov 7, 2025

When we ignore a users explicit request to an rpcbind, i think it warrants for a warning instead of a debug log.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 7, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33813.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #33960 (log: Use more severe log level (warn/err) where appropriate by maflcko)
  • #29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member

fanquake commented Nov 7, 2025

https://github.com/bitcoin/bitcoin/actions/runs/19159736145/job/54788142845?pr=33813#step:5:171:

 Duplicate include(s) in src/httpserver.cpp:
#include <node/interface_ui.h>

Can you also shorten the length of the commit title, and add a proper prefix:.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 7, 2025

🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19159736145/job/54788142845
LLM reason (✨ experimental): Lint failures caused the CI to fail (duplicate include in src/httpserver.cpp and all_python_linters errors).

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@DrahtBot DrahtBot mentioned this pull request Nov 8, 2025
Merged
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Nov 9, 2025
@Ataraxia009 @luke-jr
Changing the rpcbind argument being ignored to a pop up warning, inst…
7498da0 
…ead of a debug log

Github-Pull: bitcoin#33813
Rebased-From: 0cca5b772a9a76edc2f15e2f476ea138046c9cb8
laanwj
laanwj reviewed Nov 11, 2025
View reviewed changes
src/httpserver.cpp
@@ -375,7 +376,7 @@ static bool HTTPBindAddresses(struct evhttp* http)
LogPrintf("WARNING: option -rpcallowip was specified without -rpcbind; this doesn't usually make sense\n");
Copy link
Member

@laanwj laanwj Nov 11, 2025

Choose a reason for hiding this comment

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

Do we want to change this one to a warning as well? It even say "WARNING".

Copy link
Author

@Ataraxia009 Ataraxia009 Nov 14, 2025

Choose a reason for hiding this comment

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

Not necessary since its a weird case imo? Why would you ever hit it?

Copy link
Member

@maflcko maflcko Nov 19, 2025

Choose a reason for hiding this comment

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

Why would you ever hit it?

the warning can be hit when -rpcallowip was specified without -rpcbind?

Copy link
Author

@Ataraxia009 Ataraxia009 Nov 24, 2025

Choose a reason for hiding this comment

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

right but is there an actual case where people do that? The only reason you would need to -rpcallowip is if you are exposing the rpc server with -rpcbind?

@Ataraxia009 Ataraxia009 changed the title Changing the rpcbind argument being ignored to a pop up warning, inst… init: Changing the rpcbind argument being ignored to a pop up warning, inst… Nov 14, 2025
@Ataraxia009 Ataraxia009 changed the title init: Changing the rpcbind argument being ignored to a pop up warning, inst… init: Changing the rpcbind argument being ignored to a pop up warning Nov 14, 2025
This was referenced Nov 22, 2025
Open
Merged
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 2, 2025

🐙 This pull request conflicts with the target branch and needs rebase.

@Ataraxia009
Copy link
Author

Ataraxia009 commented Dec 4, 2025

closing this, this is resolved by: fa45a15

@Ataraxia009 Ataraxia009 closed this Dec 4, 2025
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 13, 2026
@Ataraxia009 @luke-jr
Capitalise rpcbind-ignored warning message
7bd80ba 
Github-Pull: bitcoin#33813
Rebased-From: 335a05c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@laanwj laanwj laanwj left review comments

+1 more reviewer

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Needs rebase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Ataraxia009 @DrahtBot @fanquake @laanwj @maflcko