💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1467815704)
But if there's only one name here, it's unclear what name the bool type is for? Or we're assuming it can only be used as a positional param at that point?

💬 michaelfolkson commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1912282572)
> I use the data from a few recent blocks to find out median of fees of transactions that were included. I also consider broadcasting a duplicated transaction with higher fee if the first one doesn't get in a block for too long.
>
> Besides, nobody uses fee estimation by Bitcoin Core in my circles.

Ok fair enough. That is a criticism of the Bitcoin Core wallet's fee estimation. But the "data from a few recent blocks" you are using for fee estimation includes transactions that have been subm
...

💬 GregTonoski commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1912291528)
Let's not divert from the subject. Fee estimation techniques and tradeoffs are beside the point.

💬 michaelfolkson commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1912300846)
> Let's not divert from the subject. Fee estimation techniques and tradeoffs are beside the point.

Just pointing out your inconsistency @GregTonoski. If you don't want people to have some of these high fee rate transactions in their mempools so they can do better fee estimation you should filter out these transactions from mined blocks in your fee estimation data. You want them to keep their heads in the sand that some of these transactions will be mined and hence you should keep your head in
...

📝 maflcko opened a pull request: "fuzz: Print coverage summary after run_once"
(https://github.com/bitcoin/bitcoin/pull/29329)
This can be used to quickly check the coverage effects of a code change or qa-assets change.

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467853130)
not sure I like this suggested text. anyone else want to give it a shot?

💬 maflcko commented on pull request "fuzz: Print coverage summary after run_once":
(https://github.com/bitcoin/bitcoin/pull/29329#issuecomment-1912328327)
Can be quickly tested with: `$ ./test/fuzz/test_runner.py ..folder.. addition_overflow multiplication_overflow`

🤔 mzumsande reviewed a pull request: "fuzz: Test headers pre-sync through p2p interface"
(https://github.com/bitcoin/bitcoin/pull/28043#pullrequestreview-1846077000)
just fyi, I started reviewing at some point, my problem was that I don't like test-specific / mocking code in production code (especially the consensus-critical parts), so I wasn't really comfortable (concept)-acking it.
On the other hand, I didn't see a better way to do it and fuzzing is great obviously...

As a result, I was undecided and didn't write anything. Maybe it'd be worth to have a general discussion about the extent of mocking / test-specific production code we want as a project,
...

👍 dergoegge approved a pull request: "fuzz: Print coverage summary after run_once"
(https://github.com/bitcoin/bitcoin/pull/29329#pullrequestreview-1846088199)
tACK fa74d2d54f3a102221fb24790c0dfb5c60758535

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1912341078)
For your consideration, I have pushed a variant with some recommended changes here: https://github.com/luke-jr/bitcoin/commits/rpccookieperms-26%2Bknots/

Feel free to cherry-pick or squash into your PR as you deem best.

💬 luke-jr commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1912353847)
Concept NACK to restricting it to only manual connections. This will cause random breakage when automatic peering happens to connect to a manual peer first.

🚀 fanquake merged a pull request: "fuzz: also set MSAN_SYMBOLIZER_PATH"
(https://github.com/bitcoin/bitcoin/pull/29327)

💬 mzumsande commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1912368151)
> Concept NACK to restricting it to only manual connections. This will cause random breakage when automatic peering happens to connect to a manual peer first.

did you see #28895? We don't make automatic connections to manually specified peers anymore.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467890537)
I've made it "We don't check whether the sibling is to-be-replaced (done in ApplyV3Rules) because that doesn't apply in a package."

💬 glozow commented on pull request "doc: update `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1467894205)
You've deleted the comment entirely?

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467907205)
Cleaned up, thanks

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467908805)
Added "Should be called for package transactions to fail more quickly" to the doc (as in: not must call but should call).

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909437)
Or I guess we could turn it off for `AcceptMultiple`?

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909613)
Done

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909813)
Done