💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1603770501)
Huh interesting. i didn't know netlink worked for multiple operating systems, that's much better.

💬 ryanofsky commented on pull request "JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/29946#issuecomment-2115854243)
Code review ACK f90a84d61505443fd3bb83253c091590b3dc7f45, but I think it would be helpful to change description to something like "doc: specify json content type in rpc examples" because the current description doesn't make it obvious that this is a documentation change, not a change in behavior.

PR also needs to be rebased since #27101 was just merged and it conflicts

📝 theStack opened a pull request: "test: improve BDB parser (handle internal/overflow pages, support all page sizes)"
(https://github.com/bitcoin/bitcoin/pull/30125)
This PR adds missing features to our test framework's BDB parser with the goal of hopefully being able to read all legacy wallets that are created with current and past versions of Bitcoin Core. This could be useful both for making review of https://github.com/bitcoin/bitcoin/pull/26606 easier and to also possibly improve our functional tests for the wallet BDB-ro parser by additionally validating it with an alternative implementation. The second commits introduces a test that create a legacy wa
...

💬 laanwj commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2115934921)
Concept ACK.

Agree with @ryanofsky that if copy is something expensive (or generally undesirable) to do, it would make sense to make copy explicit, so that it stands out in code review.

💬 1ma commented on issue "contrib/signet/miner: grind will fail for high difficulty chain":
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2115939051)
Thanks for the pointer. A bit off-topic, but since there are not so many people yet who are familiar with operating a signet let me ask you a follow up question:

I'm don't really understand why the signet miner has so much logic and is so opinionated. My initial idea was to keep the difficulty as low as possible then fire a cronjob every 10 minutes to insta-mine a block at the current timestamp, so the consensus algorithm would be tricked into thinking that is the real difficulty.

I was ne
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1603876259)
Is it possible to get the `scope_id` for IPv6 addresses? At least my router gives me an link-scope address.

✅ achow101 closed an issue: "Wrong block mined time in testnet"
(https://github.com/bitcoin/bitcoin/issues/30121)

💬 achow101 commented on issue "Wrong block mined time in testnet":
(https://github.com/bitcoin/bitcoin/issues/30121#issuecomment-2115970264)
This is expected behavior. `UpdateTip` logs the timestamp stored in the block header, which can be in the future and not actually match the time the block was mined. Testnet miners are known to be messing around and doing weird things like this.

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1603902055)
`m_package_feerates` is experiencing mission creep here. Really this is asking "was I called in AcceptSingleTransaction"?

Should we bikeshed this to make it clearer @glozow ?

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1603904807)
Good catch. It looks like BDB doesn't actually set last_pgno to the correct value, so this check is too strict. I've commented it out entirely.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2115990626)
Replaced Linux-specific `QueryDefaultGateway` with @vasild's netlink implementation for Linux and FreeBSD. This may even generalize to more UNIX variants.

Will tackle Windows next.

👍 hebasto approved a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095#pullrequestreview-2061691479)
re-ACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d.

👍 hebasto approved a pull request: "bench: enable wallet creation benchmarks on all platforms"
(https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2061694682)
re-ACK b6b2d822eb06d705e6ec4318211163e5862008a6.

💬 hebasto commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2116022140)
> Or would you prefer waiting for [bitcoin-core/secp256k1#1529](https://github.com/bitcoin-core/secp256k1/pull/1529)?

Not at all. [bitcoin-core/secp256k1#1529](https://github.com/bitcoin-core/secp256k1/pull/1529) is not critical because the Bitcoin Core's CMake staging branch does not use the `PROJECT_IS_TOP_LEVEL` variable.

💬 edilmedeiros commented on issue "contrib/signet/miner: grind will fail for high difficulty chain":
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2116027691)
Yeah, I had some difficulty understanding its behavior and I believe I grasped after studying the miner source code.

It has an internal timer whose logic is capable of doing what you want by itself. Instead of using the `calibrate` feature, go straight to using `--min-nbits`. It will mine a block very fast and wait for 10 minutes until it mines the next. That way, it keeps difficulty at minimum but can keep the network pace.

See https://edil.com.br/blog/creating-a-custom-bitcoin-signet.

👍 jonatack approved a pull request: "net: make the list of known message types a compile time constant"
(https://github.com/bitcoin/bitcoin/pull/29421#pullrequestreview-2061710382)
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef

💬 hebasto commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2116031458)
> @real-or-random @jonasnick

Can you confirm that the default value of the new `--with-ecmult-gen-kb` option is optimal for Bitcoin Core?

💬 laanwj commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1603933797)
Might want to note that the implemenation handles only bdb files with the same endian as the host.

💬 laanwj commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1603934797)
Forgot `=`

💬 laanwj commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-2116045523)
Code review ACK, from what i learned about how BDB databases work internally, implementation looks correct on first glance.