💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978035081)
You should remove the old version of the release notes from the first commit too.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978035081)
You should remove the old version of the release notes from the first commit too.
💬 glozow commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695314625)
There were some opinions about the release notes
> I left comments there but I don't want to bikeshed the release notes too much, so feel free to ignore them unless you have to retouch or don't want to address them.
> (I didn't study the release notes again, but those can be changed later anyway)
> Further editing of the release notes can be done as part of the actual release process.
> It's explained in BlockAssembler::Options ClampOptions, though we should probably point to that fr
...
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695314625)
There were some opinions about the release notes
> I left comments there but I don't want to bikeshed the release notes too much, so feel free to ignore them unless you have to retouch or don't want to address them.
> (I didn't study the release notes again, but those can be changed later anyway)
> Further editing of the release notes can be done as part of the actual release process.
> It's explained in BlockAssembler::Options ClampOptions, though we should probably point to that fr
...
💬 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."