🚀 fanquake merged a pull request: "build, bench, msvc: Add missing benchmarks"
(https://github.com/bitcoin/bitcoin/pull/29773)
(https://github.com/bitcoin/bitcoin/pull/29773)
💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2095990357)
> Found via `$ git grep 5092929b74`
Nice! Pushed a second commit to update the existing tests.
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2095990357)
> Found via `$ git grep 5092929b74`
Nice! Pushed a second commit to update the existing tests.
💬 Cinnamonrou commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1591001090)
- [ ] - []()~~~~
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1591001090)
- [ ] - []()~~~~
💬 Cinnamonrou commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1591000672)
- [ ]
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1591000672)
- [ ]
💬 sipa commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2096017270)
This seems unnecessarily unnatural and inefficient to use as a default sort order for this type.
If BIP352 defines its own normative sort order, I think it's better to have a specialized BIP352Comparator class that implements it, and use that in the BIP352 code?
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2096017270)
This seems unnecessarily unnatural and inefficient to use as a default sort order for this type.
If BIP352 defines its own normative sort order, I think it's better to have a specialized BIP352Comparator class that implements it, and use that in the BIP352 code?
💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1591046220)
I've been trying to reproduce in my local setup but I'm unable. Did you manage to trigger this on your updated branch, or with this one?
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1591046220)
I've been trying to reproduce in my local setup but I'm unable. Did you manage to trigger this on your updated branch, or with this one?
💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1591045131)
Yep
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1591045131)
Yep
💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096075601)
Added @wiz as the second seed node 🚀
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096075601)
Added @wiz as the second seed node 🚀
💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096079112)
> I don't think that's necessary. However, you could use a recent block hash as a provably unspendable public key (instead of all zeros).
Well, we now have a genesis block with the hash in the message and the all zeros public key. I don't think that makes a difference anymore?
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096079112)
> I don't think that's necessary. However, you could use a recent block hash as a provably unspendable public key (instead of all zeros).
Well, we now have a genesis block with the hash in the message and the all zeros public key. I don't think that makes a difference anymore?
💬 jlopp commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096133300)
I consider closing the block storm loophole to be rather important - it's the primary reason why TBTC became scarce and accrued value far faster than ought to have happened. Given that the patch is rather trivial, it would be a huge missed opportunity not to fix it at this time.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096133300)
I consider closing the block storm loophole to be rather important - it's the primary reason why TBTC became scarce and accrued value far faster than ought to have happened. Given that the patch is rather trivial, it would be a huge missed opportunity not to fix it at this time.
💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096167567)
> I consider closing the block storm loophole to be rather important - it's the primary reason why TBTC became scarce and accrued value far faster than ought to have happened. Given that the patch is rather trivial, it would be a huge missed opportunity not to fix it at this time.
I agree. Could you say precisely which (combination) of the [ideas given here so far](https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2094300149) do you consider a fix? Currently this PR includes the fix
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096167567)
> I consider closing the block storm loophole to be rather important - it's the primary reason why TBTC became scarce and accrued value far faster than ought to have happened. Given that the patch is rather trivial, it would be a huge missed opportunity not to fix it at this time.
I agree. Could you say precisely which (combination) of the [ideas given here so far](https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2094300149) do you consider a fix? Currently this PR includes the fix
...
💬 luke-jr commented on pull request "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition":
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-2096168450)
reopen please
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-2096168450)
reopen please
📝 fanquake reopened a pull request: "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition"
(https://github.com/bitcoin/bitcoin/pull/29086)
Instead of duplicating mempool options two places, just include the Options struct directly on the CTxMemPool
(https://github.com/bitcoin/bitcoin/pull/29086)
Instead of duplicating mempool options two places, just include the Options struct directly on the CTxMemPool
💬 sr-gi commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2096174963)
ACK [75d27fe](https://github.com/bitcoin/bitcoin/pull/26326/commits/75d27fefc7a04ebdda7be5724a014b6a896e7325)
The only differences are the ones mentioned, which align with the approach after considering https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2096174963)
ACK [75d27fe](https://github.com/bitcoin/bitcoin/pull/26326/commits/75d27fefc7a04ebdda7be5724a014b6a896e7325)
The only differences are the ones mentioned, which align with the approach after considering https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2040944697)
Code review ACK d447bdcfb0e38353940e4a7fc89d09482d8d39c3. Thanks for the updates!
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2040944697)
Code review ACK d447bdcfb0e38353940e4a7fc89d09482d8d39c3. Thanks for the updates!
✅ josibake closed a pull request: "Compare `COutPoints` lexicographically"
(https://github.com/bitcoin/bitcoin/pull/30046)
(https://github.com/bitcoin/bitcoin/pull/30046)
💬 josibake commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2096195301)
> This seems unnecessarily unnatural and inefficient to use as a default sort order for this type.
>
> If BIP352 defines its own normative sort order, I think it's better to have a specialized BIP352Comparator class that implements it, and use that in the BIP352 code?
>
> That also has the advantage of not tying the two together; someone might come in and change the order back to the current ordering for performance or simplicity reasons, and break the BIP352 code.
Personally, I find it
...
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2096195301)
> This seems unnecessarily unnatural and inefficient to use as a default sort order for this type.
>
> If BIP352 defines its own normative sort order, I think it's better to have a specialized BIP352Comparator class that implements it, and use that in the BIP352 code?
>
> That also has the advantage of not tying the two together; someone might come in and change the order back to the current ordering for performance or simplicity reasons, and break the BIP352 code.
Personally, I find it
...
💬 jlopp commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096201740)
I like all of the currently proposed changes.
So the question is this: if a high hashrate miner deliberately stalls testnet and we limp along for an entire difficulty adjustment period with minimum difficulty blocks, what should happen?
I think, in that case, perhaps we SHOULD reset the actual difficulty to the minimum difficulty (or to a sane ASIC difficulty like 1,000,000) and allow the network to find the real hashrate again.
Thus, it would still technically be possible for a block s
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2096201740)
I like all of the currently proposed changes.
So the question is this: if a high hashrate miner deliberately stalls testnet and we limp along for an entire difficulty adjustment period with minimum difficulty blocks, what should happen?
I think, in that case, perhaps we SHOULD reset the actual difficulty to the minimum difficulty (or to a sane ASIC difficulty like 1,000,000) and allow the network to find the real hashrate again.
Thus, it would still technically be possible for a block s
...
💬 mzumsande commented on issue "RfC: increase minimum prune target?":
(https://github.com/bitcoin/bitcoin/issues/30037#issuecomment-2096202995)
> since nodes with limited space would believe bitcoind was being limited/constrained to a particular allocated amount of space but would be exceeding the allocated amount
This is already the case today, because the 288 blocks rule takes precedence over the user-provided number. With average block sizes of 1.6MiB currently, my synced `prune=550` node usually takes up between 700MiB and 900MiB of space for `*.blk` and `*.rev` files.. The target is only respected when possible (during the earli
...
(https://github.com/bitcoin/bitcoin/issues/30037#issuecomment-2096202995)
> since nodes with limited space would believe bitcoind was being limited/constrained to a particular allocated amount of space but would be exceeding the allocated amount
This is already the case today, because the 288 blocks rule takes precedence over the user-provided number. With average block sizes of 1.6MiB currently, my synced `prune=550` node usually takes up between 700MiB and 900MiB of space for `*.blk` and `*.rev` files.. The target is only respected when possible (during the earli
...
💬 mzumsande commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2096263893)
> Could this be a race between "clicking on the peer to ban it in the GUI" and "the peer disconnecting"?
I thought the same last week and tried to reproduce it with node by adding a sleep after a node disconnected, but didn't manage to reproduce the crash (I might have put the sleep in the wrong place though...).
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2096263893)
> Could this be a race between "clicking on the peer to ban it in the GUI" and "the peer disconnecting"?
I thought the same last week and tried to reproduce it with node by adding a sleep after a node disconnected, but didn't manage to reproduce the crash (I might have put the sleep in the wrong place though...).