💬 bigshiny90 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874819011)
> @BootsStribling
>
> > This PR achieves none of the stated benefits of #32359 which were:
> >
> > * reduction of harm through the use of OP_RETURN rather than witness storage
> >
> > The ultimate result of this particular PR will be:
> >
> > * continued use of witness data field for arbitrary data incentivized by lower cost from witness discount
>
> The harm reduction argument is that OP_RETURN use allows L2 users an alternative to stuffing arbitrary data into non-provably unspen
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2874819011)
> @BootsStribling
>
> > This PR achieves none of the stated benefits of #32359 which were:
> >
> > * reduction of harm through the use of OP_RETURN rather than witness storage
> >
> > The ultimate result of this particular PR will be:
> >
> > * continued use of witness data field for arbitrary data incentivized by lower cost from witness discount
>
> The harm reduction argument is that OP_RETURN use allows L2 users an alternative to stuffing arbitrary data into non-provably unspen
...
🤔 Kixunil reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2835095098)
Concept ACK, the code looks sensible from brief look, though it's unclear what the intention of the parameter was.
I'd also like to ask the opposition to address the numerous arguments about the limit driving centralization and causing long-term storage problems as well as causing the size of the chain to **increase** in the **mailinglist**. I haven't seen a single person attempting to do that yet.
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2835095098)
Concept ACK, the code looks sensible from brief look, though it's unclear what the intention of the parameter was.
I'd also like to ask the opposition to address the numerous arguments about the limit driving centralization and causing long-term storage problems as well as causing the size of the chain to **increase** in the **mailinglist**. I haven't seen a single person attempting to do that yet.
💬 Kixunil commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2085799758)
This doesn't account for the length of serialization of value (8B). So after this change if one wants to keep the 80B limit, the transaction can actually increase by a lot more (80*9 if I count correctly). While I find the arguments for keeping the limit unconvincing this seems to not follow the intention. (But it is true that the amount cannot contain *arbitrary* data.)
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2085799758)
This doesn't account for the length of serialization of value (8B). So after this change if one wants to keep the 80B limit, the transaction can actually increase by a lot more (80*9 if I count correctly). While I find the arguments for keeping the limit unconvincing this seems to not follow the intention. (But it is true that the amount cannot contain *arbitrary* data.)
💬 monlovesmango commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2874964921)
reACK 8673e8f01917b48a5f5476792f759f44ea49d5a5
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2874964921)
reACK 8673e8f01917b48a5f5476792f759f44ea49d5a5
💬 1440000bytes commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875133307)
@FractalEncrypt you can use python to create a transaction with multiple OP_RETURN, `fundrawtransaction` to add inputs, `signrawtransactionwithwallet` to sign and `sendrawtransaction` to broadcast it later. It will be listed in the output for `getrawmempool` once broadcasted.
```python
from bitcoin.core import CTransaction, CTxOut, CScript, b2x
from bitcoin.core.script import OP_RETURN
inputs = []
outputs = [
CTxOut(0, CScript([OP_RETURN, b'deadbeef'])),
CTxOut(0, CScript([OP
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875133307)
@FractalEncrypt you can use python to create a transaction with multiple OP_RETURN, `fundrawtransaction` to add inputs, `signrawtransactionwithwallet` to sign and `sendrawtransaction` to broadcast it later. It will be listed in the output for `getrawmempool` once broadcasted.
```python
from bitcoin.core import CTransaction, CTxOut, CScript, b2x
from bitcoin.core.script import OP_RETURN
inputs = []
outputs = [
CTxOut(0, CScript([OP_RETURN, b'deadbeef'])),
CTxOut(0, CScript([OP
...
💬 delta1 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875159636)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875159636)
Concept ACK
💬 maflcko commented on pull request "build: enable libc++ hardening in debug builds":
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2875226336)
Seems fine. I tested not this pull, but `rm -rf ./bld-cmake && cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-std=c++23 -O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++;-stdlib=libc++;-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG'` and found that some benchmarks are slower by 10x (`--filter=EvictionProtection3Networks100Candidates`), however this should be fine, as it can be disabled. Funnily, some are even *faster*, like (`--
...
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2875226336)
Seems fine. I tested not this pull, but `rm -rf ./bld-cmake && cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-std=c++23 -O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++;-stdlib=libc++;-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG'` and found that some benchmarks are slower by 10x (`--filter=EvictionProtection3Networks100Candidates`), however this should be fine, as it can be disabled. Funnily, some are even *faster*, like (`--
...
📝 maflcko opened a pull request: "refactor: Remove unused HaveKey and HaveWatchOnly"
(https://github.com/bitcoin/bitcoin/pull/32476)
removes some dead code
(https://github.com/bitcoin/bitcoin/pull/32476)
removes some dead code
💬 maflcko commented on pull request "refactor: Removals after bdb removal":
(https://github.com/bitcoin/bitcoin/pull/32438#issuecomment-2875411238)
The final possible bdb removals can be found via `git grep -i 'legacy wallet' src/bitcoin-wallet.cpp src/wallet src/script/`.
Some may be fine to keep as historic comment and others are not a clean removal, some may need to replaced by a Assert/throw or `util::Error`.
In any case, if the untested code isn't removed, it should be tested:
* https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/db.cpp.gcov.html
* https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/dump.cpp.
...
(https://github.com/bitcoin/bitcoin/pull/32438#issuecomment-2875411238)
The final possible bdb removals can be found via `git grep -i 'legacy wallet' src/bitcoin-wallet.cpp src/wallet src/script/`.
Some may be fine to keep as historic comment and others are not a clean removal, some may need to replaced by a Assert/throw or `util::Error`.
In any case, if the untested code isn't removed, it should be tested:
* https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/db.cpp.gcov.html
* https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/dump.cpp.
...
💬 maflcko commented on pull request "refactor: Remove unused HaveKey and HaveWatchOnly":
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875412918)
(follow-up to https://github.com/bitcoin/bitcoin/pull/32438)
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875412918)
(follow-up to https://github.com/bitcoin/bitcoin/pull/32438)
💬 maflcko commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2086208814)
> Yes, this one definitely, but in the other cases I'm worried about introducing a strong bias.
Again, it would be good to list the benchmark that needs this. Also, serialization itself shouldn't care if the data is synthetic (random) or if it exactly matches a real past block. If you worry about a bias, it should actually be easier to provide synthetic data, than to try to find a fitting past block. In any case, there will always be a bias, even if the data is fully synthetic, as the real ch
...
(https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2086208814)
> Yes, this one definitely, but in the other cases I'm worried about introducing a strong bias.
Again, it would be good to list the benchmark that needs this. Also, serialization itself shouldn't care if the data is synthetic (random) or if it exactly matches a real past block. If you worry about a bias, it should actually be easier to provide synthetic data, than to try to find a fitting past block. In any case, there will always be a bias, even if the data is fully synthetic, as the real ch
...
🤔 maflcko reviewed a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2835753705)
I wonder if `-reindex` could be taught to do this instead. This may be simpler for users to set. Also, it will be atomic, safe and interruptible. However, it will eat more CPU and thus take a longer time (I'd say this is probably fine).
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2835753705)
I wonder if `-reindex` could be taught to do this instead. This may be simpler for users to set. Also, it will be atomic, safe and interruptible. However, it will eat more CPU and thus take a longer time (I'd say this is probably fine).
💬 maflcko commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2086226509)
too bad simply getting a few bytes of os rand is still experimental and only in rust nightly. Maybe using the `getrandom` crate directly can cut down the dependencies?
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2086226509)
too bad simply getting a few bytes of os rand is still experimental and only in rust nightly. Maybe using the `getrandom` crate directly can cut down the dependencies?
💬 fanquake commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2875540449)
```bash
/Users/runner/work/bitcoin/bitcoin/src/wallet/test/wallet_tests.cpp:69:10: error: unused variable 'spk_manager' [-Werror,-Wunused-variable]
auto spk_manager = *Assert(wallet.AddWalletDescriptor(w_desc, provider, "", false));
^
1 error generated.
```
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2875540449)
```bash
/Users/runner/work/bitcoin/bitcoin/src/wallet/test/wallet_tests.cpp:69:10: error: unused variable 'spk_manager' [-Werror,-Wunused-variable]
auto spk_manager = *Assert(wallet.AddWalletDescriptor(w_desc, provider, "", false));
^
1 error generated.
```
💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875574267)
Rebased and moved `SIGNET_HEADER` into `blocktools.py`.
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875574267)
Rebased and moved `SIGNET_HEADER` into `blocktools.py`.
💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875626028)
Encoded it as bytes: `SIGNET_HEADER = b"\xec\xc7\xda\xa2"` and using the constant in `contrib/signet/miner`
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875626028)
Encoded it as bytes: `SIGNET_HEADER = b"\xec\xc7\xda\xa2"` and using the constant in `contrib/signet/miner`
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2875639340)
@ryanofsky I added your libmultiprocess patch (and rebased).
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2875639340)
@ryanofsky I added your libmultiprocess patch (and rebased).
👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2835944933)
ACK 4e1aae19512df82af584a064640c2143c5c5fa4f
Good to see the manual quoting go away and `bitcoin daemon -daemon` be renamed to `bitcoin node -daemon`.
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2835944933)
ACK 4e1aae19512df82af584a064640c2143c5c5fa4f
Good to see the manual quoting go away and `bitcoin daemon -daemon` be renamed to `bitcoin node -daemon`.
💬 Sjors commented on pull request "refactor: Remove unused HaveKey and HaveWatchOnly":
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875734524)
utACK fabdc5ad06bce8cd3b0e525071d10f616a30bb04
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875734524)
utACK fabdc5ad06bce8cd3b0e525071d10f616a30bb04
💬 ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086377216)
This is only a limit on the arbitrary data you're encoding in OP_RETURN scriptPubKeys, not a limit on the overall transaction size.
If you want to encode additional data, there are three approaches: extend an existing PUSH with extra data, add an additional PUSH, or add an additional output. These have an overhead of roughly 0, 1 and 2 with respect to this limit, and an overhead of 0, 1 and 10 vbytes wrt transaction weight with a corresponding impact on fees needed. If you did indeed have 83
...
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086377216)
This is only a limit on the arbitrary data you're encoding in OP_RETURN scriptPubKeys, not a limit on the overall transaction size.
If you want to encode additional data, there are three approaches: extend an existing PUSH with extra data, add an additional PUSH, or add an additional output. These have an overhead of roughly 0, 1 and 2 with respect to this limit, and an overhead of 0, 1 and 10 vbytes wrt transaction weight with a corresponding impact on fees needed. If you did indeed have 83
...