💬 ccdle12 commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1573387265)
> Rebased 3rd_place_medal
>
> @ccdle12 Thanks! Mind re-acking? As to motivation, I think it's a mix of both reasons you mentioned, with the first one being the direction to go when doing so -- see `doc/design/libraries.md` and merged changes like [c741d74](https://github.com/bitcoin/bitcoin/commit/c741d748d4d9836940b99091cc7be09c65efcb79).
Of course! re-tACK at 8efd76b, thanks for explaining the motivation and pointing out the design/doc :)
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1573387265)
> Rebased 3rd_place_medal
>
> @ccdle12 Thanks! Mind re-acking? As to motivation, I think it's a mix of both reasons you mentioned, with the first one being the direction to go when doing so -- see `doc/design/libraries.md` and merged changes like [c741d74](https://github.com/bitcoin/bitcoin/commit/c741d748d4d9836940b99091cc7be09c65efcb79).
Of course! re-tACK at 8efd76b, thanks for explaining the motivation and pointing out the design/doc :)
💬 willcl-ark commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1573396315)
It's a little unclear to me how the ASM output is used on the consumer side, and therefore how breaking any changes to it might be. However I still think we have a few options to resolve this:
1) Fully hex-encoded output in the ASM repr:
```shell
$ /home/will/src/bitcoin/src/bitcoin-cli -regtest decodescript 0512121212120457c74942
{
"asm": "1212121212 57c74942",
```
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1573396315)
It's a little unclear to me how the ASM output is used on the consumer side, and therefore how breaking any changes to it might be. However I still think we have a few options to resolve this:
1) Fully hex-encoded output in the ASM repr:
```shell
$ /home/will/src/bitcoin/src/bitcoin-cli -regtest decodescript 0512121212120457c74942
{
"asm": "1212121212 57c74942",
```
🚀 fanquake merged a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/27802)
(https://github.com/bitcoin/bitcoin/pull/27802)
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1573408760)
`b6fd6b525d...92fb45b5ef`: rebase and address suggestions
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1573408760)
`b6fd6b525d...92fb45b5ef`: rebase and address suggestions
🚀 fanquake merged a pull request: "[25.x] build: disable boost multi index safe mode"
(https://github.com/bitcoin/bitcoin/pull/27775)
(https://github.com/bitcoin/bitcoin/pull/27775)
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214126425)
Added this test, but in `feature_config_args.py` because it checks interaction between some config args. That fits better than `p2p_local_tx_relay.py`:
> Test how locally submitted transactions are sent to the network when sensitive relay is used.
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214126425)
Added this test, but in `feature_config_args.py` because it checks interaction between some config args. That fits better than `p2p_local_tx_relay.py`:
> Test how locally submitted transactions are sent to the network when sensitive relay is used.
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214127166)
Added a comment in the code in case somebody else wonders the same.
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214127166)
Added a comment in the code in case somebody else wonders the same.
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214128768)
I removed `new_p2p_index()` because it was used in just one place. Earlier it was used also for all listeners behind the SOCKS5 proxy, but I had to stop using `p2p_port()` for that because it exceeded `MAX_NODES` (12).
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214128768)
I removed `new_p2p_index()` because it was used in just one place. Earlier it was used also for all listeners behind the SOCKS5 proxy, but I had to stop using `p2p_port()` for that because it exceeded `MAX_NODES` (12).
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214131440)
Yes. That is a bit hidden/implicit in the creation of the grant, in `master`:
```cpp
CSemaphoreGrant grant(*semOutbound);
```
this would hang if there are too many connections, waiting for a free slot to be available (somebody to disconnect).
Added a mention to it in the description of `-maxconnections`.
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214131440)
Yes. That is a bit hidden/implicit in the creation of the grant, in `master`:
```cpp
CSemaphoreGrant grant(*semOutbound);
```
this would hang if there are too many connections, waiting for a free slot to be available (somebody to disconnect).
Added a mention to it in the description of `-maxconnections`.
🚀 fanquake merged a pull request: "streams: Drop confusing DataStream::Serialize method and << operator"
(https://github.com/bitcoin/bitcoin/pull/27800)
(https://github.com/bitcoin/bitcoin/pull/27800)
💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1573450367)
Using alternate distros, or the pre-compiled LLVM bins do not currently seem to be viable options. Follow up in the qa-assets repo is here: https://github.com/bitcoin-core/qa-assets/pull/129.
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1573450367)
Using alternate distros, or the pre-compiled LLVM bins do not currently seem to be viable options. Follow up in the qa-assets repo is here: https://github.com/bitcoin-core/qa-assets/pull/129.
🚀 fanquake merged a pull request: "ci: compile Clang and compiler-rt in msan jobs"
(https://github.com/bitcoin/bitcoin/pull/27737)
(https://github.com/bitcoin/bitcoin/pull/27737)
🤔 glozow reviewed a pull request: "Fuzz: Mitigate timeout in CalculateTotalBumpFees"
(https://github.com/bitcoin/bitcoin/pull/27803#pullrequestreview-1457035240)
ACK 5d718f6913219d3ebe8394a17ddee81915e6f0ac
The code changes make sense to me. Reproduced the slow input from #27799, and it is dramatically faster with this change.
(https://github.com/bitcoin/bitcoin/pull/27803#pullrequestreview-1457035240)
ACK 5d718f6913219d3ebe8394a17ddee81915e6f0ac
The code changes make sense to me. Reproduced the slow input from #27799, and it is dramatically faster with this change.
✅ glozow closed an issue: "fuzz: mini_miner: Timeout in mini_miner"
(https://github.com/bitcoin/bitcoin/issues/27799)
(https://github.com/bitcoin/bitcoin/issues/27799)
🚀 glozow merged a pull request: "Fuzz: Mitigate timeout in CalculateTotalBumpFees"
(https://github.com/bitcoin/bitcoin/pull/27803)
(https://github.com/bitcoin/bitcoin/pull/27803)
💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214180938)
Can you explain the rationale behind the 60 hours chosen? Also perhaps you can update OP as it still mentions 12 hours :)
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214180938)
Can you explain the rationale behind the 60 hours chosen? Also perhaps you can update OP as it still mentions 12 hours :)
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214199866)
Thanks, included the move back of the `UnloadWallet` call and added you as co-author.
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214199866)
Thanks, included the move back of the `UnloadWallet` call and added you as co-author.
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214200031)
Thanks, done.
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214200031)
Thanks, done.
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#issuecomment-1573510436)
@jonatack: Oh sorry, wasn't aware you already put time into this. Pulled in your documentation fixup commit and took also your other suggestions for the removal and release note commits.
(https://github.com/bitcoin/bitcoin/pull/27757#issuecomment-1573510436)
@jonatack: Oh sorry, wasn't aware you already put time into this. Pulled in your documentation fixup commit and took also your other suggestions for the removal and release note commits.
💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1573521731)
> > redownloading orphans if we cannot afford to protect them
>
> Is the idea here that protection is a bandiwdth optimization to avoid fetching an orphan twice in a row in the presence of malicious peers or random churn? Reasonable if so, just want this to be explicitly stated if so.
Yes exactly. Worse is we want to avoid a situation where we requested the rest of the package while the orphan was in our orphanage, but then it fell out while we were waiting for the rest of the transactions
...
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1573521731)
> > redownloading orphans if we cannot afford to protect them
>
> Is the idea here that protection is a bandiwdth optimization to avoid fetching an orphan twice in a row in the presence of malicious peers or random churn? Reasonable if so, just want this to be explicitly stated if so.
Yes exactly. Worse is we want to avoid a situation where we requested the rest of the package while the orphan was in our orphanage, but then it fell out while we were waiting for the rest of the transactions
...
💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214254203)
I don't really understand the improvement in the context of achieving step 2 of the kernel library project between the solution you implemented now and what is implemented currently here in the first commit, besides it being a big step towards de-globalizing. One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a st
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214254203)
I don't really understand the improvement in the context of achieving step 2 of the kernel library project between the solution you implemented now and what is implemented currently here in the first commit, besides it being a big step towards de-globalizing. One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a st
...