💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233680450)
It doesn't just initialize the state of the object, but modifies the global state as well - can we make that more obvious to make sure the users understand that?
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233680450)
It doesn't just initialize the state of the object, but modifies the global state as well - can we make that more obvious to make sure the users understand that?
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233674509)
maybe we could reuse the previous setters like we do in other methods:
```suggestion
void operator()(std::chrono::seconds d) { set(m_t + d); }
```
or maybe even:
```suggestion
void operator()(std::chrono::seconds d)
{
Assert(d >= 0s); // Elapse time cannot move backward.
set(m_t + d);
}
```
nit: I don't find the operator to be intuitive here, maybe we could call it `Advance` instead?
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233674509)
maybe we could reuse the previous setters like we do in other methods:
```suggestion
void operator()(std::chrono::seconds d) { set(m_t + d); }
```
or maybe even:
```suggestion
void operator()(std::chrono::seconds d)
{
Assert(d >= 0s); // Elapse time cannot move backward.
set(m_t + d);
}
```
nit: I don't find the operator to be intuitive here, maybe we could call it `Advance` instead?
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233676772)
In the constructors we're using the setters, can we use it here, too, to minimize the places where we're adjusting the local/global state?
```suggestion
~ElapseTime() { set(0s); }
```
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233676772)
In the constructors we're using the setters, can we use it here, too, to minimize the places where we're adjusting the local/global state?
```suggestion
~ElapseTime() { set(0s); }
```
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233681737)
I think the part that a setter modifies both the local and global state should be made more obvious, it's not intuitive:
```suggestion
void Set(NodeSeconds t) noexcept
{
m_t = t;
SetMockTime(m_t); // Set the global mock time.
}
```
nit: capitalized + noexcept
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233681737)
I think the part that a setter modifies both the local and global state should be made more obvious, it's not intuitive:
```suggestion
void Set(NodeSeconds t) noexcept
{
m_t = t;
SetMockTime(m_t); // Set the global mock time.
}
```
nit: capitalized + noexcept
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233691602)
are we planning on using the new declared object? Could we rather skip the name, or call it a `time_mock` or something to obviate why it's unused?
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233691602)
are we planning on using the new declared object? Could we rather skip the name, or call it a `time_mock` or something to obviate why it's unused?
⚠️ OreoCookieRustDev opened an issue: "Pruned node missing some coinbase UTXOs (version: 28.1)"
(https://github.com/bitcoin/bitcoin/issues/33071)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Bitcoin Core version: 28.1
Node state: Fully synced, pruned
Observed issue:
Using a fully synced pruned node (prune=550), I scanned the UTXO set using bitcoin-cli scantxoutset for the address 1J6PYEzr4CUoGbnXrELyHszoTSz3wCsCaj (known to contain the original 50 BTC coinbase from satoshis block 8).
The result returns only small non-coinbase dust UTXOs totaling 0.00012281 BTC - every transac
...
(https://github.com/bitcoin/bitcoin/issues/33071)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Bitcoin Core version: 28.1
Node state: Fully synced, pruned
Observed issue:
Using a fully synced pruned node (prune=550), I scanned the UTXO set using bitcoin-cli scantxoutset for the address 1J6PYEzr4CUoGbnXrELyHszoTSz3wCsCaj (known to contain the original 50 BTC coinbase from satoshis block 8).
The result returns only small non-coinbase dust UTXOs totaling 0.00012281 BTC - every transac
...
💬 Sjors commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2233868340)
I think initially it would be similar to [HWI](https://github.com/bitcoin-core/HWI): separately maintained and released. (but see discussion below)
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2233868340)
I think initially it would be similar to [HWI](https://github.com/bitcoin-core/HWI): separately maintained and released. (but see discussion below)
💬 Bost commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-3124271015)
FYI my build fails:
```
bost@ecke ~/dev/guix$ guix describe | grep -B 1 -A 2 codeberg
guix fe71182
repository URL: https://codeberg.org/guix/guix.git
branch: master
commit: fe7118239d45f032d78c86f62c6b3d7119ee77e2
bost@ecke ~/dev/guix$ git show fe7118239d45f032d78c86f62c6b3d7119ee77e2
commit fe7118239d45f032d78c86f62c6b3d7119ee77e2 (HEAD -> master, codeberg/master, codeberg/HEAD)
Author: Andreas Enge <andreas@enge.fr>
Date: Sat Jul 26 23:41:51 2025
gnu: b
...
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-3124271015)
FYI my build fails:
```
bost@ecke ~/dev/guix$ guix describe | grep -B 1 -A 2 codeberg
guix fe71182
repository URL: https://codeberg.org/guix/guix.git
branch: master
commit: fe7118239d45f032d78c86f62c6b3d7119ee77e2
bost@ecke ~/dev/guix$ git show fe7118239d45f032d78c86f62c6b3d7119ee77e2
commit fe7118239d45f032d78c86f62c6b3d7119ee77e2 (HEAD -> master, codeberg/master, codeberg/HEAD)
Author: Andreas Enge <andreas@enge.fr>
Date: Sat Jul 26 23:41:51 2025
gnu: b
...
✅ OreoCookieRustDev closed an issue: "Pruned node missing some coinbase UTXOs (version: 28.1)"
(https://github.com/bitcoin/bitcoin/issues/33071)
(https://github.com/bitcoin/bitcoin/issues/33071)
💬 pinheadmz commented on issue "Pruned node missing some coinbase UTXOs (version: 28.1)":
(https://github.com/bitcoin/bitcoin/issues/33071#issuecomment-3124295188)
> Using a fully synced pruned node (prune=550), I scanned the UTXO set using bitcoin-cli scantxoutset for the address 1J6PYEzr4CUoGbnXrELyHszoTSz3wCsCaj (known to contain the original 50 BTC coinbase from satoshis block 8).
First of all I think you mean block 9 ;-)
And second of all, this is not known and is in fact incorrect! Satoshi did not use a pay-to-pubkey-hash address. If you check that coinbase transaction on an [explorer](https://mempool.space/tx/0437cd7f8525ceed2324359c2d0ba26006d92d8
...
(https://github.com/bitcoin/bitcoin/issues/33071#issuecomment-3124295188)
> Using a fully synced pruned node (prune=550), I scanned the UTXO set using bitcoin-cli scantxoutset for the address 1J6PYEzr4CUoGbnXrELyHszoTSz3wCsCaj (known to contain the original 50 BTC coinbase from satoshis block 8).
First of all I think you mean block 9 ;-)
And second of all, this is not known and is in fact incorrect! Satoshi did not use a pay-to-pubkey-hash address. If you check that coinbase transaction on an [explorer](https://mempool.space/tx/0437cd7f8525ceed2324359c2d0ba26006d92d8
...
💬 pinheadmz commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3124309346)
Did a rough benchmark test. Froze a full node at height `906551` by restarting with `-noconnect` and also `-txindex -blockfilterindex`. First test was from master, after 48 hours I aborted the process after reaching only:
```
$ bitcoin-cli getindexinfo
{
"txindex": {
"synced": false,
"best_block_height": 770289
},
"basic block filter index": {
"synced": true,
"best_block_height": 906551
}
}
```
Running this PR (restarting with empty `indexes/`), both ind
...
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3124309346)
Did a rough benchmark test. Froze a full node at height `906551` by restarting with `-noconnect` and also `-txindex -blockfilterindex`. First test was from master, after 48 hours I aborted the process after reaching only:
```
$ bitcoin-cli getindexinfo
{
"txindex": {
"synced": false,
"best_block_height": 770289
},
"basic block filter index": {
"synced": true,
"best_block_height": 906551
}
}
```
Running this PR (restarting with empty `indexes/`), both ind
...
💬 pinheadmz commented on issue "Pruned node missing some coinbase UTXOs (version: 28.1)":
(https://github.com/bitcoin/bitcoin/issues/33071#issuecomment-3124328203)
finally...
```
$ bitcoin-cli -rpcclienttimeout=999999999999999 scantxoutset start '[{"desc":"pk(0496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858ee)"}]'
{
"success": true,
"txouts": 167415131,
"height": 907405,
"bestblock": "000000000000000000017573b428d3f13ad8944f368b53ad195ad03b3bdbb1a1",
"unspents": [
{
"txid": "0e3e2357e806b6cdb1f70b54c3a3a17b6714ee1f0e68bebb44a74b1efd512098",
"vout": 0
...
(https://github.com/bitcoin/bitcoin/issues/33071#issuecomment-3124328203)
finally...
```
$ bitcoin-cli -rpcclienttimeout=999999999999999 scantxoutset start '[{"desc":"pk(0496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858ee)"}]'
{
"success": true,
"txouts": 167415131,
"height": 907405,
"bestblock": "000000000000000000017573b428d3f13ad8944f368b53ad195ad03b3bdbb1a1",
"unspents": [
{
"txid": "0e3e2357e806b6cdb1f70b54c3a3a17b6714ee1f0e68bebb44a74b1efd512098",
"vout": 0
...
💬 macgyver13 commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2234032395)
This is the format I used to import scan and spend private keys:
sp([fingerprint/352'/1'/0'/1'/0]WIF(private scan key),[fingerprint/352'/1'/0'/0'/0]WIF(private spend key)#checksum
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2234032395)
This is the format I used to import scan and spend private keys:
sp([fingerprint/352'/1'/0'/1'/0]WIF(private scan key),[fingerprint/352'/1'/0'/0'/0]WIF(private spend key)#checksum
🤔 cedwies reviewed a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/33064#pullrequestreview-3059599768)
Tested on MacOS 15.5 (Debug build).
- unit tests pass (0/143 failures)
- ./test/functional/wallet_transactiontime_rescan.py passes (11 s)
Question: would it make sense to also test "abortrescan" **during** an active rescan to hit the True path and ensure the scan halts as expected?
ACK 8aed477
(https://github.com/bitcoin/bitcoin/pull/33064#pullrequestreview-3059599768)
Tested on MacOS 15.5 (Debug build).
- unit tests pass (0/143 failures)
- ./test/functional/wallet_transactiontime_rescan.py passes (11 s)
Question: would it make sense to also test "abortrescan" **during** an active rescan to hit the True path and ensure the scan halts as expected?
ACK 8aed477
💬 yuvicc commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#issuecomment-3124693888)
Concep ACK
This would make the test consistent with `mempool_accept`
(https://github.com/bitcoin/bitcoin/pull/33067#issuecomment-3124693888)
Concep ACK
This would make the test consistent with `mempool_accept`
💬 goodgollyholly commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2234123982)
Hi need help my laptop was stollen and all i have is my cell phone from like the ice age do i need to download my core wallet to execute this
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2234123982)
Hi need help my laptop was stollen and all i have is my cell phone from like the ice age do i need to download my core wallet to execute this
💬 ariard commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3124747814)
> but BIP editors have been highly resistant to it, hence doc/policy.
I think it’s def worthy to have BIP even for tx-relay policy once it has spent the test of time (a la BIP125 for replacement mechanism), though here it’s a bastard situation of a policy rule that is intended to be consensus sooner or latter, and which is already documented in its own BIP. But during the meanwhile, there can be downside effects on multi-party tx flows.
It’s not clear if it’s worthy to document in doc/poli
...
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3124747814)
> but BIP editors have been highly resistant to it, hence doc/policy.
I think it’s def worthy to have BIP even for tx-relay policy once it has spent the test of time (a la BIP125 for replacement mechanism), though here it’s a bastard situation of a policy rule that is intended to be consensus sooner or latter, and which is already documented in its own BIP. But during the meanwhile, there can be downside effects on multi-party tx flows.
It’s not clear if it’s worthy to document in doc/poli
...
💬 jonatack commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3124756789)
> I think there are a lot of cases where a document would be helpful but BIP editors have been highly resistant to it, hence doc/policy.
More recently, BIPs involving policy R&D and interoperability have been given BIP numbers and merged. Perhaps propose on the ML for community feedback.
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3124756789)
> I think there are a lot of cases where a document would be helpful but BIP editors have been highly resistant to it, hence doc/policy.
More recently, BIPs involving policy R&D and interoperability have been given BIP numbers and merged. Perhaps propose on the ML for community feedback.
💬 Zeegaths commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3124757121)
I ran these commands on Ubuntu 22:
sudo ip addr add 1.1.1.1/32 dev wlo1
sudo ip addr add 2.2.2.2/32 dev wlo1
./build/bin/bitcoind -regtest -bind=0.0.0.0:18444 -discover -daemon
./build/bin/bitcoin-cli -regtest getnetworkinfo | grep -A 10 "localaddresses"
**Result on master:**
"localaddresses": [
],
"warnings": [
"This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
]
**Result on PR:**
"localaddresses": [
{
...
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3124757121)
I ran these commands on Ubuntu 22:
sudo ip addr add 1.1.1.1/32 dev wlo1
sudo ip addr add 2.2.2.2/32 dev wlo1
./build/bin/bitcoind -regtest -bind=0.0.0.0:18444 -discover -daemon
./build/bin/bitcoin-cli -regtest getnetworkinfo | grep -A 10 "localaddresses"
**Result on master:**
"localaddresses": [
],
"warnings": [
"This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
]
**Result on PR:**
"localaddresses": [
{
...
💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2234167309)
done, thanks
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2234167309)
done, thanks