🤔 mzumsande reviewed a pull request: "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors"
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2529778939)
Concept ACK

💬 mzumsande commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1902209558)
did you consider using the existing `hasBlocks()` function instead to adding this function? (I didn't check if it works, but at a glance it looks like it might).

The reason I looked for other functions because I wasn't sure if it's good to expose RPC function such as `GetPruneHeight()` through the chain interface, so that non-RPC code could use it too.

💬 mzumsande commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1902211186)
similar here: could you use the existing `hasAssumedValidChain`?

💬 mzumsande commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1902202946)
this went into the wrong commit (2nd instead of 1st)

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2569993729)
> Making the default 4000 instead of 8000 provides 0.1% extra fee revenue for miners who don't read the release notes. But there could be a miner out there losing 100% of the block revenue (subsidy + 100% of fees)

Fair enough.

I've pushed an update [diff](https://github.com/bitcoin/bitcoin/compare/f7a2f125c7b23c4c3c2e5253a6d3e2dcd8d3fe97..cd48e2ccaf24c9594f5987a29e02b924573134ed):

1. Set the default to 8000, based on [this suggestion](https://github.com/bitcoin/bitcoin/pull/31384#issuec
...

💬 achow101 commented on pull request "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor":
(https://github.com/bitcoin/bitcoin/pull/31590#issuecomment-2570023831)
> q: why not store the id belonging to the pubkey with the correct parity bit inside the `SigningProvider` arg at the point where such a structure is loaded? (which most likely happens inside `ParsePubkeyInner()`).

The pubkey in the descriptor has no parity bit information at all, and parsing is context independent.

💬 i-am-yuvi commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2570079166)
Tested ACK f1b40b4de460173e927ebe93cdb5f4ad2ac02859

I tried testing using `./src/bitcoind -signetchallenge=512102ee856c56a5aaadd1656f849bafa4c9dacc86a2878fe546c6189185f842ae2c1851ae`

👍 tdb3 approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2530058120)
Code review re ACK b35ed41b03578586e380cb73aece14046ec2da93

Sync from genesis was successful (to 877742, `000000000000000000008bac930b7efd54a7ddef0b9ec5b1c0c00b6a6c8e0cfb`)

fyi, conf used:
```
[main]
connect=<sister node on LAN>
checkpoints=0
prune=53000
dbcache=24000
```

⚠️ thebignaught opened an issue: "Fake bitcoin core website at the top of duckduckgo"
(https://github.com/bitcoin/bitcoin/issues/31602)
Hello all,

If you search "bitcoin core" on duckduckgo.com the top search result is a fake bitcoin core website with the URL: btcore . cc

I haven't actually visited the site but I am assuming it is offering malware downloads to the unsuspecting. Is there anything that can be done about this?

I have crossposted this also on the bitcoin-core github on the website repo.

💬 TheCharlatan commented on pull request "refactor: cache block[undo] serialized size for consecutive calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1902837379)
In commit 0dd386994bafd67081c4521673018ddd22e2d63c

Can we call this `ReadBlockUndo` instead?

💬 TheCharlatan commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1902903280)
In commit 3b66b7e20ddba94d251c97818700d46030b14cc5

`s/Spit/Split` in commit message.

💬 TheCharlatan commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2571248802)
Concept ACK

💬 laanwj commented on pull request "test: Remove non-portable IPv6 test":
(https://github.com/bitcoin/bitcoin/pull/31580#issuecomment-2571265003)
i think this makes sense, relying on a "default scope" is brittle at most. Better to add the interface number explicitly.
We don't use that anywhere in our code outside the test, right? If so i think it's okay to remove this test unconditionally.

👍 theStack approved a pull request: "wallet, desc spkm: Return SigningProvider only if we have the privkey"
(https://github.com/bitcoin/bitcoin/pull/31242#pullrequestreview-2530727962)
re-ACK f6a6d912059c66792f48689632d2a7f14f8bdad9

💬 l0rinc commented on pull request "refactor: cache block[undo] serialized size for consecutive calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1903089183)
That was inconsistent, indeed - [fixed](https://github.com/bitcoin/bitcoin/compare/0dd386994bafd67081c4521673018ddd22e2d63c..2ff0ea366c61b2fcb80ad1f711480915c7a7aa2e)!

💬 sebm123 commented on pull request "optimization: buffer reads(23%)/writes(290%) in [undo]block [de]serialization, 6% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31539#issuecomment-2571272017)
> An alternative implementation of #31551, where the block is read into memory in chunks, resulting in more modest gains, is slightly more complicated, but uses less memory.
>
> This is a follow-up to #31490 (first few commits duplicated here, hence a draft) and a precursor to #31144.
>
> Currently, obfuscation operations are performed byte-by-byte during serialization. Buffering the reads allows batching these operations (implemented in #31144) and improves file access efficiency by reduc
...

💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1903091109)
Hah, indeed, proofread all other commit messages:
```bash
git range-diff 836d733a6788c7f7e66da6f8007fb62f15161429..6983d82c84d4ca351a7cd9d1e0e20ab878da6475 ae40cf1a8e16462a8b9dfd076d440bc8ec796c2b..f919d919eb8425ef2bb25aa0ebf61c90ab9b07fa
```

💬 laanwj commented on pull request "refactor: Allow std::byte in (Read/Write)(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#issuecomment-2571273109)
Post-merge ACK fa83bec78ef3e86445e522afa396c97b58eb1902

💬 i-am-yuvi commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2571280097)
> > I am unable to crash the fuzz test when `setmocktime()` is removed from one of the targets.
>
> @i-am-yuvi which target was it?
>
> It could be that running the test from scratch doesn't quickly reach code where `now()` is called. Try running the target on a corpus of inputs:
>
> ```
> FUZZ=load_external_block_file ./mocktimefuzzbuild/src/test/fuzz/fuzz ../qa-assets/fuzz_corpora/load_external_block_file -runs=1
> ```
>
> You can find the corpora here: https://github.com/bitcoin
...

💬 i-am-yuvi commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2571280802)
Tested ACK fdc5224d205c69ebcb2d2b7bcd95386ef89181ca on x86_64

- removing `setmocktime ()` from any one of the targets would cause a crash.