💬 brunoerg commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#discussion_r1221729333)
Make sense. I'm going to address it

💬 brunoerg commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1580980099)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/26969#discussion_r1221594636

💬 MarcoFalke commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1581058082)
> In other words, imo this PR fixes a bug.

Not sure. I don't think it makes sense to change the behavior of the `-datacarrier` setting. There is no reason for it to exist in the first place and it should just be removed. It is redundant with the `-datacarriersize` setting, because setting `-datacarriersize=0` is equivalent. Having two ways to achieve the same is just confusing and can lead to issues.
Also, unrelated to removing the option, the `std::optional<unsigned>` should just be flatten
...

👍 dergoegge approved a pull request: "ci: Nuke Android APK task, Use credits for tsan"
(https://github.com/bitcoin/bitcoin/pull/27834#pullrequestreview-1467998616)
ACK fa22538e481fa2c4f0b5d6f91166335e60b67fe9 - nuke it

I have only ever seen this task pass (even if all others fail) or fail intermittently.

📝 Sjors opened a pull request: "doc: clarify full Xcode download is not needed"
(https://github.com/bitcoin/bitcoin/pull/27835)

💬 grdddj commented on pull request "doc: clarify full Xcode download is not needed":
(https://github.com/bitcoin/bitcoin/pull/27835#issuecomment-1581067617)
LGTM!

💬 MarcoFalke commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-1581091453)
Not sure what is up with CI, but lint says:

```
contrib/devtools/special-instruction-check.py:55: error: "Binary" has no attribute "has_section" [attr-defined]
```

Maybe rebase?

💬 MarcoFalke commented on pull request "Refactor: Improve API design of `ScriptToUniv`, `TxToUniv` etc to return the `UniValue` instead of mutating a parameter/reference":
(https://github.com/bitcoin/bitcoin/pull/23507#discussion_r1221850091)
I think this is a bugfix for the fuzz test, which eats too much memory (especially under msan)

💬 mjdietzx commented on pull request "Refactor: Improve API design of `ScriptToUniv`, `TxToUniv` etc to return the `UniValue` instead of mutating a parameter/reference":
(https://github.com/bitcoin/bitcoin/pull/23507#discussion_r1221863188)
Oh man I'm curious how you ended up back in this PR!? Is that just from your memory?

Anyways lmk if you want me to do anything. I'd be happy to rebase this, open a PR with just this commit, etc.. whatever you suggest. Also feel free to just cherry-pick that commit or whatever is most efficient for you as well incase you are just commenting this for documentation purposes

💬 glozow commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1221909729)
> Happy to add an assert, though

Maybe add this? `assert(!available_coins.empty());` above this line

💬 MarcoFalke commented on pull request "doc: clarify full Xcode download is not needed":
(https://github.com/bitcoin/bitcoin/pull/27835#issuecomment-1581194624)
Not sure if this is needed? Anyone to copy-paste the command shouldn't run into issues and if you do random other stuff, you are on your own anyway?

💬 MarcoFalke commented on pull request "Refactor: Improve API design of `ScriptToUniv`, `TxToUniv` etc to return the `UniValue` instead of mutating a parameter/reference":
(https://github.com/bitcoin/bitcoin/pull/23507#discussion_r1221913157)
Maybe just split out the `TxToUniv` change in a new pull?

💬 ryanofsky commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#issuecomment-1581200866)
Approach ACK d44dd73bcabc694880dfc966fad21980cd3b4c64. Need to review more closely, but these changes look very good and should help with #27711

💬 jarolrod commented on pull request "ci: Nuke Android APK task, Use credits for tsan":
(https://github.com/bitcoin/bitcoin/pull/27834#issuecomment-1581305041)
Fine with me to repurpose to something more useful for this repo 👍

💬 jarolrod commented on pull request "ci: Nuke Android APK task, Use credits for tsan":
(https://github.com/bitcoin/bitcoin/pull/27834#issuecomment-1581325077)
It should be noted the primary role of the task is to check that a change doesn’t prevent us from building for android, a platform with 3 billion users and a platform that we care about building for. A manual check will be needed for build system changes to ensure that this compatibility is kept.

💬 willcl-ark commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-1581330720)
> Not sure what is up with CI, but lint says:
>
> ```
> contrib/devtools/special-instruction-check.py:55: error: "Binary" has no attribute "has_section" [attr-defined]
> ```
>
> Maybe rebase?

mypy decided it didn't like that line. Got rid of it with `# type: ignore[attr-defined]` as I don't know any better

Did the CI randomly re-run on this?

💬 fanquake commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-1581333578)
> Got rid of it with

There shouldn't be any need for changes, as that issue was fixed with upgrading to LIEF 0.13.1. So should be resolved after rebasing.

📝 furszy opened a pull request: "rpc: getblockfrompeer, introduce fetch from "any" functionality"
(https://github.com/bitcoin/bitcoin/pull/27836)
Coming from #27652. Implementing the first part of it.

The idea is to let users call `getblockfrompeer` without providing any peer id.
The node will internally select one peer at random and make the `getdata` request.

This also fixes a bug where the user is allowed to call `getblockfrompeer` providing
an id of a peer that signals a "limited" service. As limited peers cannot provide historical
blocks, it is not correct to allow the user to do that.

💬 willcl-ark commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-1581334980)
Ah I see. I rebased an re-ran the lint to see it fail locally, but didn't update my lief version!

💬 willcl-ark commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-1581339396)
It does in fact, appear to need changes:

```log
will@ubuntu in ~/src/bitcoin-18603_si_check on  18603_si_check [$!?] : C v16.0.5-clang : 🐍 3.8.16
$ pip3 install lief==0.13.1
Collecting lief==0.13.1
Using cached lief-0.13.1-cp38-cp38-manylinux_2_24_x86_64.whl (4.1 MB)
Installing collected packages: lief
Successfully installed lief-0.13.1

will@ubuntu in ~/src/bitcoin-18603_si_check on  18603_si_check [$!?] : C v16.0.5-clang : 🐍 3.8.16
$ ./test/lint/all-lint.py
src/addrdb.cpp:21
...