✅ ryanofsky closed a pull request: "assumeutxo: Remove translated strings added in 30267"
(https://github.com/bitcoin/bitcoin/pull/30417)
(https://github.com/bitcoin/bitcoin/pull/30417)
💬 ryanofsky commented on pull request "assumeutxo: Remove translated strings added in 30267":
(https://github.com/bitcoin/bitcoin/pull/30417#issuecomment-2218345172)
> This is also done in #30395.
Oh, thanks I missed that. Closing.
(https://github.com/bitcoin/bitcoin/pull/30417#issuecomment-2218345172)
> This is also done in #30395.
Oh, thanks I missed that. Closing.
💬 Christewart commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218349630)
IIUC these txs will not get relayed widely across the network until there is widespread adoption of `bitcoind` versions that adopt these txs as standard?
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218349630)
IIUC these txs will not get relayed widely across the network until there is widespread adoption of `bitcoind` versions that adopt these txs as standard?
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218354375)
@Christewart spends of these outputs will not propagate until some 10%+ of the network updates, correct.
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218354375)
@Christewart spends of these outputs will not propagate until some 10%+ of the network updates, correct.
💬 Christewart commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218357340)
> @Christewart spends of these outputs will not propagate until some 10%+ of the network updates, correct.
Worth mentioning this in the release notes as this feature is intended for time sensitive protocols. Perhaps putting the cart before the horse, but it is a potential foot gun.
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218357340)
> @Christewart spends of these outputs will not propagate until some 10%+ of the network updates, correct.
Worth mentioning this in the release notes as this feature is intended for time sensitive protocols. Perhaps putting the cart before the horse, but it is a potential foot gun.
💬 melvincarvalho commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2218357984)
Great analysis, thanks
I have seen two instances where the diff drops to 1. The first is when the mining dries up for 20 minutes, and then someone will mine a block, just to keep the chain going (normally wiz :)).
The other is when people are deliberately propagating blocks with a future timestamp, up to about 6 blocks. Coincidentally 6 * 20 = 2 hours. I do not know what triggers this.
The 20 minute rule when used properly is quite nice as it keeps the chain flowing. The future spoo
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2218357984)
Great analysis, thanks
I have seen two instances where the diff drops to 1. The first is when the mining dries up for 20 minutes, and then someone will mine a block, just to keep the chain going (normally wiz :)).
The other is when people are deliberately propagating blocks with a future timestamp, up to about 6 blocks. Coincidentally 6 * 20 = 2 hours. I do not know what triggers this.
The 20 minute rule when used properly is quite nice as it keeps the chain flowing. The future spoo
...
💬 ryanofsky commented on pull request "rpc: Use untranslated error strings in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/30395#discussion_r1670972864)
In commit "rpc: Use untranslated error strings in loadtxoutset" (fa5b8920be041380fbfa4c7b443918637423d7a0)
Not important, but you could change this from `bilingual_str&&` to just `bilingual_str` so it is simpler, and function can be called naturally with both lvalues and rvalues, not just rvalues. Rvalues will be moved in any case.
(https://github.com/bitcoin/bitcoin/pull/30395#discussion_r1670972864)
In commit "rpc: Use untranslated error strings in loadtxoutset" (fa5b8920be041380fbfa4c7b443918637423d7a0)
Not important, but you could change this from `bilingual_str&&` to just `bilingual_str` so it is simpler, and function can be called naturally with both lvalues and rvalues, not just rvalues. Rvalues will be moved in any case.
👍 ryanofsky approved a pull request: "rpc: Use untranslated error strings in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/30395#pullrequestreview-2167052588)
Code review ACK fa5b8920be041380fbfa4c7b443918637423d7a0
(https://github.com/bitcoin/bitcoin/pull/30395#pullrequestreview-2167052588)
Code review ACK fa5b8920be041380fbfa4c7b443918637423d7a0
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218389213)
@Christewart done, since it would need a release note anyways
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2218389213)
@Christewart done, since it would need a release note anyways
👍 itornaza approved a pull request: "refactor: add coinbase constraints to BlockAssembler::Options"
(https://github.com/bitcoin/bitcoin/pull/30356#pullrequestreview-2167070268)
Concept ACK d89637eceab0145a64c2d1af1139ace204a3ab3c
Giving the option to assign the values of `coinbase_max_additional_weight` and `coinbase_output_max_additional_sigops` needed by the Stratum V2 protocol is essential for its integration. I understand the risks involved by having two parameters for the same thing:
> 1. The max block weight demanded by the user via -blockmaxweight
> 2. Weight units reserved for the coinbase
but it seems unavoidable at a first glance due to thei
...
(https://github.com/bitcoin/bitcoin/pull/30356#pullrequestreview-2167070268)
Concept ACK d89637eceab0145a64c2d1af1139ace204a3ab3c
Giving the option to assign the values of `coinbase_max_additional_weight` and `coinbase_output_max_additional_sigops` needed by the Stratum V2 protocol is essential for its integration. I understand the risks involved by having two parameters for the same thing:
> 1. The max block weight demanded by the user via -blockmaxweight
> 2. Weight units reserved for the coinbase
but it seems unavoidable at a first glance due to thei
...
💬 itornaza commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670984170)
I take it you wanted to rename it to `coinbase_max_additional_weight` instead
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670984170)
I take it you wanted to rename it to `coinbase_max_additional_weight` instead
💬 itornaza commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670987152)
Again, should it be `coinbase_max_additional_weight?`?
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670987152)
Again, should it be `coinbase_max_additional_weight?`?
💬 itornaza commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670985574)
Same here, did you meant to name it `coinbase_max_additional_weight`?
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670985574)
Same here, did you meant to name it `coinbase_max_additional_weight`?
💬 itornaza commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670987990)
`coinbase_max_additional_weight`?
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1670987990)
`coinbase_max_additional_weight`?
💬 mzumsande commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218418674)
I also think that 2) would be the best approach, because 1) doesn't seem to remove the race, just makes it more unlikely: If a version message was received after `InitializeNode()` and before `SendInitialMessages()` I think the race would still be happening.
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218418674)
I also think that 2) would be the best approach, because 1) doesn't seem to remove the race, just makes it more unlikely: If a version message was received after `InitializeNode()` and before `SendInitialMessages()` I think the race would still be happening.
🚀 ryanofsky merged a pull request: "rpc: Use untranslated error strings in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/30395)
(https://github.com/bitcoin/bitcoin/pull/30395)
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1671033583)
Fixed
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1671033583)
Fixed
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2218456437)
> I'm not sure if it's the intended behavior, but passing the same `NUM` twice to `importdescriptors` does not fail and ends up with a single _change_ descriptor:
Good catch.
The BIP does not specify either way yet, but I don't think there's any reason to allow duplicates here, so I've implemented that and added a test.
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2218456437)
> I'm not sure if it's the intended behavior, but passing the same `NUM` twice to `importdescriptors` does not fail and ends up with a single _change_ descriptor:
Good catch.
The BIP does not specify either way yet, but I don't think there's any reason to allow duplicates here, so I've implemented that and added a test.
💬 achow101 commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-2218543678)
ACK ad1e3620af88f73b869b4a2dbb74e03f5cd93517
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-2218543678)
ACK ad1e3620af88f73b869b4a2dbb74e03f5cd93517
💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218643656)
The unit test failure in the CI was caused by the [`ConnmanTestMsg::Handshake`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/util/net.cpp#L20) helper (used in the unit test [`outbound_slow_chain_eviction`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/denialofservice_tests.cpp#L45)), where a version handshake is done manually by calling the relevant connman/peerman methods. Due to a missing call to `SendMess
...
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218643656)
The unit test failure in the CI was caused by the [`ConnmanTestMsg::Handshake`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/util/net.cpp#L20) helper (used in the unit test [`outbound_slow_chain_eviction`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/denialofservice_tests.cpp#L45)), where a version handshake is done manually by calling the relevant connman/peerman methods. Due to a missing call to `SendMess
...