💬 MarcoFalke commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1655489674)
In the meantime it may be good to do 10 runs and then check how many of them fail

💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1655491023)
I still have to rebase this, will do soon(tm).

🚀 fanquake merged a pull request: "refactor: Remove unused raw-pointer read helper from univalue"
(https://github.com/bitcoin/bitcoin/pull/28168)

💬 Sjors commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1277413296)
Should we disable the adding of new inputs when `reduce_output` is set? If someone really intends to do that, they should probably use `outputs`.

🤔 stickies-v reviewed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174#pullrequestreview-1551889123)
NACK.

I think documentation needs to be as concise and simple as possible, but I feel like the current version does a good job at that already. The current language is factual and not more informal than it needs to be.

In a world of overmarketed crypto projects, I think maintaining sober documentation is a nice differentiation.

💬 stickies-v commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1277416439)
I don't like this current rewording, but I think I agree with your intent of adding some more information of what the Bitcoin Core project stands for, e.g. highly reviewed, robust and stable code, one-stop-shop for running a node and wallet to fully interface with the Bitcoin Network, ...

⚠️ Sjors opened an issue: "Unsafe reduce_output when new coins are added"
(https://github.com/bitcoin/bitcoin/issues/28180)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

See inline discussion at https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1269177446

@whitslack:

> Consider the case where I am paying someone on chain under the stipulation that _they_ will eat the mining fee. (Thus, I specify _their_ address in `subtractfeefrom` when I'm constructing my transaction.) Later, they tell me that they can't wait for confirmation any longer and
...

💬 Sjors commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1277417319)
Made a separate issue to track this: #28180

👍 TheCharlatan approved a pull request: "ci: Keep system env vars as-is (bugfix)"
(https://github.com/bitcoin/bitcoin/pull/28138#pullrequestreview-1551904341)
lgtm ACK fabc04a4d96c4fe70e60d365aa28031d149094f3

💬 Sjors commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1277425106)
Dropping a constant but then hardcoding it all over the place with no historical context is not a good idea.

🤔 Sjors requested changes to a pull request: "Remove arbitrary restrictions on OP_RETURN by default"
(https://github.com/bitcoin/bitcoin/pull/28130#pullrequestreview-1551912890)
Concept NACK unless you keep the default at `MAX_OP_RETURN_RELAY`. Depending on adoption we can always drop that default later. No need to make pull requests more controversial than necessary.

🚀 fanquake merged a pull request: "refactor: Revert additional univalue check in ParseSighashString"
(https://github.com/bitcoin/bitcoin/pull/28162)

💬 aureleoules commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#issuecomment-1655530271)
I understand the points you have raised and do somewhat agree with them. Closing this PR.

✅ aureleoules closed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174)

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1655540354)
With `-mutate_depth=14` being worse (be aware that the y-axis no longer matches to all previous plots)


💬 MarcoFalke commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1655543843)
Thanks, generated some data and edited the description to support the `mutate_depth` choice. (Limited to [1,10] for now, but this can be changed back to [1,15], if there is at least one data point showing that values larger than 10 are useful)

💬 Sjors commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1655548325)
I guess both changes are a strict improvement, but won't put an end to the whack-a-mole grind.

💬 theStack commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913)
tiny-nit (or more, a curiosity question): Was it intended to derive the bit position within the byte randomly? Due to the choice of having 8 error values for this case, I was assuming that the idea was to systematically damage with all bit positions 0..7.
```suggestion
to_decrypt[InsecureRandRange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
```
But in practice it shouldn't really matter and I guess both are fine.

👍 theStack approved a pull request: "BIP324 ciphersuite"
(https://github.com/bitcoin/bitcoin/pull/28008#pullrequestreview-1551988560)
ACK 1c7582ead6e1119899922041c1af2b4169b0bc74

Finished reviewing. Code looks good to me, checked carefully that the implementation matches BIP324 and verified the test vectors (see above). :heavy_check_mark:

💬 Sjors commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1655594515)
@vostrnad's comment makes sense to me. I'll hold off re-reviewing until that's either done or decided against.