💬 glozow commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2519835223)
nit: missing parens

Also, I wonder if it would be confusing that an RPC result named "weight" is adjusted in some RPCs but not in others... should we be calling this "adjusted_weight" ? Or perhaps "clusterweight" and "chunkweight" are enough to indicate that this is in the mempool context and thus not just BIP 141 weight.

💬 glozow commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2519515632)
```suggestion
// Use ChangeSet interface to check whether the cluster count
// limits would be violated. Note that the changeset will be destroyed
// when it goes out of scope.
```

💬 glozow commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2535383593)
57cb99c82504693601709eeb47eba9cd7ab7e387 requires that `GetCluster` output txns in a deterministic order, which could be worth documenting.

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3560813571)
@glozow I actually have one concern around the value of POST_CHANGE_WORK; I think I'm going to suggest we drop it to `5 * ACCEPTABLE_ITERS` to avoid using too much CPU when encountering a cluster that our current linearization code cannot quickly find an optimal ordering.

(Likely this concern will go away completely with SFL (#32545), so I'm doing some benchmarks with and without that code to make sure things look good.)

💬 glozow commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2548163119)
Some documentation that could be worth having in doc/policy/ ?

### Fee and Size Terminology in Mempool Policy

- Each transaction has a **weight** and virtual size as defined in BIP 141 (different from serialized size for witness
transactions as witness data is discounted and the value is rounded up to the nearest integer).

- In the RPCs, "weight" refers to the weight as defined in BIP 141.

- A transaction has a **sigops size**, defined as its sigop cost multiplied by the node's
...

⚠️ byLAEV opened an issue: "Maximum Bitcoin price based on LAEV "
(https://github.com/bitcoin-core/gui/issues/912)
@gregwebs @ldenman @casey @eklitzke @rion @diegoviola @diegoviola @richardkiss @Empact @lian @petertodd @Sjors @witten @nothingmuch @vegard @takinbo

It's very simple, the electronic banking money system has a functional basis and worldwide adoption.Bitcoin was created to be electronic money exactly like traditional bank money with the only difference bein
...

⚠️ byLAEV opened an issue: "100k Bitcoin price: compelling fundamentals "
(https://github.com/bitcoin-core/gui/issues/913)
@gregwebs @ldenman @casey @eklitzke @rion @diegoviola @diegoviola @richardkiss @Empact @lian @petertodd @Sjors @witten @nothingmuch @vegard @takinbo
)

It's very simple, the electronic banking money system has a functional basis and worldwide adoption.Bitcoin was created to be electronic money exactly like traditional bank money with the only difference being that it prevents double spending and is decentralized.Therefore, the value of the satoshi should not be linked to values like 0.001 or 0.
...

💬 Sammie05 commented on pull request "ci: Run GUI unit tests in cross-Windows task":
(https://github.com/bitcoin/bitcoin/pull/33919#issuecomment-3560855219)
The workflow correctly attempts to execute the GUI test binary, ./bin/test_bitcoin-qt.exe. but the CI logs do not show any Qt test output and there are no lines such as ********* Start testing of <TestSuite> *********
PASS / FAIL entries
********* Finished testing of <TestSuite> *********

It would be helpful to confirm that test_bitcoin-qt.exe is executing as intended in the cross-Windows environment.

💬 Sammie05 commented on pull request "test: Retry download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/33915#issuecomment-3561716043)
I verified the retry mechanism by testing both successful downloads (v25.0) and failure scenarios (invalid_version_123), the retry logic correctly activates on HTTP 404 errors and provides clear error message


Fetching: https://bitcoincore.org/bin/bitcoin-core-nvalid_version_123/bitcoin-nvalid_version_123-osx64.tar.gz

Download failed: HTTP Error 404: Not Found
Retrying download after failure ...

Download failed a second time: HTTP Error 404: Not Found

💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2548796423)
I'm working on making it be:

* have node/miner generate two <4MW `CBlock` results, rather than a single <8MW one.
* send a `TEMPLATE` message with both blocks encoded as compact blocks
* request missing txns from the first block; request missing txns from the second block
* receive missing txns from the first block
* receive missing txns from the second block; add the txs from both blocks as that peer's template

Little bit fiddly to make it all work, but I'm not seeing any showst
...

💬 maflcko commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2548811262)
It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.

My hope was that one toolchain could be dropped while adding the new one relatively quickly, or even at the same time.

Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?

💬 janb84 commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2548815278)
Weird, dev-mode should have those flags on.

Your build is reporting this [config](https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19550555322/job/55988016371#step:6:1874).
```sh
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```

💬 hebasto commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2549088989)
> It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.

It appears to be a WIP: https://packages.ubuntu.com/search?keywords=ucrt

> Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?

From https://learn.microsoft.com/en-us/cpp/porting/upgrade-your-code-to-the-universal-crt:
> Many functions were added or updated in the UCRT to improve ISO C99 conformance, and to address code qualit
...

🤔 janb84 reviewed a pull request: "test: Retry download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3491910763)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899

PR adds one retry to download a prev. release.

The single retry is somewhat crude solution but a good first step to solve the intermitted download issues, good KISS sollution. If this isn't enough we can always fallback on more elegant solutions, but this also increases the complexity.

💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3562209741)
review ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f 🚏

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 55c6a69f777a
...

💬 hebasto commented on pull request "Add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3562241063)
https://github.com/bitcoin-core/gui/pull/882#issue-3276693433:

> Provides explicit, privacy-orientd ways to clear console history and output without altering default behavior.
> Useful in shared or long-running sessions.

I’m not convinced that building a privacy-focused feature around the idea of a “shared session” is sound. I’m not even sure what exactly qualifies as a "shared session" in this context, and if privacy is the aim, preventing such scenarios altogether seems the more appropr
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549174545)
Wow! Taken :) plus a comment/note that now the declaration order is now relevant, to prevent somebody reordering those in the future or inserting new members at the beginning.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549185247)
Done, even though `CService{addr}` is also slicing according to https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#es63-dont-slice.

💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2549200835)
Hmm, this was fixed by setting `DYLD_LIBRARY_PATH`.

Fixed in https://github.com/maflcko/bitcoin-core-nightly/commit/c35885a1904c16918af9602e47a456b15b15e941

💬 hebasto commented on pull request "Prevent re-execution of sensitive commands from console history":
(https://github.com/bitcoin-core/gui/pull/909#issuecomment-3562283264)
> Sensitive RPC commands such as `walletpassphrase` or `createwallet`
> have their arguments redacted when stored in the console history.
> Even though their parameters are hidden, these commands could still
> be recalled and executed again, which might lead to unintended or
> harmful actions.

If running the same RPC command twice "might lead to unintended or harmful actions", then the issue should be addressed in that command’s implementation, as it isn’t specific to the GUI console.


...