✅ fanquake closed an issue: "build: configure using depends by default if config.site exists "
(https://github.com/bitcoin/bitcoin/issues/16692)
(https://github.com/bitcoin/bitcoin/issues/16692)
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2208466255)
> * We add a new zmq publisher, e.g. `-zmqpubtemplate`, which publishes block templates as soon as they become available.
> * We add a new rpc `updatetemplatepub` to configure the template publisher, e.g. for setting the coinbase output data size.
This approach would mostly work. Two downsides that come to mind:
1. it limits the number of connected stratum clients to 1, since there's only one ZMQ template feed.
2. it also precludes the ability to make the template provider public facing
...
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2208466255)
> * We add a new zmq publisher, e.g. `-zmqpubtemplate`, which publishes block templates as soon as they become available.
> * We add a new rpc `updatetemplatepub` to configure the template publisher, e.g. for setting the coinbase output data size.
This approach would mostly work. Two downsides that come to mind:
1. it limits the number of connected stratum clients to 1, since there's only one ZMQ template feed.
2. it also precludes the ability to make the template provider public facing
...
💬 fanquake 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-2208479289)
Is this fixed after #18554?
(https://github.com/bitcoin/bitcoin/issues/16107#issuecomment-2208479289)
Is this fixed after #18554?
💬 hebasto commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#issuecomment-2208481950)
> Use CXX/CXXFLAGS rather than CC/CFLAGS to test our actual compiler for binary checks rather than the one we only forward to secp256k1.
Concept ACK on that.
> From hebasto's CMake repo. See discussion here: [hebasto#252 (comment)](https://github.com/hebasto/bitcoin/pull/252#discussion_r1664657488)
While the discussion was raised in the CMake project, I'd like to note that this change is _not required_ for it.
(https://github.com/bitcoin/bitcoin/pull/30387#issuecomment-2208481950)
> Use CXX/CXXFLAGS rather than CC/CFLAGS to test our actual compiler for binary checks rather than the one we only forward to secp256k1.
Concept ACK on that.
> From hebasto's CMake repo. See discussion here: [hebasto#252 (comment)](https://github.com/hebasto/bitcoin/pull/252#discussion_r1664657488)
While the discussion was raised in the CMake project, I'd like to note that this change is _not required_ for it.
💬 fanquake 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-2208517806)
@willcl-ark was this fixed by #27101 ?
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2208517806)
@willcl-ark was this fixed by #27101 ?
💬 maflcko commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208567773)
For a few reasons:
* There is nothing relevant in the discussion. Some comment, such as https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-1907735929 are about other (real, fixed) bugs.
* Actually the comment about not being able to reproduce: https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-1968819534. Seems relevant, but this is the only one in a total of 14.
* Most of the GHA links are stale, as the cache is cleared by Microsoft
* I don't have the edit permission o
...
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208567773)
For a few reasons:
* There is nothing relevant in the discussion. Some comment, such as https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-1907735929 are about other (real, fixed) bugs.
* Actually the comment about not being able to reproduce: https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-1968819534. Seems relevant, but this is the only one in a total of 14.
* Most of the GHA links are stale, as the cache is cleared by Microsoft
* I don't have the edit permission o
...
💬 fanquake commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208584721)
Ok, we'll continue to keep track of the (sporadic) failures in the new issue. I guess the issue mostly remains a GH/infra one?
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208584721)
Ok, we'll continue to keep track of the (sporadic) failures in the new issue. I guess the issue mostly remains a GH/infra one?
👍 dergoegge approved a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2158527393)
utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2158527393)
utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce
💬 maflcko commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208586729)
Yes, this is my understanding as well.
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2208586729)
Yes, this is my understanding as well.
💬 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.
(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
...
(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)
(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?
(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`.
(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.
(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
...
(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?
(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)
```
(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
(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...
(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.
(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.