🤔 glozow reviewed a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-3034612261)
light code review ACK 96da68a38fa, looks correct to me

💬 glozow commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2216772534)
When I changed this to `false`, none of the tests failed - maybe worth adding a multisig case?

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2216808190)
This line is counting ops in the scriptSig. I'm not sure it makes sense to add a test case with an `OP_CHECKMULTISIG` sneaked in the scriptSig because this is already non-standard.

💬 achow101 commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3090534874)
light ACK 50024620b909fc30b68a3715680e963f048482a5

🚀 achow101 merged a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829)

💬 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