💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3436089301)
I took your suggestion, make the test robust by generating a transaction after interrupting, and took @furszy suggestion by initializing the waiting inside the log.
cc @Eunovo
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3436089301)
I took your suggestion, make the test robust by generating a transaction after interrupting, and took @furszy suggestion by initializing the waiting inside the log.
cc @Eunovo
🚀 glozow merged a pull request: "test: p2p block malleability"
(https://github.com/bitcoin/bitcoin/pull/33172)
(https://github.com/bitcoin/bitcoin/pull/33172)
💬 maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3436092184)
> subpoena
Would be trivial to address by just using a more general tag like `assisted-by-llm`. I don't think we care about the exact model and parameters, and on which cloud provider (or locally) it was run?
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3436092184)
> subpoena
Would be trivial to address by just using a more general tag like `assisted-by-llm`. I don't think we care about the exact model and parameters, and on which cloud provider (or locally) it was run?
👋 fanquake's pull request is ready for review: "ci: run native fuzz with MSAN job"
(https://github.com/bitcoin/bitcoin/pull/33626)
(https://github.com/bitcoin/bitcoin/pull/33626)
💬 ajtowns commented on issue "rfc: only relay transactions to v2 encrypted peers":
(https://github.com/bitcoin/bitcoin/issues/32373#issuecomment-3436119899)
This seems inferior to private broadcast #29415 which separates the new txs from the mempool until they make it out to the wider network. Extending it to also relay via a much-less-private v2 transitory encrypted might be worth considering though?
(https://github.com/bitcoin/bitcoin/issues/32373#issuecomment-3436119899)
This seems inferior to private broadcast #29415 which separates the new txs from the mempool until they make it out to the wider network. Extending it to also relay via a much-less-private v2 transitory encrypted might be worth considering though?
💬 l0rinc commented on pull request "crypto: optimize SipHash Write() method with chunked processing":
(https://github.com/bitcoin/bitcoin/pull/33325#issuecomment-3436142482)
I have looked into this again.
I have printed the actual span sizes during runtime (IBD or reindex or reindex chainstate) and it seems to me most of the span sizes are tiny:
<img width="855" height="516" alt="image" src="https://github.com/user-attachments/assets/826de36e-53a1-4386-a4a1-353028eafab6" />
The benchmarks however use bigger values of different distribution (e.g. 16, 22, 23, 25, 32, 34).
They do show some speedup for your mentioned cases:
<img width="1489" height="859" alt="
...
(https://github.com/bitcoin/bitcoin/pull/33325#issuecomment-3436142482)
I have looked into this again.
I have printed the actual span sizes during runtime (IBD or reindex or reindex chainstate) and it seems to me most of the span sizes are tiny:
<img width="855" height="516" alt="image" src="https://github.com/user-attachments/assets/826de36e-53a1-4386-a4a1-353028eafab6" />
The benchmarks however use bigger values of different distribution (e.g. 16, 22, 23, 25, 32, 34).
They do show some speedup for your mentioned cases:
<img width="1489" height="859" alt="
...
👍 stickies-v approved a pull request: "util: Abort on failing CHECK_NONFATAL in debug builds"
(https://github.com/bitcoin/bitcoin/pull/32588#pullrequestreview-3369219621)
ACK fa37153288ca420420636046ef6b8c4ba7e5a478
(https://github.com/bitcoin/bitcoin/pull/32588#pullrequestreview-3369219621)
ACK fa37153288ca420420636046ef6b8c4ba7e5a478
🚀 glozow merged a pull request: "p2p: Correct unrealistic headerssync unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579)
(https://github.com/bitcoin/bitcoin/pull/32579)
👍 TheCharlatan approved a pull request: "refactor: Improve assumeutxo state representation"
(https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-3369269782)
Re-ACK 48e7e82808da43a52e31fb3ff95cdf763c077c20
(https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-3369269782)
Re-ACK 48e7e82808da43a52e31fb3ff95cdf763c077c20
💬 maflcko commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#issuecomment-3436307799)
Hmm, so I did a fresh run in a repo with a clean GHA cache, and it now says:
> Approaching total cache storage limit (8.89 GB of 10 GB Used)
So I guess we are still close to overlapping the cache and certainly will reach cache churn with more than one push to the default branch.
(https://github.com/bitcoin/bitcoin/pull/33639#issuecomment-3436307799)
Hmm, so I did a fresh run in a repo with a clean GHA cache, and it now says:
> Approaching total cache storage limit (8.89 GB of 10 GB Used)
So I guess we are still close to overlapping the cache and certainly will reach cache churn with more than one push to the default branch.
💬 laanwj commented on pull request "wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members":
(https://github.com/bitcoin/bitcoin/pull/32763#discussion_r2454827136)
Maybe `wtx.comment_to.value_or("")` to avoid undefined behavior if it happens to not be set.
(https://github.com/bitcoin/bitcoin/pull/32763#discussion_r2454827136)
Maybe `wtx.comment_to.value_or("")` to avoid undefined behavior if it happens to not be set.
💬 stickies-v commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3436638183)
ACK 944e5ff848f656d2ee6202b2330f3ae178ad0fbe
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3436638183)
ACK 944e5ff848f656d2ee6202b2330f3ae178ad0fbe
💬 Sjors commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3436655686)
@glozow thanks, can you add a backport label for 30.x?
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3436655686)
@glozow thanks, can you add a backport label for 30.x?
💬 dergoegge commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3436677670)
rfm
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3436677670)
rfm
📝 real-or-random opened a pull request: "test: Update BIP324 test vectors"
(https://github.com/bitcoin/bitcoin/pull/33688)
This updates the hardcoded test vectors from BIP324. The test vectors had to be regenerated (in the aux files of the BIP) because there was a bug in the script used for generating them (https://github.com/bitcoin/bips/pull/2016).
This is a draft because https://github.com/bitcoin/bips/pull/2016 has not been merged.
(https://github.com/bitcoin/bitcoin/pull/33688)
This updates the hardcoded test vectors from BIP324. The test vectors had to be regenerated (in the aux files of the BIP) because there was a bug in the script used for generating them (https://github.com/bitcoin/bips/pull/2016).
This is a draft because https://github.com/bitcoin/bips/pull/2016 has not been merged.
💬 l0rinc commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3436715527)
I like the new version better, you have indeed identified an inconsistency that we could fix in a way that would make the source could also more readable.
However, I think the current version still makes the code less readable than it was before - now the loop iterations depend on each other by reusing the same vector. However, `vSolutions` that you're reusing isn't actually needed at all (maybe that's what the compiler detected after your change). The cleaner alternative (that I also bumped
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3436715527)
I like the new version better, you have indeed identified an inconsistency that we could fix in a way that would make the source could also more readable.
However, I think the current version still makes the code less readable than it was before - now the loop iterations depend on each other by reusing the same vector. However, `vSolutions` that you're reusing isn't actually needed at all (maybe that's what the compiler detected after your change). The cleaner alternative (that I also bumped
...
👍 darosior approved a pull request: "test: descriptor: cover invalid multi/multi_a cases"
(https://github.com/bitcoin/bitcoin/pull/32504#pullrequestreview-3369771171)
utACK 58e55b17e632dbd4425dd64825b087f242ac4b7b
(https://github.com/bitcoin/bitcoin/pull/32504#pullrequestreview-3369771171)
utACK 58e55b17e632dbd4425dd64825b087f242ac4b7b
💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3436769067)
You're right, `vSolutions` is completely unnecessary in `AreInputsStandard()`. I'm not a fan of raw pointers but in this case it seems the best practical solution overall. smart pointers or optionals don't fit quite well.
I'll use your commits with small tweaks and add you as co-author for now
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3436769067)
You're right, `vSolutions` is completely unnecessary in `AreInputsStandard()`. I'm not a fan of raw pointers but in this case it seems the best practical solution overall. smart pointers or optionals don't fit quite well.
I'll use your commits with small tweaks and add you as co-author for now
🤔 ajtowns reviewed a pull request: "log: print every script verification state change"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3369808963)
ACK 45bd8914658a675d00aa9c83373d6903a8a9ece8
nits don't need fixing
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3369808963)
ACK 45bd8914658a675d00aa9c83373d6903a8a9ece8
nits don't need fixing
💬 ajtowns commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2455049702)
nit: Better as an `if` than ` .. ? .. : ..` IMO.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2455049702)
nit: Better as an `if` than ` .. ? .. : ..` IMO.