💬 BitcoinMechanic commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851220667)
@Sjors this is materially different from the other PR as it at least allows users the ability to configure. Not sure it's appropriate to invoke same reasoning in both cases?

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2851224058)
`61ba3fc494...9057ab8b80`:

> shouldn't `Truncate` and `Commit` also set `m_was_written = true`?

Good question! I think `Truncate()` - yes because it changes the contents of the file. Changed.

`Commit()` does not, so I left it alone. In other words - if a file is opened, none of the `write()`, `write_buffer()` or `Truncate()` is called, `Commit()` is called, and the file object is destroyed, could that be a problem? I think no because the contents of the file was not changed.

💬 BitcoinMechanic commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851232986)
> @nsvrn this PR specifically enables users to have a divergent mempool. I don't understand the use case for that

The use case for users maintaining control over their mempools is self evident.

💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851233257)
@BitcoinMechanic wrote:

> allows users the ability to configure

It briefly retains it, typically deprecation is followed by removal one release later.

So if you're opposed to direct removal, then everyone will understand that you're also opposed to deprecation, so there's nothing new to say.

🤔 polespinasa reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2815021971)
cACK

I like this approach, is far less problematic than https://github.com/bitcoin/bitcoin/pull/32359
Not going to add any more argument than the ones given there.

Left two small non-blocking comments

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073561758)
I think this should be a warning- something like:
```c++
warnings.push_back(_("Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in future versions."));
```
See #31278

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073586466)
A personal opinion to read the code (non blocking)

Maybe would be more clear to have it as a constant and override it if the option is set?
If not, maybe a comment would be good.

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073604279)
functional tests were failing on shutdown when `InitWarning` was being triggered on an earlier version of my code. Is there a way to make the tests ok with it being set for specific tests?

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2851264287)
Some more "buggy" examples

- Find the file descriptor for the debug log, then log some bootleg messages (needs Linux due to `/proc/.../fd` usage)
```python
#!/usr/bin/env python3
# evil-signer1.py
import os, socket, sys

fd_root = f'/proc/{os.getpid()}/fd'
open_files = []
for fname in os.listdir(fd_root):
try:
fd = int(fname)
if fd < 3:
continue
link = os.readlink(os.path.join(fd_root, fname))
open_files.append((fd, link))
exc
...

🤔 ismaelsadeeq reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2814921270)
Concept ACK on relaxing the maximum capacity of the `OP_RETURN` size to the current transaction standardness limit.

1. This change will improve fee estimation in Bitcoin Core, as users will now be able to see the fee pressure from "data-carier" transactions that were previously broadcast out of band.

2. It will also improve block propagation time and make mining fairer, as data carrier transactions below standardness limit will now be relayed in nodes' mempools before being included in blo
...

💬 ismaelsadeeq commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073589388)
From what I see from previous PR is that after deprecation; the feature will be removed in next release cycle, so maybe indicate that?
https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1838749101

If we will eventually deprecate this then let's indicate that.

💬 ismaelsadeeq commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073509605)
Contrary to the PR title and description we still have a capacity of the maximum standard tx weight?

So could this be clarified to indicate that we still maintain the standardness limit?

---
_"hypothetical" but what if we see in the future "transaction landscape has rendered this cap ineffective" as well?_

💬 ismaelsadeeq commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073574032)
nit:
```suggestion
// Multiple TxoutType::NULL_DATA are permitted
```

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073627850)
This should be a no-op at default due to `MAX_STANDARD_TX_WEIGHT`, but if it's clearer to set it to int max or similar, I can do that

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2073633213)
Personally, I'm fine with the option being subset over a longer period. I'll let other people weigh in on that as a possibility.

💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2851339649)
@pinheadmz The failure is from commit https://github.com/starius/bitcoin/commit/60a7042a7219fcfc35e3fea28d038f96a11efcea
It doesn't include this PR, it just the master (slightly outdated, commit eba5f9c4b63fe46261fbb3e71b9a94832d105b23) + the patch I posted above.
The command I'm running is `build/test/functional/test_runner.py feature_signet.py --nocleanup`

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2851344013)
My Guix build:
```
164958d8c5292ef876e3454725888a488565a414af690a568caedae31215db6c guix-build-b074ae1b4f52/output/dist-archive/bitcoin-b074ae1b4f52.tar.gz
145be7d02fd3d967351ad5944ffe4ceb885e3f7bed6677be657b1c0aa125b40c guix-build-b074ae1b4f52/output/x86_64-w64-mingw32/SHA256SUMS.part
2794ba8c24e64ca1068e04b73748e1406df51089a149a8b0a70637d0bf8e120a guix-build-b074ae1b4f52/output/x86_64-w64-mingw32/bitcoin-b074ae1b4f52-win64-codesigning.tar.gz
627119d12ba969849ed7727d105ec7d756e201415c99
...

👋 hebasto's pull request is ready for review: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396)

💬 moth-oss commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851349215)
Concept NACK

I've read the justifications but remain unconvinced the timing as well as the decision to altogether remove the option to configure is the right one.

Increasing the default and giving node runners the option to change that is the right way to go. And a few versions layers maybe the discussion to remove it altogether can be restarted. Current PR is way too sudden. It seems we did not learn the lesson from the unintentional effect lifting the witness script size limit had on th
...

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2851366217)
Just tested with `-dbcache=45000 -connect=<local_node>` and after about an hour I have
`[warning] Flushing large (7 GiB) UTXO set to disk, it may take several minutes`.