Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 maflcko commented on issue "test: Intermittent issue in feature_init.py", line 88, in run_test with node.wait_for_debug_log([terminate_line]): AssertionError: [node 0] Expected messages "[b'scheduler thread start']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30011#issuecomment-2088093064)
Looks like it ignored the terminate signal
💬 maflcko commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2088103940)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
💬 maflcko commented on issue "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs":
(https://github.com/bitcoin/bitcoin/issues/30001#issuecomment-2088108958)
The gdb txt is empty. You can also post it in a comment, if it is shorter than 100 lines.
💬 WaffleApe commented on issue "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs":
(https://github.com/bitcoin/bitcoin/issues/30001#issuecomment-2088111750)
@maflcko apologies, here is the dgb extract https://pastebin.com/LQri1rtg
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585991033)
The idea is to return `GetChildrenFromSamePeer` in recency order. This one could be returned that way as well, as the goal is to have a random order (and we randomize the indices), but all I wanted to do here was deduplicate.

As for the 2 functions having duplicate code, for a longer explanation: I'm hoping we can remove 1p1cs with different providers by tracking all orphan announcers, which would mean deleting `GetChildrenFromDifferentPeer`. I think it'll be easier to delete the function + u
...
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1586000826)
> Is there a reason to convert the wtxids into hex strings for this comparison? That seems kind of expensive, especially when Wtxid already has an operator<.

> I surmise that this has to do with the "concatenated in lexicographical order (treating the wtxids as little endian encoded uint256, smallest to largest)" and BIP 331 defines the same thing, but it's not clear to me why it must be like that (and couldn't also be changed in the BIP).

The code was originally using the operator< define
...
💬 fanquake commented on pull request "refactor: Avoid unused-variable warning in init.cpp":
(https://github.com/bitcoin/bitcoin/pull/29968#issuecomment-2088128784)
Could be:
```diff
diff --git a/src/init.cpp b/src/init.cpp
index 4d7638cd6e..ebfcb9cf8e 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1312,7 +1312,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
{"-zmqpubsequence", true}
}) {
const std::string arg{port_option.first};
- const bool unix{port_option.second};
+ [[maybe_unused]] const bool unix{port_option.second};
for (const std::string& socket
...
💬 ismaelsadeeq commented on issue "Change estimate_mode default to "ECONOMICAL" in these RPC calls":
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2088162169)
> Therefore, I suggest changing the default fee estimation mode to "ECONOMICAL" in the following RPCs

Unfortunately even "economic" mode still overestimate so changing default mode to "economic" does not fix this issue.

There is an ongoing effort "see [delving post](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703)" where contributors are discussing the challenges of `CBlockPolicyEstimator` which is the fee estimation strategy used by all those RPC's.

Yes th
...
willcl-ark closed a pull request: "net: disconnect inside AttemptToEvictConnection"
(https://github.com/bitcoin/bitcoin/pull/27912)
💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-2088163916)
Going to close this for now as I don't have time to finish it.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1586034211)
`PCKG_POLICY` doesn't populate `m_tx_results` so we wouldn't have results for those transactions - it's meant for situations in which the package is invalid, but the individual transactions could be if submitted in a different context. However, going over the failures that are marked as `PCKG_POLICY`, I agree there's a few failures that are classified as `PCKG_POLICY` but are actually more definitively invalid, and we should bubble that up to p2p.

Having both mempool + package v3 parents, v2-
...
🤔 ismaelsadeeq reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2033245669)
reACK 69deec6fd074e8524dd11103834739b02f50e814
💬 willcl-ark commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2088171543)
Can we close this now then? We're not going to find a more appropriate word than "migrate" for this context, and it seems like there's therefore no action to be taken here.
👍 willcl-ark approved a pull request: "test: Add test case for spending bare multisig"
(https://github.com/bitcoin/bitcoin/pull/29120#pullrequestreview-2033256084)
reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f

Appears to only a rebase on master since my previous ack
```
git range-diff 9c7b2d0d37b31f01403f7a0f2ea25b60b126c841...e504b1fa1fa4d014b329dea81bfdf1aa55db238f
```
💬 maflcko commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2088192746)
I think context can be added to translations, if it isn't already there. See for example 3868ba3a27d594dd2968548fff3db457cbeb0080.
💬 willcl-ark commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2088193794)
Oh ok, great! I wasn't aware that we could add context like that.
💬 maflcko commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2088194592)
An alternative may be to call it "Migrate Wallet Database", not sure which one is better, if any at all. I think if someone cares, they can open a pull request?
👍 willcl-ark approved a pull request: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052#pullrequestreview-2033326580)
ACK faaf3cf535999c2c9a84337414c8b35435384862

Nice change with the same rationale as the previous mempool XOR changes (preventing anti-virus falsely flagging/modifying block files).

One non-blocking comment left as a result of manual testing.
💬 willcl-ark commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1586058508)
Jjust noting here that if the user specifies `-blocksxor=01234567` and a file-based key already exists in the datadir, the key from the file is silently used.

This is the correct behaviour IMO, but wonder if we should log that the user-set option has been overridden?

As @maflcko pointed out to me previously, even if a user mistakenly thought they were using a custom xor key set as a CL option and removed the key file, they could still restore it. So I don't see any dangers with this curren
...
💬 maflcko commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-2088216432)
utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f