🤔 ryanofsky reviewed a pull request: "refactor: Add util::Result failure values, multiple error and warning messages"
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1561722494)
Updated 08f5febfc571220043436bbec96a326beebdee22 -> b0beb4c504da29c27358d4602a045aaab39305f6 ([`pr/bresult2.37`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.37) -> [`pr/bresult2.38`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.38), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult2.37..pr/bresult2.38)) moving a lot more functionality from the `Result` class to the `ResultBase` class so the new code can be compatible with the `ResultPtr` class from #26022
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1283630635)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1282487687

> Here and line 192 below: "earning"?

Thanks, rewrote these comments now

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1283624276)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1282502389

> also, per the lint CI / spelling linter

Thanks, fixed spelling

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1283623667)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1282503773

> Tidy CI iwyu suggestions

Thanks applied these

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1283623420)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1282499219

> [584e3fa](https://github.com/bitcoin/bitcoin/commit/584e3fa4ea8ca15b3a8b46e023f218c5e0ed73b0) Not sure, but these `if (!result) return result;` idioms (here and lines 228-229 below) seem "odd" enough that an explanatory comment might be helpful.

I think this will probably be a common pattern and it would be too verbose to explain what is happening each place it is used. If the code were misleading, I think I would w
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1283629856)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1282507096

> [6ddcc9b](https://github.com/bitcoin/bitcoin/commit/6ddcc9bec8f64148b7f4b6d03b8dd85a5e748175) `GetErrors()` and `GetWarnings()` don't seem to have any test coverage yet. Note that there is also already a `GetWarnings()` in `src/warnings.{h.cpp}`, perhaps disambiguate naming/grepping-wise in addition to namespace-wise.

Thanks, added some test coverage. On the `warnings.h` overlap, I think that `GetWarnings()` is not
...

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1283703466)
Done

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1283703631)
Done

✅ fanquake closed a pull request: "p2p: diversity network outbounds follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28189)

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1664631599)
Force-pushed:

Addressed:
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1272359281
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1272371310

Also, I added support for `GetSelectedValue()`, asserting that its value is always greater or equal than `target` in a valid result.

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1283710640)
Does it worth? BnB internally calls `ComputeSetAndWaste` which checks change. Any unexpected action regarding it would affect the assertion - `assert(best_waste == result.GetWaste());`.

💬 brunoerg commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283716489)
I don't think it's worth to mention this in a release note, seems like a specific code nuance to be exposed. Isn't it?

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#issuecomment-1664651486)
> https://github.com/bitcoin/bitcoin/commit/68e48a0185751d24eecb194b8efd7028c8b590f3: can you elaborate a bit in the commit description why nSendOffset can be dropped?

In the current codebase, the send buffering (= remembering the to-be-sent bytes which we haven't managed to send yet) is done `vSendMsg` (a queue of *byte arrays*) + `nSendOffset` (the position within `vSendMsg[0]` up to where we've sent things). After this PR, `vSendMsg` is turned into a queue of *messages*, which have not yet
...

💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664652771)
> > Why do you believe that expanding a publication mechanism that is 4x more expensive than the heavily used taproot witness mechanism will "increase the spam problems significantly"?
>
> @ChristopherA just said that he's paying you to push this because it makes it cheaper and easier for him to write his non-bitcoin stuff into the chain.

Some clarification: I said I would contribute $1K toward a bounty for a real PR. Others said they might contribute to that bounty as well. My real goal w
...

💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664659414)
A question to those who object to this PR, would any of you change your mind if a new `is_standard` was simply a little larger? Maybe a modest 256 or 512 bytes?

Though I believe there are other reasons OP_RETURN shouldn't be limited, if advancing a modest limit is an option, it is at least a way for devs who "want to do the right thing" rather than enormous inscriptions and other hacks, I would support it.

-- Christopher Allen

💬 paplorinc commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283740534)
```Python
# Test that 1-of-1000 and 999-of-1000 raise exception
for k in [1, 999]:
assert_raises_rpc_error(-5, "Cannot have 1000 keys in multi_a; must have between 1 and 999 keys, inclusive", self.do_test_k_of_n, k, 1000)
```

💬 Empact commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1664679951)
Concept ACK

💬 ariard commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1664750667)
> I'm not aware of any second layer protocols that are improved by having full-rbf more broadly available.

@TheBlueMatt See https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html and https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-May/003920.html

💬 brunoerg commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283810102)
I don't think it's really a bug. It seems like this was the expected behavior (due to examples and docs - e.g. having to specify port). However, I also think it should be able to disconnect all nodes from an IP. I'm gonna address it.

💬 1ma commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664757776)
> My goal is not "non-bitcoin stuff". My specific challenge is that Lightning and Nostr use public keys in a persistent way that are not rotatable, which is a bad cryptographic practice. By committing to a rotated key and some other cryptographic in advance in L1, they could be. I believe this could be as small as a signed hash and a little metadata.

@ChristopherA can you clarify why it is imperative for this use case to store that data on the Bitcoin blockchain?

To give you some insight i
...