Bitcoin Core Github
44 subscribers
121K links
Download Telegram
bombastictranz closed a pull request: "bombastictranz/bitcoin"
(https://github.com/bitcoin/bitcoin/pull/30323)
💬 romanz commented on pull request "rest: don't copy block data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#discussion_r1649767869)
Thanks for the suggestion!

What do you think about the following modification, in order to support `Span`?
```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 54fec59468..adc6040d08 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -634,7 +634,7 @@ void HTTPRequest::WriteHeader(const std::string& hdr, const std::string& value)
* Replies must be sent in the main loop in the main http thread,
* this cannot be done from worker threads.
*/
-void HTTPReques
...
🤔 TheCharlatan reviewed a pull request: "guix: build with glibc 2.31"
(https://github.com/bitcoin/bitcoin/pull/29987#pullrequestreview-2133927461)
Guix build (riscv64)
```
aa0ac90d4abc930dee2327985fafdbee9c40c8eeea5738383e798f076db36766 guix-build-b5fc6d46a385/output/aarch64-linux-gnu/SHA256SUMS.part
c25dec340e71ba13b238cc743cb86cb6ee564b05f47a1d3df2240f90b8cdf1cd guix-build-b5fc6d46a385/output/aarch64-linux-gnu/bitcoin-b5fc6d46a385-aarch64-linux-gnu-debug.tar.gz
b7e9627b89d06e2646fcc5430f2eb08ab920039cfa9d4ce14917830bc48ba129 guix-build-b5fc6d46a385/output/aarch64-linux-gnu/bitcoin-b5fc6d46a385-aarch64-linux-gnu.tar.gz
3afe6f97bc0
...
👍 TheCharlatan approved a pull request: "Enable clang-tidy checks for self-assignment"
(https://github.com/bitcoin/bitcoin/pull/30234#pullrequestreview-2133941453)
ACK 26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19
💬 fjahr commented on issue "AssumeUTXO Mainnet Readiness Tracking":
(https://github.com/bitcoin/bitcoin/issues/29616#issuecomment-2184214743)
Moved up #29996 and added #30320 and #30288
👍 tdb3 approved a pull request: "rest: don't copy block data when sending binary response"
(https://github.com/bitcoin/bitcoin/pull/30321#pullrequestreview-2133999354)
ACK 07770609afddaa1c730b431c207015f34b3a5f5f

Simple and efficent. Looks good.

Tested with REST calls on two different nodes with signet block 201185 (one running PR 30321 and one running master (538363738e9e30813cf3e76ca4f71c1aaff349e7 at the time)).

PR 30321:
```
curl -v http://127.0.0.1:38332/rest/block/0000005d9cd746eeae8078c9bdb40866e02ddde50680c4c2d9d396dfdb6d3351.bin --output pr30321.bin
sha256sum pr30321.bin
0f88a3cda3b44b2f0fc66006f5ac1349cc8e6a2e7a70f7d5f756355c61d58c76
...
💬 iw4p commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/29398#issuecomment-2184941436)
For some type of errors like this, why we're not containing the values (in this case fee_needed and current_fee) to show the users more clear error message (So he can realize faster what's wrong) and also easier for debugging?
🤔 rkrux requested changes to a pull request: "[test]: prevent `create_self_transfer` failure when target weight is below tx weight"
(https://github.com/bitcoin/bitcoin/pull/30322#pullrequestreview-2134242207)
tACK [d0a0408](https://github.com/bitcoin/bitcoin/pull/30322/commits/d0a0408fcfd3c184a25ca5330dc74f8ecd61f29f)

`make, test/functional` are successful.

Thanks @ismaelsadeeq for picking this up, I like the introduction of few assertions here. Added couple comment nits, and suggested one debug log change.
💬 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
```
💬 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.
💬 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?
💬 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!
💬 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}")
```
💬 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)
💬 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).
```
💬 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.
💬 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.
💬 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.
📝 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 |              
...
👋 paplorinc's pull request is ready for review: "Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324)