💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211030320)
I don't think there is any risk in reading directly into the field. The deser throws internally on key size validation, so it safe.
Handling with a vector separately just seems like extra handling/code?
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211030320)
I don't think there is any risk in reading directly into the field. The deser throws internally on key size validation, so it safe.
Handling with a vector separately just seems like extra handling/code?
💬 w0xlt commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3079524346)
If I understand correctly, the `walletversion` (`CWallet::nWalletVersion`) is not used by descriptor wallets (as can be checked in https://github.com/bitcoin/bitcoin/pull/32977), so it has no real effect on whether or not this field is updated after a migration.
Unless we intend to use this field in the future, I don't see the point in keeping it or this RPC.
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3079524346)
If I understand correctly, the `walletversion` (`CWallet::nWalletVersion`) is not used by descriptor wallets (as can be checked in https://github.com/bitcoin/bitcoin/pull/32977), so it has no real effect on whether or not this field is updated after a migration.
Unless we intend to use this field in the future, I don't see the point in keeping it or this RPC.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211041788)
Seems simpler and I assumed copy elision kicks in, but I'll change it next time I touch the code, thanks.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211041788)
Seems simpler and I assumed copy elision kicks in, but I'll change it next time I touch the code, thanks.
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2211042123)
Thanks! Makes sense, I'll add them.
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2211042123)
Thanks! Makes sense, I'll add them.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211055488)
We're not exposing the internals of `Obfuscation`, we can't currently call `GetRandBytes(new_key)` on the internals, we have to first read the vector or create a random vector to be able to construct the object.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211055488)
We're not exposing the internals of `Obfuscation`, we can't currently call `GetRandBytes(new_key)` on the internals, we have to first read the vector or create a random vector to be able to construct the object.
💬 glozow commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2211062513)
Yes. Should I reword?
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2211062513)
Yes. Should I reword?
💬 glozow commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3079572074)
> op and commit message: `test_mid_package_evaluation` doesn't exist, it's named `test_mid_package_eviction`
Fixed
> The bull case for keeping `test_mid_package_replacement` is that it involves an RBF, not a mempool overflow, right? The comment in that test seems slightly erroneous, mentioning `replaced_tx` needs to be at the bottom of the mempool, when I think it just has to be low feerate enough to be RBF'd?
Thanks, yeah, I think it had an incorrect copy-paste from the other test. I t
...
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3079572074)
> op and commit message: `test_mid_package_evaluation` doesn't exist, it's named `test_mid_package_eviction`
Fixed
> The bull case for keeping `test_mid_package_replacement` is that it involves an RBF, not a mempool overflow, right? The comment in that test seems slightly erroneous, mentioning `replaced_tx` needs to be at the bottom of the mempool, when I think it just has to be low feerate enough to be RBF'd?
Thanks, yeah, I think it had an incorrect copy-paste from the other test. I t
...
💬 l0rinc commented on pull request "[IBD] log start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#issuecomment-3079579547)
Ran a reindex until 890k without `-assumevalid` to make sure it works for an actual IBD/reindex as well:
```python
2025-07-16T04:43:03Z UpdateTip: new best=00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77 height=886157 version=0x22024000 log2_work=95.475345 tx=1161875261 date='2025-03-03T15:52:21Z' progress=0.955815 cache=4682.3MiB(34814627txo)
2025-07-16T04:43:03Z Started signature validation at block 000000000000000000022614dc170eadb7b544824cbcdb0732bdeca42839aa37.
2025-07-
...
(https://github.com/bitcoin/bitcoin/pull/32975#issuecomment-3079579547)
Ran a reindex until 890k without `-assumevalid` to make sure it works for an actual IBD/reindex as well:
```python
2025-07-16T04:43:03Z UpdateTip: new best=00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77 height=886157 version=0x22024000 log2_work=95.475345 tx=1161875261 date='2025-03-03T15:52:21Z' progress=0.955815 cache=4682.3MiB(34814627txo)
2025-07-16T04:43:03Z Started signature validation at block 000000000000000000022614dc170eadb7b544824cbcdb0732bdeca42839aa37.
2025-07-
...
💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211075810)
`FastRandomContext` calls `GetRandBytes` internally, so I think it is fine to use (it is also used in the blockmanager).
At least for me the following compiles/links: (Obviously the code is wrong, I just wanted to check compilation)
```diff
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
index 2b33e04468..79c909b552 100644
--- a/src/dbwrapper.cpp
+++ b/src/dbwrapper.cpp
@@ -252,13 +252,13 @@ CDBWrapper::CDBWrapper(const DBParams& params)
{
assert(!m_obfuscation); //
...
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211075810)
`FastRandomContext` calls `GetRandBytes` internally, so I think it is fine to use (it is also used in the blockmanager).
At least for me the following compiles/links: (Obviously the code is wrong, I just wanted to check compilation)
```diff
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
index 2b33e04468..79c909b552 100644
--- a/src/dbwrapper.cpp
+++ b/src/dbwrapper.cpp
@@ -252,13 +252,13 @@ CDBWrapper::CDBWrapper(const DBParams& params)
{
assert(!m_obfuscation); //
...
💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211078238)
It only checks `m_rotations[0]` against a hardcoded memcpy, it doesn't check `ToKey` in the top commit, so I don't think the unit test adds any new checks. Otherwise, it would be good to know what exact bug this could possibly catch that the other test isn't catching.
In fact the unit test seems a bit confusing, because it constructs the obfuscation from an uint64_t, whereas production code only uses byte spans.
If you really want to directly test `Serialize` uses `m_rotations[0]` and `Un
...
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211078238)
It only checks `m_rotations[0]` against a hardcoded memcpy, it doesn't check `ToKey` in the top commit, so I don't think the unit test adds any new checks. Otherwise, it would be good to know what exact bug this could possibly catch that the other test isn't catching.
In fact the unit test seems a bit confusing, because it constructs the obfuscation from an uint64_t, whereas production code only uses byte spans.
If you really want to directly test `Serialize` uses `m_rotations[0]` and `Un
...
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2211078898)
Added to followup
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2211078898)
Added to followup
💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211080401)
Just a nit, anything is fine here, as it is optimized out anyway.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211080401)
Just a nit, anything is fine here, as it is optimized out anyway.
💬 gmaxwell commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079665606)
> It's not obvious that the price going up 10x means we get to lower the default by 10x, because it's a more important system now and the stakes are simply higher.
Sure, though at the same time the situation being dicussed here isn't price up 10x means default down 10x. Compared to when this was last adjusted the price is up 500x and this proposes down by 10x. :P The magnitudes matter-- particularly in that it answers your drawdown comment: Even if bitcoin prices drop by 90% they'll still
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079665606)
> It's not obvious that the price going up 10x means we get to lower the default by 10x, because it's a more important system now and the stakes are simply higher.
Sure, though at the same time the situation being dicussed here isn't price up 10x means default down 10x. Compared to when this was last adjusted the price is up 500x and this proposes down by 10x. :P The magnitudes matter-- particularly in that it answers your drawdown comment: Even if bitcoin prices drop by 90% they'll still
...
💬 sybot99 commented on issue "GUI bitcoin core shows wrong amount":
(https://github.com/bitcoin/bitcoin/issues/32976#issuecomment-3079684889)
> You can use the coin selection expert feature, or the RPC interface
Can you give some manual ? I need to get btc from that address, from change address
(https://github.com/bitcoin/bitcoin/issues/32976#issuecomment-3079684889)
> You can use the coin selection expert feature, or the RPC interface
Can you give some manual ? I need to get btc from that address, from change address
📝 l0rinc opened a pull request: "log: unify `UpdateTip` values"
(https://github.com/bitcoin/bitcoin/pull/32996)
The most frequently seen log line prints tip-update data after each successful block connection. During IBD this produces ~900k lines (~250 characters per line), totaling ~220 MB so far.
Most `key=value` pairs are written directly, but block time is emitted as `date='ISO8601DateTime'`, adding unnecessary single quotes because the value contains no spaces.
Removing those quotes shortens every line and reduces a full-IBD debug log by ~1% (about ~2MB).
Before:
> 2025-07-16T04:50:47Z Updat
...
(https://github.com/bitcoin/bitcoin/pull/32996)
The most frequently seen log line prints tip-update data after each successful block connection. During IBD this produces ~900k lines (~250 characters per line), totaling ~220 MB so far.
Most `key=value` pairs are written directly, but block time is emitted as `date='ISO8601DateTime'`, adding unnecessary single quotes because the value contains no spaces.
Removing those quotes shortens every line and reduces a full-IBD debug log by ~1% (about ~2MB).
Before:
> 2025-07-16T04:50:47Z Updat
...
💬 achow101 commented on pull request "wallet: Set migrated wallet name only on success":
(https://github.com/bitcoin/bitcoin/pull/32984#issuecomment-3079713773)
> > I was unable to write a working functional test for this behavior.
>
> It should be pretty trivial, see #32988
Ah, thanks. Pulled in that commit with some changes.
(https://github.com/bitcoin/bitcoin/pull/32984#issuecomment-3079713773)
> > I was unable to write a working functional test for this behavior.
>
> It should be pretty trivial, see #32988
Ah, thanks. Pulled in that commit with some changes.
💬 achow101 commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3079716543)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3079716543)
Concept ACK
👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for tx scanning"
(https://github.com/bitcoin/bitcoin/pull/32993)
(https://github.com/bitcoin/bitcoin/pull/32993)
💬 brunoerg commented on pull request "fuzz: wallet: add target for tx scanning":
(https://github.com/bitcoin/bitcoin/pull/32993#issuecomment-3079741494)
> https://cirrus-ci.com/task/4897452429410304?logs=ci#L6267:
>
> ```shell
> [10:48:30.374] Run wallet_scan with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
> [10:48:30.374] INFO: Seed: 1927962085
> [10:48:30.374] INFO: Loaded 1 modules (634026 inline 8-bit counters): 634026 [0x5573df666fb8, 0x5573df701c62),
> [10:48:30.374] INFO: Loaded 1 PC tables (634026 PCs): 634026 [0x5573df70
...
(https://github.com/bitcoin/bitcoin/pull/32993#issuecomment-3079741494)
> https://cirrus-ci.com/task/4897452429410304?logs=ci#L6267:
>
> ```shell
> [10:48:30.374] Run wallet_scan with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
> [10:48:30.374] INFO: Seed: 1927962085
> [10:48:30.374] INFO: Loaded 1 modules (634026 inline 8-bit counters): 634026 [0x5573df666fb8, 0x5573df701c62),
> [10:48:30.374] INFO: Loaded 1 PC tables (634026 PCs): 634026 [0x5573df70
...
💬 maflcko commented on issue "GUI bitcoin core shows wrong amount":
(https://github.com/bitcoin/bitcoin/issues/32976#issuecomment-3079753017)
coin control is an expert feature, but you can enable it in
Settings > Options > Wallet
Check "Enable coin control features (experts only!)"
The RPC has a manual, you can just go to "Window"->"Console" and type `help`.
(https://github.com/bitcoin/bitcoin/issues/32976#issuecomment-3079753017)
coin control is an expert feature, but you can enable it in
Settings > Options > Wallet
Check "Enable coin control features (experts only!)"
The RPC has a manual, you can just go to "Window"->"Console" and type `help`.