💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978049278)
Sorry, what do you mean? There's only one commit.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978049278)
Sorry, what do you mean? There's only one commit.
💬 glozow commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2695318085)
Unsure if a release note was written, apologies if I missed it. @hebasto @theuni @fanquake could you add to https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft?
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2695318085)
Unsure if a release note was written, apologies if I missed it. @hebasto @theuni @fanquake could you add to https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft?
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978050716)
added to the PR description to have that in mind
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978050716)
added to the PR description to have that in mind
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978052592)
I think this is outdated as we agreed to not hide anything?
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978052592)
I think this is outdated as we agreed to not hide anything?
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978057684)
Squashing is also fine! I just thought you intended to keep them in a separate commit like suggested by @ismaelsadeeq here: https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977593279
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978057684)
Squashing is also fine! I just thought you intended to keep them in a separate commit like suggested by @ismaelsadeeq here: https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977593279
💬 glozow commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695347851)
Removing the "Needs release note label" since there are notes for #31130. Lmk if this doesn't sound right.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2695347851)
Removing the "Needs release note label" since there are notes for #31130. Lmk if this doesn't sound right.
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978074740)
I don't think we have something like `deprecatedstartupoption`.
I did it with a warn trying to follow the way it's done with Legacy wallets.
https://github.com/bitcoin/bitcoin/blob/79bbb381a1fd13ad456fde736b3c195a791d4e58/src/wallet/wallet.cpp#L484-L487
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978074740)
I don't think we have something like `deprecatedstartupoption`.
I did it with a warn trying to follow the way it's done with Legacy wallets.
https://github.com/bitcoin/bitcoin/blob/79bbb381a1fd13ad456fde736b3c195a791d4e58/src/wallet/wallet.cpp#L484-L487
💬 stratospher commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r1978075230)
looks like we always break here since `old_tip`, `best_tip` and `chain.Tip()` is the genesis block initially.
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r1978075230)
looks like we always break here since `old_tip`, `best_tip` and `chain.Tip()` is the genesis block initially.
💬 jonatack commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695354214)
Yes, for the avoidance of doubt, I ran the script with no manual curation (as done previously from v21 to v27).
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695354214)
Yes, for the avoidance of doubt, I ran the script with no manual curation (as done previously from v21 to v27).
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978092613)
Done :)
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978092613)
Done :)
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978079620)
Nit: I would have liked it a bit more if you had added that where `-paytxfee=` is tested directly (above) or here it could use a small comment that this doesn't have anything to do with the check mention in the info log. But since this is temporary it's fine to keep it as is.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978079620)
Nit: I would have liked it a bit more if you had added that where `-paytxfee=` is tested directly (above) or here it could use a small comment that this doesn't have anything to do with the check mention in the info log. But since this is temporary it's fine to keep it as is.
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978103312)
Please add a test for this in `test/functional/rpc_deprecated.py`
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978103312)
Please add a test for this in `test/functional/rpc_deprecated.py`
🤔 fjahr reviewed a pull request: "wallet, rpc: deprecate settxfee and paytxfee"
(https://github.com/bitcoin/bitcoin/pull/31278#pullrequestreview-2655058161)
Looks good to me aside from the missing functional deprecation test.
(https://github.com/bitcoin/bitcoin/pull/31278#pullrequestreview-2655058161)
Looks good to me aside from the missing functional deprecation test.
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978061581)
formatting nit (no need to fix unless you repush, can be done in editing of the release notes in the wiki)
``` suggestion
The `-paytxfee` startup option and the `settxfee` RPC are now deprecated and will be removed in Bitcoin Core 31.0. They allowed users to set a static fee rate for wallet transactions, which is discouraged in a dynamic fee environment as it can lead to overpaying or underpaying. Users should instead rely on fee estimation or specify a fee rate per transaction using the fee
...
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978061581)
formatting nit (no need to fix unless you repush, can be done in editing of the release notes in the wiki)
``` suggestion
The `-paytxfee` startup option and the `settxfee` RPC are now deprecated and will be removed in Bitcoin Core 31.0. They allowed users to set a static fee rate for wallet transactions, which is discouraged in a dynamic fee environment as it can lead to overpaying or underpaying. Users should instead rely on fee estimation or specify a fee rate per transaction using the fee
...
💬 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
...