Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 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
...
💬 theuni commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290493781)
Ok, I think I see. I can force a similar error on Linux by adding "-Wl,-no-undefined" to the link-line.

I'm guessing ld64 is opinionated about undefined symbols by default. And in this case we're relying on them.

Could you try messing with undefined symbol behavior and seeing what happens?


`/opt/homebrew/opt/ccache/libexec/c++ -O3 -DNDEBUG -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -bundle -Wl,-headerpad_max
...
💬 stevenroose commented on pull request "wallet: Allow users to create a wallet that encrypts all database records":
(https://github.com/bitcoin/bitcoin/pull/28142#issuecomment-1673672150)
Concept ACK :) (Concept Request, in fact)
💬 stevenroose commented on pull request "wallet: Allow users to create a wallet that encrypts all database records":
(https://github.com/bitcoin/bitcoin/pull/28142#issuecomment-1673671724)
> Concept NACK, I think. Wallet encryption is not going to help you if someone steals your wallet, only if someone gains very temporary access to your keyboard/mouse only. I don't think this full-encryption you're proposing has a real use case.
>
> (What might make sense is to support fully encrypting backups.)

I requested this because I have a real use case.. I run a full node that is on an unencrypted external drive. Bitcoin data is public data, it shouldn't be encrypted. I want to use t
...
💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1290509079)
I don't think we need to prepend the zero here.
👍 SambhavsCreation approved a pull request: "test: verify spend from 999-of-999 taproot multisig wallet"
(https://github.com/bitcoin/bitcoin/pull/28212#pullrequestreview-1572527277)
I think it looks good. I think the concerns about testing with different keys are not of critical importance right now. Something could be done about it in the future.