Bitcoin Core Github
42 subscribers
125K links
Download Telegram
💬 Sjors commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#issuecomment-1673353862)
Maybe cherry-pick 461259c4ecc1e48d028eaea56061e72a6667ce4f instead of 34aca2d5f26f0441a366942dc56bd220e806d63c. Seems to compile and pass all tests.
💬 theuni commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290259830)
Hmm, LLVM should be header-only, so I think this is indicative of some real problem.

Could you please paste your link line generated by `make VERBOSE=1`? For Linux mine is:
`/usr/bin/c++ -fPIC -O3 -DNDEBUG -shared -o libbitcoin-tidy.so "CMakeFiles/bitcoin-tidy.dir/bitcoin-tidy.cpp.o" "CMakeFiles/bitcoin-tidy.dir/logprintf.cpp.o"`

---

A few possibilities:
1. Something about the way it's being compiled/linked makes ld64 grumpy.
2. Something related to [this magic line](https://github.
...
🤔 dergoegge reviewed a pull request: "assumeutxo (2)"
(https://github.com/bitcoin/bitcoin/pull/27596#pullrequestreview-1572046251)
The net processing changes look much better (no chainstate pointers on CNodeState, yay!).
💬 dergoegge commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1290225497)
This look suspiciously similar to `FindNextBlocksToDownload`, is there no way to unify the two?

(sorry if I missed previous discussion on this)
💬 dergoegge commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1290241176)
propbably not worth it to pass the chainstate role then?
💬 dergoegge commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1290237668)
`NewPoWValidBlock` is already gated with `m_highest_fast_announce` but I think it would make sense to also ignore bg chainstate events explicitly.
💬 jonatack commented on issue "Raise maximum -dbcache setting":
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1673442897)
There is some related discussion and testing in https://github.com/bitcoin/bitcoin/pull/6102 containing the commit you linked to and more recently in https://github.com/bitcoin/bitcoin/pull/25325.
🤔 instagibbs reviewed a pull request: "validation: avoid mempool eviction mid-package evaluation"
(https://github.com/bitcoin/bitcoin/pull/28251#pullrequestreview-1572237699)
conditional ACK https://github.com/bitcoin/bitcoin/pull/28251/commits/c2c9dfe95b3c5b8332521d9a691725ee55ec3450

on having a test case catching the crash included. Tested with my own test case in https://github.com/bitcoin/bitcoin/pull/26403/commits/e3622f7ec944ec0e4c48ec3f29835d9f90d9d55c
💬 instagibbs commented on pull request "validation: avoid mempool eviction mid-package evaluation":
(https://github.com/bitcoin/bitcoin/pull/28251#discussion_r1290355065)
was this always supposed to be true?
💬 instagibbs commented on pull request "validation: avoid mempool eviction mid-package evaluation":
(https://github.com/bitcoin/bitcoin/pull/28251#discussion_r1290350524)
Declaration comments for `SubmitPackage` should reflect that it doesn't limit anymore, or just not mention limiting.
💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290358956)
Thank you @fanquake and @theuni. I think I'm using clang/llvm from homebrew (and have `export PATH="/opt/homebrew/opt/llvm/bin:$PATH"` in `.zshrc`) but could be missing something. Tried 1. and 2. above and still see `Undefined symbols for architecture arm64` at the second step.

<details><summary>output of <code>make VERBOSE=1</code></summary><p>

```bash
jon|master =:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ clang --version
Homebrew clang version 16.0.6
Target: arm64-apple-darwi
...
🤔 furszy reviewed a pull request: "fuzz: improve `coinselection`"
(https://github.com/bitcoin/bitcoin/pull/27585#pullrequestreview-1572286861)
utACK 1942ea0
🤔 jonatack reviewed a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1572270154)
ACK

Non-blockers, consider changing the commit message to something like `doc: update help for -rpcallowip config option` (it's not an RPC help), and changing the two `#21070` to `PR 21070` so GitHub doesn't add mentions in that issue on every push.
💬 jonatack commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1290372278)
```suggestion
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid values for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
```
💬 furszy commented on issue "wallet_importdescriptors.py: can't decode bytes in position 228861-228863: unexpected end of data":
(https://github.com/bitcoin/bitcoin/issues/28030#issuecomment-1673537303)
#28035 was merged, can this be closed?
📝 dergoegge opened a pull request: "[no merge, meta] refactor: net/net processing split"
(https://github.com/bitcoin/bitcoin/pull/28252)
This PR is supposed to provide context for some of the refactoring PRs I've been working on (#25268, #26621, etc).

The aim is to completely decouple CConnman and its internals from PeerManager to allow for isolated testing of our message processing code (isolated from net only as isolating from validation is another can of worms). To get there, this work refactors CConnman's API to use `NodeId` as the primary handle for managing connections and defines a new interface `ConnectionsInterface` t
...
💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#discussion_r1290433669)
I think that's the reason why OP_VER and its variants are evaluated as disabled opcode while `OP_RESERVED` is evaluated as reserved.
actually other disabled opcodes(which throw disabled err) are based on history.
💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1673617042)
I move the logic to default clause following @ajtowns opinion, but still include `OP_VER` to throw disabled error(because it's disabled opcode).
I also added `reserved` for bad opcode error in 17a0270da5536547850dca06885a278d3e0d3269 which is for reserved opcode.

I mentioned above, but If there's not any meaningful difference btw disabled and reserved ones, it would make sense to throw disabled error for both of them as it's how it works for disabled opcodes.
like below

```cpp
...

...
💬 brunoerg commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1290455071)
nit:
```suggestion
* Analyze and log current health of ASMap based buckets.
```
💬 RicYashiroLee commented on pull request "Inscriptions option":
(https://github.com/bitcoin/bitcoin/pull/27589#issuecomment-1673632015)
> @Retropex: Ask on [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) whether what you are suggesting is a good idea (short answer is any default policy change or custom policy option doesn't necessarily stop inscriptions as you can still submit consensus compatible transactions directly to a miner). There is already an [ordinals tag](https://bitcoin.stackexchange.com/questions/tagged/ordinals) with previous Q&A on this topic.

The key word is 'cumbersome', to make hash-anchoring(st
...