Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 paplorinc opened a pull request: "Optimization: Switch CTxMemPool::CalculateDescendants from set to deque to reduce transaction hash calculations"
(https://github.com/bitcoin/bitcoin/pull/30325)
Profiling `BlockAssemblerAddPackageTxns` indicated excessive hash recalculations which were eliminated by the deque.

<img src="https://github.com/bitcoin/bitcoin/assets/1841944/e1687162-30b5-47b6-83b3-d234cd156395">

The original implementation used a set to manage the stack of transactions for a depth-first traversal of its child transactions.
However, since we're already filtering by `setDescendants` count, duplicates cannot exist in stage anyway.
Therefore, we can change it to a double
...
💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1650116355)
Incorporated.
📝 paplorinc reopened a pull request: "refactor: Preallocate addresses in GetAddr based on nNodes"
(https://github.com/bitcoin/bitcoin/pull/29608)
Closely related to https://github.com/bitcoin/bitcoin/pull/29578/files

Before:
```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 76,852.79 | 13,011.89 | 0.4% | 1.07 | `AddrManGetAddr`
| 76,598.21 | 13,055.14 | 0.2% | 1.07 | `AddrManGetAddr`
| 76,296.32 | 13,106.79 | 0.1% | 1.07 | `AddrManGetAd
...
💬 paplorinc commented on pull request "refactor: Preallocate addresses in GetAddr based on nNodes":
(https://github.com/bitcoin/bitcoin/pull/29608#issuecomment-2185115697)
Reopening since https://github.com/bitcoin/bitcoin/pull/29578 was closed without merging
💬 furszy commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1650127334)
q: is the "undo data" term well known outside of core? It seems to be an implementation detail to me. Maybe, could write this differently: "The block data will be stored alone; there will be no available information associated with the outputs this block spent. This limits the usage.. etc".
💬 techy2 commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2185253218)
Did all that, does not make any difference
executing this command line for depends
make HOST=i686-pc-linux-gnu -j8

however, config.log says this, see attached
configure:3745: $? = 0
configure:3734: gcc -m32 -v >&5
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/10/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa:hsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 10.3.0
...
💬 Andredreyer1 commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2185254617)
Thanks
👋 paplorinc's pull request is ready for review: "optimization: Switch CTxMemPool::CalculateDescendants from set to deque to reduce transaction hash calculations"
(https://github.com/bitcoin/bitcoin/pull/30325)
💬 techy2 commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2185279562)
Last run was attempt at cross compile on ubuntu 18.04 64 bit host attempting cross compile
Xeon X5690
192 gigs memory
1 T disk
autoconf 2.69
automake 1.16.1
gcc 10.3.0
python3 3.6.9

seems like a no-win situation, X64 won't do the 32 bit build and the focal-32 system gcc-10.5 can not find 'cc1' gcc -print-prog-name=cc1 does not provide a path

suggestions?
💬 techy2 commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2185325521)
taking a look at focal32 expected path for cc1, it is not there. Installed gcc 10.5.0 once again and succeeded in populating /usr/lib/gcc/i686-linux-gnu/10/cc1
works now on focal32

However the failure to cross compile on the x64 platform is an issue

--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
from the config log does not look right to me for an 1686 cross build
📝 paplorinc opened a pull request: "optimizations: Reduce cache lookups in CCoinsViewCache::FetchCoin"
(https://github.com/bitcoin/bitcoin/pull/30326)
Enhanced efficiency and readability of `CCoinsViewCache::FetchCoin` by replacing separate `find()` and `emplace()` calls with a single `try_emplace()`, reducing map lookups and potential insertions.

`AssembleBlock` shows `FetchCoin` as one of its bottlenecks:
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/79c7f480-aac2-46da-9ac9-526a02a8eafa">

These changes result in a modest performance improvement:

Before:
| ns/op | op/s | err% | tota
...
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1650196258)
Hm, not sure how much it is known but it is referenced in other places already, like the `getblock` RPC help. And I'm not sure if a more generic text helps more people because it's a quite technical reason and I don't know how to explain it in a way that works for everyone and is still concise. With the term undo data people at least have something they can search on Bitcoin SE etc.
💬 edilmedeiros commented on pull request "[26.x] upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30319#issuecomment-2185496229)
ACK 10413ac46c07d3a45dc9d71818f59ffdb1129032
💬 romanz commented on pull request "rest: don't copy block data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#discussion_r1650328599)
Added https://github.com/bitcoin/bitcoin/commit/9c9375cf3b581c31f047b87a2c05507dc7b31804 with a few more fixes.
👍 rkrux approved a pull request: "wallet: notify when preset + automatic inputs exceed max weight"
(https://github.com/bitcoin/bitcoin/pull/30309#pullrequestreview-2134800855)
tACK [72b2268](https://github.com/bitcoin/bitcoin/pull/30309/commits/72b226882fe2348a9a66aee1d8d21b4e2d275e68)

Reran make and all functional tests. Thanks @furszy for addressing my comments quickly!

> Basically, we are erroring out early when the automatic coin selection process exceeds the maximum weight

I ended up going through the coin selection algorithms and found all of them honouring the max weight criteria in case of automatic coin selection in `coinselection`:
- https://githu
...
💬 rkrux commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1650435604)
Any specific reason to generate 1472 outputs when 1471 are picked?
💬 rkrux commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1650434123)
Thanks for adding a functional test for `send()` as well.
👍 rkrux approved a pull request: "[test]: prevent `create_self_transfer` failure when target weight is below tx weight"
(https://github.com/bitcoin/bitcoin/pull/30322#pullrequestreview-2134883494)
reACK [7d5afbb](https://github.com/bitcoin/bitcoin/pull/30322/commits/7d5afbb5627d754af5db3a2bd4beb5cf0f400b2c)

@ismaelsadeeq Thanks for quickly addressing my comments, these multiple small commits helped in thorough review of the PR.
👍 willcl-ark approved a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2135153143)
ACK a2fc9ddee9cbaeffb3c460973bab3c2dd734db55

Compared changes since last review using `git range-diff 8135632...a2fc9ddee9cbaeffb3c460973bab3c2dd734db55`, which added the `difflib` helper and updated the commit message.
📝 paplorinc converted_to_draft a pull request: "optimization: Switch CTxMemPool::CalculateDescendants from set to vector to reduce transaction hash calculations"
(https://github.com/bitcoin/bitcoin/pull/30325)
Profiling `BlockAssemblerAddPackageTxns` indicated excessive hash recalculations:
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/e1687162-30b5-47b6-83b3-d234cd156395">

The original implementation used a set to manage the stack of transactions for a depth-first traversal of its child transactions. However, since we're already filtering via `setDescendants` before adding them to `stage`, duplicates cannot exist anyway. Therefore, we can change it to a simple vector, providing effi
...