💬 polespinasa commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2383565725)
While some miners may want to not include large OP_RETURNS, I think we should discourage it to miners too.

Miners filtering transactions will probably take more time to start working on the new tip, which is bad for them. This should be a reason to discourage the use for miners.

> And the miner use case doesn't have a natural end of life argument...

See that my proposal is to only discourage the use (always) not to plan for removal.

💬 achow101 commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3340629533)
Can you do the version bump stuff here?

🤔 l0rinc reviewed a pull request: "node: Add --debug_runs/-waitfordebugger + --debug_cmd"
(https://github.com/bitcoin/bitcoin/pull/31723#pullrequestreview-3273781326)
Concept ACK, I'm always struggling with debugging these tests with multiple nodes, I usually end up just printing their internal states.

Thanks for working on this. I went through the code, tried to find simpler solutions to minimize and modernize the code we need to add to production code. I didn't get to trying this out yet, so I'm not sure my suggestions make sense, I just checked if it compiles on a Mac or Linux. Will try to test it properly next week.

Since we're editing production code f
...

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383388859)
nit: comment is redundant

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383387627)
the `0-based` part may need some explanation

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383397007)
[This](https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-isdebuggerpresent) is funny, I wonder why other platforms don't have this.

Seems C++26 will have this standardized: https://en.cppreference.com/w/cpp/utility/is_debugger_present.html, maybe we can add it as a comment (and adjust the naming to match that, if needed)

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383391233)
nice - it might be overkill, but how difficult do you think it would be to add a test for whether we can start a test with debugging? Would help with checking it on other platforms as well

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383394089)
learning from https://github.com/bitcoin/bitcoin/pull/33435 we may want to add support for BSD as well (not sure anyone will use it, but at least we should be aware that it's an option)

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383440187)
nit: any reason for not sorting these?

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383468341)
I wonder if there's a better way than sleep. The [internets](https://www.oreilly.com/library/view/secure-programming-cookbook/0596003943/ch12s13.html) indicates that `raise(SIGSTOP)` + `SIGTRAP` would also pause and revive the process safely - I don't have Linux locally, but maybe we could give it a try

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383424565)
is it possible to add an assert somewhere that this isn't enabled when we're not in debug mode, just to be sure?

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383472403)
Most of this is hard to test, but maybe we can test this safely

nit: had to read the sentence a few times, maybe we can change it to:
```suggestion
return InitError(Untranslated("-waitfordebugger is unavailable in this build. Rebuild with -DWAIT_FOR_DEBUGGER=ON."));
```

💬 ajtowns commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3341177765)
> No because the high latency statistically hurts small miners disproportionately relative to large, and that effect is made worse by selfish mining attacks which benefit large miners.

The difference between large miners and small miners here is that large miners are (a) more likely to get two blocks in a row, meaning they have ~0 latency, and (b) may have more resources to spend setting up their own custom FIBRE relay network or similar, to improve latency for themselves. But the effect of d
...

👍 rkrux approved a pull request: "doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness)"
(https://github.com/bitcoin/bitcoin/pull/33484#pullrequestreview-3274671517)
Ah crACK ff05bebcc4262966b117082a67dc4c63a3f67d2d

There are two different kinds of spellings in the `decodepsbt` response.

💬 maflcko commented on pull request "multiprocess: Add bitcoin-wallet -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/19460#issuecomment-3341361282)
I guess on the next rebase, this would have to revert https://github.com/bitcoin/bitcoin/pull/33459 ?

🤔 kevkevinpal reviewed a pull request: "Rollback for dumptxoutset without invalidating blocks"
(https://github.com/bitcoin/bitcoin/pull/33477#pullrequestreview-3274904950)
Concept ACK [6d409d5](https://github.com/bitcoin/bitcoin/pull/33477/commits/6d409d59704a026a56d18856ab2f9e90eea62186)

This approach makes more sense. I reviewed the code a bit and made some comments, but nothing blocking

I also added comments on possible functional tests for the new `JSONRPCError` but these can be done in a followup

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384138037)
It may be worth noting that "network activity will **_not_** be suspended during this process."

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384142999)
Maybe this text would make more sense

"For deep rollbacks, make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0) as it may take several minutes."

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175011)
Might be able to add a functional test for this rpc error

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175151)
Might be able to add a functional test for this rpc error