💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2868847845)
Re https://github.com/bitcoin/bitcoin/pull/32427#pullrequestreview-2823207472
> In my personal experience of running a node on crappy hardware a long time ago, it was usually 2. that would happen (I knew that because the reindex wouldn't scan all block files but abort with an error somewhere, and switch to IBD from peers).
That is interesting, I don't think I've ever run into blk file corruption. I agree with you that if that is something we need to be able to salvage from, that the reind
...
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2868847845)
Re https://github.com/bitcoin/bitcoin/pull/32427#pullrequestreview-2823207472
> In my personal experience of running a node on crappy hardware a long time ago, it was usually 2. that would happen (I knew that because the reindex wouldn't scan all block files but abort with an error somewhere, and switch to IBD from peers).
That is interesting, I don't think I've ever run into blk file corruption. I agree with you that if that is something we need to be able to salvage from, that the reind
...
💬 jb55 commented on pull request "tracing: fix invalid argument in mempool_monitor":
(https://github.com/bitcoin/bitcoin/pull/32454#issuecomment-2868895291)
that makes sense to me
(https://github.com/bitcoin/bitcoin/pull/32454#issuecomment-2868895291)
that makes sense to me
💬 JeremyRubin commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2868941155)
if anyone is looking for an annex in the wild, here is the first one on mainnet:
https://mempool.space/tx/b78748f2ee3222d9bf4b23ab917136c745531ccc5ef8097235d11e16483e466f?mode=details
seems to be keypath, so it's signed.
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2868941155)
if anyone is looking for an annex in the wild, here is the first one on mainnet:
https://mempool.space/tx/b78748f2ee3222d9bf4b23ab917136c745531ccc5ef8097235d11e16483e466f?mode=details
seems to be keypath, so it's signed.
🚀 fanquake merged a pull request: "build: simplify *ifaddr handling"
(https://github.com/bitcoin/bitcoin/pull/32446)
(https://github.com/bitcoin/bitcoin/pull/32446)
💬 fanquake commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2868953442)
Can be rebased now.
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2868953442)
Can be rebased now.
💬 fjahr commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2868975059)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2868975059)
Concept ACK
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2869001802)
Updated 76a8f22c5c5cf48a9c36cc40db9224f0454917d0 -> 16a695fbff4a92eba3eb72ec6aa766e71c52f773 ([spendblock_0](https://github.com/TheCharlatan/bitcoin/tree/spendblock_0) -> [spendblock_1](https://github.com/TheCharlatan/bitcoin/tree/spendblock_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_0..spendblock_1))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2078426205), now using `std::span<T>` as templated argument.
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2869001802)
Updated 76a8f22c5c5cf48a9c36cc40db9224f0454917d0 -> 16a695fbff4a92eba3eb72ec6aa766e71c52f773 ([spendblock_0](https://github.com/TheCharlatan/bitcoin/tree/spendblock_0) -> [spendblock_1](https://github.com/TheCharlatan/bitcoin/tree/spendblock_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_0..spendblock_1))
* Addressed @stringintech's [comment](https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2078426205), now using `std::span<T>` as templated argument.
💬 ajtowns commented on pull request "miner: don't needlessly append dummy OP_0 to bip34":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-2869006108)
> Our miner code adds `OP_0` to the coinbase `scriptSig` in order to avoid triggering the `bad-cb-length` consensus error on test networks.
Correct me if I'm wrong, please! I think what's actually going on here is:
* scriptSigs in the coinbase have had to be 2 bytes or more since before bitcoin's git history began
* this is perhaps because mainnet bitcoin blocks will usually require some extranonce stuff, and the coinbase tx's scriptSig is a sensible place to do that
* in any event, s
...
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-2869006108)
> Our miner code adds `OP_0` to the coinbase `scriptSig` in order to avoid triggering the `bad-cb-length` consensus error on test networks.
Correct me if I'm wrong, please! I think what's actually going on here is:
* scriptSigs in the coinbase have had to be 2 bytes or more since before bitcoin's git history began
* this is perhaps because mainnet bitcoin blocks will usually require some extranonce stuff, and the coinbase tx's scriptSig is a sensible place to do that
* in any event, s
...
🤔 LarryRuane reviewed a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2830996924)
This is a great tool, thanks for implementing it!
Consider obtaining the `.lock` file in the data directory in the same way `bitcoind` does. This will prevent `bitcoind` from starting if this tool is running, and vice versa.
Just to illustrate, if I try to start a second `bitcoind`, I get this error, so maybe do the same thing here.
```
$ bitcoind
Error: Cannot obtain a lock on directory /home/larry/.bitcoin. Bitcoin Core is probably already running.
```
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2830996924)
This is a great tool, thanks for implementing it!
Consider obtaining the `.lock` file in the data directory in the same way `bitcoind` does. This will prevent `bitcoind` from starting if this tool is running, and vice versa.
Just to illustrate, if I try to start a second `bitcoind`, I get this error, so maybe do the same thing here.
```
$ bitcoind
Error: Cannot obtain a lock on directory /home/larry/.bitcoin. Bitcoin Core is probably already running.
```
💬 LarryRuane commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2083250679)
Consider accepting this argument in the same way it's specified with `bitcoind` and `bitcoin-cli`, that is, `-datadir=<dir>`. Besides being consistent with these existing executables, it would also be cleaner if more arguments were added in the future (because the data directory wouldn't be specified in a different way from other arguments).
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2083250679)
Consider accepting this argument in the same way it's specified with `bitcoind` and `bitcoin-cli`, that is, `-datadir=<dir>`. Besides being consistent with these existing executables, it would also be cleaner if more arguments were added in the future (because the data directory wouldn't be specified in a different way from other arguments).
📝 polespinasa opened a pull request: "rpc:generatetomany"
(https://github.com/bitcoin/bitcoin/pull/32468)
Adds `generatetomany`. A rpc call to mine on regtest creating multiple outputs on the coinbase transaction.
The block reward and fees are split equally between the addresses provided.
The RPC can be used like:
```bash
sliv3r@sliv3r-tuxedo:~/BitcoinCore/bitcoin/build/bin$ ./bitcoin-cli -rpcport=18443 generatetomany 2 '["bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v", "bcrt1qvr3qgyhw6y0e0zj97v0j5yc40xtpea4wqj0g43"]'
[
"425749260d4292e860db13f4bb78c4aa586363228907ac2ba44dba4de407cea7",
...
(https://github.com/bitcoin/bitcoin/pull/32468)
Adds `generatetomany`. A rpc call to mine on regtest creating multiple outputs on the coinbase transaction.
The block reward and fees are split equally between the addresses provided.
The RPC can be used like:
```bash
sliv3r@sliv3r-tuxedo:~/BitcoinCore/bitcoin/build/bin$ ./bitcoin-cli -rpcport=18443 generatetomany 2 '["bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v", "bcrt1qvr3qgyhw6y0e0zj97v0j5yc40xtpea4wqj0g43"]'
[
"425749260d4292e860db13f4bb78c4aa586363228907ac2ba44dba4de407cea7",
...
💬 LarryRuane commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2869044934)
> > as long as they let it completely finish once before starting bitcoind
>
> That seems likely to cause issues. It seems better if we track the height at which xor starts. A script could then work backwards and lower the height. And in that case it might as well be an RPC command.
I have an idea for a follow-up PR (haven't started implementing yet) which would cause `bitcoind` to, upon opening a block file, look at the first 4 bytes, and if it's MAGIC, then don't xor anything while readi
...
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2869044934)
> > as long as they let it completely finish once before starting bitcoind
>
> That seems likely to cause issues. It seems better if we track the height at which xor starts. A script could then work backwards and lower the height. And in that case it might as well be an RPC command.
I have an idea for a follow-up PR (haven't started implementing yet) which would cause `bitcoind` to, upon opening a block file, look at the first 4 bytes, and if it's MAGIC, then don't xor anything while readi
...
💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869057493)
I would like to discuss how to implement reward distribution. Do we ask the user for the exact number of bitcoins for each address or do we opt to use percentages of the reward?
IMHO it is better to use percentages, it makes things easier for the user.
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869057493)
I would like to discuss how to implement reward distribution. Do we ask the user for the exact number of bitcoins for each address or do we opt to use percentages of the reward?
IMHO it is better to use percentages, it makes things easier for the user.
💬 andrewtoth commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869062598)
The `generateblock` command takes either an address or descriptor as its first argument. Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs? That way we won't need a new RPC.
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869062598)
The `generateblock` command takes either an address or descriptor as its first argument. Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs? That way we won't need a new RPC.
💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869063575)
> Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs?
That would be easy as the code is already done on the new RPC. But this change would not be backwards compatible, idk if we want to keep it that way.
There are some softwares like [Polar](https://lightningpolar.com/) that would be affected by this.
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869063575)
> Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs?
That would be easy as the code is already done on the new RPC. But this change would not be backwards compatible, idk if we want to keep it that way.
There are some softwares like [Polar](https://lightningpolar.com/) that would be affected by this.
💬 andrewtoth commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869075020)
> But this change would not be backwards compatible
Sure it would be. Just parse an array first and if that fails parse a string.
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869075020)
> But this change would not be backwards compatible
Sure it would be. Just parse an array first and if that fails parse a string.
💬 TheCharlatan commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2083272753)
Nit: `s/pqueue/queue/`
The relevant lines are touched already, so maybe just do it in the same commit?
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2083272753)
Nit: `s/pqueue/queue/`
The relevant lines are touched already, so maybe just do it in the same commit?
💬 1440000bytes commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869146364)
This pull request won't get merged. If you really have any point that is worth considering please do it in #32406
I might get banned for this comment because Bitcoin Core contributors cannot read disagreements or dissent here or on other forums.
However, I would like to read your comments in #32406 if they make sense.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869146364)
This pull request won't get merged. If you really have any point that is worth considering please do it in #32406
I might get banned for this comment because Bitcoin Core contributors cannot read disagreements or dissent here or on other forums.
However, I would like to read your comments in #32406 if they make sense.
💬 iicuriosity commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2869147520)
Concept NACK for the same reason as #32359. Why would you remove a feature with a valid use case?
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2869147520)
Concept NACK for the same reason as #32359. Why would you remove a feature with a valid use case?
🤔 monlovesmango reviewed a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2830971855)
ACK 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7
All my other comments are just small nits.
Sorry it took a while to do my review, just wanted to make sure I actually felt like I had a good understanding of all the changes (which took a while).
Unrelated to this PR and more of an FYI, in src/test/fuzz/txgraph.cpp `pick_fn` I think that:
```c
size_t choices = tx_count[0] + sims[0].removed.size() + 1;
```
can just be:
```c
size_t choices = tx_count[0] + sims[0].removed.size();
```
an
...
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2830971855)
ACK 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7
All my other comments are just small nits.
Sorry it took a while to do my review, just wanted to make sure I actually felt like I had a good understanding of all the changes (which took a while).
Unrelated to this PR and more of an FYI, in src/test/fuzz/txgraph.cpp `pick_fn` I think that:
```c
size_t choices = tx_count[0] + sims[0].removed.size() + 1;
```
can just be:
```c
size_t choices = tx_count[0] + sims[0].removed.size();
```
an
...