Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2208589062)
Given that no one has reproduced this locally yet, this may be an issue limited to GHA.
💬 maflcko commented on pull request "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options":
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2208611605)
E.g.:

```diff
commit 94ed6bf4575abee5200e7fc7054a47d66bebd56c
Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed Jul 3 18:05:21 2024 +0200

move-only: Default values in MemPoolLimits

diff --git a/src/kernel/mempool_limits.h b/src/kernel/mempool_limits.h
index 8d4495c3cb..eeeaedd233 100644
--- a/src/kernel/mempool_limits.h
+++ b/src/kernel/mempool_limits.h
@@ -4,9 +4,17 @@
#ifndef BITCOIN_KERNEL_MEMPOOL_LIMITS_H
#define BITCOIN_KERNEL_MEMPOOL_LIMITS_H

-#includ
...
🚀 fanquake merged a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625)
💬 maflcko commented on issue "Bitcoin Core on mainnet shows testnet3 dir as a wallet to open and allows opening it":
(https://github.com/bitcoin/bitcoin/issues/16107#issuecomment-2208645483)
> Is this fixed after https://github.com/bitcoin/bitcoin/pull/18554?

The "allows opening" part should be fixed by this. Not sure about the listing part.

However, if it wasn't fixed by now, my understanding is that this will also be fixed by migrating the BDB wallets to sqlite wallets, because they are placed in dedicated folders, and they can not exists as standalone files?
💬 jeandudey commented on issue "build: use UCRT runtime for Windows (release) binaries":
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-2208648139)
I've sent a patchset a while ago when working on cross-base updating MinGW to 12.0.0: <https://issues.guix.gnu.org/71630>.

Using `UCRT` will likely need a new target triplet for Guix though like MSYS2 does, e.g. `x86_64-w64-ucrt-mingw32`.
💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1665534798)
I don't agree with this suggestion - we are checking the result of a transaction that *was* accepted here. So "would replace" is incorrect and "replaced" is correct.
🤔 stickies-v reviewed a pull request: "[WIP] net: return result from addnode RPC"
(https://github.com/bitcoin/bitcoin/pull/30381#pullrequestreview-2158639334)
There's a lot of stuff happening in one commit:
1. behaviour change: allow removing a v2 connection when `!node_v2transport`
2. behaviour change: explicitly throwing a a `JSONRPCError` when opening the connection for a `onetry` fails, instead of returning NULL (which makes it indistinguishable from a success operation)
3. behaviour change: always returning a results object with input values and `result==success`
4. clang-tidy fixes

I would suggest breaking things up a bit and describing t
...
💬 stickies-v commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#discussion_r1665533399)
nit: is this necessary?
💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665523241)
Renaming would help distinguish the variables which are currently reassigned later. Also, since fee doesn't matter.

```suggestion
_, placeholder_txns3 = self.create_simple_package(coin)
```
💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665526245)
nit: variable naming could be more clear, e.g. `fail_package_hex3` and `success_package_hex3` instead of just numbers
💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665529930)
nit: out of scope for this PR, but I think it's high time we create a constant that represents 1 satoshi as a `Decimal`. Would save a lot of time counting zeroes...
💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665536084)
I think adcfd69e443475f941c0b0f9d1a0246801ab5dad can be dropped.

See https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1665534798. The error strings are correct to be in past tense instead of conditional.
💬 maflcko commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2208691414)
> However there is also a log message in OP from `ReadBlockFromDisk()` which was passed a flat file position it was unable to read from (which is a server error, and should be 500 IMO)

The issue explained in OP can normally happen in normal operation. See https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-1463557219 on how to reproduce.

This is still the case. Header exists, but block has no data:

```
$ curl 'http://__cookie__:a@127.0.0.1:18442' --data '{"method":"getblock",
...
💬 glozow commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2208699739)
Needs rebase
📝 ismaelsadeeq opened a pull request: "BlockAssembler: return selected packages vsize and feerate"
(https://github.com/bitcoin/bitcoin/pull/30391)
This PR enables `BlockAssembler` to add all selected packages' feerate and vsize to a vector, and then return the vector as `vFeeratePerSize` in the `CBlockTemplate` struct.

This PR is the first step in the #30157 project.

The packages' vsize and fee rates are used in #30157 to determine the percentile fee rate of the top block in the mempool.

The first commit changes the data type of `CFeeRate` size from `uint32_t` to `uint64_t` because the `BlockAssembler::addPackageTxs` package size
...
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2208702356)
Addressed @dergoegge and @vasild comments. Ready for review again, reACKs would be appreciated!
💬 glozow commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2208707191)
concept ACK
💬 fanquake commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2208745034)
@instagibbs @sr-gi maybe @mzumsande. This will likely affect you. Any feedback?
⚠️ ismaelsadeeq opened an issue: "Improving Fee estimation tracking issue"
(https://github.com/bitcoin/bitcoin/issues/30392)
This is a tracking issue for an effort to improve fee estimation on Bitcoin Core.

For the conceptual motivation behind this work, see issue #27995 and the previous discussion on Delving Bitcoin: [Mempool-Based Fee Estimation on Bitcoin Core](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/13),

What to Review

- #30391
- #30275

Roadmap

- [ ] #30391
- [ ] Add Fee estimator Module / Mempool forecaster and unit tests
- [ ] Create `estimatefee` rpc an
...
💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2208753220)
So the `Sv2Connman` is called, but `start()` isn't, so the mock `BindListenPort` doesn't happen.
💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1665606867)
I looked into catching specifically `EAI_ADDRFAMILY` errors but this only applies on GNU Libc and is not POSIX. I checked windows and they have different errors. Rather than try to catalogue every possible system and libc implementation, the code now retries on any error (assuming you first tried with `AI_ADDRCONFIG` in the first place).