💬 1440000bytes commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1949572923)
Are ocean contributors (miners) complying with the nodes?

💬 ariard commented on pull request "Add imbued v3 based on template-matching":
(https://github.com/bitcoin/bitcoin/pull/29427#issuecomment-1949579407)
one design feedback, look on how any imbuance mechanism would have to fit with dynamic upgrades, whatever the bitcoin use-cases. especially matters for time-sensitive flows: https://github.com/lightning/bolts/pull/1117

💬 mrsteve0924 commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-1949611717)
after 4 years why not close this? obvious no one is going to fix it

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493302062)
Maybe weshould also check that we can have more than two directly conflicting transactions as long as they are all in a cluster size <=2.

<details>
<summary>diff </summary>

```diff
diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp
index e15fd29da3..561ebd3881 100644
--- a/src/test/rbf_tests.cpp
+++ b/src/test/rbf_tests.cpp
@@ -431,6 +431,53 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
BOOST_CHECK(new_diagram[1] == FeeFrac(high_fee, low_size
...

🤔 ismaelsadeeq reviewed a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1886534107)
Thanks for adding the test!
re-ACK d57fbda350ee9051931d9a0dad4beb55f6b2e574

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493302276)
here an another place below!

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1949937829)
Yeah, good point. So I guess the only way to remove unused includes would be to write another check in the linter. I'll drop the last commit and leave everything else for a follow-up.

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1949962146)
re-ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 🌄

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK d5228efb5391b31a9a06
...

🤔 hebasto reviewed a pull request: "Prevent weird focus rect on inital sync"
(https://github.com/bitcoin-core/gui/pull/795#pullrequestreview-1886576526)
I can actually see two distinct issues here:

1. The "Hide" button doesn't have focus when the overlay shows for the first time. This can be resolved with `ui->closeButton->setFocus();` in the `ModalOverlay` constructor.

2. The other issue is a bit convoluted. As a child widget, the overlay also follows its parent's tab order when processing "Tab" or "Shift+Tab" keys. Isolating the overlay's tab order might also be a solution to the issue.

This PR indeed forces focus on the "Hide" as lon
...

💬 hebasto commented on pull request "Prevent weird focus rect on inital sync":
(https://github.com/bitcoin-core/gui/pull/795#discussion_r1493340467)
Using the same variable across all conditions for the same action improves readability:

```suggestion
if (layerIsVisible) {
```

💬 jadijadi commented on pull request "Keep focus on "Hide" while ModalOverlay is visible":
(https://github.com/bitcoin-core/gui/pull/795#issuecomment-1950222937)
> I can actually see two distinct issues here:
>
> 1. The "Hide" button doesn't have focus when the overlay shows for the first time. This can be resolved with `ui->closeButton->setFocus();` in the `ModalOverlay` constructor.
> 2. The other issue is a bit convoluted. As a child widget, the overlay also follows its parent's tab order when processing "Tab" or "Shift+Tab" keys. Isolating the overlay's tab order might also be a solution to the issue.
>
> This PR indeed forces focus on the "Hi
...

💬 hebasto commented on pull request "Keep focus on "Hide" while ModalOverlay is visible":
(https://github.com/bitcoin-core/gui/pull/795#issuecomment-1950228993)
cc @jarolrod @furszy

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493358857)
I think we should drop the `Assume()` calls, as otherwise I think we ought to do more to ensure that using operator- on FeeFrac's is safe everywhere that might be invoked.

💬 daniel-btc-nym commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1950254374)
> I like bugfixes the most, when they are touching only a single line of code. But if more changes are required, then that is fine, too.

My changes have a few orthogonal, but related fixes and test additions. We could make different pull-requests for them. Let me know what you prefer.

For example:
1. Check strncmp will not go out of bound. This is lines 99-115 in univalue_read.cpp ; unit tests that call `no_reading_beyond_end_test`
1. Change parsing of numbers, where there were two plac
...

🤔 tdb3 reviewed a pull request: "Update rpcauth.py"
(https://github.com/bitcoin/bitcoin/pull/29433#pullrequestreview-1886951173)
Concept ACK. Inline comments provided.

💬 tdb3 commented on pull request "Update rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/29433#discussion_r1493397894)
The `share/rpcauth/README.md` file should also be updated to explain the argument and capability change in `rpcauth.py`.

nit:
Recommend making the argument be a single letter (e.g. `-j` for json) to follow the existing style with `-h` for help and prevent conflation with `-j -s -o -n`.

💬 bstin commented on pull request "Update rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/29433#discussion_r1493398180)
thank you. yes single "-" makes more sense.

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1950323631)
> Being able to specify a start_height sounds like something that would be generally useful for all indexes, not just this one. I've been toying with the idea of what it would take to have a "pruned" txindex and I would need to be able to start from a specific height for that. @fjahr I took the branch you started and got it working with a little tinkering here: https://github.com/josibake/bitcoin/tree/add-start-height-to-base-index . With a little more work this could probably be its own PR. Fee
...

🤔 sdaftuar reviewed a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1886598977)
Still reviewing all the tests -- leaving some nits that I've noticed so far.

In addition:
* I commented [here](https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493358857) that I think we should drop the `Assume(size != 0 || fee == 0)` calls in feefrac.h, so that we don't have to police all the places we invoke operator- to make sure we can never accidentally violate that.
* Another nit: in the commit message for commit 860823fb93212fa78ab928589bd403da462be222, it should say "Fee
...

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493359169)
nit: As the functions here don't relate to the `FeeFrac` implementation, perhaps this should be in a different file? `feerate_diagram.cpp` maybe?