💬 foolbear commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-2107575480)
keywords: not watch-only wallet, prune mode, importprivkey, migratewallet.

"And same procedure works fine in another wallet of test net", but I cannot make an address with UTXOs from years ago(maybe have something to do with prune mode?).

💬 maflcko commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1598483565)
> no way to know if the compiler becomes happier:

If it compiles locally, the code should be fine. If the CI warning persists, it should be disabled in the CI task config. (you can test by pushing, or running locally)

💬 Sjors commented on pull request "contrib: use ENV flags in get_arch":
(https://github.com/bitcoin/bitcoin/pull/30074#issuecomment-2107600543)
The second commit b59a027d957a4cffd225a681e6c85f9ae7fd77f3 seems trivially correct since `get_machine` is unused.

I got a bit confused about its history, so here it is:

1. It first became unused in #21255 (merged Feb 22, 2021)
2. It was then removed in #21428 (merged Mar 18, 2021)
3. It was reintroduced in #22381 (merged Jul 9, 2021) in order to skip RISCV in `TestSymbolChecks`
4. That skipping was no longer needed as of #27029 (merged Feb 17, 2023)

I'll do a guix build...

📝 willcl-ark opened a pull request: "rpc: move UniValue in blockToJSON"
(https://github.com/bitcoin/bitcoin/pull/30094)
Fixes: #24542

Without explicitly declaring the move, these `UniValues` get copied, causing increased memory usage. Fix this by explicitly moving the `UniValue` objects.

Used by `rest_block` and `getblock` RPC.

💬 willcl-ark commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2107608396)
Before and after of RSS:


💬 maflcko commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2107608765)
review ACK b77bad309e92f176f340598eec056eb7bff86f5f

Should be fine either way, but did you also test it?

💬 willcl-ark commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-2107609517)
> Can you explain this? It has a move constructor, ...

I think you are correct here, the compiler implicitly generates move-contructors which are enough to fix this issue, if combined with appropriate `std::move`. I did not think that was the case, but I was using a non-reproducible test. I made my test reproducible and tested:

1. v27.0
2. addition of `std::move` only
3. addition of `std::move` + excplicit move constructor(s)
4. addition of `std::move` + excplicit move constructor(s) +
...

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598499179)
>
> I should add that I was using `IsDust()` with `DUST_RELAY_TX_FEE`, which takes SegWit into account to decide the exact dust limit.
>

Ah I'm not sure how this should affect the results tbh. In my index I just looked at the UTXO value, no modifications.

>
> I looked at the largest output, not all outputs.

Yeah but my current understanding is that in order to see no difference on the index size at all, all of those UTXOs must have been in combination with bigger UTXOs, not eligib
...

💬 TheCharlatan commented on pull request "contrib: use ENV flags in get_arch":
(https://github.com/bitcoin/bitcoin/pull/30074#issuecomment-2107613473)
Guix build (aarch64):
```
fcb75f2a3b61befeabe143301e5db490d23b1ee4a5edb109f219dea575395b49 guix-build-b59a027d957a/output/aarch64-linux-gnu/SHA256SUMS.part
15310e3aff2a9ef71bad06315fcd425e56611d5bed7f6c394f3fe2248140e17b guix-build-b59a027d957a/output/aarch64-linux-gnu/bitcoin-b59a027d957a-aarch64-linux-gnu-debug.tar.gz
8e60146d39a47e9c332e3b842e239f1ca7773ca2e6788876155377f4568ab1fa guix-build-b59a027d957a/output/aarch64-linux-gnu/bitcoin-b59a027d957a-aarch64-linux-gnu.tar.gz
10432256ea
...

👍 TheCharlatan approved a pull request: "contrib: use ENV flags in get_arch"
(https://github.com/bitcoin/bitcoin/pull/30074#pullrequestreview-2052757206)
ACK b59a027d957a4cffd225a681e6c85f9ae7fd77f3

💬 maflcko commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-2107624153)
> either use `std::move` or really use `.copy()` if a copy is needed.

Style-wise I found it a bit unfortunate, because it forces code to either specify `move` or `copy` explicitly. (There is some beauty to rust, in that it implicitly moves)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598508290)
Dust limits are not stored in the index. https://github.com/bitcoin/bitcoin/pull/28241/commits/835dd4ba1ca4cc1abfd338e6bc38e25a116f5f8b stores the maximum output amount for each transaction. You can then use the RPC with any dust limit value you like.

💬 maflcko commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2107629353)
Also "fixes" #30052

💬 willcl-ark commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2107634197)
> Also "fixes" #30052

Ah yes, the actual reason I started investigating this in the first place! Thanks.

I was slightly unsure whether I should claim that this is fully fixing #24525, but it will fix #30052

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598514498)
My bad, that's what I meant. So you can just say give me everything where at least one UTXO exists above that threshold value. That's what my implementation does as well. Should be comparable then.

💬 maflcko commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2107635441)
Unrelated to the blockToJSON changes here, as a follow-up, it may be possible to "fix" https://github.com/bitcoin/bitcoin/issues/25229 in the same way?

🤔 ismaelsadeeq reviewed a pull request: "rpc: move UniValue in blockToJSON"
(https://github.com/bitcoin/bitcoin/pull/30094#pullrequestreview-2052778427)
ACK b77bad309e92f176f340598eec056eb7bff86f5f

🤔 vasild reviewed a pull request: "doc: i2p: improve `-i2pacceptincoming` mention"
(https://github.com/bitcoin/bitcoin/pull/29813#pullrequestreview-2052781798)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d

👍 TheCharlatan approved a pull request: "rpc: move UniValue in blockToJSON"
(https://github.com/bitcoin/bitcoin/pull/30094#pullrequestreview-2052782996)
ACK b77bad309e92f176f340598eec056eb7bff86f5f

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598526584)
Ok I've added a commit to fix this and another log to say "transaction(s)". I took the opportunity to add a few nice things to the logs as well.