📝 sr-gi opened a pull request: "[DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation)"
(https://github.com/bitcoin/bitcoin/pull/30277)
Erlay Project Tracking: https://github.com/bitcoin/bitcoin/issues/30249
---
This is a full implementation of Erlay. Its purpose is to check the integrity and correctness of the implementation against changes/additions that may originate from the review process and/or rebases on top of newer functionality.
This is not to be merged. Functionality will be spread across multiple smaller PRs to ease the review process.
(https://github.com/bitcoin/bitcoin/pull/30277)
Erlay Project Tracking: https://github.com/bitcoin/bitcoin/issues/30249
---
This is a full implementation of Erlay. Its purpose is to check the integrity and correctness of the implementation against changes/additions that may originate from the review process and/or rebases on top of newer functionality.
This is not to be merged. Functionality will be spread across multiple smaller PRs to ease the review process.
📝 sr-gi converted_to_draft a pull request: "[DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation)"
(https://github.com/bitcoin/bitcoin/pull/30277)
Erlay Project Tracking: https://github.com/bitcoin/bitcoin/issues/30249
---
This is a full implementation of Erlay. Its purpose is to check the integrity and correctness of the implementation against changes/additions that may originate from the review process and/or rebases on top of newer functionality.
This is not to be merged. Functionality will be spread across multiple smaller PRs to ease the review process.
(https://github.com/bitcoin/bitcoin/pull/30277)
Erlay Project Tracking: https://github.com/bitcoin/bitcoin/issues/30249
---
This is a full implementation of Erlay. Its purpose is to check the integrity and correctness of the implementation against changes/additions that may originate from the review process and/or rebases on top of newer functionality.
This is not to be merged. Functionality will be spread across multiple smaller PRs to ease the review process.
💬 m3dwards commented on issue "RPC wont bind without an IP address on a non-localhost interface":
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-2163548438)
I believe this is the same issue as in https://github.com/bitcoin/bitcoin/pull/30245 but in libevent.
If you pass `::1` to `getaddrinfo` when configured with `AI_ADDRCONFIG` it will check that there is a non loopback IPV6 interface configured. This was to prevent IPV6 DNS lookups on IPV4 only machines which could be slow.
Please see https://github.com/bitcoin/bitcoin/pull/30245 for a more full write up and some references.
As I understand it, there are efforts to replace libevent so rat
...
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-2163548438)
I believe this is the same issue as in https://github.com/bitcoin/bitcoin/pull/30245 but in libevent.
If you pass `::1` to `getaddrinfo` when configured with `AI_ADDRCONFIG` it will check that there is a non loopback IPV6 interface configured. This was to prevent IPV6 DNS lookups on IPV4 only machines which could be slow.
Please see https://github.com/bitcoin/bitcoin/pull/30245 for a more full write up and some references.
As I understand it, there are efforts to replace libevent so rat
...
💬 theuni commented on issue "Won't compile with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2163549076)
Looks like we need to update for https://github.com/miniupnp/miniupnp/commit/c0a50ce33e3b99ce8a96fd43049bb5b53ffac62f I'll have a look at this tomorrow.
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2163549076)
Looks like we need to update for https://github.com/miniupnp/miniupnp/commit/c0a50ce33e3b99ce8a96fd43049bb5b53ffac62f I'll have a look at this tomorrow.
💬 m3dwards commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2163577539)
Updated `test_runner.py` so that:
`--exclude "rpc_bind.py --IPV6` will only exclude that variant.
`--exclude rpc_bind.py` will exclude all variants
Green test run with `TEST_RUNNER_EXTRA: --exclude "rpc_bind.py --ipv6, feature_proxy.py"`: https://github.com/m3dwards/bitcoin/actions/runs/9485948819/job/26139127116. Note that `rpc_bind.py --ipv4` and `rpc_bind.py --nonloopback` were both run but `rpc_bind.py --ipv6` and `feature_proxy.py` did not run.
Spaces between test names now also
...
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2163577539)
Updated `test_runner.py` so that:
`--exclude "rpc_bind.py --IPV6` will only exclude that variant.
`--exclude rpc_bind.py` will exclude all variants
Green test run with `TEST_RUNNER_EXTRA: --exclude "rpc_bind.py --ipv6, feature_proxy.py"`: https://github.com/m3dwards/bitcoin/actions/runs/9485948819/job/26139127116. Note that `rpc_bind.py --ipv4` and `rpc_bind.py --nonloopback` were both run but `rpc_bind.py --ipv6` and `feature_proxy.py` did not run.
Spaces between test names now also
...
👋 m3dwards's pull request is ready for review: "ci: parse TEST_RUNNER_EXTRA into an array"
(https://github.com/bitcoin/bitcoin/pull/30244)
(https://github.com/bitcoin/bitcoin/pull/30244)
💬 luke-jr commented on pull request "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options":
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2163670519)
Besides making the code cleaner, I'm hoping to get to a point where it's practical to fix the remaining vsize bugs.
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2163670519)
Besides making the code cleaner, I'm hoping to get to a point where it's practical to fix the remaining vsize bugs.
💬 furszy commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636519093)
Structural topic (not really for this PR):
These two functions, and also `CRecipient` seem to fit better on a new `spend_util.h/cpp` file rather than here.
Also `TransactionChangeType` seem to fit better inside `spend.h/cpp` rather than here.
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636519093)
Structural topic (not really for this PR):
These two functions, and also `CRecipient` seem to fit better on a new `spend_util.h/cpp` file rather than here.
Also `TransactionChangeType` seem to fit better inside `spend.h/cpp` rather than here.
💬 furszy commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636591875)
I must also be missing something here because you should be able to write this as:
```c++
size_t GetSerializeSizeForRecipient(const CRecipient& recipient)
{
return ::GetSerializeSize(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest)));
}
```
And the follow-up PR should also compile this code:
```c++
size_t GetSerializeSizeForRecipient(const CRecipient& recipient)
{
// A Silent Payements address is instructions on how to create a WitnessV1Taproot output
...
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636591875)
I must also be missing something here because you should be able to write this as:
```c++
size_t GetSerializeSizeForRecipient(const CRecipient& recipient)
{
return ::GetSerializeSize(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest)));
}
```
And the follow-up PR should also compile this code:
```c++
size_t GetSerializeSizeForRecipient(const CRecipient& recipient)
{
// A Silent Payements address is instructions on how to create a WitnessV1Taproot output
...
💬 furszy commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636555459)
This include should still be needed. The `CreateTransactionInternal` still calls `GetDustThreshold`.
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1636555459)
This include should still be needed. The `CreateTransactionInternal` still calls `GetDustThreshold`.
💬 mzumsande commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636908381)
commit 4a7f541054d634a34e5f3cbda427e400dc1569f1:
This comment is no longer true and should be removed - the send happens below instead.
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636908381)
commit 4a7f541054d634a34e5f3cbda427e400dc1569f1:
This comment is no longer true and should be removed - the send happens below instead.
💬 mzumsande commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636906205)
unrelated whitespace change?
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636906205)
unrelated whitespace change?
💬 mzumsande commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636917686)
commit 4a7f541054d634a34e5f3cbda427e400dc1569f1:
I think that this could fail intermittently (can put a sleep before this line to trigger).
I think we have to first set `can_data_be_received`, and only then send the rest of the data.
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636917686)
commit 4a7f541054d634a34e5f3cbda427e400dc1569f1:
I think that this could fail intermittently (can put a sleep before this line to trigger).
I think we have to first set `can_data_be_received`, and only then send the rest of the data.
💬 mzumsande commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636934801)
commit 12b9aa22d6a74e4f6e9ef9672569f19d33d4f52b:
I think that the test would fail if `generate_keypair_and_garbage()` returns 0 garbage bytes, so `MAX_GARBAGE_LEN + 1` might be needed here.
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1636934801)
commit 12b9aa22d6a74e4f6e9ef9672569f19d33d4f52b:
I think that the test would fail if `generate_keypair_and_garbage()` returns 0 garbage bytes, so `MAX_GARBAGE_LEN + 1` might be needed here.
💬 Scamreno commented on issue "Enable `importprivkey`, `addmultisigaddress` in descriptor wallets":
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2163749506)
U want me to reply
On Wed, Jun 12, 2024, 9:28 AM maflcko ***@***.***> wrote:
> I think the issue remains that the rescan argument is boolean and optional
> in importaddress, which IIRC was one of the reasons to move toward
> importdescriptors. Yes, the importdescriptors interface is non-trivial,
> but I don't see how the burden can be taken off the user to think about the
> rescan timestamp of the descriptor. It needs to be accurate, because if it
> is too late, funds/txs may be misse
...
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2163749506)
U want me to reply
On Wed, Jun 12, 2024, 9:28 AM maflcko ***@***.***> wrote:
> I think the issue remains that the rescan argument is boolean and optional
> in importaddress, which IIRC was one of the reasons to move toward
> importdescriptors. Yes, the importdescriptors interface is non-trivial,
> but I don't see how the burden can be taken off the user to think about the
> rescan timestamp of the descriptor. It needs to be accurate, because if it
> is too late, funds/txs may be misse
...
💬 Scamreno commented on issue "Enable `importprivkey`, `addmultisigaddress` in descriptor wallets":
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2163753965)
#29772
On Wed, Jun 12, 2024, 3:25 PM Scam Reno ***@***.***> wrote:
> U want me to reply
>
> On Wed, Jun 12, 2024, 9:28 AM maflcko ***@***.***> wrote:
>
>> I think the issue remains that the rescan argument is boolean and
>> optional in importaddress, which IIRC was one of the reasons to move
>> toward importdescriptors. Yes, the importdescriptors interface is
>> non-trivial, but I don't see how the burden can be taken off the user to
>> think about the rescan timestamp of the descr
...
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2163753965)
#29772
On Wed, Jun 12, 2024, 3:25 PM Scam Reno ***@***.***> wrote:
> U want me to reply
>
> On Wed, Jun 12, 2024, 9:28 AM maflcko ***@***.***> wrote:
>
>> I think the issue remains that the rescan argument is boolean and
>> optional in importaddress, which IIRC was one of the reasons to move
>> toward importdescriptors. Yes, the importdescriptors interface is
>> non-trivial, but I don't see how the burden can be taken off the user to
>> think about the rescan timestamp of the descr
...
💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1636990489)
Yes, you're right. Since i2p is `CThreadInterrupt*`. Thanks.
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1636990489)
Yes, you're right. Since i2p is `CThreadInterrupt*`. Thanks.
💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2163773284)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1636502596
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2163773284)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1636502596
💬 luke-jr commented on pull request "Allow to configure custom libzmq prefix":
(https://github.com/bitcoin/bitcoin/pull/30256#issuecomment-2163798160)
Concept ~0. Doesn't seem worth it if we're moving to cmake soon.
(https://github.com/bitcoin/bitcoin/pull/30256#issuecomment-2163798160)
Concept ~0. Doesn't seem worth it if we're moving to cmake soon.
📝 brunoerg opened a pull request: "test: cover more errors for `signrawtransactionwithkey` RPC"
(https://github.com/bitcoin/bitcoin/pull/30278)
This PR adds test coverage for the following errors for the `signrawtransactionwithkey` RPC:
- Invalid private key
- TX decode failed
For reference: https://maflcko.github.io/b-c-cov/total.coverage/src/rpc/rawtransaction.cpp.gcov.html
(https://github.com/bitcoin/bitcoin/pull/30278)
This PR adds test coverage for the following errors for the `signrawtransactionwithkey` RPC:
- Invalid private key
- TX decode failed
For reference: https://maflcko.github.io/b-c-cov/total.coverage/src/rpc/rawtransaction.cpp.gcov.html