👍 TheCharlatan approved a pull request: "test: simple reordering to reduce run time"
(https://github.com/bitcoin/bitcoin/pull/31396#pullrequestreview-2472777174)
ACK 62f6d9e1a48e3b63c504996e914075cacfdcaedc
(https://github.com/bitcoin/bitcoin/pull/31396#pullrequestreview-2472777174)
ACK 62f6d9e1a48e3b63c504996e914075cacfdcaedc
💬 fanquake commented on pull request "util: Drop boost posix_time in ParseISO8601DateTime":
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2511632283)
Can also drop `boost-date-time` from `vcpkg.json`.
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2511632283)
Can also drop `boost-date-time` from `vcpkg.json`.
🤔 willcl-ark reviewed a pull request: "[28.x] Backports & 28.1rc1"
(https://github.com/bitcoin/bitcoin/pull/31104#pullrequestreview-2472783190)
These backports look clean to me.
Due to how my helper script parses the diffs between originals and backports I noticed that in: "addrman: change nid_type from int to int64_t" 4c1d74b60c8aee9702e2fe53ea9e680cc74c38dc, the commit message in this backport references the corrent PR, but wrong commit, it should be:
```diff
- Rebased-From: 051ba3290e30e210bfc50dea974063053313ad3e
+ Rebased-From: 51f7668d31e2624e41c7ce77fe33162802808f3f
```
(https://github.com/bitcoin/bitcoin/pull/31104#pullrequestreview-2472783190)
These backports look clean to me.
Due to how my helper script parses the diffs between originals and backports I noticed that in: "addrman: change nid_type from int to int64_t" 4c1d74b60c8aee9702e2fe53ea9e680cc74c38dc, the commit message in this backport references the corrent PR, but wrong commit, it should be:
```diff
- Rebased-From: 051ba3290e30e210bfc50dea974063053313ad3e
+ Rebased-From: 51f7668d31e2624e41c7ce77fe33162802808f3f
```
💬 willcl-ark commented on pull request "[28.x] Backports & 28.1rc1":
(https://github.com/bitcoin/bitcoin/pull/31104#discussion_r1865903526)
Would it be worth adding a bit more context here? For example something like:
```
Belt-and-suspenders fix for a potential remote crash due to `addr` message spam.
For more context see the bitcoincore.org [disclosure](bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow)
```
(https://github.com/bitcoin/bitcoin/pull/31104#discussion_r1865903526)
Would it be worth adding a bit more context here? For example something like:
```
Belt-and-suspenders fix for a potential remote crash due to `addr` message spam.
For more context see the bitcoincore.org [disclosure](bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow)
```
🚀 fanquake merged a pull request: "test: simple reordering to reduce run time"
(https://github.com/bitcoin/bitcoin/pull/31396)
(https://github.com/bitcoin/bitcoin/pull/31396)
👍 fanquake approved a pull request: "doc: Use more precise anchor link to codesigning docs"
(https://github.com/bitcoin/bitcoin/pull/31387#pullrequestreview-2472804575)
ACK 19f49c7489d226e1cebc754fbbae3e4bebc360af
(https://github.com/bitcoin/bitcoin/pull/31387#pullrequestreview-2472804575)
ACK 19f49c7489d226e1cebc754fbbae3e4bebc360af
🚀 fanquake merged a pull request: "doc: Use more precise anchor link to codesigning docs"
(https://github.com/bitcoin/bitcoin/pull/31387)
(https://github.com/bitcoin/bitcoin/pull/31387)
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865920849)
These definitions aren't fully consistent currently, the follow-up PR will partially clean this up I think (cc: @andrewtoth )
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865920849)
These definitions aren't fully consistent currently, the follow-up PR will partially clean this up I think (cc: @andrewtoth )
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865920939)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865920939)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921120)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921120)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921273)
I must have had a reason for changing it, but it does make sense to have the cache value before the modify value in the row -> done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921273)
I must have had a reason for changing it, but it does make sense to have the cache value before the modify value in the row -> done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921490)
Optional is an implementation detail
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921490)
Optional is an implementation detail
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921632)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921632)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921841)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865921841)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922183)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922183)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922298)
Done
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922298)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922400)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922400)
Done, thanks
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922643)
enum class is safer than enum https://agrawalsuneet.github.io/blogs/enum-vs-enum-class-in-c++ and I didn't like the result of `using enum State`
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865922643)
enum class is safer than enum https://agrawalsuneet.github.io/blogs/enum-vs-enum-class-in-c++ and I didn't like the result of `using enum State`
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2511657756)
Thanks @hodlinator, updated the PR:
* Removed redundant `inlines` from `struct CCoinsCacheEntry`
* Moved constants above `CoinEntry`and used const and brace init in more places in `coins_tests.cpp`
* Migrated changes inside the commits as soon as possible to minimize churn
* Added `AccessCoin` return value check in `CheckAccessCoin`
* Change `CheckAddCoin` param order to have cache values before the modified values in the row
* Fixed remaining `SetDirty`/`SetFresh` static reference in Fuzz
...
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2511657756)
Thanks @hodlinator, updated the PR:
* Removed redundant `inlines` from `struct CCoinsCacheEntry`
* Moved constants above `CoinEntry`and used const and brace init in more places in `coins_tests.cpp`
* Migrated changes inside the commits as soon as possible to minimize churn
* Added `AccessCoin` return value check in `CheckAccessCoin`
* Change `CheckAddCoin` param order to have cache values before the modified values in the row
* Fixed remaining `SetDirty`/`SetFresh` static reference in Fuzz
...
💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2511693820)
... https://cirrus-ci.com/task/4678171796176896?logs=ci#L2624
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2511693820)
... https://cirrus-ci.com/task/4678171796176896?logs=ci#L2624