💬 Sjors commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1798278012)
> I think it's not a convenient way to call `txin.scriptSig.GetOp(pc, opcode, vch);` three times.

Indeed it's not. But I don't understand what you're trying to do here.

I would suggest first fixing the tests, so that it's clear if the code is correct. And then worry about making the code nicer.

💬 dergoegge commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798293005)
> I think it would be better to fix the underlying timeouts and crashes, and treat them as bugs.

I mean for crashes sure but for timeouts I have the suspicion that this happens on almost all targets that use `TestingSetup`. There might even be another reason for why these files stick around besides timeouts.

There is just no nice way of preventing this besides "don't create tmp files" and I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we
...

💬 Sjors commented on pull request "index: blockfilter initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-1798294413)
> Could also give it a run rebased on top #27006.

That PR currently does not cleanly cherry-pick on top of this PR, not can I (trivially) rebase this PR on top top of it. Happy to try if you can make a branch.

👍 brunoerg approved a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1717431352)
utACK fa7ba926300f44b350d04262e569cc607e4991d8

🚀 fanquake merged a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789)

💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772837)
taken

💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772909)
taken

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798342200)
No, I am saying, as explained in my first comment, that this pull request isn't needed. If it was needed, a reason should be given in the pull request description. So far I haven't seen a valid reason, reading the whole thread.

💬 maflcko commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798350581)
> I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we add a new one.

In essence, this is identical to the rpc timeout issue in functional tests. I don't think a cheat code is available to sidestep maintenance here.

If you only care about not creating leftover tmp files, you can increase the timeout massively. If temp files are still created, it should be reported and treated as a bug and fixed.

However, I think we should also treat lower
...

💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798369856)
Addressed https://github.com/bitcoin/bitcoin/pull/28785#pullrequestreview-1716255880 and fixed the ci error

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798372461)
@maflcko I copy-pasted @luke-jr's comment in the PR description, if that helps?

💬 Sjors commented on issue "assumeutxo: Ensure transactions are not presented as confirmed until background sync is complete":
(https://github.com/bitcoin/bitcoin/issues/28598#issuecomment-1798373704)
@pablomartin4btc I also added that in #28616

💬 theStack commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798376058)
Concept ACK

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798385890)
reACK https://github.com/bitcoin/bitcoin/pull/28785/commits/1147e00e59e47f27024ec96629993c66a3ce4ef0

took reconsiderable variable renames, tests using CheckPackageMempoolAcceptResult more, unsure why ci was failing though :)

💬 maflcko commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798390693)
> Though after #24748 we might also want to consider running the entire suite with `--v2transport` in some CI.

I presume this will fix itself, once and if v2 is enabled by default?

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798410685)
Before the background sync is finished, we can only check the proof-of-work for new blocks and few other rules. It's better than SPV, but not the same as knowing the transaction is in a fully validated block. We should display that difference somehow.

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1384865925)
I think wallet tests should be separate from consensus tests. Also, it would be good to check what happens if loading a wallet file from a backup, whose transactions happened before the utxo snapshot.

💬 aureleoules commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1384873465)
Thanks for the ping, I re-ran the job. AWS preempted the job and I haven't figured out how to rerun them automatically yet.
Anyone in this org can re-run corecheck jobs by signing in as well!

💬 dergoegge commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798477310)
The fact that timeouts or crashes lead to left over files on disk is a bug and we should fix that. Otherwise it isn't really possible to conduct long running fuzzing campaigns using afl++ (or other engines that use the fork-server model) without running out of disk space.

We should also fix the timeouts and crashes themselves but that is not what I wanted to document with this issue.

💬 sipa commented on pull request "test: python cryptography required for BIP 324 functional tests":
(https://github.com/bitcoin/bitcoin/pull/28374#issuecomment-1798520540)
utACK c534c0871038ded72dc9078cc91e030ceb746196