💬 tdb3 commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1645219537)
Nice approach to test the boundary and handle non-`bad-txns-oversize` reject reasons.

Exercised this by inducing a failure with:
```diff
- auto maxPayloadSize = maxTransactionSize - oversizedTransactionBaseSize;
+ auto maxPayloadSize = maxTransactionSize - oversizedTransactionBaseSize +1;
```
The test failed as expected.

👍 tdb3 approved a pull request: "test: Validate oversized transactions or without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2126734529)
ACK 969e047cfbab86e5819a2c9056e8d2dab17513a8
Left comments showing what was exercised.

💬 tdb3 commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1645225922)
Similarly, exercised this by inducing a failure with:
```diff
- maxPayloadSize += 1;
+ //maxPayloadSize += 1;
```

The test failed as expected.

💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2177289493)
> It's about 25 GB. Not sure if it's supposed to be deterministic, but here you go:
>
> ```
> 99450bac6e081e03751c064034b3d10e17b463ca56dfc35797fc6b5fe0de8ac4 ../utxo-snapshots/utxo-840000.sqlite
> ```

Pretty sure that the .sqlite file as a whole is not deterministic, as there are e.g. different page sizes supported (similar to BDB) and the header seems to include the concrete SQLite version that was used to create the file, see https://www.sqlite.org/fileformat.html#the_database_header
...

⚠️ kosuodhmwa opened an issue: "What means "Mempool limited" ?"
(https://github.com/bitcoin/bitcoin/issues/30303)
https://github.com/Mirobit/bitcoin-node-manager/issues/92

💬 S3RK commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#issuecomment-2177884124)
reACK a9c7300135f0188daa5bce5491e2daf2dd8da8ae

Looks even simpler now, thanks for incorporating feedback.

💬 Sjors commented on pull request "upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2177898915)
Guix hashes

```
2f879df88862b94c211670a451f442eb951a6ec963d841be683d1ee7c14cf49a guix-build-8acdf6654083/output/aarch64-linux-gnu/SHA256SUMS.part
00f163274f35e955eb5ef00cb1e6e81e828b3b7415d15f594ea61bc921c000b9 guix-build-8acdf6654083/output/aarch64-linux-gnu/bitcoin-8acdf6654083-aarch64-linux-gnu-debug.tar.gz
e83b5155ab258164204fd117f7974af90b580c2f0f0da713a1066eb2caecf651 guix-build-8acdf6654083/output/aarch64-linux-gnu/bitcoin-8acdf6654083-aarch64-linux-gnu.tar.gz
21ac9b233e814a1393
...

💬 S3RK commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1645529346)
> It also enforces that the sequences are either MAX_SEQUENCE_NONFINAL or MAX_BIP125_RBF_SEQUENCE, so this needs to be checking that as well otherwise an assertion will be hit. Or a commit could be added to relax those checks.

It seems like this part of the comment wasn't addressed

✅ maflcko closed an issue: "What means "Mempool limited" ?"
(https://github.com/bitcoin/bitcoin/issues/30303)

💬 maflcko commented on issue "What means "Mempool limited" ?":
(https://github.com/bitcoin/bitcoin/issues/30303#issuecomment-2177906842)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.

💬 kosuodhmwa commented on issue "What means "Mempool limited" ?":
(https://github.com/bitcoin/bitcoin/issues/30303#issuecomment-2177910080)
ok thx

💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2177915400)
That results in 32de3c3dc85e602878f9e5b7e6ebcd7fe917d4345f7a07c0bfd4c82b4b5639db.

So your other approach would be to use all the CSV fields to calculate the MuHash for the entire UTXO set at block 840,000 and then compare that to the `muhash` value of `gettxoutsetinfo muhash 840000` (with `-coinstatsindex` enabled)?

💬 S3RK commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1645537740)
I'm not clear why do we need a wallet lock here?

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1645611240)
If it is too much hassle to set up an arm, one could look into moving it to GHA. However, I am not sure if this is possible. Can macOS ARM run ARM Linux containers in docker?

⚠️ maflcko opened an issue: "ci: Move more tasks to GHA?"
(https://github.com/bitcoin/bitcoin/issues/30304)
Motivated by https://github.com/bitcoin/bitcoin/pull/29274 to make it easier to run the CI on forks, more tasks could be moved to GHA, similar to d97ddbe797f5b8b3bca0ee71b692e542b8990195?

The downside would be that it is harder to re-run a task (only maintainers can do it, not the pull request author).

Another downside would be that caching depends artefacts and docker images is hard on GHA. So ideally only tasks with `NO_DEPENDS=1` are moved for now. It would be:

* `ci/test/00_setup_en
...

💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2178023089)
cc @Sjors @m3dwards

🤔 hodlinator reviewed a pull request: "fuzz: have package_rbf always make small txns"
(https://github.com/bitcoin/bitcoin/pull/30300#pullrequestreview-2127472207)
ACK 4ccb3d6d0d576d32da8a1b9c6e70962cbd0f19fe

💬 hodlinator commented on pull request "fuzz: have package_rbf always make small txns":
(https://github.com/bitcoin/bitcoin/pull/30300#discussion_r1645663034)
Might as well
```suggestion
parent.vin.emplace_back(g_outpoints[iter++]);
```

💬 hodlinator commented on pull request "fuzz: have package_rbf always make small txns":
(https://github.com/bitcoin/bitcoin/pull/30300#discussion_r1645714573)
Good that you removed the unnecessary `vin.empty()` check.

Until the first time `ConsumeBool()` returns true here, we'll be using a `child` with a default-initialized `vin[0]`. This was true before the change as well. Does some later code hook up default-constructed `CTxIn` to coinbase transactions or something?

It seems unnecessary to have created a `parent` at all if the `ConsumeBool()` returns false. But I guess it's good to test adding unrelated transactions between each `child`.

💬 hodlinator commented on pull request "fuzz: have package_rbf always make small txns":
(https://github.com/bitcoin/bitcoin/pull/30300#discussion_r1645678247)
I understand this comment as explaining why a random number of `vin`s of `parent` generated from fuzz data are overwritten with one input. Time to change/remove?