💬 l0rinc commented on pull request "ci: Run unit test parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3120840192)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3120840192)
Concept ACK
💬 l0rinc commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3120847592)
> Looks like this was introduced in https://github.com/bitcoin/bitcoin/commit/421218d3040279c84616891e8d14b05576b07c57
Maybe @sipa can help us out here: should we try making read/write symmetric (either by `write` returning, or `read` throwing) or is the current refactor the cleanest direction?
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3120847592)
> Looks like this was introduced in https://github.com/bitcoin/bitcoin/commit/421218d3040279c84616891e8d14b05576b07c57
Maybe @sipa can help us out here: should we try making read/write symmetric (either by `write` returning, or `read` throwing) or is the current refactor the cleanest direction?
📝 w0xlt opened a pull request: "wallet: Add address validation in `sendtoaddress` RPC"
(https://github.com/bitcoin/bitcoin/pull/33068)
Is there a specific reason not to validate the address in the `sendtoaddress` RPC?
This PR adds that.
(https://github.com/bitcoin/bitcoin/pull/33068)
Is there a specific reason not to validate the address in the `sendtoaddress` RPC?
This PR adds that.
💬 ajtowns commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232582690)
```c++
static void check_script_has_static_size(CScript& script, unsigned int size)
{
BOOST_CHECK_EQUAL(script.size(), size);
BOOST_CHECK_EQUAL(script.capacity(), CScriptBase::STATIC_SIZE);
BOOST_CHECK_EQUAL(script.allocated_memory(), 0);
}
static void check_script_has_dynamic_size(CScript& script, unsigned int size)
{
BOOST_CHECK_EQUAL(script.size(), size);
BOOST_CHECK_EQUAL(script.capacity(), size);
BOOST_CHECK_EQUAL(script.allocated_memory(), size);
}
``
...
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232582690)
```c++
static void check_script_has_static_size(CScript& script, unsigned int size)
{
BOOST_CHECK_EQUAL(script.size(), size);
BOOST_CHECK_EQUAL(script.capacity(), CScriptBase::STATIC_SIZE);
BOOST_CHECK_EQUAL(script.allocated_memory(), 0);
}
static void check_script_has_dynamic_size(CScript& script, unsigned int size)
{
BOOST_CHECK_EQUAL(script.size(), size);
BOOST_CHECK_EQUAL(script.capacity(), size);
BOOST_CHECK_EQUAL(script.allocated_memory(), size);
}
``
...
💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232583047)
you think that would be easier to follow?
If it fails I would see it logged in `check_script_has_dynamic_size` - unless I make it a macro.
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232583047)
you think that would be easier to follow?
If it fails I would see it logged in `check_script_has_dynamic_size` - unless I make it a macro.
💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232585256)
Thanks, pushed, let me know what you think!
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232585256)
Thanks, pushed, let me know what you think!
✅ w0xlt closed a pull request: "wallet: Add address validation in `sendtoaddress` RPC"
(https://github.com/bitcoin/bitcoin/pull/33068)
(https://github.com/bitcoin/bitcoin/pull/33068)
💬 ajtowns commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232640266)
Can get errors like:
```
test/script_tests.cpp(1246): error: in "script_tests/script_size_and_capacity_test": check script_dynamic_alloc( script, 64, 64 ) has failed for ( CScript(size=67,cap=67), 64, 64 )
test/script_tests.cpp(1253): error: in "script_tests/script_size_and_capacity_test": check script_dynamic_alloc( script, 10, 1000 ) has failed for ( CScript(size=71,cap=103), 10, 1000 )
```
with this approach:
```diff
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cp
...
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2232640266)
Can get errors like:
```
test/script_tests.cpp(1246): error: in "script_tests/script_size_and_capacity_test": check script_dynamic_alloc( script, 64, 64 ) has failed for ( CScript(size=67,cap=67), 64, 64 )
test/script_tests.cpp(1253): error: in "script_tests/script_size_and_capacity_test": check script_dynamic_alloc( script, 10, 1000 ) has failed for ( CScript(size=71,cap=103), 10, 1000 )
```
with this approach:
```diff
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cp
...
📝 w0xlt opened a pull request: "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver"
(https://github.com/bitcoin/bitcoin/pull/33069)
This PR implements [BIP-353](https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki) support in Bitcoin Core's wallet, enabling users to send Bitcoin to human-readable usernames instead of traditional addresses.
### What is BIP-353?
BIP-353 specifies a method for resolving human-friendly payment instructions via DNS. Instead of sharing long Bitcoin addresses, users can share memorable usernames like `alice._bitcoin-payment.example.com`.
### Key Features
* New `-dnsresolver` opt
...
(https://github.com/bitcoin/bitcoin/pull/33069)
This PR implements [BIP-353](https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki) support in Bitcoin Core's wallet, enabling users to send Bitcoin to human-readable usernames instead of traditional addresses.
### What is BIP-353?
BIP-353 specifies a method for resolving human-friendly payment instructions via DNS. Instead of sharing long Bitcoin addresses, users can share memorable usernames like `alice._bitcoin-payment.example.com`.
### Key Features
* New `-dnsresolver` opt
...
📝 w0xlt converted_to_draft a pull request: "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver"
(https://github.com/bitcoin/bitcoin/pull/33069)
This PR implements [BIP-353](https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki) support in Bitcoin Core's wallet, enabling users to send Bitcoin to human-readable usernames instead of traditional addresses.
### What is BIP-353?
BIP-353 specifies a method for resolving human-friendly payment instructions via DNS. Instead of sharing long Bitcoin addresses, users can share memorable usernames like `alice._bitcoin-payment.example.com`.
### Key Features
* New `-dnsresolver` opt
...
(https://github.com/bitcoin/bitcoin/pull/33069)
This PR implements [BIP-353](https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki) support in Bitcoin Core's wallet, enabling users to send Bitcoin to human-readable usernames instead of traditional addresses.
### What is BIP-353?
BIP-353 specifies a method for resolving human-friendly payment instructions via DNS. Instead of sharing long Bitcoin addresses, users can share memorable usernames like `alice._bitcoin-payment.example.com`.
### Key Features
* New `-dnsresolver` opt
...
💬 Sjors commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3121464147)
@achow101 there was some discussion about `test_bitcoin` above: https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2807539249
https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3027233699
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3121464147)
@achow101 there was some discussion about `test_bitcoin` above: https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2807539249
https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3027233699
💬 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_r2232783849)
So after #28201 this would no longer be a failure?
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2232783849)
So after #28201 this would no longer be a failure?
💬 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_r2232784061)
Maybe return an array of strings, because there might be multiple options we can pay to (regular address, silent payment, some future thing).
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2232784061)
Maybe return an array of strings, because there might be multiple options we can pay to (regular address, silent payment, some future thing).
💬 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_r2232784859)
Can you add this to the send RPC as well? Ideally the code should reusable by the GUI too, so maybe move it to CWallet.
(https://github.com/bitcoin/bitcoin/pull/33069#discussion_r2232784859)
Can you add this to the send RPC as well? Ideally the code should reusable by the GUI too, so maybe move it to CWallet.
💬 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#issuecomment-3121475171)
Concept ACK
Using `RunCommandParseJSON()` is similar to how we handle external signers. It's a good way to add this functionality without adding a lot of code and dependencies.
It won't be useful until we have silent payments send support #24897 (we don't want to encourage address reuse), so this PR could be contingent on that.
Node in a box applications like [Rspiblitz](https://github.com/raspiblitz/raspiblitz) could easily ship and configure a resolver, so this can be used not just by
...
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3121475171)
Concept ACK
Using `RunCommandParseJSON()` is similar to how we handle external signers. It's a good way to add this functionality without adding a lot of code and dependencies.
It won't be useful until we have silent payments send support #24897 (we don't want to encourage address reuse), so this PR could be contingent on that.
Node in a box applications like [Rspiblitz](https://github.com/raspiblitz/raspiblitz) could easily ship and configure a resolver, so this can be used not just by
...
💬 HowHsu commented on pull request "truc: optimize the in package relation calculation":
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2232787217)
Hi glozow,
> I think most of the benefits of precomputing this would be realized in the loop a few lines down looking for a package_tx sibling.
Could you elaborate on this?
As is, I'm not sure how much faster precomputation is: n=2 in normal cases, and we don't need to continue computing parents once we've realized it's oversize.
Yep, for n=2, I think no much difference here.
(https://github.com/bitcoin/bitcoin/pull/33062#discussion_r2232787217)
Hi glozow,
> I think most of the benefits of precomputing this would be realized in the loop a few lines down looking for a package_tx sibling.
Could you elaborate on this?
As is, I'm not sure how much faster precomputation is: n=2 in normal cases, and we don't need to continue computing parents once we've realized it's oversize.
Yep, for n=2, I think no much difference here.
💬 fanquake commented on issue "intermittent timeout in wallet_signer.py : sendall timed out":
(https://github.com/bitcoin/bitcoin/issues/33015#issuecomment-3121553822)
https://cirrus-ci.com/task/4962760091500544?logs=ci#L1710
(https://github.com/bitcoin/bitcoin/issues/33015#issuecomment-3121553822)
https://cirrus-ci.com/task/4962760091500544?logs=ci#L1710
💬 fanquake commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3121554126)
ACK faa1c3e80d95552bdc2c0e717065ebf8d510138f
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3121554126)
ACK faa1c3e80d95552bdc2c0e717065ebf8d510138f
✅ fanquake closed an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode"
(https://github.com/bitcoin/bitcoin/issues/32524)
(https://github.com/bitcoin/bitcoin/issues/32524)
🚀 fanquake merged a pull request: "util: Revert "common: Close non-std fds before exec in RunCommandJSON""
(https://github.com/bitcoin/bitcoin/pull/33063)
(https://github.com/bitcoin/bitcoin/pull/33063)