Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 achow101 merged a pull request: "Fix -netinfo backward compat with getpeerinfo pre-v26"
(https://github.com/bitcoin/bitcoin/pull/29212)
📝 fanquake opened a pull request: "doc: refer to "Node relay options" in policy/README"
(https://github.com/bitcoin/bitcoin/pull/29235)
Fixed up #29095, to refer to `-help`, rather than listing every option.
fanquake closed a pull request: "Update doc/policy/README.md"
(https://github.com/bitcoin/bitcoin/pull/29095)
💬 fanquake commented on pull request "Update doc/policy/README.md":
(https://github.com/bitcoin/bitcoin/pull/29095#issuecomment-1887707194)
Cherry-picked into #29235.
💬 fanquake commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1887708435)
Maybe draft for now, while we hash out #29189?
📝 maflcko opened a pull request: "log: Nuke error(...)"
(https://github.com/bitcoin/bitcoin/pull/29236)
`error(...)` has many issues:

* It is often used in the context of `return error(...)`, implying that it has a "fancy" type, creating confusing with `util::Result/Error`
* `-logsourcelocations` does not work with it, because it will pretend the error happened inside of `logging.h`
* The log line contains `ERROR: `, as opposed to `[error]`, like for other errors logged with `LogError`.

Fix all issues by removing it.
💬 fanquake commented on pull request "libconsensus: adapt API header to be compliant to ANSI C":
(https://github.com/bitcoin/bitcoin/pull/28661#issuecomment-1887738422)
Could draft for now, dependant on the outcome of #29189?
💬 mzumsande commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1887747143)
Concept ACK. I remember being surprised in the past that >5 minutes after starting up the node, my mempool was still loading.
📝 alfonsoromanz opened a pull request: "[depends] Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237)
The goal of this PR is to help close https://github.com/bitcoin/bitcoin/pull/28733. I reverted the change on `depends/config.guess` based on the feedback provided in the previous PR. I've also incorporated the test mentioned by @maflcko
💬 alfonsoromanz commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1887761563)
Hi @maflcko! I opened this PR for this issue: https://github.com/bitcoin/bitcoin/pull/29237
fanquake closed a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/28733)
💬 maflcko commented on pull request "[depends] Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/29237#issuecomment-1887780628)
lgtm ACK 4756114e505cff8848fb6344ef9a48d8822066c1

Thank you!
💬 maflcko commented on pull request "Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#discussion_r1449283143)
Can remove the useless `__func__` when changing the log output anyway?
💬 jonatack commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1887823265)
Thanks for reporting. Ran into it as well.

https://github.com/bitcoin/bitcoin/actions/runs/7492061657/job/20394747721?pr=29230#step:27:7866
💬 jamesob commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#issuecomment-1887882541)
Big Concept ACK. `error()` confuses me basically every time I run into it.
👍 stickies-v approved a pull request: "Update to new logging API"
(https://github.com/bitcoin/bitcoin/pull/29231#pullrequestreview-1816554251)
ACK 2632d038754d975e76926b7c068309c7aadc82f5

Since we don't have significant merge conflicts I would this support this PR having a non-scripted diff commit to first fix incorrect levels (instead of having to fix and then re-fix) but am okay with this approach too.

Suggested fixes in https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:2024-01/fix-log-level-29231 - happy to open a separate pull for it too?
💬 ryanofsky commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1449355955)
> which is trivial to do with `grep`?

This doesn't seem trivial if some log messages have categories and some don't. Like there is no way anymore to only see a list of all the messages only from wallet, or all the messages only from net_processing. I don't think this is terrible, but it does seem weird, and I wonder if any log systems work like this where they go out of their way to define categories, but then choose not to include categories in the most important messages.
💬 maflcko commented on pull request "ci: Rename tasks (previous releases, macOS cross)":
(https://github.com/bitcoin/bitcoin/pull/29218#issuecomment-1887924197)
CI failure can be ignored
💬 GregTonoski commented on issue "[Bug]: Blockspace price shouldn't be higher for a simple transaction (price discrimination against simple txs)":
(https://github.com/bitcoin/bitcoin/issues/29146#issuecomment-1887925923)
> 2024-01-11T14:31:31 <@sipa> sigh, there is economic demand for block space, whether you like it or not - the buyers and sellers of that space both want the transaction to occur, i don't see what anyone thinks they can do about that

Of course, there is economic demand for data storage. All the more so if it is permanent, distributed, replicated, immutable and branded by Bitcoin.

At the same time, there is economic demand for money (satoshi) transfers.

The block space is limited and
...
💬 ajtowns commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1449401248)
> This doesn't seem trivial if some log messages have categories and some don't.

If you want all log messages to have categories for easier grepping (or for importing into a database), there's the `-loglevelalways=1` option introduced in this PR.

> Like there is no way anymore to only see a list of all the messages only from wallet,

If you want to focus on messages from some section of the code, then that's what the `-logsourcelocations=1` option is for.

Your wording suggests that
...