Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hebasto commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790406668)
```
$ make -C depends capnp MULTIPROCESS=1 HOST=x86_64-w64-mingw32
make: Entering directory '/home/hebasto/git/bitcoin/depends'
Building capnp...
make[1]: Entering directory '/home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/capnp/1.0.1-f19a52e23f7'
make all-am
make[2]: Entering directory '/home/hebasto/git/bitcoin/depends/work/build/x86_64-w64-mingw32/capnp/1.0.1-f19a52e23f7'
/bin/bash ./libtool --tag=CXX --mode=link x86_64-w64-mingw32-g++-posix -I./src -I./src -DKJ_HEA
...
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1379847330)
The idea here is just to make sure a descendant doesn't come before its child. If A is an ancestor of B, then all of A's ancestors are also B's ancestors. It must be that A.ancestor_count < B.ancestor_count. Using total ancestor vsize would also work, but count is much easier to check.
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1379853153)
If the sorting is broken, it means that `FilteredTxns` etc. might return transactions out of order, but this means validation will get a "missing inputs" error and nothing will be submitted. It can also mean that `FilteredTxns` doesn't give the more incentive compatible transactions first, which can affect whether we accept or reject these transactions.

This function being incorrect can't make us produce a consensus-invalid block template. Transactions still need to pass validation to be subm
...
💬 fanquake commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790453392)
While we are changing this, could also add the following. I assume we don't use the TLS lib for anything, so we can skip the configure checks, and building the library if OpenSSL happens to be found:
```diff
--- a/depends/packages/native_capnp.mk
+++ b/depends/packages/native_capnp.mk
@@ -5,6 +5,10 @@ $(package)_download_file=capnproto-c++-$($(package)_version).tar.gz
$(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz
$(package)_sha256_hash=0f7f4b8a76a2cdb284fddef20de8306450
...
📝 maflcko opened a pull request: "refactor: Remove unused circular include dependency from kernel/coinstats.cpp"
(https://github.com/bitcoin/bitcoin/pull/28773)
Also, iwyu
💬 fanquake commented on pull request "refactor: Remove unused circular include dependency from kernel/coinstats.cpp":
(https://github.com/bitcoin/bitcoin/pull/28773#issuecomment-1790469398)
https://github.com/bitcoin/bitcoin/pull/28773/checks?check_run_id=18295730879:
> A new circular dependency in the form of "kernel/coinstats -> node/blockstorage -> validation -> kernel/coinstats" appears to have been introduced.
💬 maflcko commented on pull request "refactor: Remove unused circular include dependency from kernel/coinstats.cpp":
(https://github.com/bitcoin/bitcoin/pull/28773#issuecomment-1790492880)
> A new circular dependency in the form of "kernel/coinstats -> node/blockstorage -> validation -> kernel/coinstats" appears to have been introduced.

Yeah, this is identical to, and already tracked as `"node/blockstorage -> validation -> node/blockstorage"`. I guess I can close for now, until the other one is fixed.
maflcko closed a pull request: "refactor: Remove unused circular include dependency from kernel/coinstats.cpp"
(https://github.com/bitcoin/bitcoin/pull/28773)
💬 fanquake commented on pull request "refactor: [tidy] modernize-type-traits":
(https://github.com/bitcoin/bitcoin/pull/28664#issuecomment-1790517127)
Might come back to this when bear/tidy are working.
fanquake closed a pull request: "refactor: [tidy] modernize-type-traits"
(https://github.com/bitcoin/bitcoin/pull/28664)
🤔 glozow reviewed a pull request: "tests, bug fix: DisconnectedBlockTransactions rewrite followups"
(https://github.com/bitcoin/bitcoin/pull/28530#pullrequestreview-1709976459)
ACK 9b3da70bd06b45482e7211aa95637a72bd115553
💬 glozow commented on pull request "tests, bug fix: DisconnectedBlockTransactions rewrite followups":
(https://github.com/bitcoin/bitcoin/pull/28530#discussion_r1379932340)
For future reference for b2d04479647af64ad7cf5ebfb6175251b2f6b72e

Imagining `DynamicMemoryUsage` for a `DisconnectedBlockTransactions` with 1 transaction (ignoring the `iters_by_txid` part since that doesn't change),

before
```
DynamicUsage(queuedTx) + cachedInnerUsage
=MallocUsage(sizeof(list_node<CTransactionRef>) + cachedInnerUsage
=MallocUsage(sizeof(4 pointers) + RecursiveDynamicUsage(CTransaction)
=MallocUsage(sizeof(4 pointers) + DynamicUsage(vin) + DynamicUsage(vout) + sum([R
...
🚀 glozow merged a pull request: "tests, bug fix: DisconnectedBlockTransactions rewrite followups"
(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
...
glozow closed an issue: "EstimateMedianVal returns higher fee for higher confTarget"
(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)
💬 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 \

...
💬 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.
💬 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?
maflcko closed an issue: "Add generate method to debug console"
(https://github.com/bitcoin-core/gui/issues/55)