💬 willcl-ark commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978121178)
addressed in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978121178)
addressed in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b
💬 willcl-ark commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978121463)
changed to "should" in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978121463)
changed to "should" in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b
💬 fjahr commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2695419909)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2695419909)
Concept ACK
💬 fjahr commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2695420685)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2695420685)
Concept ACK
💬 jonatack commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978124505)
also s/fee_rate/"fee_rate"/
"discouraged in a dynamic fee environment" -> this leads to the question whether the fee environment is more "dynamic" than before, e.g. when this RPC was introduced, to justify removal. Would be shorter and possibly preferable to write: "They allowed users to set a static fee rate for wallet transactions, which could potentially lead to overpaying or underpaying."
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978124505)
also s/fee_rate/"fee_rate"/
"discouraged in a dynamic fee environment" -> this leads to the question whether the fee environment is more "dynamic" than before, e.g. when this RPC was introduced, to justify removal. Would be shorter and possibly preferable to write: "They allowed users to set a static fee rate for wallet transactions, which could potentially lead to overpaying or underpaying."
💬 willcl-ark commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978128755)
Thanks for the check! I actually called `invalidateblock` and then `gettxoutsetinfo` when testing this. I figured `gettxoutsetinfo <hash> <block_height>` was permitted from skimming the RPC helptext, but I didnt' verify!
As this doesn't work, I reverted to my tested method of `invalidateblock` followed by `gettxoutsetinfo` in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b (please let me know if there is a better method I'm missing)
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978128755)
Thanks for the check! I actually called `invalidateblock` and then `gettxoutsetinfo` when testing this. I figured `gettxoutsetinfo <hash> <block_height>` was permitted from skimming the RPC helptext, but I didnt' verify!
As this doesn't work, I reverted to my tested method of `invalidateblock` followed by `gettxoutsetinfo` in 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b (please let me know if there is a better method I'm missing)
💬 fjahr commented on issue "RFC: when to drop testnet3":
(https://github.com/bitcoin/bitcoin/issues/31975#issuecomment-2695434964)
I'm guessing we need to weigh the potential pain for those that have built infrastructure on Testnet3 for a while (wallets with lot's of UTXOs for testing elaborate multisig setups, lightning nodes etc.) with the fact that Testnet3 doesn't seem to be usable for anyone according to some commenters. It think a mailing list post is in order and if nobody complains for good reason (not "I'm rich there") then I would even be fine with removal in v30. We can also do a Github grep and reach out to open
...
(https://github.com/bitcoin/bitcoin/issues/31975#issuecomment-2695434964)
I'm guessing we need to weigh the potential pain for those that have built infrastructure on Testnet3 for a while (wallets with lot's of UTXOs for testing elaborate multisig setups, lightning nodes etc.) with the fact that Testnet3 doesn't seem to be usable for anyone according to some commenters. It think a mailing list post is in order and if nobody complains for good reason (not "I'm rich there") then I would even be fine with removal in v30. We can also do a Github grep and reach out to open
...
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695437679)
i've checked the 31330 release notes, they're kind of brief but get the gist of the change.
We might want to add a sentence that the NAT-PMP/PCP implementation was replaced. It's not super important to end users but if they encounter problems they'll want to report it.
Also "Setting `-upnp` will now return an error" won't be correct after #31916 is merged.
i'll open a PR for this.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695437679)
i've checked the 31330 release notes, they're kind of brief but get the gist of the change.
We might want to add a sentence that the NAT-PMP/PCP implementation was replaced. It's not super important to end users but if they encounter problems they'll want to report it.
Also "Setting `-upnp` will now return an error" won't be correct after #31916 is merged.
i'll open a PR for this.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2695441852)
@instagibbs So... it seems that the "optimal linearization, truncate, postlinearize, split" does not actually result in minimal chunks (but it does result in feerate-diagram-optimal ones, and connected ones):
```mermaid
graph BT;T18["A: 4"];T19["B: 5"];T20["C: 7"];T20-->T18;T20-->T13;T9["D: 7"];T9-->T18;T9-->T19;T13["E: 6"];T13-->T19;
```
The original (optimal) linearization is [ABEDC]. It's just a single chunk, so really any ordering is equally good.
Truncation removes C. Postlineari
...
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2695441852)
@instagibbs So... it seems that the "optimal linearization, truncate, postlinearize, split" does not actually result in minimal chunks (but it does result in feerate-diagram-optimal ones, and connected ones):
```mermaid
graph BT;T18["A: 4"];T19["B: 5"];T20["C: 7"];T20-->T18;T20-->T13;T9["D: 7"];T9-->T18;T9-->T19;T13["E: 6"];T13-->T19;
```
The original (optimal) linearization is [ABEDC]. It's just a single chunk, so really any ordering is equally good.
Truncation removes C. Postlineari
...
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695461472)
Thanks for taking a look!
> Seems fine to add this, though it also takes up a line for information
Right, when calling `-netinfo` without a details level (without the peers list).
I began by adding the services in the versions line, in the same format as in the peers list:
```
Bitcoin Core client v28.99.0 - server 70016/Satoshi:28.99.0/ - services nbwcl2
<-> type net serv v mping ping send recv txn blk hb addrp addrl age asmap id version
in onion nwl2
...
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695461472)
Thanks for taking a look!
> Seems fine to add this, though it also takes up a line for information
Right, when calling `-netinfo` without a details level (without the peers list).
I began by adding the services in the versions line, in the same format as in the peers list:
```
Bitcoin Core client v28.99.0 - server 70016/Satoshi:28.99.0/ - services nbwcl2
<-> type net serv v mping ping send recv txn blk hb addrp addrl age asmap id version
in onion nwl2
...
💬 glozow commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695473173)
Thank you @laanwj! Would you mind adding it to the release notes draft actually, instead of opening a PR? https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695473173)
Thank you @laanwj! Would you mind adding it to the release notes draft actually, instead of opening a PR? https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft
💬 mzumsande commented on issue "compact blocks in IBD resets m_stalling_since":
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-2695499701)
I don't think this would work with blocks below the minchainwork threshold, we should abort [here](https://github.com/bitcoin/bitcoin/blob/3c1f72a36700271c7c1293383549c3be29f28edb/src/net_processing.cpp#L4309) -
so large parts of IBD shouldn't be affected. We will still be in IBD for a while after reaching that threshold though, depending on how up-to-date the bitcoind release is.
Also, the node schedules the blocks it downloads during IBD not as an answer to received headers messages, but on
...
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-2695499701)
I don't think this would work with blocks below the minchainwork threshold, we should abort [here](https://github.com/bitcoin/bitcoin/blob/3c1f72a36700271c7c1293383549c3be29f28edb/src/net_processing.cpp#L4309) -
so large parts of IBD shouldn't be affected. We will still be in IBD for a while after reaching that threshold though, depending on how up-to-date the bitcoind release is.
Also, the node schedules the blocks it downloads during IBD not as an answer to received headers messages, but on
...
📝 fjahr opened a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977)
The comment in `functional/rpc_deprecated.py` says "This test should be used to verify correct behaviour of deprecated RPC methods with and without the -deprecatedrpc flags." I think we can get rid of the "with" part since we can assume that every RPC is already tested in at least one other functional test. (I didn't look but I could verify in our coverage if someone has doubts about that.) In order for this test to continue working, the flag will need to be used there. Otherwise this seems to p
...
(https://github.com/bitcoin/bitcoin/pull/31977)
The comment in `functional/rpc_deprecated.py` says "This test should be used to verify correct behaviour of deprecated RPC methods with and without the -deprecatedrpc flags." I think we can get rid of the "with" part since we can assume that every RPC is already tested in at least one other functional test. (I didn't look but I could verify in our coverage if someone has doubts about that.) In order for this test to continue working, the flag will need to be used there. Otherwise this seems to p
...
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978196051)
Note that I think it's enough to test the deprecation error there, see #31977
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978196051)
Note that I think it's enough to test the deprecation error there, see #31977
💬 laanwj commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695543354)
> Nevertheless, I could revert that change if you prefer it remain appended to the versions header, WDYT?
i agree an alphabet soup like "nbwcl2" does not say anything without a legend what every character means.
To be clear, i'm not against adding it, it's only one line, it was more of a general concern about the direction.
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695543354)
> Nevertheless, I could revert that change if you prefer it remain appended to the versions header, WDYT?
i agree an alphabet soup like "nbwcl2" does not say anything without a legend what every character means.
To be clear, i'm not against adding it, it's only one line, it was more of a general concern about the direction.
📝 glozow opened a pull request: "kernel: pre-29.x chainparams and headerssync update"
(https://github.com/bitcoin/bitcoin/pull/31978)
Update chainparams and headerssync config for v29.0 (see [release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off)).
(https://github.com/bitcoin/bitcoin/pull/31978)
Update chainparams and headerssync config for v29.0 (see [release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off)).
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695553587)
I've updated the pull description to be clearer about it.
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695553587)
I've updated the pull description to be clearer about it.
💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2695559684)
These are from my nodes except for the mainnet `m_assumed_blockchain_size` which is from @sr-gi (thanks!)
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2695559684)
These are from my nodes except for the mainnet `m_assumed_blockchain_size` which is from @sr-gi (thanks!)
💬 pinheadmz commented on issue "test: WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=32090)":
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2695593851)
I might be seeing this today as well: https://gist.github.com/pinheadmz/08758d268ec1741afc8c4f181c0e97d2
My branch forks off of master at db36a92c02
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2695593851)
I might be seeing this today as well: https://gist.github.com/pinheadmz/08758d268ec1741afc8c4f181c0e97d2
My branch forks off of master at db36a92c02
💬 fjahr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695596738)
I gave it a shot and changed it to this:
"Due to a bug the default block reserved weight for fixed-size block header, transactions count, and coinbase transaction was reserved twice and could not be lowered further. As a result the total reserved weight was always `8,000 WU`, meaning that even when specifying a `-blockmaxweight` higher than the default (even to the max `4,000,000 WU`), the actual block size never exceeded `3,992,000 WU`. The fix consolidates the reservation into a single pla
...
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695596738)
I gave it a shot and changed it to this:
"Due to a bug the default block reserved weight for fixed-size block header, transactions count, and coinbase transaction was reserved twice and could not be lowered further. As a result the total reserved weight was always `8,000 WU`, meaning that even when specifying a `-blockmaxweight` higher than the default (even to the max `4,000,000 WU`), the actual block size never exceeded `3,992,000 WU`. The fix consolidates the reservation into a single pla
...
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2695598930)
I split 5d6aa13cf047e31005afca919ca9cf181e30905b into smaller commits. If we decide to punt this until after v30 then I'll PR some of these separately. E.g. there's some tests that can use testnet4 instead of testnet3 and perhaps we can drop GUI support earlier.
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2695598930)
I split 5d6aa13cf047e31005afca919ca9cf181e30905b into smaller commits. If we decide to punt this until after v30 then I'll PR some of these separately. E.g. there's some tests that can use testnet4 instead of testnet3 and perhaps we can drop GUI support earlier.