🚀 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.

💬 Sjors commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#issuecomment-1655597008)
> If #17487 gets merged I will rebase this

It has been merged.

💬 darosior commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1277494461)
That's a large increase in runtime isn't it? Won't 100k iterations take much less than 100 minutes to complete on most of our targets?

We've got 115 targets at the moments, this change makes it so running this script with `--generate` for all targets would take around 8 days.