✅ achow101 closed an issue: "`combinepsbt` RPC does not work with P2TR inputs"
(https://github.com/bitcoin/bitcoin/issues/27219)
(https://github.com/bitcoin/bitcoin/issues/27219)
🤔 mzumsande reviewed a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3190527540)
I have now pushed an update - I went with the approach of @davidgumberg because it's simpler - when we use mocktime, `MAX_REPEATS` should no longer be necessary.
I made small changes to the suggestion above:
- used current time with `setmocktime`, going back in time seems brittle.
- I didn't understand the comment "Checking that the inv contains the wtxid would have a 1 in 10^6 chance of spuriously failing (...)", so I didn't take that part.
If `NextInvToInbounds` doesn't fire, we should
...
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3190527540)
I have now pushed an update - I went with the approach of @davidgumberg because it's simpler - when we use mocktime, `MAX_REPEATS` should no longer be necessary.
I made small changes to the suggestion above:
- used current time with `setmocktime`, going back in time seems brittle.
- I didn't understand the comment "Checking that the inv contains the wtxid would have a 1 in 10^6 chance of spuriously failing (...)", so I didn't take that part.
If `NextInvToInbounds` doesn't fire, we should
...
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2325948813)
I think this can be removed now, it was added to be used with `BOOST_CHECK_THROW` / `BOOST_CHECK_NO_THROW` which are no longer used after https://github.com/bitcoin/bitcoin/pull/33011.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2325948813)
I think this can be removed now, it was added to be used with `BOOST_CHECK_THROW` / `BOOST_CHECK_NO_THROW` which are no longer used after https://github.com/bitcoin/bitcoin/pull/33011.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326001691)
Done.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326001691)
Done.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326001953)
Added a comment to explain that.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326001953)
Added a comment to explain that.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326002614)
I've rewritten this using the "loop of comand-- approach".
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326002614)
I've rewritten this using the "loop of comand-- approach".
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326002899)
I have rewritten this.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2326002899)
I have rewritten this.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3259692570)
I have implemented the "abstract Cluster" approach anyway, as special-casing singletons outside of the Cluster logic was too cumbersome.
Old numbers:
* 384 bytes per singleton cluster
* 512 or 576 bytes per pair cluster
New numbers:
* 224 bytes per singleton cluster
* 520 or 584 bytes per pair cluster
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3259692570)
I have implemented the "abstract Cluster" approach anyway, as special-casing singletons outside of the Cluster logic was too cumbersome.
Old numbers:
* 384 bytes per singleton cluster
* 512 or 576 bytes per pair cluster
New numbers:
* 224 bytes per singleton cluster
* 520 or 584 bytes per pair cluster
📝 ryanofsky opened a pull request: "Update libmultiprocess subtree to improve build and logs"
(https://github.com/bitcoin/bitcoin/pull/33322)
Includes:
- https://github.com/bitcoin-core/libmultiprocess/pull/197
- https://github.com/bitcoin-core/libmultiprocess/pull/202
- https://github.com/bitcoin-core/libmultiprocess/pull/203
- https://github.com/bitcoin-core/libmultiprocess/pull/200
- https://github.com/bitcoin-core/libmultiprocess/pull/205
These changes should give better feedback when there are build errors, and also make IPC logs more readable.
The changes can be verified by running `test/lint/git-subtree-check.sh sr
...
(https://github.com/bitcoin/bitcoin/pull/33322)
Includes:
- https://github.com/bitcoin-core/libmultiprocess/pull/197
- https://github.com/bitcoin-core/libmultiprocess/pull/202
- https://github.com/bitcoin-core/libmultiprocess/pull/203
- https://github.com/bitcoin-core/libmultiprocess/pull/200
- https://github.com/bitcoin-core/libmultiprocess/pull/205
These changes should give better feedback when there are build errors, and also make IPC logs more readable.
The changes can be verified by running `test/lint/git-subtree-check.sh sr
...
💬 ryanofsky commented on issue "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/issues/33315#issuecomment-3259719642)
Subtree update is #33322
(https://github.com/bitcoin/bitcoin/issues/33315#issuecomment-3259719642)
Subtree update is #33322
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2326052487)
I removed the `destroy` functions, like you said, there is no good reason to keep them, if we only have `const` pointers anyway. I also removed the `nullptr` checks altogether. I don't think there is really a good reason to keep them.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2326052487)
I removed the `destroy` functions, like you said, there is no good reason to keep them, if we only have `const` pointers anyway. I also removed the `nullptr` checks altogether. I don't think there is really a good reason to keep them.
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2326052516)
The handle type is not exposed to the user (other than being defined in the header), so I don't think it matters that much. I think I prefer the current approach though, since the difference between the two is pretty clear. I think there is a decent reason why would not want to copy a `ContextOptions`: We transfer potential ownership of a user-space object to it, which could be a double free if we just copy it to another object. We could make it a shared pointer internally, but then it might als
...
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2326052516)
The handle type is not exposed to the user (other than being defined in the header), so I don't think it matters that much. I think I prefer the current approach though, since the difference between the two is pretty clear. I think there is a decent reason why would not want to copy a `ContextOptions`: We transfer potential ownership of a user-space object to it, which could be a double free if we just copy it to another object. We could make it a shared pointer internally, but then it might als
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3259751728)
Thank you for the review @stickies-v! Took most of your suggestions and added a few minor additional cleanups.
82c503641a3a848234bba5c9979b9c4df5e53ea7 -> 89f9518af5c92a5daac49b9ed4b8b9ad81c2c354 ([kernelApi_62](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_62) -> [kernelApi_63](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_63), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_62..kernelApi_63))
* Addressed @stickies-v's [comment](https://github.com/bit
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3259751728)
Thank you for the review @stickies-v! Took most of your suggestions and added a few minor additional cleanups.
82c503641a3a848234bba5c9979b9c4df5e53ea7 -> 89f9518af5c92a5daac49b9ed4b8b9ad81c2c354 ([kernelApi_62](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_62) -> [kernelApi_63](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_63), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_62..kernelApi_63))
* Addressed @stickies-v's [comment](https://github.com/bit
...
💬 davidgumberg commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3259772527)
> If `NextInvToInbounds` doesn't fire, we should just get no inv and the `wait_until` should fail (I also used 120s timeout here to make that less likely), but if we receive an inv, shouldn't it always contain the wtxid?
My understanding of [`Peer::TxRelay::m_tx_inventory_known_filter`](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/net_processing.cpp#L297-L300) is that it will [occasionally indicate](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf219
...
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3259772527)
> If `NextInvToInbounds` doesn't fire, we should just get no inv and the `wait_until` should fail (I also used 120s timeout here to make that less likely), but if we receive an inv, shouldn't it always contain the wtxid?
My understanding of [`Peer::TxRelay::m_tx_inventory_known_filter`](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/net_processing.cpp#L297-L300) is that it will [occasionally indicate](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf219
...
💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3259826186)
I was able to get AFL++ multi-core fuzzing to work by adding a static `FuzzedDirectoryWrapper` instance that deletes the passed path in its destructor as well as using a random element to each statically named datadir. Each instance is in the high 98-99% stability and does not crash after hitting 100,000 iterations (yay). I was not able to use multiple `TestingSetup`s at the same time without causing several panics due to assertions about globals. I will fix the lint, tidy jobs and see if the de
...
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3259826186)
I was able to get AFL++ multi-core fuzzing to work by adding a static `FuzzedDirectoryWrapper` instance that deletes the passed path in its destructor as well as using a random element to each statically named datadir. Each instance is in the high 98-99% stability and does not crash after hitting 100,000 iterations (yay). I was not able to use multiple `TestingSetup`s at the same time without causing several panics due to assertions about globals. I will fix the lint, tidy jobs and see if the de
...
👋 hebasto's pull request is ready for review: "Release: 30.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/33275)
(https://github.com/bitcoin/bitcoin/pull/33275)
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3259983275)
Rebased. Updated.
Ready for review.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3259983275)
Rebased. Updated.
Ready for review.
💬 hebasto commented on pull request "Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3260012080)
Concept ACK.
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3260012080)
Concept ACK.
🚀 hebasto merged a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
(https://github.com/bitcoin-core/gui/pull/884)
⚠️ Bottegatecnologica opened an issue: "Potential systemic risk: OP_RETURN outputs as unintended governance signaling"
(https://github.com/bitcoin/bitcoin/issues/33323)
## Context
In `policy/policy.cpp`, inside `IsStandardTx`, OP_RETURN (NULL_DATA) outputs are limited in size (`nMaxDatacarrierBytes`, default 80 bytes). This effectively prevents arbitrarily large payloads but does not fully eliminate systemic risks from structured misuse.
## Description of risk
Even within the allowed size, OP_RETURN can be systematically repurposed as a signaling channel. For example:
- Each UTXO could embed a short marker in its OP_RETURN.
- These markers could be aggregated
...
(https://github.com/bitcoin/bitcoin/issues/33323)
## Context
In `policy/policy.cpp`, inside `IsStandardTx`, OP_RETURN (NULL_DATA) outputs are limited in size (`nMaxDatacarrierBytes`, default 80 bytes). This effectively prevents arbitrarily large payloads but does not fully eliminate systemic risks from structured misuse.
## Description of risk
Even within the allowed size, OP_RETURN can be systematically repurposed as a signaling channel. For example:
- Each UTXO could embed a short marker in its OP_RETURN.
- These markers could be aggregated
...
📝 l0rinc opened a pull request: "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup"
(https://github.com/bitcoin/bitcoin/pull/33324)
### Draft for now - comments are welcome
* Should we repurpose the existing `-blocksxor` arg instead?
* Would multithreading help enough to justify the extra complexity (ordering, extra memory), given the current IO-bound profile?
* Should we split `ObfuscateBlocks` out of init?
* Which additional scenarios would you like covered in testing (resume mid-run, explicit 16-hex key path, large datadirs, pruned)?
* Release notes, translations?
* Given user concern about what they store on disk
...
(https://github.com/bitcoin/bitcoin/pull/33324)
### Draft for now - comments are welcome
* Should we repurpose the existing `-blocksxor` arg instead?
* Would multithreading help enough to justify the extra complexity (ordering, extra memory), given the current IO-bound profile?
* Should we split `ObfuscateBlocks` out of init?
* Which additional scenarios would you like covered in testing (resume mid-run, explicit 16-hex key path, large datadirs, pruned)?
* Release notes, translations?
* Given user concern about what they store on disk
...