💬 achow101 commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3090586828)
ACK 96da68a38fa295d2414685739c41b8626e198d27

🚀 achow101 merged a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521)

💬 caesrcd commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3090651380)
> Also, i don't think there is a good reason for decreasing the incremental relay fee at the same time as the min relay fee.

If `DEFAULT_INCREMENTAL_RELAY_FEE` is not lowered along with `DEFAULT_MIN_RELAY_TX_FEE`, users will be forced to pay fees above the estimates during periods of low transaction demand. It’s as if `DEFAULT_INCREMENTAL_RELAY_FEE` were now effectively set to 10,000 sat/kvB.

That’s exactly what I’m seeing. Yesterday I broadcast a transaction with a 0.2 sat/vB fee, and wit
...

💬 naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325)
I was testing https://github.com/bitcoin/bitcoin/pull/32928 and made an attempt to reproduce this issue by hanging a getdescriptors call

```git diff
index aa9d286417..edbd5359ee 100755
--- a/test/functional/mock_external_signer.py
+++ b/test/functional/mock_external_signer.py
@@ -97,6 +97,15 @@ parser_signtx.add_argument('psbt', metavar='psbt')
parser_signtx.set_defaults(func=signtx)

if not sys.stdin.isatty():
+ log.debug("About to read from stdin")
+ log.debug(f"Current args: {sys.a
...

💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2216903066)
Done. Thanks for the suggestion, this is quite a bit simpler. Let me know if there's something that can be improved/fixed.

📝 darosior opened a pull request: "Backport #32521 to 29"
(https://github.com/bitcoin/bitcoin/pull/33013)
This backports PR #32521 to make the change available to miners who can't (or don't want to) upgrade past version 29.

💬 tnndbtc commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090725560)
@naiyoma I also suspect this might be the cause and put comment in https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3071393448 . As stated in that comment, one possible reason is that the close of `err_wr_pipe`, which should send the empty string to stdin for the python process (signer.py) to confirm, could return `-1` but bitcoind didn't check the return result.
`subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`

Typically, close of file descr
...

💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216970656)
I agree, sounds good to me!

👍 ryanofsky approved a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-3033783450)
Code review ACK 248b6a27c351690d3596711cc36b8102977adeab. Looks good! Thanks for adapting this and considering all the suggestions. I did leave more comments below but non are important and this looks good as-is

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216425882)
In commit "scripted-diff: unify xor-vs-obfuscation nomenclature" (0b8bec8aa6260c499c2663ab7a1c905da0d312c3)

Given latest changes where the obfuscation object is what's serialized instead of the key (e.g. `Read(OBFUSCATION_KEY_KEY, m_obfuscation)`), I think it probably makes sense to rename OBFUSCATE_KEY_KEY to OBFUSCATION_KEY

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216849672)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211613705

> I think the current way is probably simpler. Please let me know if you disagree.

I think current way is ok, but serialization test in the diff has some benefits over the current one. Diff suggested:

```c++
BOOST_AUTO_TEST_CASE(obfuscation_serialize)
{
Obfuscation obfuscation{};

// Test loading a key.
std::vector<std::byte> key_in{m_rng.randbytes<std::byte>(Obfuscation::KEY_SIZE)};
DataStr
...

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216948127)
In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

reinterpret_cast seems more appropriate than bit_cast here, though either are probably ok. IIUC reinterpret cast is guaranteed to convert the pointer to an integer that can be round tripped back into a valid pointer, while bit_cast is looser and doesn't provide any guarantees.

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216935443)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210851772

In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

I was about to ask the same question about why min was there to handle an impossible condition and would suggest either removing the `min` or removing the `if (target.size() > KEY_SIZE)` check if the latter wouldn't hurt performance . Either of these would make the code easier to follow, IMO

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216962117)
In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

Might be clearer to s/64-byte/8*KEY_SIZE/ here and s/64-bit/KEY_SIZE/ below

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216850853)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211627396

Thanks! New commit and explanations make this much clearer to me

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216460569)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211605825

In commit "refactor: prepare `DBWrapper` for obfuscation key change" (6bbf2d9311b47a8a15c17d9fe11828ee623d98e0)

> I have added them after you have requested [simplifying a more complicated optimization commit](https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1879140048)

Thanks for trying this. Looking at these two "prepare for" commits I don't think they simply the optimization commit and just create compl
...

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216848225)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211610091

> Do you recommend we apply any of that after the recent changes?

Nope, new implementation looks better and this should be resolved.

💬 ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216198711)
In commit "test: compare util::Xor with randomized inputs against simple impl" (618a30e326e9bcfd72e0e2645ce49f8b2a88714d)

re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211596259

Thanks for the detail in the commit message clarifying the reasons for the changes.

I would still suggest simplifying the tests. I think the first test is simple and easy to understand but has a major gap in coverage, and the second test provides good coverage but is written in a convoluted styl
...

💬 yuvicc commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3090820186)
Fixed conflicts and rebased to master.

🤔 fjahr reviewed a pull request: "index: Deduplicate HashKey / HeightKey handling"
(https://github.com/bitcoin/bitcoin/pull/32997#pullrequestreview-3034913148)
Looks good, aside from the documentation cleanup in `blockfilterindex.cpp` you can consider my comments optional.