💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2851174297)
@Sjors I just tested this by modifying the lines [here](https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L97-L98) to be `1min` and `2min`, deleted my `blocks`, `chainstate`, and `indexes` directories and ran `./build/bin/bitcoind -maxconnections=0 -addnode=<local_node> -debug=coindb`. I can confirm it flushes between 1 and 2 minutes after startup and continuously after, due to the change in `chainstate` directory size and the `coindb` logs.

💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2851176101)
What commit is that failure from?

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

If fee estimation, block propagation etc has issues with mempool diversity specially for defense against spam/DoS then the position of not fixing that or making a future economic judgement based on theoretical assumptions is a fallacy. This broad position of Bitcoin Core can clearly increase spam and centralization regardless of economic incentives of miners which are all speculations and projections to modify how Bitcoin works instead of being realistic about the end goal of Bit
...

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851218711)
FWIW I will not engage in meta discussion here. I will respond to any deficiencies in the code itself.

> https://github.com/bitcoin/bitcoin/commit/3ba7449f6c335026b752366c53f9c309f09e6c64 could be split between a commit that allows multiple outputs and one that switches the default.

Touching tests twice is kind of annoying, but I can do if it makes code history easier and others agree.

💬 pinheadmz commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2851220227)
@nsvrn this PR specifically enables users to have a divergent mempool. I don't understand the use case for that, but the proposal here addresses feedback on #32359 and adds a feature specifically for users who do not want their mempool to anticipate miner behavior.

💬 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.