💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043501)
The condition seems correct but the corresponding comment doesn't, it was a bit difficult for me to understand.
How about the following?
```
Prevent bulking tx when target weight (along with offset) is less than or equal to the tx weight
```
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043501)
The condition seems correct but the corresponding comment doesn't, it was a bit difficult for me to understand.
How about the following?
```
Prevent bulking tx when target weight (along with offset) is less than or equal to the tx weight
```
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650044105)
Good addition imo.
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650044105)
Good addition imo.
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043843)
Isn't this debug log incorrect?
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043843)
Isn't this debug log incorrect?
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041303)
Always nice to see getting rid of magic numbers!
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041303)
Always nice to see getting rid of magic numbers!
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043938)
```suggestion
self.log.debug(f"-> tx weight: {tx.get_weight()} is always greater than or equal target weight: {target_weight}")
```
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650043938)
```suggestion
self.log.debug(f"-> tx weight: {tx.get_weight()} is always greater than or equal target weight: {target_weight}")
```
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041868)
➕
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041868)
➕
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041737)
```
"""Pad a transaction with extra outputs until it reaches a target weight (or higher by
at most BULK_TX_PADDING_OFFSET WU).
```
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041737)
```
"""Pad a transaction with extra outputs until it reaches a target weight (or higher by
at most BULK_TX_PADDING_OFFSET WU).
```
💬 rkrux commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041551)
Thanks for this change, I recall highlighting in the previous PR.
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650041551)
Thanks for this change, I recall highlighting in the previous PR.
💬 ismaelsadeeq commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650068106)
Yes, updated.
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650068106)
Yes, updated.
💬 ismaelsadeeq commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650068246)
good catch, updated thanks.
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1650068246)
good catch, updated thanks.
📝 paplorinc opened a pull request: "Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324)
`AddToBlock` was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time.
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/6fd89647-7b6c-4f44-bd04-98d16cd2a938">
This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a measurable speed increase for many iterations.
> ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=1000
before:
| ns/op |
...
(https://github.com/bitcoin/bitcoin/pull/30324)
`AddToBlock` was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time.
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/6fd89647-7b6c-4f44-bd04-98d16cd2a938">
This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a measurable speed increase for many iterations.
> ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=1000
before:
| ns/op |
...
👋 paplorinc's pull request is ready for review: "Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324)
(https://github.com/bitcoin/bitcoin/pull/30324)
📝 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
...
(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.
(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
...
(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
(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".
(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
...
(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
(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)
(https://github.com/bitcoin/bitcoin/pull/30325)