💬 l0rinc commented on pull request "RPC: Fix invalid txid in `gettransaction` example":
(https://github.com/bitcoin/bitcoin/pull/31610#issuecomment-2600988779)
@luke-jr, what's wrong with the current approach?

✅ l0rinc closed a pull request: "coins: Add move operations to Coin and CCoinsCacheEntry"
(https://github.com/bitcoin/bitcoin/pull/30643)

🤔 mzumsande reviewed a pull request: "optimization: `CheckBlock` input duplicate detection"
(https://github.com/bitcoin/bitcoin/pull/31682#pullrequestreview-2561075956)
This is not just affecting `CheckBlock`, it is called whenever a tx is validated (so also during mempool acceptance).

The reason that this check is increased significantly percent-wise by these changes might just be that the non-contextual checks are extremely fast in the first place (compared with contextual checks such as script verification / input lookup).
Since the non-contextual checks aren't really ever performed in isolation, an interesting benchmark for me would be how much this imp
...

🤔 fjahr reviewed a pull request: "test: assumeutxo: add missing tests in wallet_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-2561068645)
Did another pass, mostly minor comments, otherwise looks pretty good. Will do a final pass when my comments have been addressed.

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921611162)
I would prefer if the logging here and below were comments because these are printed multiple times since this is a helper function and that means they are less helpful for debugging/showing progress.

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921614979)
nit: looks like there is a double space before `error_message`

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921612541)
Should check the balance of w2 here once more I think

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921615367)
nit: `pruneblockchain` returns the new pruneheight, so I think you could shorten this to

``` suggestion
assert_equal(n3.pruneblockchain(FINAL_HEIGHT), 299)
```

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921619130)
nit: I would have used this, it's a little more readable and a little more strict

``` suggestion
assert 'snapshot_blockhash' not in normal
```

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921617304)
nit: Here also two spaces

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921616676)
nit: Would name that wallet here `w_alt` or so, just because w2 is sort of reserved for the w2 backup

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921616285)
nit: similar to below

``` suggestion
assert_greater_than(n3.pruneblockchain(START_HEIGHT), 0)
```

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921615824)
nit: I would call it `complete_background_validation` or something similar. `wait_for_*` implies that the sync was started before somewhere else, but the sync is part of the function.

💬 luke-jr commented on pull request "RPC: Fix invalid txid in `gettransaction` example":
(https://github.com/bitcoin/bitcoin/pull/31610#issuecomment-2600994475)
> @luke-jr, what's wrong with the current approach?

It won't work without txindex enabled

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921645830)
Done

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921645869)
Done

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921645907)
Fixed

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921646155)
Done. Actually I changed it to `assert_equal(n3.pruneblockchain(FINAL_HEIGHT), 298)`. This is because the `prubeblockchain` method returns the height of the last block pruned (298 in this case) while `pruneheight` from `getblockchaininfo` returns the oldest block stored in disk (299 in our case). I added a comment to clarify this.

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921646227)
Done

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1921646239)
Fixed