💬 achow101 commented on pull request "test: Add time-timewarp-attack boundary cases":
(https://github.com/bitcoin/bitcoin/pull/30698#issuecomment-2310813586)
ACK 31378d44f44cabc576bf92ddd61afde4b528de77
Going to merge this now in the interest of having test coverage of the boundary before the 28.x branch off.
(https://github.com/bitcoin/bitcoin/pull/30698#issuecomment-2310813586)
ACK 31378d44f44cabc576bf92ddd61afde4b528de77
Going to merge this now in the interest of having test coverage of the boundary before the 28.x branch off.
🚀 achow101 merged a pull request: "test: XORed blocks test follow up"
(https://github.com/bitcoin/bitcoin/pull/30669)
(https://github.com/bitcoin/bitcoin/pull/30669)
🤔 hodlinator requested changes to a pull request: "test: Improve clarity of subsidy limit test"
(https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2261348316)
Think a possible way forward would be to undo most changes to `subsidy_limit_test`.
* The change from `2099999997690000` to using `'`-separators is nice. Have you considered the slightly more BTC-native `20'999'999'9769'0000` or `20'999'999'97'690'000`? (Only found some vague precedence for the former in `util_ParseMoney` test case and none for the latter).
* As I already [mentioned](https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2255459494), I think it would be an improvement
...
(https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2261348316)
Think a possible way forward would be to undo most changes to `subsidy_limit_test`.
* The change from `2099999997690000` to using `'`-separators is nice. Have you considered the slightly more BTC-native `20'999'999'9769'0000` or `20'999'999'97'690'000`? (Only found some vague precedence for the former in `util_ParseMoney` test case and none for the latter).
* As I already [mentioned](https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2255459494), I think it would be an improvement
...
🚀 achow101 merged a pull request: "test: Add time-timewarp-attack boundary cases"
(https://github.com/bitcoin/bitcoin/pull/30698)
(https://github.com/bitcoin/bitcoin/pull/30698)
💬 murchandamus commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731663858)
Sorry, I have to withdraw `limitancestorcount`, `limitancestorsize`, `limitdescendantcount`, and `limitdescendantsize`. Both parties that I suspected to use these startup options, in fact, do not use them. We definitely used `stopatheight` at a prior employer while indexing the blockchain, and my understanding was that `deprecatedrpc` was meant to give users a heads-up that an RPC would be going away with the next release, but not completely requiring an immediate transitioning away from it.
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731663858)
Sorry, I have to withdraw `limitancestorcount`, `limitancestorsize`, `limitdescendantcount`, and `limitdescendantsize`. Both parties that I suspected to use these startup options, in fact, do not use them. We definitely used `stopatheight` at a prior employer while indexing the blockchain, and my understanding was that `deprecatedrpc` was meant to give users a heads-up that an RPC would be going away with the next release, but not completely requiring an immediate transitioning away from it.
💬 achow101 commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2310853293)
ACK 1b41d45d462d856a9d0b44ae0039bbb2cd78407c
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2310853293)
ACK 1b41d45d462d856a9d0b44ae0039bbb2cd78407c
💬 achow101 commented on pull request "devtools, utxo-snapshot: Fix block height out of range in script":
(https://github.com/bitcoin/bitcoin/pull/30690#issuecomment-2310859905)
ACK 5b4f34006dbd76223b55b156421f87d2241ac296
(https://github.com/bitcoin/bitcoin/pull/30690#issuecomment-2310859905)
ACK 5b4f34006dbd76223b55b156421f87d2241ac296
🚀 achow101 merged a pull request: "devtools, utxo-snapshot: Fix block height out of range in script"
(https://github.com/bitcoin/bitcoin/pull/30690)
(https://github.com/bitcoin/bitcoin/pull/30690)
💬 achow101 commented on pull request "qt: 28.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/30715#issuecomment-2310874299)
ACK a0cdf43c4dde4f8d1983311fee2393fcde9123fe
(https://github.com/bitcoin/bitcoin/pull/30715#issuecomment-2310874299)
ACK a0cdf43c4dde4f8d1983311fee2393fcde9123fe
🚀 achow101 merged a pull request: "qt: 28.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/30715)
(https://github.com/bitcoin/bitcoin/pull/30715)
🤔 ryanofsky reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2261368799)
Updated 10770c164485324af8acdd3b932b51d32862c41f -> a1c9d23ff587b84d37175b2993ea6760f9762177 ([`pr/listset.3`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.3) -> [`pr/listset.4`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/listset.3..pr/listset.4)) with suggested changes
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2261368799)
Updated 10770c164485324af8acdd3b932b51d32862c41f -> a1c9d23ff587b84d37175b2993ea6760f9762177 ([`pr/listset.3`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.3) -> [`pr/listset.4`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/listset.3..pr/listset.4)) with suggested changes
💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731662812)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1710072169
> maybe use the term outbound connections instead of addrman connections?
Thanks, used and updated this comment
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731662812)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1710072169
> maybe use the term outbound connections instead of addrman connections?
Thanks, used and updated this comment
💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731662703)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1730774338
> would `IsArgSet` be simpler compared to `GetArgs()/IsArgNegated()` here?
It's true double negation behavior, treating `-noconnect=0` like `-connect=1`, is confusing and not obvious. But ArgsManager code goes out of its way to treat double negated settings the same as setting 1, so you generally do not need to think about this case when you are looking at code calling ArgsManager methods. If the code is doing the ri
...
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731662703)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1730774338
> would `IsArgSet` be simpler compared to `GetArgs()/IsArgNegated()` here?
It's true double negation behavior, treating `-noconnect=0` like `-connect=1`, is confusing and not obvious. But ArgsManager code goes out of its way to treat double negated settings the same as setting 1, so you generally do not need to think about this case when you are looking at code calling ArgsManager methods. If the code is doing the ri
...
💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731683649)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1719789856
> [ea7dbfb](https://github.com/bitcoin/bitcoin/commit/ea7dbfba8a62f3ba32b4386392721679d802e8ed):
Thanks! That might have caused problems if more tests were added after this
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1731683649)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1719789856
> [ea7dbfb](https://github.com/bitcoin/bitcoin/commit/ea7dbfba8a62f3ba32b4386392721679d802e8ed):
Thanks! That might have caused problems if more tests were added after this
💬 TheCharlatan commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1731725112)
Thanks ryanofsky, that is exactly what I had in mind for a potential followup - happy to have this in one go. I still need to think through the trade-offs for the block template rpc, but I think this might actually be better.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1731725112)
Thanks ryanofsky, that is exactly what I had in mind for a potential followup - happy to have this in one go. I still need to think through the trade-offs for the block template rpc, but I think this might actually be better.
📝 achow101 opened a pull request: "Pre-28.x branch off version bump and doc updates"
(https://github.com/bitcoin/bitcoin/pull/30719)
* Bump to 28.99 in preparation for the 28.x branching
* Remove current release note fragments. They've been moved to [draft release notes]((https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft).) in the wiki.
* Updated bips.md with missing BIPs that were implemented a while ago.
(https://github.com/bitcoin/bitcoin/pull/30719)
* Bump to 28.99 in preparation for the 28.x branching
* Remove current release note fragments. They've been moved to [draft release notes]((https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft).) in the wiki.
* Updated bips.md with missing BIPs that were implemented a while ago.
📝 achow101 opened a pull request: "chainparams: Remove seed.bitcoinstats.com"
(https://github.com/bitcoin/bitcoin/pull/30720)
This seeder no longer appears to be serving sufficient addresses.
Fixes #29911
(https://github.com/bitcoin/bitcoin/pull/30720)
This seeder no longer appears to be serving sufficient addresses.
Fixes #29911
🚀 achow101 merged a pull request: "seeds: Pull additional nodes from my seeder and update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/30008)
(https://github.com/bitcoin/bitcoin/pull/30008)
💬 Rob1Ham commented on issue "An "output descriptor" should not have many different checksums":
(https://github.com/bitcoin/bitcoin/issues/30632#issuecomment-2310966130)
I'm seeing an issue related to this.
When I get checksums of two descriptors (an internal and external one) individually, I get these checksum values:
```
getdescriptorinfo "wsh(andor(multi(2,[a0d3c79c/48'/1'/83'/2']tpubDFGoZLGBUQDBPzYHppiNcmX8hg2BkJvaanhUUyQHQCvkbjmqvb5akMW5AQKdYxSHbkaYPZR4JMMSMF7qSW3iERxPoVKSjdttnmEvwhpDAC7/0/*,[ea2484f9/48'/1'/83'/2']tpubDEzGdYvznBEvmWDgo8aJznu74ZRcQct2d2k6VEVtcgKJvCjCVitPVTtxgAfM2Hd5QVscv2jN8AjN6Ch69NhXYiceZ7eR8Sth2Sq6UND18So/0/*,[93f245d7/48'/1'/83'/
...
(https://github.com/bitcoin/bitcoin/issues/30632#issuecomment-2310966130)
I'm seeing an issue related to this.
When I get checksums of two descriptors (an internal and external one) individually, I get these checksum values:
```
getdescriptorinfo "wsh(andor(multi(2,[a0d3c79c/48'/1'/83'/2']tpubDFGoZLGBUQDBPzYHppiNcmX8hg2BkJvaanhUUyQHQCvkbjmqvb5akMW5AQKdYxSHbkaYPZR4JMMSMF7qSW3iERxPoVKSjdttnmEvwhpDAC7/0/*,[ea2484f9/48'/1'/83'/2']tpubDEzGdYvznBEvmWDgo8aJznu74ZRcQct2d2k6VEVtcgKJvCjCVitPVTtxgAfM2Hd5QVscv2jN8AjN6Ch69NhXYiceZ7eR8Sth2Sq6UND18So/0/*,[93f245d7/48'/1'/83'/
...
💬 furszy commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1731749995)
I don't think this alias is needed. It isn't really a complex function and forces devs to scroll / search it. It can just be in another line in the function declaration.
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1731749995)
I don't think this alias is needed. It isn't really a complex function and forces devs to scroll / search it. It can just be in another line in the function declaration.