💬 jonatack commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695206796)
Updated to take @laanwj's review feedback and re-ran the seeds scripts.
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695206796)
Updated to take @laanwj's review feedback and re-ran the seeds scripts.
💬 ismaelsadeeq commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977976554)
My thinking of deprecating a feature is discouraging its use without outright disabling it, only allowing it when explicitly required.
Hiding it from the help text is a good step in this direction.
You can still access details about the RPC by running:
bitcoin-cli settxfee --help
You just won’t see the feature description in the overview of available RPCs.
As for paytxfee, it is still listed in the startup option help text.
AFAICT, it is not deprecated it's just warnings th
...
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977976554)
My thinking of deprecating a feature is discouraging its use without outright disabling it, only allowing it when explicitly required.
Hiding it from the help text is a good step in this direction.
You can still access details about the RPC by running:
bitcoin-cli settxfee --help
You just won’t see the feature description in the overview of available RPCs.
As for paytxfee, it is still listed in the startup option help text.
AFAICT, it is not deprecated it's just warnings th
...
💬 jonatack commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978004417)
Software clients that use the deprecated RPC API will see breakage when they update and look for why. It may be more confusing to no longer see the RPC in the list and potentially think it has been removed outright ("core devs broke my app without warning") rather than deprecated behind a flag. Hopefully, people read the release notes but 🤷.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978004417)
Software clients that use the deprecated RPC API will see breakage when they update and look for why. It may be more confusing to no longer see the RPC in the list and potentially think it has been removed outright ("core devs broke my app without warning") rather than deprecated behind a flag. Hopefully, people read the release notes but 🤷.
💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2695256954)
Rebased the previous conflict and #30901 has been merged now as well 🥳
I also added the `-DWITH_EMBEDDED_ASMAP=NO` to one of the CI jobs.
> would it make sense to provide the ability for a user (who doesn't necessarily have easy access to the source code) to extract the asmap data from a running node?
Let me address that in a follow-up. I think it's a nice-to-have since the necessary data for diffing should always be available in https://github.com/asmap/asmap-data when we follow the
...
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2695256954)
Rebased the previous conflict and #30901 has been merged now as well 🥳
I also added the `-DWITH_EMBEDDED_ASMAP=NO` to one of the CI jobs.
> would it make sense to provide the ability for a user (who doesn't necessarily have easy access to the source code) to extract the asmap data from a running node?
Let me address that in a follow-up. I think it's a nice-to-have since the necessary data for diffing should always be available in https://github.com/asmap/asmap-data when we follow the
...
💬 ismaelsadeeq commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978010680)
fair point cc @polespinasa
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978010680)
fair point cc @polespinasa
💬 laanwj commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695280949)
Code review ACK b2dc4458ac86c0a633ca3eb87827ac2731993f97 on the script changes
mainnet
```
Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
IPv4 IPv6 Onion I2P CJDNS Pass
17173 3638 21303 3116 12 Initial
17173 3638 21303 3116 12 Skip entries with invalid address
8378 1755 14661 2343 10 After removing duplicates
8337 1736 14656 2343 10 Enforce minimal number of blocks
7986 1612 14654 2343
...
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695280949)
Code review ACK b2dc4458ac86c0a633ca3eb87827ac2731993f97 on the script changes
mainnet
```
Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
IPv4 IPv6 Onion I2P CJDNS Pass
17173 3638 21303 3116 12 Initial
17173 3638 21303 3116 12 Skip entries with invalid address
8378 1755 14661 2343 10 After removing duplicates
8337 1736 14656 2343 10 Enforce minimal number of blocks
7986 1612 14654 2343
...
💬 polespinasa commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978024595)
> It may be more confusing to no longer see the RPC in the list and potentially think it has been removed.
That's a fair point and a good reason to not hide it. I will just revert that.
> It would be good to have clear guidance on this.
Would be good to have a written procedure on the dev notes to follow always the same procedure in the future and avoid repeating the discussion.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978024595)
> It may be more confusing to no longer see the RPC in the list and potentially think it has been removed.
That's a fair point and a good reason to not hide it. I will just revert that.
> It would be good to have clear guidance on this.
Would be good to have a written procedure on the dev notes to follow always the same procedure in the future and avoid repeating the discussion.
📝 glozow opened a pull request: "delete release note fragments for v29"
(https://github.com/bitcoin/bitcoin/pull/31976)
Delete release note fragments in preparation for 29.x branch-off.
Everything here has been copied over to the draft release notes at https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft
(https://github.com/bitcoin/bitcoin/pull/31976)
Delete release note fragments in preparation for 29.x branch-off.
Everything here has been copied over to the draft release notes at https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft
💬 ismaelsadeeq commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978034199)
> Would be good to have a written procedure on the dev notes to follow always the same procedure in the future and avoid repeating the discussion.
Good idea for a follow up PR, a section on removing a feature will be nice.
But we have different feature category.
1. RPC
2. Startup option
3. Rest
4. ZMQ interface
5. Wallet settings
6. In the future ( multiprocess interface)
We should document how we remove each.
Documenting for RPC is a good first step.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978034199)
> Would be good to have a written procedure on the dev notes to follow always the same procedure in the future and avoid repeating the discussion.
Good idea for a follow up PR, a section on removing a feature will be nice.
But we have different feature category.
1. RPC
2. Startup option
3. Rest
4. ZMQ interface
5. Wallet settings
6. In the future ( multiprocess interface)
We should document how we remove each.
Documenting for RPC is a good first step.
💬 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).