✅ achow101 closed an issue: "Wrong block mined time in testnet"
(https://github.com/bitcoin/bitcoin/issues/30121)
(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.
(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 ?
(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.
(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.
(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.
(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.
(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.
(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.
(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
(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?
(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.
(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 `=`
(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.
(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.
💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2116084230)
There seems to be a conflict with bitcoin/bitcoin#30098. Maybe rebase?
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2116084230)
There seems to be a conflict with bitcoin/bitcoin#30098. Maybe rebase?
📝 sipa opened a pull request: "Low-level cluster linearization code"
(https://github.com/bitcoin/bitcoin/pull/30126)
Based on a part of #29625.
This introduces low-level optimized cluster linearization code, including tests and some benchmarks. It is currently not hooked up to anything.
Roughly the commits are organized into 3 groups:
* Repeat of part of #29625.
* Introduce unoptimized versions of candidate finding and linearizations, plus benchmarks and tests.
* Add various optimizations step by step.
(https://github.com/bitcoin/bitcoin/pull/30126)
Based on a part of #29625.
This introduces low-level optimized cluster linearization code, including tests and some benchmarks. It is currently not hooked up to anything.
Roughly the commits are organized into 3 groups:
* Repeat of part of #29625.
* Introduce unoptimized versions of candidate finding and linearizations, plus benchmarks and tests.
* Add various optimizations step by step.
🤔 jonatack reviewed a pull request: "depends: Remove Qt build-time dependencies"
(https://github.com/bitcoin/bitcoin/pull/29923#pullrequestreview-2061817228)
Concept ACK. This looks like amazing work, great!
I'm not sure how to help test. Using arm64 macOS 14.4.1, this branch builds cleanly and the GUI runs fine. Don't see any issues on first read of the code.
```
$ clang --version
Homebrew clang version 18.1.5
Target: arm64-apple-darwin23.4.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
```
(https://github.com/bitcoin/bitcoin/pull/29923#pullrequestreview-2061817228)
Concept ACK. This looks like amazing work, great!
I'm not sure how to help test. Using arm64 macOS 14.4.1, this branch builds cleanly and the GUI runs fine. Don't see any issues on first read of the code.
```
$ clang --version
Homebrew clang version 18.1.5
Target: arm64-apple-darwin23.4.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
```
💬 sipa commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2116140509)
I think the 86 kB option is a bit faster, and the change in binary size/memory is immaterial for Bitcoin Core, so from that perspective maybe we want to use that. On the other hand, signing speed is not super important for us, but still, seems like 86 is the no-downside option.
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2116140509)
I think the 86 kB option is a bit faster, and the change in binary size/memory is immaterial for Bitcoin Core, so from that perspective maybe we want to use that. On the other hand, signing speed is not super important for us, but still, seems like 86 is the no-downside option.
👍 TheCharlatan approved a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2061831963)
Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2061831963)
Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
💬 naiyoma commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2116143536)
Test passes for all three scenarios:
- [x] https://github.com/bitcoin/bitcoin/pull/29996/commits/a7501f779eb84effd5579070b8b1c1a3020273d6 loading a snapshot when chain tip isn't ancestor but has less work
- [x] https://github.com/bitcoin/bitcoin/pull/29996/commits/3f033542b869dfbe037cda705c3b8f7faa32da5d loading a snapshot when chain tip isn't ancestor/descendant, has more work and A valid snapshot file & snapshot block, but the block is not on the most-work chain
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2116143536)
Test passes for all three scenarios:
- [x] https://github.com/bitcoin/bitcoin/pull/29996/commits/a7501f779eb84effd5579070b8b1c1a3020273d6 loading a snapshot when chain tip isn't ancestor but has less work
- [x] https://github.com/bitcoin/bitcoin/pull/29996/commits/3f033542b869dfbe037cda705c3b8f7faa32da5d loading a snapshot when chain tip isn't ancestor/descendant, has more work and A valid snapshot file & snapshot block, but the block is not on the most-work chain