Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621795298)
Thanks for the confirmation, appreciate it!
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621797908)
> To my knowledge, weight is always measured in weight units, so I don't think adding the _wu suffix here would add any new information.
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621812773)
Agreed. However if it had not been for the absolute fee, we could come up with a more generic function `get_effective_unspent_value` that calculates based on the `fee_rate` and the `vsize` of the unspent.

Can't include the absolute fee in this because it is paid for the whole transaction and not just one unspent - still valid for transactions spending only one unspent.
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621826033)
I see, nice!
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621826528)
I can pick it up in a follow-up.
💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1621830415)
Actually, I can't reproduce it so far. So a corrupt CI machine is more likely.
💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621836458)
IMO, this 3 deserves a constant soon because it has started showing up in a comment in the functional test as well now besides being in few other places too in this diff. Otherwise, the mind of the code reader wanders off to figuring out where this 3 comes from.
👍 rkrux approved a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2090023300)
tACK [39d135e](https://github.com/bitcoin/bitcoin/pull/30162/commits/39d135e79f3f0c40dfd8fad2c53723d533cd19b4)

@theStack Thanks for addressing the comments and answering my questions!
📝 zoupingshi opened a pull request: "chore: fix some comments"
(https://github.com/bitcoin/bitcoin/pull/30208)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 maflcko commented on pull request "chore: fix some comments":
(https://github.com/bitcoin/bitcoin/pull/30208#issuecomment-2141386493)
You'll have to submit them upstream, see the CI failure (lint)
maflcko closed a pull request: "chore: fix some comments"
(https://github.com/bitcoin/bitcoin/pull/30208)
⚠️ Sjors opened an issue: "Make Transport independent of CNetMessage and CSerializedNetMsg"
(https://github.com/bitcoin/bitcoin/issues/30209)
Currently `class Transport` uses `CNetMessage` in one place:

https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/net.h#L278-L285

And `CSerializedNetMsg` here:

https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/net.h#L287-L295

This creates a problem in my Stratum v2 Template Provider implementation in #28983 which introduces `class Sv2Transport : Transport`. Because the sv2 noise protocol (#29346) is very similar to BI
...
💬 fanquake commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621965700)
Pushed that now.
⚠️ fanquake opened an issue: "build: use UCRT runtime for Windows (release) binaries"
(https://github.com/bitcoin/bitcoin/issues/30210)
Switching to the modern runtime would be good, because the old runtime is missing features, which has meant writing workarounds for Windows in our code: i.e #29014/#29357.

[Mingw-w64 12.0.0 has been released](https://sourceforge.net/p/mingw-w64/mailman/message/58776404/), which now defaults to the `UCRT` runtime, over `MSVCRT`. See https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-doc/howto-build/ucrt-vs-msvcrt.txt for more details. This makes using it somewhat easier, because we c
...
💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622010745)
Wow completely missed the messages here. Yeah, was able to recreate the issue. I'll figure out what the issue is and come back to you after I've got a fix for the problem.
💬 moonsettler commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-2141561465)
> Is it still relevant?

yes!

> Did the author lose interest or time to work on this?

opening this PR against master may not really make sense all things considered. i don't suppose anyone is under the delusion that it will be merged.

here is a branch that will be maintained for an activation client:
https://github.com/lnhance/bitcoin/tree/lnhance-26.x
💬 fanquake commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2141598317)
Using:
```diff
commit 76726e1131e15161ca1d9f90ffd2d337bb9aacdb (HEAD -> repro_29754)
Author: Your Name <you@example.com>
Date: Fri May 31 09:16:25 2024 +0000

repro: 29754

diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk
index a64008d6aa..3cfe333264 100644
--- a/depends/hosts/darwin.mk
+++ b/depends/hosts/darwin.mk
@@ -107,7 +107,7 @@ darwin_CXX=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \

darwin_CFLAGS=-pipe -std=$(C_STANDARD) -mmacosx-version-min=$(OSX_MI
...
💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622171433)
@Sjors it's fixed https://github.com/setavenger/blindbit-oracle/pull/14 now. There was no taproot filter on the biggest utxo selection.
💬 hebasto commented on pull request "depends: qt 5.15.14 and fix macOS build with Clang 18":
(https://github.com/bitcoin/bitcoin/pull/30198#issuecomment-2141709227)
My Guix builds:
```
x86_64
1efa3e0205032d6cfe5517ce36ab63379afa22268dcbb0b92150719baa030682 guix-build-0a3631fc352e/output/aarch64-linux-gnu/SHA256SUMS.part
e3db302484148c00cab7150e3451d2c95dae14062cb4165c8e5a28d71d77e630 guix-build-0a3631fc352e/output/aarch64-linux-gnu/bitcoin-0a3631fc352e-aarch64-linux-gnu-debug.tar.gz
f9792f8db8635bb19f6a2d7c73f1b0780e5776734c12912c5e7172f357f7fa1f guix-build-0a3631fc352e/output/aarch64-linux-gnu/bitcoin-0a3631fc352e-aarch64-linux-gnu.tar.gz
708374dc
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622190959)
Thanks, trying again...