🚀 glozow merged a pull request: "tests, bug fix: DisconnectedBlockTransactions rewrite followups"
(https://github.com/bitcoin/bitcoin/pull/28530)
(https://github.com/bitcoin/bitcoin/pull/28530)
💬 maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790540168)
> make: *** [funcs.mk:291: /home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/capnp/1.0.1-f19a52e23f7/./.stamp_built] Error 2
I also tried `0.9.2`, with another error:
```
# make capnp MULTIPROCESS=1 HOST=x86_64-w64-mingw32
Building capnp...
make[1]: Entering directory '/bitcoin-core/depends/work/build/x86_64-w64-mingw32/capnp/0.9.2-e1454ca26e8'
make all-am
make[2]: Entering directory '/bitcoin-core/depends/work/build/x86_64-w64-mingw32/capnp/0.9.2-e1454ca26e8'
depbase
...
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790540168)
> make: *** [funcs.mk:291: /home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/capnp/1.0.1-f19a52e23f7/./.stamp_built] Error 2
I also tried `0.9.2`, with another error:
```
# make capnp MULTIPROCESS=1 HOST=x86_64-w64-mingw32
Building capnp...
make[1]: Entering directory '/bitcoin-core/depends/work/build/x86_64-w64-mingw32/capnp/0.9.2-e1454ca26e8'
make all-am
make[2]: Entering directory '/bitcoin-core/depends/work/build/x86_64-w64-mingw32/capnp/0.9.2-e1454ca26e8'
depbase
...
✅ glozow closed an issue: "EstimateMedianVal returns higher fee for higher confTarget"
(https://github.com/bitcoin/bitcoin/issues/20725)
(https://github.com/bitcoin/bitcoin/issues/20725)
🚀 glozow merged a pull request: "Fee estimation: extend bucket ranges consistently"
(https://github.com/bitcoin/bitcoin/pull/21161)
(https://github.com/bitcoin/bitcoin/pull/21161)
💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1790564148)
> Qt still fails to build the same way as described in the PR description.
The following patch fixes the Qt build for me:
```diff
--- a/depends/hosts/darwin.mk
+++ b/depends/hosts/darwin.mk
@@ -1,4 +1,5 @@
OSX_MIN_VERSION=11.0
+_OSX_MIN_VERSION=110000
OSX_SDK_VERSION=14.0
XCODE_VERSION=15.0.1
XCODE_BUILD_ID=15A507
@@ -75,7 +76,7 @@ $(foreach TOOL,$(cctools_TOOLS),$(eval darwin_$(TOOL) = $$(build_prefix)/bin/$$(
darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \
...
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1790564148)
> Qt still fails to build the same way as described in the PR description.
The following patch fixes the Qt build for me:
```diff
--- a/depends/hosts/darwin.mk
+++ b/depends/hosts/darwin.mk
@@ -1,4 +1,5 @@
OSX_MIN_VERSION=11.0
+_OSX_MIN_VERSION=110000
OSX_SDK_VERSION=14.0
XCODE_VERSION=15.0.1
XCODE_BUILD_ID=15A507
@@ -75,7 +76,7 @@ $(foreach TOOL,$(cctools_TOOLS),$(eval darwin_$(TOOL) = $$(build_prefix)/bin/$$(
darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \
...
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1790575315)
> The following patch fixes the Qt build for me:
We wont be taking that patch, but if the problem is that qt fails to respect `-mmacosx-version-min` for some reason, I would assume that should make it fairly obvious where the problem is.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1790575315)
> The following patch fixes the Qt build for me:
We wont be taking that patch, but if the problem is that qt fails to respect `-mmacosx-version-min` for some reason, I would assume that should make it fairly obvious where the problem is.
💬 maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790587775)
The compile error also happens outside of depends, so I guess someone can reported it upstream?
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790587775)
The compile error also happens outside of depends, so I guess someone can reported it upstream?
✅ maflcko closed an issue: "Add generate method to debug console"
(https://github.com/bitcoin-core/gui/issues/55)
(https://github.com/bitcoin-core/gui/issues/55)
💬 maflcko commented on issue "Add generate method to debug console":
(https://github.com/bitcoin-core/gui/issues/55#issuecomment-1790608649)
Closing for now, due to lack of progress on this test-only feature request. Discussion can continue on the pull request.
(https://github.com/bitcoin-core/gui/issues/55#issuecomment-1790608649)
Closing for now, due to lack of progress on this test-only feature request. Discussion can continue on the pull request.
💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1790613552)
Re ACK 105a0f4db4ffdc25d3ad30300c949d46d5d8e647
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1790613552)
Re ACK 105a0f4db4ffdc25d3ad30300c949d46d5d8e647
💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1380032491)
Changes in this commit is identical to the one in https://github.com/bitcoin/bitcoin/pull/28762/commits/d525998b07be066c97a36536b58f0b98d816be39 , but here the `txmempool.h` dependency from `mini_miner.h`, and the imports are not updated?
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1380032491)
Changes in this commit is identical to the one in https://github.com/bitcoin/bitcoin/pull/28762/commits/d525998b07be066c97a36536b58f0b98d816be39 , but here the `txmempool.h` dependency from `mini_miner.h`, and the imports are not updated?
💬 maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790674841)
Disabled openssl, should be easy to re-ACK
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790674841)
Disabled openssl, should be easy to re-ACK
📝 vasild opened a pull request: "Avoid returning references to mutex guarded members"
(https://github.com/bitcoin/bitcoin/pull/28774)
Avoid this unsafe pattern from `ArgsManager` and `CWallet`:
```cpp
class A
{
Mutex mutex;
Foo member GUARDED_BY(mutex);
const Foo& Get()
{
LOCK(mutex);
return member;
} // callers of `Get()` will have access to `member` without owning the mutex.
```
(https://github.com/bitcoin/bitcoin/pull/28774)
Avoid this unsafe pattern from `ArgsManager` and `CWallet`:
```cpp
class A
{
Mutex mutex;
Foo member GUARDED_BY(mutex);
const Foo& Get()
{
LOCK(mutex);
return member;
} // callers of `Get()` will have access to `member` without owning the mutex.
```
💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1790706698)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1790706698)
Concept ACK
💬 jrakibi commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1790709630)
Is anyone working on this? I'd be happy to take a look
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1790709630)
Is anyone working on this? I'd be happy to take a look
🤔 glozow reviewed a pull request: "Fuzz: Check individual and package transaction invariants"
(https://github.com/bitcoin/bitcoin/pull/28764#pullrequestreview-1710219708)
ACK b847d48f70b5a67263c362f9d28ee6d092068e27, thanks for shaving!
(https://github.com/bitcoin/bitcoin/pull/28764#pullrequestreview-1710219708)
ACK b847d48f70b5a67263c362f9d28ee6d092068e27, thanks for shaving!
💬 glozow commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1380088102)
c1319395f74c0016b367e860050c0ff5a99dea85 nit: the whitespace here is weird, should probably align with parentheses
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1380088102)
c1319395f74c0016b367e860050c0ff5a99dea85 nit: the whitespace here is weird, should probably align with parentheses
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1790719912)
Some test commits have been split off into #28764.
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1790719912)
Some test commits have been split off into #28764.
💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1380105208)
Since `fanout` is already initialized `true`, couldn't we simplify it?
```diff
@@ -5878,19 +5878,17 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
if (reconciles_txs) {
auto txiter = m_mempool.GetIter(txinfo.tx->GetHash());
if (txiter) {
- if ((*txiter)->GetCountWithDescendants() > 1) {
- // If a transaction has in-mempool children, alwa
...
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1380105208)
Since `fanout` is already initialized `true`, couldn't we simplify it?
```diff
@@ -5878,19 +5878,17 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
if (reconciles_txs) {
auto txiter = m_mempool.GetIter(txinfo.tx->GetHash());
if (txiter) {
- if ((*txiter)->GetCountWithDescendants() > 1) {
- // If a transaction has in-mempool children, alwa
...
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1380107969)
> > In other words, if the peer's time deviates slightly from the node time
>
> I don't think `GetAdjustedTime` offers this precision even. Also, it is not a per-peer offset, but a global one.
Yeah I know. I just thought it was better to do that than nothing.
Still, I'm quite sure we cannot trigger the edge case I mentioned above only using core vanilla alone. The pruning process isn't that aggressive for the time being. It might arise across different implementations but we can revisit i
...
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1380107969)
> > In other words, if the peer's time deviates slightly from the node time
>
> I don't think `GetAdjustedTime` offers this precision even. Also, it is not a per-peer offset, but a global one.
Yeah I know. I just thought it was better to do that than nothing.
Still, I'm quite sure we cannot trigger the edge case I mentioned above only using core vanilla alone. The pruning process isn't that aggressive for the time being. It might arise across different implementations but we can revisit i
...
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1790729762)
Rebased 4052c2d5b9f84289752093981f475ea2ca5b64e7 -> 1a589335617944b755235597aafc254e28e4acf9 ([kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_3) -> [kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_3..kernelInternalLib_4))
* Fixed merge conflict.
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1790729762)
Rebased 4052c2d5b9f84289752093981f475ea2ca5b64e7 -> 1a589335617944b755235597aafc254e28e4acf9 ([kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_3) -> [kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_3..kernelInternalLib_4))
* Fixed merge conflict.