💬 ajtowns commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106772366)
(Signet adopts essentially the same rules as mainnet, so the only way to sustain blocks that are more frequent than once every 10 minutes is to continually raise the difficulty, which results in real costs, or to make use of the timewarp bug)
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106772366)
(Signet adopts essentially the same rules as mainnet, so the only way to sustain blocks that are more frequent than once every 10 minutes is to continually raise the difficulty, which results in real costs, or to make use of the timewarp bug)
💬 russeree commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106776587)
tACK - 06c2c713c52b60231efc3e00d2c5eb0bf9e345f9
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106776587)
tACK - 06c2c713c52b60231efc3e00d2c5eb0bf9e345f9
💬 craigraw commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106789221)
tACK https://github.com/bitcoin/bitcoin/commit/06c2c713c52b60231efc3e00d2c5eb0bf9e345f9
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106789221)
tACK https://github.com/bitcoin/bitcoin/commit/06c2c713c52b60231efc3e00d2c5eb0bf9e345f9
💬 maflcko commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1597970957)
```suggestion
port=ToIntegral<uint16_t>(rpcport_arg.value_or(0));
if (port == 0) {
```
if you treat `0` as invalid, there is no need to also have nullopt be invalid as well, no?
This should also fix the false-positive compiler warning?
An alternative would be to suppress the false-positive warning in the CI task config.
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1597970957)
```suggestion
port=ToIntegral<uint16_t>(rpcport_arg.value_or(0));
if (port == 0) {
```
if you treat `0` as invalid, there is no need to also have nullopt be invalid as well, no?
This should also fix the false-positive compiler warning?
An alternative would be to suppress the false-positive warning in the CI task config.
💬 Emzy commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106805681)
tACK 06c2c71
I'm just tested mining with CPUs and a S9 ASIC.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106805681)
tACK 06c2c71
I'm just tested mining with CPUs and a S9 ASIC.
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1597984017)
> only send me stuff where the UTXOs have a value of >= X
So now we need an index and efficient sorting.
> It might also be case of trying to fit a square peg into a round hole
For the index without cut-through I think BIP157 is fine. But to get the extra bandwidth savings of cut-through and a (custom) dust limit, it indeed seems we need something else. But ideally that shouldn't block shipping something that works, even if it uses a bit more bandwidth that we'd like.
It's worth cons
...
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1597984017)
> only send me stuff where the UTXOs have a value of >= X
So now we need an index and efficient sorting.
> It might also be case of trying to fit a square peg into a round hole
For the index without cut-through I think BIP157 is fine. But to get the extra bandwidth savings of cut-through and a (custom) dust limit, it indeed seems we need something else. But ideally that shouldn't block shipping something that works, even if it uses a bit more bandwidth that we'd like.
It's worth cons
...
💬 maflcko commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598001674)
> Should this change be in a new commit or should I force it on the previous commit?
Squashing seems fine?
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598001674)
> Should this change be in a new commit or should I force it on the previous commit?
Squashing seems fine?
💬 fanquake commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598004575)
If this is part of the standard library, I don't think you need to list it as a requirement.
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598004575)
If this is part of the standard library, I don't think you need to list it as a requirement.
💬 iw4p commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598009105)
To rewrite the git history for having the last change on the last commit and ignore old ones, I forced it. I used squashing only for merging before that.
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1598009105)
To rewrite the git history for having the last change on the last commit and ignore old ones, I forced it. I used squashing only for merging before that.
💬 Sjors commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1598013264)
I think that would require adding [sage](https://www.sagemath.org) to our test dependencies.
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1598013264)
I think that would require adding [sage](https://www.sagemath.org) to our test dependencies.
💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598018340)
> For the index without cut-through I think BIP157 is fine
Agree.
> It's worth considering at least adding the 546 sat dust limit to BIP352
I'd argue these are all implementation details/optimizations. The BIP itself should be reserved for describing the protocol. There also might be use cases where where the value of the output is not the primary concern.
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598018340)
> For the index without cut-through I think BIP157 is fine
Agree.
> It's worth considering at least adding the 546 sat dust limit to BIP352
I'd argue these are all implementation details/optimizations. The BIP itself should be reserved for describing the protocol. There also might be use cases where where the value of the output is not the primary concern.
💬 willcl-ark commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106890826)
Ah I see!
So the 2:30 block times are expected then, and at the first difficulty adjustment I would be expecting the difficulty to increase to such a point that it would take my CPU 600 seconds to generate a block.
I did discuss this possiblilty with @pinheadmz late last night, but was too impatient to let my signet miner run to an adjustment to find out. Thanks @ajtowns
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106890826)
Ah I see!
So the 2:30 block times are expected then, and at the first difficulty adjustment I would be expecting the difficulty to increase to such a point that it would take my CPU 600 seconds to generate a block.
I did discuss this possiblilty with @pinheadmz late last night, but was too impatient to let my signet miner run to an adjustment to find out. Thanks @ajtowns
💬 S3RK commented on pull request "wallet: add coin selection parameter `add_excess_to_recipient_position` for changeless txs with excess that would be added to fees":
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2106892196)
Here are my initial thoughts on each of the options individually
`change_target`
IIUC you want a specific min value for the change UTXO. Would a following alternative work? You provide your change address in the list of recipients with desired min amount and specify it as change UTXO using `change_position`parameter. The downside compared to a new option is that we always have a change output. Not sure if that's a problem in your case.
`disable_algos`
This makes sense, and could be a fir
...
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2106892196)
Here are my initial thoughts on each of the options individually
`change_target`
IIUC you want a specific min value for the change UTXO. Would a following alternative work? You provide your change address in the list of recipients with desired min amount and specify it as change UTXO using `change_position`parameter. The downside compared to a new option is that we always have a change output. Not sure if that's a problem in your case.
`disable_algos`
This makes sense, and could be a fir
...
💬 TheCharlatan commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1598055160)
Adding something to exercise `Erase` would also be nice.
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1598055160)
Adding something to exercise `Erase` would also be nice.
👍 TheCharlatan approved a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-2051980268)
ACK ffee43efe845cbbfbf16d5e61a1d541cb316ef56
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-2051980268)
ACK ffee43efe845cbbfbf16d5e61a1d541cb316ef56
💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598073431)
Done.
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598073431)
Done.
💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598077078)
Would prefer to keep this PR focused on the `limit` and `CHECKSUM_SIZE` change, but perhaps a good change for #29607
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598077078)
Would prefer to keep this PR focused on the `limit` and `CHECKSUM_SIZE` change, but perhaps a good change for #29607
💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598077219)
Done.
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598077219)
Done.
💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598078613)
Yes, this is checking that "The data part, which is at least 6 characters long" (from BIP173). Changed `pos + 7 >` to `pos + CHECKSUM_SIZE >=` throughout.
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598078613)
Yes, this is checking that "The data part, which is at least 6 characters long" (from BIP173). Changed `pos + 7 >` to `pos + CHECKSUM_SIZE >=` throughout.
💬 0xB10C commented on pull request "[DO NOT MERGE] Schnorr batch verification for blocks":
(https://github.com/bitcoin/bitcoin/pull/29491#issuecomment-2106968575)
@willcl-ark and I ran a IBD benchmark vs master for this: IBD from a local node to height 840000 and currently only looking at the connect_block duration.
In the current draft implementation:
- master is currently faster
- batch validation is attempted even before taproot activated
- after taproot activation, batch validation is significantly slower than non-batch validation
@willcl-ark and I ran a IBD benchmark vs master for this: IBD from a local node to height 840000 and currently only looking at the connect_block duration.
In the current draft implementation:
- master is currently faster
- batch validation is attempted even before taproot activated
- after taproot activation, batch validation is significantly slower than non-batch validation
![image](htt
...