💬 Prabhat1308 commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2757479417)
I suspect this issue is because of the `-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld"` flag.

Other than my default llvm19 , I used llvm@18 downloaded via brew which comes with clang 18 I used 2 different configs

```
make --preset=libfuzzer \
-DCMAKE_C_COMPILER="$(brew --prefix llvm@18)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm@18)/bin/clang++" \
-DAPPEND_LDFLAGS="-Wl,-no_w
...

💬 oren-z0 commented on issue "Feature Request: testmempoolaccept with an argument to ignore nLocktime/nSequence time-locking":
(https://github.com/bitcoin/bitcoin/issues/32142#issuecomment-2757493641)
Thanks @darosior , no I did not see that discussion before (only searched for related issues, not related PRs).
In the other discussion, the main use case for the feature is to simplify the development of L2 layers - which use scripts with pre-known structure.
In my use case, I'm building a service that could broadcast future recovery-transactions uploaded by customers, and I want to verify in advance that these transactions could indeed be broadcasted when the time comes (unless, of course, th
...

📝 musaHaruna opened a pull request: "test: add p2pk support to wallet_implicity_segwit.py"
(https://github.com/bitcoin/bitcoin/pull/32152)
This pull request modifies `wallet_implicity_segwit.py` by adding support for Pay-to-Public-Key (P2PK) transactions. It addresses a previously noted TODO, which suggested that P2PK transactions will be nice to be tested as part of the wallet implicit segwit functionality. The modification expands the test to include transactions involving P2PK outputs, which are handled by Bitcoin Core in a non-native way (simulated using legacy addresses). The updated test ensures that Bitcoin Core correctly pr
...

📝 rkrux opened a pull request: "wallet: remove redundant `Assert` call when block is disconnected"
(https://github.com/bitcoin/bitcoin/pull/32153)
It was highlighted in a PR discussion previously that the recently moved `Assert` macro call inside the block disconnected loop had been redundant for quite a while because of the presence of the `assert` macro call at the start of the function. Therefore, it is removed now.

refs #https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1995416821

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
...

💬 laanwj commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2757604990)
> Fair points! My reasoning to consider this (somewhat hacky) "import code from test framework" approach as acceptable was the fact that we already do it in other contrib scripts as well (e.g. the [signet miner](https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/contrib/signet/miner#L17-L19)), but yeah, that doesn't necessarily mean it's a very good idea.

Yes, to be fair i've used that hack plenty of times in my own code outside of bitcoin core, the test framewor
...

🤔 l0rinc reviewed a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2720758773)
lightweight, code review only ACK 6539598b42005f453b358a0076287db99d1b02eb

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2016268165)
nit: for consistency I'd make this either a `r"...\[node 0\]` as well or do a `re.escape` if we want to be explicit. Though I think we're probably matching way too much here, it doesn't hint at which part of the error is important and which part is irrelevant and subject to change

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2016198629)
Is this the issue? https://github.com/python/cpython/issues/109601 If so, it doesn't seem to be Windows related.

📝 maflcko opened a pull request: "fuzz: Avoid integer sanitizer warnings in policy_estimator target"
(https://github.com/bitcoin/bitcoin/pull/32154)
It seems odd to write a fuzz target to trigger integer sanitizer warnings in `CBlockPolicyEstimator::processBlockTx` and then suppress them. If the scenario can happen in reality, the code should be properly fixed to handle the cases. If not, it seems better to fix the fuzz target to not trigger meaningless traces.

Do that here by keeping track of the current height and limiting mempool entries to at most this entry height.

🤔 maflcko reviewed a pull request: "test: add p2pk support to wallet_implicity_segwit.py"
(https://github.com/bitcoin/bitcoin/pull/32152#pullrequestreview-2720936932)
the bdb wallet will be removed, so i guess it seems easier to just wait for that to happen instead of modifying code that will be deleted

💬 musaHaruna commented on pull request "test: add p2pk support to wallet_implicity_segwit.py":
(https://github.com/bitcoin/bitcoin/pull/32152#issuecomment-2757692617)
> the bdb wallet will be removed, so i guess it seems easier to just wait for that to happen instead of modifying code that will be deleted

Okay thanks for the heads up, should I close the PR then?

✅ maflcko closed a pull request: "test: add p2pk support to wallet_implicity_segwit.py"
(https://github.com/bitcoin/bitcoin/pull/32152)

💬 maflcko commented on pull request "test: add p2pk support to wallet_implicity_segwit.py":
(https://github.com/bitcoin/bitcoin/pull/32152#issuecomment-2757697397)
See https://github.com/bitcoin/bitcoin/pull/31250

💬 maflcko commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2016324443)
Also, the commit message seems to imply this is a bugfix for an issue observed on Windows. However, I don't think anyone has observed this on current master. The first commit of this pull claims to be a "refactor", so maybe the bug this is fixing was introduced there instead?

🤔 l0rinc reviewed a pull request: "wallet: remove redundant `Assert` call when block is disconnected"
(https://github.com/bitcoin/bitcoin/pull/32153#pullrequestreview-2720994104)
The initial `assert(block.data)` at the function start already establishes that `block.data` is non-null + the loop already dereferences it

crACK ae6b6ea296a228f342c3c635dc9e14c101e9534d

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2757728856)
Rebased after the merge of #31363.

💬 maflcko commented on pull request "test: Add functional test for bitcoin-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32145#issuecomment-2757731395)
This is only adding a test case, so should be rfm with three acks?

💬 sipa commented on pull request "Follow-ups for txgraph #31363":
(https://github.com/bitcoin/bitcoin/pull/32151#discussion_r2016333169)
Thanks, done!

💬 maflcko commented on pull request "test: remove strict restrictions on rpc_deprecated test":
(https://github.com/bitcoin/bitcoin/pull/32139#discussion_r2016337190)
My suggestion to use "(if any)" was to be able to just leave everything in the skeleton as-is forever. In the future only new test cases are added or removed. This should also reduce merge conflicts.

The suggestion was, but I am not sure if it was self-explanatory: (If not, maybe add a comment to not remove or otherwise touch the log line)

```
self.log.info("Tests for deprecated RPC methods (if any)")

if self.is_wallet_compiled():
self.log.info("Tests for deprecated wallet-related R
...

💬 maflcko commented on pull request "test: get rid of redundant TODO tag":
(https://github.com/bitcoin/bitcoin/pull/32058#issuecomment-2757746751)
lgtm ACK d065208f0f06309d776114d777bb16b8c38af3f1