💬 TheBlueMatt 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#issuecomment-3123472162)
One advantage of doing a separate process is that it sidesteps either dealing with validation integration [1], which will invariably stall progress ~forever. Using a separate binary for now would let the work streams progress independently and figuring out how to in-house validation can come separately.
[1] I think there's several ways, so even analyzing them will probably stall things ~forever - Rust integration, trying to write secp256r1/RSA plus DNSSEC validation in C++, or taking on some de
...
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3123472162)
One advantage of doing a separate process is that it sidesteps either dealing with validation integration [1], which will invariably stall progress ~forever. Using a separate binary for now would let the work streams progress independently and figuring out how to in-house validation can come separately.
[1] I think there's several ways, so even analyzing them will probably stall things ~forever - Rust integration, trying to write secp256r1/RSA plus DNSSEC validation in C++, or taking on some de
...
💬 achow101 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#issuecomment-3123744788)
> One advantage of doing a separate process is that it sidesteps either dealing with validation integration
It introduces a potential security issue though since we can't validate the output of the separate process. We're just trusting whatever it spits out, and if it were malicious, it could be making fake addresses which results in funds being sent to an attacker., and the user basically wouldn't know.
There's also the question of how to distribute that binary which I think would result
...
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3123744788)
> One advantage of doing a separate process is that it sidesteps either dealing with validation integration
It introduces a potential security issue though since we can't validate the output of the separate process. We're just trusting whatever it spits out, and if it were malicious, it could be making fake addresses which results in funds being sent to an attacker., and the user basically wouldn't know.
There's also the question of how to distribute that binary which I think would result
...
💬 luke-jr commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2233596644)
`{self.config['environment']['CLIENT_NAME']}`
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2233596644)
`{self.config['environment']['CLIENT_NAME']}`
💬 luke-jr commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2233596661)
`{self.config['environment']['CLIENT_NAME']}`
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2233596661)
`{self.config['environment']['CLIENT_NAME']}`
💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2233730246)
@macgyver13 Did you use the same private key for scan and spend?
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2233730246)
@macgyver13 Did you use the same private key for scan and spend?
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233666164)
nit: the code isn't in exact alignment with the comment - and since we *are* calling it with `0`, maybe something like:
```suggestion
Assert(d >= 0ms); // Steady time cannot move backward.
```
(nit2: to obviate that we don't just mean that jumps cannot be 0.5 seconds, we might want to use `0ms` instead)
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233666164)
nit: the code isn't in exact alignment with the comment - and since we *are* calling it with `0`, maybe something like:
```suggestion
Assert(d >= 0ms); // Steady time cannot move backward.
```
(nit2: to obviate that we don't just mean that jumps cannot be 0.5 seconds, we might want to use `0ms` instead)
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233663906)
typo in commit message:
> no state *is* leaked between test cases
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233663906)
typo in commit message:
> no state *is* leaked between test cases
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233658117)
👍 for using the built-in `min` instead.
Could we add it to the commit message that this isn't just a simple refactor (and not just about the return value), but as @hodlinator stated in a different PR where he did the same:
> That way we can always proceed instead of just exiting the fuzz target, wasting cycles. Also reduces code complexity.
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233658117)
👍 for using the built-in `min` instead.
Could we add it to the commit message that this isn't just a simple refactor (and not just about the return value), but as @hodlinator stated in a different PR where he did the same:
> That way we can always proceed instead of just exiting the fuzz target, wasting cycles. Also reduces code complexity.
🤔 l0rinc reviewed a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430#pullrequestreview-3058931165)
Thanks for fixing these, left quite a few comments, hope you'll find them useful
(https://github.com/bitcoin/bitcoin/pull/32430#pullrequestreview-3058931165)
Thanks for fixing these, left quite a few comments, hope you'll find them useful
💬 l0rinc commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233667188)
nit: I understand that it's meant to be symmetric with `ElapseSteady`, but I don't find the comment helpful as it is.
(https://github.com/bitcoin/bitcoin/pull/32430#discussion_r2233667188)
nit: I understand that it's meant to be symmetric with `ElapseSteady`, but I don't find the comment helpful as it is.
💬 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
...