💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719917818)
nit in https://github.com/bitcoin/bitcoin/commit/1dddb4fb0ff88162e58e50e56d5e889bb5727a81: Use the safe `UCharCast` while touching this, instead of the possibly unsafe pointer cast?
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719917818)
nit in https://github.com/bitcoin/bitcoin/commit/1dddb4fb0ff88162e58e50e56d5e889bb5727a81: Use the safe `UCharCast` while touching this, instead of the possibly unsafe pointer cast?
🤔 glozow reviewed a pull request: "wallet: fix blank legacy detection"
(https://github.com/bitcoin/bitcoin/pull/30621#pullrequestreview-2243021368)
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2
(https://github.com/bitcoin/bitcoin/pull/30621#pullrequestreview-2243021368)
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2
🚀 glozow merged a pull request: "wallet: fix blank legacy detection"
(https://github.com/bitcoin/bitcoin/pull/30621)
(https://github.com/bitcoin/bitcoin/pull/30621)
💬 fjahr commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720045465)
> Can that repo somehow have a permalink to the most recent asmap? Otherwise we would run into the same problem with outdated asmaps.
I hadn't really thought about this so far. This is a bit hacky but for now I copied the latest asmap file there to `latest_asmap.dat` and I would update that file as we create new maps so [this link](https://github.com/fjahr/asmap-data/raw/main/latest_asmap.dat) should always point to the latest file. I will look into making this a bit nicer by having latest.as
...
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720045465)
> Can that repo somehow have a permalink to the most recent asmap? Otherwise we would run into the same problem with outdated asmaps.
I hadn't really thought about this so far. This is a bit hacky but for now I copied the latest asmap file there to `latest_asmap.dat` and I would update that file as we create new maps so [this link](https://github.com/fjahr/asmap-data/raw/main/latest_asmap.dat) should always point to the latest file. I will look into making this a bit nicer by having latest.as
...
💬 0xB10C commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2293782491)
I tested with 67b1e236334f38ec4e4d2251dbdfb790f20ed88b that I'm able to do a nix build of the `bitcoind` (no GUI) and `bitcoin` (GUI) nix packages with the following modifications to the current nix package: https://github.com/0xB10C/nixpkgs/commit/fb77e0cf5194e48563e58eb418de8c2f9e0b48de. I also tested doing a development build with https://github.com/0xB10C/nix-bitcoin-core.
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2293782491)
I tested with 67b1e236334f38ec4e4d2251dbdfb790f20ed88b that I'm able to do a nix build of the `bitcoind` (no GUI) and `bitcoin` (GUI) nix packages with the following modifications to the current nix package: https://github.com/0xB10C/nixpkgs/commit/fb77e0cf5194e48563e58eb418de8c2f9e0b48de. I also tested doing a development build with https://github.com/0xB10C/nix-bitcoin-core.
💬 achow101 commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720070423)
I think you can make a symlink in the repo and just make sure it's updated every time.
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720070423)
I think you can make a symlink in the repo and just make sure it's updated every time.
💬 achow101 commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720088411)
Changed to use that asmap, although did not regenerate the seeds.
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1720088411)
Changed to use that asmap, although did not regenerate the seeds.
📝 mzumsande opened a pull request: "validation: improve m_best_header tracking"
(https://github.com/bitcoin/bitcoin/pull/30666)
`m_best_header` (the most-work header not known to be on an invalid chain) can be wrong in the context of invalidation / reconsideration of blocks. This can happen naturally (a valid header is received and stored in our block tree db; when the full block arrives, it is found to be invalid) or triggered by the user with the `invalidateblock` / `reconsiderblock` rpc.
We don't currently use `m_best_header` for any critical things (see OP of #16974 for a list that still seem up-to-date), so it be
...
(https://github.com/bitcoin/bitcoin/pull/30666)
`m_best_header` (the most-work header not known to be on an invalid chain) can be wrong in the context of invalidation / reconsideration of blocks. This can happen naturally (a valid header is received and stored in our block tree db; when the full block arrives, it is found to be invalid) or triggered by the user with the `invalidateblock` / `reconsiderblock` rpc.
We don't currently use `m_best_header` for any critical things (see OP of #16974 for a list that still seem up-to-date), so it be
...
💬 mzumsande commented on pull request "validation: Improve, document and test logic for chains building on invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/30207#issuecomment-2293902754)
I'll close this for now, large parts of this PR are contained in #30666 which actually fixes m_best_header tracking instead of just documenting its limitations.
The last two commits of this PR are not included in #30666 and fix a separate issue - I might open a separate PR for this at a later time.
(https://github.com/bitcoin/bitcoin/pull/30207#issuecomment-2293902754)
I'll close this for now, large parts of this PR are contained in #30666 which actually fixes m_best_header tracking instead of just documenting its limitations.
The last two commits of this PR are not included in #30666 and fix a separate issue - I might open a separate PR for this at a later time.
✅ mzumsande closed a pull request: "validation: Improve, document and test logic for chains building on invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/30207)
(https://github.com/bitcoin/bitcoin/pull/30207)
💬 sipa commented on pull request "validation: improve m_best_header tracking":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1720132804)
Is `m_best_header` necessarily a descendant of `pindexNew` here? If not, this loop will walk past genesis.
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1720132804)
Is `m_best_header` necessarily a descendant of `pindexNew` here? If not, this loop will walk past genesis.
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2294005237)
(Reposting at PR level instead of review level)
@glozow Cool, building on that:
```
$ git rebase --exec='F=$(mktemp) && make -j -C src bench/bench_bitcoin >/dev/null && src/bench/bench_bitcoin -filter=Linearize64TxWorstCase.* -min-time=60000 -output-json=$F >/dev/null 2>/dev/null && NS=$(cat $F | jq "(.results[0][\"median(elapsed)\"] - .results[1][\"median(elapsed)\"]) / 10000 * 1000000000") && TITLE=$(git show -s --format="%s") && echo "$NS $TITLE"' HEAD~11 2>/dev/null
11.60165442504903
...
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2294005237)
(Reposting at PR level instead of review level)
@glozow Cool, building on that:
```
$ git rebase --exec='F=$(mktemp) && make -j -C src bench/bench_bitcoin >/dev/null && src/bench/bench_bitcoin -filter=Linearize64TxWorstCase.* -min-time=60000 -output-json=$F >/dev/null 2>/dev/null && NS=$(cat $F | jq "(.results[0][\"median(elapsed)\"] - .results[1][\"median(elapsed)\"]) / 10000 * 1000000000") && TITLE=$(git show -s --format="%s") && echo "$NS $TITLE"' HEAD~11 2>/dev/null
11.60165442504903
...
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1720227562)
Ok, mystery solved. These adhoc initialization costs do not interact with the `sqrt(2^k)+1` rule, because that rule is at the per-candidate-set level, while the adhoc costs are applied at the linearization level. It does in theory interact with the derived $\sqrt{12 * 2^n}+n$ rule, but for $n \leq 3$ the linearization code is optimal without any search, and for $n \geq 4$, the formula is sufficiently overshooting to accomodate the ad-hoc costs. Still, I've changed the test to use precomputed lim
...
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1720227562)
Ok, mystery solved. These adhoc initialization costs do not interact with the `sqrt(2^k)+1` rule, because that rule is at the per-candidate-set level, while the adhoc costs are applied at the linearization level. It does in theory interact with the derived $\sqrt{12 * 2^n}+n$ rule, but for $n \leq 3$ the linearization code is optimal without any search, and for $n \geq 4$, the formula is sufficiently overshooting to accomodate the ad-hoc costs. Still, I've changed the test to use precomputed lim
...
💬 hebasto commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-2294101230)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/334.
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-2294101230)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/334.
💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720284745)
Looking at the actual code generated by GCC 14.2 (https://godbolt.org/z/8oTfPKE4q), I get the following diff from adding `[[likely]]`:
```diff
@@ -1,6 +1,13 @@
mov rax, rdi
shr rax, 33
- je .L8
+ jne .L2
+ movsx rax, edx
+ mov esi, esi
+ xor edx, edx
+ imul rax, rdi
+ div rsi
+ ret
+.L2:
movsx r10, edx
sub rsp, 24
mov rax, r10
@@ -24,
...
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720284745)
Looking at the actual code generated by GCC 14.2 (https://godbolt.org/z/8oTfPKE4q), I get the following diff from adding `[[likely]]`:
```diff
@@ -1,6 +1,13 @@
mov rax, rdi
shr rax, 33
- je .L8
+ jne .L2
+ movsx rax, edx
+ mov esi, esi
+ xor edx, edx
+ imul rax, rdi
+ div rsi
+ ret
+.L2:
movsx r10, edx
sub rsp, 24
mov rax, r10
@@ -24,
...
💬 paplorinc commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720294520)
I personally view it as documentation and would definitely keep it here
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720294520)
I personally view it as documentation and would definitely keep it here
💬 mzumsande commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2294229191)
I opened #30666
> @mzumsande, guess that your upcoming PR will continue containing the same shared commit "validation: mark blocks between m_best_header and invalid block as BLOCK_FAILED_CHILD" ?
Yes, I included that, without these block marked as BLOCK_FAILED_CHILD it `m_best_header` could still be incorrect.
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2294229191)
I opened #30666
> @mzumsande, guess that your upcoming PR will continue containing the same shared commit "validation: mark blocks between m_best_header and invalid block as BLOCK_FAILED_CHILD" ?
Yes, I included that, without these block marked as BLOCK_FAILED_CHILD it `m_best_header` could still be incorrect.
💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720339477)
Done!
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1720339477)
Done!
📝 allanperlee opened a pull request: "MiniWallet funcitonality for more readable code in functional test `rpc_signtransactionwithkey.py`"
(https://github.com/bitcoin/bitcoin/pull/30667)
Addressing issue #30600, this pull request simplifies the `send_to_address` functional test in rpc_signrawtransactionwithkey.py. The code is more terse with a MiniWallet object, its ADDRESS_OP_TRUE mode and `send_to` method on line 54, thus removing the "low-level" details of the transaction. Suggestions will be addressed. Thank you for reviewing!
(https://github.com/bitcoin/bitcoin/pull/30667)
Addressing issue #30600, this pull request simplifies the `send_to_address` functional test in rpc_signrawtransactionwithkey.py. The code is more terse with a MiniWallet object, its ADDRESS_OP_TRUE mode and `send_to` method on line 54, thus removing the "low-level" details of the transaction. Suggestions will be addressed. Thank you for reviewing!
✅ ryanofsky closed a pull request: "logging: Replace LogError and LogWarning with LogAlert"
(https://github.com/bitcoin/bitcoin/pull/30364)
(https://github.com/bitcoin/bitcoin/pull/30364)