π fanquake merged a pull request: "refactors for subpackage evaluation"
(https://github.com/bitcoin/bitcoin/pull/28758)
(https://github.com/bitcoin/bitcoin/pull/28758)
π fanquake merged a pull request: "depends: drop -O1 workaround from arm64 apple Qt build"
(https://github.com/bitcoin/bitcoin/pull/28778)
(https://github.com/bitcoin/bitcoin/pull/28778)
π¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1381803100)
You can specify different permissions for each connection direction.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1381803100)
You can specify different permissions for each connection direction.
π achow101 merged a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762)
(https://github.com/bitcoin/bitcoin/pull/28762)
π fanquake approved a pull request: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735#pullrequestreview-1712898349)
ACK 3333f14efac815ba3c885398a6795c7e8ce68d08 - the response from upstream is that [if we submit a PR, they can take a look](https://github.com/capnproto/capnproto/issues/1833#issuecomment-1792582206), so if anyone would like this to work for Windows, I'd suggest sending a patch.
(https://github.com/bitcoin/bitcoin/pull/28735#pullrequestreview-1712898349)
ACK 3333f14efac815ba3c885398a6795c7e8ce68d08 - the response from upstream is that [if we submit a PR, they can take a look](https://github.com/capnproto/capnproto/issues/1833#issuecomment-1792582206), so if anyone would like this to work for Windows, I'd suggest sending a patch.
π¬ vasild commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381834911)
This is still an issue in the latest version (8adbd44c9b). It seems unlikely that the fuzzer will generate a string that makes `LookupHost()` happy. I think it is best if this test keeps using `ConsumeNetAddr()` and `ConsumeSubNet()` but those functions should return valid/sensible values. Something like:
```cpp
inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
const CNetAddr net_base = ConsumeNetAddr(fuzzed_data_provider, {NET_IPV4, NET_IPV6}, /*always_
...
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381834911)
This is still an issue in the latest version (8adbd44c9b). It seems unlikely that the fuzzer will generate a string that makes `LookupHost()` happy. I think it is best if this test keeps using `ConsumeNetAddr()` and `ConsumeSubNet()` but those functions should return valid/sensible values. Something like:
```cpp
inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
const CNetAddr net_base = ConsumeNetAddr(fuzzed_data_provider, {NET_IPV4, NET_IPV6}, /*always_
...
π fanquake opened a pull request: "build: remove `-bind_at_load` usage"
(https://github.com/bitcoin/bitcoin/pull/28783)
This is deprecated on macOS:
```bash
ld: warning: -bind_at_load is deprecated on macOS
```
and likely redundant anyways, given the behaviour of dyld3.
Unfortunately libtool is still injecting a `-bind_at_load`, because it's version check is broken:
```bash
# Don't allow lazy linking, it breaks C++ global constructors
# But is supposedly fixed on 10.4 or later (yay!).
if test CXX = "$tagname"; then
case ${MACOSX_DEPLOYMENT_TARGET-10.0} in
10.[0123])
func_append com
...
(https://github.com/bitcoin/bitcoin/pull/28783)
This is deprecated on macOS:
```bash
ld: warning: -bind_at_load is deprecated on macOS
```
and likely redundant anyways, given the behaviour of dyld3.
Unfortunately libtool is still injecting a `-bind_at_load`, because it's version check is broken:
```bash
# Don't allow lazy linking, it breaks C++ global constructors
# But is supposedly fixed on 10.4 or later (yay!).
if test CXX = "$tagname"; then
case ${MACOSX_DEPLOYMENT_TARGET-10.0} in
10.[0123])
func_append com
...
π¬ fanquake commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1792622714)
@theuni @TheCharlatan you might have some libtool ideas? Couldn't see too anything obvious to prune this out.
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1792622714)
@theuni @TheCharlatan you might have some libtool ideas? Couldn't see too anything obvious to prune this out.
π¬ kristapsk commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1792625547)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1792625547)
Concept ACK
π¬ maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381846491)
> but those functions should return valid/sensible values.
I think it is good to offer a path for the fuzz engine to produce an invalid value. Otherwise, it can miss bugs that only happen when an invalid value is passed.
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381846491)
> but those functions should return valid/sensible values.
I think it is good to offer a path for the fuzz engine to produce an invalid value. Otherwise, it can miss bugs that only happen when an invalid value is passed.
π romanz opened a pull request: "rpc: keep .cookie if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28784)
Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock).
(https://github.com/bitcoin/bitcoin/pull/28784)
Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock).
π¬ maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1792658631)
Given that their CI uses cmake (and passes), maybe an alternative could be to try that?
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1792658631)
Given that their CI uses cmake (and passes), maybe an alternative could be to try that?
π¬ TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1792665279)
Rebased 65839d6267af1f5e0e04aded72cbfa23b56a1237 -> fc5e4dea6ffa21191e852a79103a2dfc9c241df1 ([simplifyMemPoolInteractions_5](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_5) -> [simplifyMemPoolInteractions_6](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_5..simplifyMemPoolInteractions_6))
* Dropped the commit that was integrated in #28762 e3b2e630b2
...
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1792665279)
Rebased 65839d6267af1f5e0e04aded72cbfa23b56a1237 -> fc5e4dea6ffa21191e852a79103a2dfc9c241df1 ([simplifyMemPoolInteractions_5](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_5) -> [simplifyMemPoolInteractions_6](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_5..simplifyMemPoolInteractions_6))
* Dropped the commit that was integrated in #28762 e3b2e630b2
...
π¬ romanz commented on pull request "rpc: keep .cookie if it was not generated":
(https://github.com/bitcoin/bitcoin/pull/28784#issuecomment-1792671356)
The bug can be reproduced manually by starting `bitcoind` twice, resulting in a missing `.cookie` file:
```
$ bitcoind -signet &
...
2023-11-03T15:38:09Z Using random cookie authentication.
2023-11-03T15:38:09Z Generated RPC authentication cookie /home/user/.bitcoin/signet/.cookie
...
$ ls ~/.bitcoin/signet/.cookie
/home/user/.bitcoin/signet/.cookie
$ bitcoind -signet
Error: Cannot obtain a lock on data directory /home/user/.bitcoin/signet. Bitcoin Core is probably already running
...
(https://github.com/bitcoin/bitcoin/pull/28784#issuecomment-1792671356)
The bug can be reproduced manually by starting `bitcoind` twice, resulting in a missing `.cookie` file:
```
$ bitcoind -signet &
...
2023-11-03T15:38:09Z Using random cookie authentication.
2023-11-03T15:38:09Z Generated RPC authentication cookie /home/user/.bitcoin/signet/.cookie
...
$ ls ~/.bitcoin/signet/.cookie
/home/user/.bitcoin/signet/.cookie
$ bitcoind -signet
Error: Cannot obtain a lock on data directory /home/user/.bitcoin/signet. Bitcoin Core is probably already running
...
π¬ fanquake commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1792674010)
> maybe an alternative could be to try that?
I don't think so? What they are doing only (genearting MinGW Makefiles) only works if you're running on Windows, as far as I can tell. That CI job is running on `Microsoft Windows Server 2022`, from what I can see.
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1792674010)
> maybe an alternative could be to try that?
I don't think so? What they are doing only (genearting MinGW Makefiles) only works if you're running on Windows, as far as I can tell. That CI job is running on `Microsoft Windows Server 2022`, from what I can see.
π€ murchandamus reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1712833868)
I guess now post-merge ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1712833868)
I guess now post-merge ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9
π¬ murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381775670)
Would it be possible for these parameters to be in the same order as the assignments below?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381775670)
Would it be possible for these parameters to be in the same order as the assignments below?
π¬ murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381839339)
It took me until the commit `[MiniMiner] track inclusion order and add Linearize() function` until I understood what this PR was aiming to achieve. Perhaps you could add another sentence to the PR description about the goal of this PR.
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381839339)
It took me until the commit `[MiniMiner] track inclusion order and add Linearize() function` until I understood what this PR was aiming to achieve. Perhaps you could add another sentence to the PR description about the goal of this PR.
π¬ murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381902909)
Isnβt it the parent that is fee-bumping?
```suggestion
// 3 pairs of grandparent + fee-bumping parent, plus 1 low-feerate child.
```
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381902909)
Isnβt it the parent that is fee-bumping?
```suggestion
// 3 pairs of grandparent + fee-bumping parent, plus 1 low-feerate child.
```
π¬ murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381893848)
Do we not know any outpoints or do we just not know whether the ancestor transactions of the submitted set are confirmed?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381893848)
Do we not know any outpoints or do we just not know whether the ancestor transactions of the submitted set are confirmed?
π¬ murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381826714)
I also found `manual_entries` vs `m_entries` make me check back a couple times. My understanding is that the `manual_entries` are just fully specified rather than read from the mempool. Would maybe `manually_submitted_entries` fit the bill?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381826714)
I also found `manual_entries` vs `m_entries` make me check back a couple times. My understanding is that the `manual_entries` are just fully specified rather than read from the mempool. Would maybe `manually_submitted_entries` fit the bill?