💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845313)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845313)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845340)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845340)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845388)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845388)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845419)
swapped them and updated tests accordingly
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845419)
swapped them and updated tests accordingly
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845454)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595845454)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2103229267)
@glozow thanks for the review, all comments addressed
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2103229267)
@glozow thanks for the review, all comments addressed
💬 wiz commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2103237059)
> that might be premature, if we decide to do another genesis block.
From reading this thread it seems like there is rough consensus for the testnet4 genesis block now, and the discussion has mostly shifted to tweaking the difficulty adjustment algorithm and what fancy test transactions to mine into blocks - so I imagine the blockchain might get re-org'd and genesis block probably doesn't change, but as @craigraw said even if it does that's totally fine too.
As for being "premature", to be
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2103237059)
> that might be premature, if we decide to do another genesis block.
From reading this thread it seems like there is rough consensus for the testnet4 genesis block now, and the discussion has mostly shifted to tweaking the difficulty adjustment algorithm and what fancy test transactions to mine into blocks - so I imagine the blockchain might get re-org'd and genesis block probably doesn't change, but as @craigraw said even if it does that's totally fine too.
As for being "premature", to be
...
👍 willcl-ark approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048629110)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
Without my earlier misunderstanding, this all looks good to me.
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048629110)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
Without my earlier misunderstanding, this all looks good to me.
💬 theuni commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#issuecomment-2103255846)
Are there some docs somewhere that suggest this?
(https://github.com/bitcoin/bitcoin/pull/30075#issuecomment-2103255846)
Are there some docs somewhere that suggest this?
💬 hebasto commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#issuecomment-2103262908)
> Are there some docs somewhere that suggest this?
https://gcc.gnu.org/onlinedocs/gcc/Gcov-and-Optimization.html
(https://github.com/bitcoin/bitcoin/pull/30075#issuecomment-2103262908)
> Are there some docs somewhere that suggest this?
https://gcc.gnu.org/onlinedocs/gcc/Gcov-and-Optimization.html
💬 theuni commented on issue "build: Unaligned libsecp256k1 flags in debug builds":
(https://github.com/bitcoin/bitcoin/issues/30055#issuecomment-2103276717)
I'm not sure this is a problem? It's hard to imagine a use-case where we'd want to be debugging Core with an un-optimized secp.
(https://github.com/bitcoin/bitcoin/issues/30055#issuecomment-2103276717)
I'm not sure this is a problem? It's hard to imagine a use-case where we'd want to be debugging Core with an un-optimized secp.
💬 achow101 commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2103348027)
ACK d53d84834747c37f4060a9ef379e0a6b50155246
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2103348027)
ACK d53d84834747c37f4060a9ef379e0a6b50155246
👍 theuni approved a pull request: "kernel: Remove key module from kernel library"
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2048736866)
Very nice improvements since my last review. The RAII context is an obvious improvement, thanks @ryanofsky.
I looked for a way to add a virtual lifetime class with on_startup/on_shutdown to replace the .init in fuzz.h, but didn't have any luck because of a (afacs?) missing teardown callback. It seems the static context approach used here is actually what's recommended.
utACK 96378fe734e5fb6167eb20036d7170572a647edb
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2048736866)
Very nice improvements since my last review. The RAII context is an obvious improvement, thanks @ryanofsky.
I looked for a way to add a virtual lifetime class with on_startup/on_shutdown to replace the .init in fuzz.h, but didn't have any luck because of a (afacs?) missing teardown callback. It seems the static context approach used here is actually what's recommended.
utACK 96378fe734e5fb6167eb20036d7170572a647edb
🚀 achow101 merged a pull request: "test: adds outbound eviction functional tests, updates comment in ConsiderEviction"
(https://github.com/bitcoin/bitcoin/pull/29122)
(https://github.com/bitcoin/bitcoin/pull/29122)
💬 achow101 commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#issuecomment-2103386014)
ACK dd8fa861939d5b8bdd844ad7cab015d08533a91a
(https://github.com/bitcoin/bitcoin/pull/29939#issuecomment-2103386014)
ACK dd8fa861939d5b8bdd844ad7cab015d08533a91a
👍 stickies-v approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048772665)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
All backports clean except for 0fcceefe22532dc6389a95d2e058599e9496003b because of #28955, but the backport looks good to me
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2048772665)
ACK 46890065e59875dd3d736d04f427e410bbd8faed
All backports clean except for 0fcceefe22532dc6389a95d2e058599e9496003b because of #28955, but the backport looks good to me
💬 stickies-v commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595946985)
I think we usually keep this header?
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595946985)
I think we usually keep this header?
💬 stickies-v commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595950409)
```suggestion
- #29776 Fix #29767, set m_synced = true after Commit()
```
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595950409)
```suggestion
- #29776 Fix #29767, set m_synced = true after Commit()
```
💬 stickies-v commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595965787)
Also, this is the commit message and not the PR title but this is a bit more descriptive so makes sense.
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595965787)
Also, this is the commit message and not the PR title but this is a bit more descriptive so makes sense.
🚀 achow101 merged a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939)
(https://github.com/bitcoin/bitcoin/pull/29939)