🤔 tdb3 reviewed a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-1999297020)
re-ACK for 858fa78637041ae704005d4b6e564dd8245f4b5d

Thanks for incorporating the suggestion.

Used a small ramdisk (1.2GB) with high jobs count (20) to force space exhaustion. As expected, received the initial warning `Running the test suite with fewer than 3026.0 MB of free space might cause tests to fail`. and also received the exiting error `Early exiting after test failure due to disk running out of space...Test execution data left in ...Additional storage is needed to execute testing`
...

📝 furszy opened a pull request: "index: race fix, lock cs_main while 'm_synced' is subject to change"
(https://github.com/bitcoin/bitcoin/pull/29867)
Fixes #29831. Thanks to Marko for the detailed description of the issue.

The race occurs because a block could be connected and its event signaled in-between reading the 'next block' and setting 'm_synced' during the index initial synchronization process.
To address this issue, the `m_synced` set has been moved under the `cs_main` guard. This ensures that new block events are not signaled while the index sync state is being determined.

💬 davidgumberg commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-2053718293)
reACK https://github.com/bitcoin/bitcoin/pull/29335/commits/858fa78637041ae704005d4b6e564dd8245f4b5d

Looks great, tested on a tmpfs 1 GiB in size, and the test runner prints the intended warning, and fails immediately when the disk is full.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2053737139)
Concept ACK.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2053738677)
> remove unused cpp-subprocess options

Are all remaining options used?

🤔 vostrnad reviewed a pull request: "[27.x] More backports and finalize"
(https://github.com/bitcoin/bitcoin/pull/29780#pullrequestreview-1999340097)
Reviewed the release notes. A few style nits/suggestions, one entry is missing its PR number.

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218367)
```suggestion
- The `mempool.dat` file created by `-persistmempool` or the savemempool RPC will
```

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218402)
```suggestion
- Opt-in Topologically Restricted Until Confirmation (TRUC) Transactions policy
```

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218594)
```suggestion
flush in order to speed up the syncing of such nodes. (#20827)
```

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218569)
```suggestion
- The addnode RPC now follows the `-v2transport` option (now on by default, see above) for making connections.
```

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218615)
```suggestion
- Improved handling of empty `settings.json` files. (#28920)
```

💬 vostrnad commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564218543)
```suggestion
(aka v3 transaction policy) is available for use on test networks when
`-acceptnonstdtxn=1` is set. By setting the transaction version number to 3, TRUC transactions
```

💬 okorye commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564292710)
- [0x2ed4De9a4c3D09A8917E4CE31dB3abE473de87a4https://t.me/Cryptocom_DeFiWallet/236016`{"SPDXID":"SPDXRef-DOCUMENT","spdxVersion":"SPDX-2.3","creationInfo":{"created":"202403-05T10:18:00Z","creators":["Tool: GitHub.com-DependencyGraph"]},"name":"com.github.okorye/codespaces-express","dataLicense":"CC01.0","documentDescribes":["SPDXRef-com.github.okorye-codespacesexpress"],"documentNamespace":"https://github.com/okorye/codespaces-
express/dependency_graph/sbom-ce3fd3ce36630cb4","packages":[{"SPDXI
...

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1564307022)
Good point! I added the message like you suggest. Logs now read:
```
2024-04-13T23:05:41.654000Z TestFramework (INFO): At block height >= 128 this multisig is 3-of-4
2024-04-13T23:05:41.674000Z TestFramework (INFO): Check that the time-locked transaction is too immature to spend with 3-of-4 at block height 103...
2024-04-13T23:05:41.675000Z TestFramework (INFO): Generate blocks to reach the time-lock block height 128 and broadcast the transaction...
2024-04-13T23:05:42.888000Z TestFramework
...

🤔 tdb3 reviewed a pull request: "test: Validate transaction without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-1999388825)
ACK for 1240610fcf0651f217fd01de387e1047dc18485f

Great work adding more coverage.
Built and ran all unit tests (including `transaction_tests`). Passed.

💬 tdb3 commented on pull request "test: Validate transaction without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1564345633)
To check that unexpected reject reasons would cause test failure, added characters to the end of the reject reason check. Test failed as expected.

```c
BOOST_CHECK(state.GetRejectReason() == "bad-txns-vin-empty_ShouldCauseTestFailure");
```

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1564361740)
I just got confused here. CLTV opcode checks against spending transaction `nLockTime`. So behavior is exactly as expected

> Do you mean the 4-signature transaction was rejected?

No I didn't mean this. 4-signature transaction would never be rejected. But I explained this confusingly so you can ignore my comment above

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-2053833598)
Thanks for the review @rkrux !

> Suggested adding a log message in the last portion of the test.

Nice suggestion, I did this just now when I rebased on latest `master`

💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1564364788)
Yeah there is def some duplicated code between the two tests. But I want to focus on making this simple and easy to understand. It's trying to demonstrate/document an example wallet and could be used as a reference. Kind of like a quick start / tutorial

If I were to combine these tests it would become harder for someone to quickly and easily follow, and would lose the purpose of being a quick example. I think it's quite common to have code duplication in these types of tests/examples for that
...

🤔 tdb3 reviewed a pull request: "Feature: Use different datadirs for different signets"
(https://github.com/bitcoin/bitcoin/pull/29838#pullrequestreview-1999403287)
Concept ACK.
Great feature to help prevent conflicting datadir conflicts when switching between signets.
Inline comments added.

Received failures for functional tests feature_signet.py and tool_signet_miner.py.
```
dev@bdev01:~/bitcoin$ test/functional/test_runner.py -u --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp test/functional/feature_signet.py test/functional/tool_signet_miner.py
Temporary test directory at /mnt/tmp/test_runner_₿_🏃_20240413_214701
Remaining jobs: [feature_signet.py
...