💬 achow101 commented on issue "importing a wallet containing an hdseed overwrites target wallet hdseed":
(https://github.com/bitcoin/bitcoin/issues/28927#issuecomment-1825170183)
`importwallet` does not import any metadata, including whether something is the seed. That it overwrites the seed metadata is expected behavior, although it should still work as the seed. You can check whether both wallets use the same seed by looking at `hdseedid` in `getwalletinfo`, and also generating some addresses to see if both wallets recognize them with `getaddressinfo`.
However, the workflow that you are using uses the legacy wallet storage format which is targeted to be removed soon
...
(https://github.com/bitcoin/bitcoin/issues/28927#issuecomment-1825170183)
`importwallet` does not import any metadata, including whether something is the seed. That it overwrites the seed metadata is expected behavior, although it should still work as the seed. You can check whether both wallets use the same seed by looking at `hdseedid` in `getwalletinfo`, and also generating some addresses to see if both wallets recognize them with `getaddressinfo`.
However, the workflow that you are using uses the legacy wallet storage format which is targeted to be removed soon
...
💬 Daniel600 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1825182273)
> @Daniel600
>
> > At GAP600 to date we have not seen any significant impact of FULLRBF on our service - we have seen circa 2 trxs which affected us, keep in mind we processed Circa 180k unique trx hash in Sept of 2023.
>
> Have you actually tested full-RBF double spends against your service? Because at the moment, about 40% of hash power is mining with full-RBF enabled. So the chance of a full-RBF double-spend succeeding is approximately 40%.
>
> As an example, here in El Salvador the
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1825182273)
> @Daniel600
>
> > At GAP600 to date we have not seen any significant impact of FULLRBF on our service - we have seen circa 2 trxs which affected us, keep in mind we processed Circa 180k unique trx hash in Sept of 2023.
>
> Have you actually tested full-RBF double spends against your service? Because at the moment, about 40% of hash power is mining with full-RBF enabled. So the chance of a full-RBF double-spend succeeding is approximately 40%.
>
> As an example, here in El Salvador the
...
💬 quakemmo commented on issue "importing a wallet containing an hdseed overwrites target wallet hdseed":
(https://github.com/bitcoin/bitcoin/issues/28927#issuecomment-1825262967)
@achow101 Thank you for your reply.
Can you please point me to the right direction as far as the "proper workflow" is concerned? My basic need is to have two nodes that I can failover between in case one goes down, with no coin loss (obviously) and reasonable downtime.
The nodes will be used for a classic mechanism of receiving user deposits (each user has its own BTC address), and sending withdrawals using "sendtoaddress" at first, then using raw transactions when I implement that. I pre
...
(https://github.com/bitcoin/bitcoin/issues/28927#issuecomment-1825262967)
@achow101 Thank you for your reply.
Can you please point me to the right direction as far as the "proper workflow" is concerned? My basic need is to have two nodes that I can failover between in case one goes down, with no coin loss (obviously) and reasonable downtime.
The nodes will be used for a classic mechanism of receiving user deposits (each user has its own BTC address), and sending withdrawals using "sendtoaddress" at first, then using raw transactions when I implement that. I pre
...
👍 TheCharlatan approved a pull request: "ci: remove `python3-setuptools` from macOS build deps"
(https://github.com/bitcoin/bitcoin/pull/28932#pullrequestreview-1747457083)
ACK 0ffcc5b680b92cf921fc11afb05e4a3607572d41
(https://github.com/bitcoin/bitcoin/pull/28932#pullrequestreview-1747457083)
ACK 0ffcc5b680b92cf921fc11afb05e4a3607572d41
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1404092298)
Improved the documentation around it a bit.
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1404092298)
Improved the documentation around it a bit.
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1825334360)
Addressed the comments, mostly refactoring. Some conversations pending above. The code is good for review.
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1825334360)
Addressed the comments, mostly refactoring. Some conversations pending above. The code is good for review.
💬 maflcko commented on issue "wallet RPC to double-check the calculated balance":
(https://github.com/bitcoin/bitcoin/issues/28898#issuecomment-1825338315)
> An alternative/parallel course of action could be making `rescanblockchain` much faster. Removing the need for additional RPC calls to identify any missing tx.
I don't think this is an alternative, because the rescan won't find the txs if the birthdate is wrong, see also https://github.com/bitcoin/bitcoin/issues/28897? Moreover, on hardware without parallel cores, building the block filter index is not going to be sped up. Finally, on wallets with a sufficient number of keys, the blockfilte
...
(https://github.com/bitcoin/bitcoin/issues/28898#issuecomment-1825338315)
> An alternative/parallel course of action could be making `rescanblockchain` much faster. Removing the need for additional RPC calls to identify any missing tx.
I don't think this is an alternative, because the rescan won't find the txs if the birthdate is wrong, see also https://github.com/bitcoin/bitcoin/issues/28897? Moreover, on hardware without parallel cores, building the block filter index is not going to be sped up. Finally, on wallets with a sufficient number of keys, the blockfilte
...
💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1404107883)
good point.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1404107883)
good point.
💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1404108378)
well, actually....you still can track it with a map<addr, bool>, no?
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1404108378)
well, actually....you still can track it with a map<addr, bool>, no?
👍 dergoegge approved a pull request: "fuzz: Limit fuzz buffer size in script_flags target"
(https://github.com/bitcoin/bitcoin/pull/28931#pullrequestreview-1747698896)
ACK faf1fb207fb6e9a12c864074f8c40d5922d93ff4
(https://github.com/bitcoin/bitcoin/pull/28931#pullrequestreview-1747698896)
ACK faf1fb207fb6e9a12c864074f8c40d5922d93ff4
💬 dergoegge commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#issuecomment-1825460284)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28929#issuecomment-1825460284)
Concept ACK
💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1825462831)
> Our service rests on the first seen rule. Based on our experience a trx with confirmed inputs, reasonable fee, not seen conflicting trx for 5-7 seconds - is reliable. Also the trxs do not happen in a vacuum but rather in commercial exchanges with expected characteristics.
Again, you still have refused to answer my question: Have you actually tested full-RBF double spends against your service?
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1825462831)
> Our service rests on the first seen rule. Based on our experience a trx with confirmed inputs, reasonable fee, not seen conflicting trx for 5-7 seconds - is reliable. Also the trxs do not happen in a vacuum but rather in commercial exchanges with expected characteristics.
Again, you still have refused to answer my question: Have you actually tested full-RBF double spends against your service?
💬 hebasto commented on pull request "ci: Avoid toolset ambiguity that MSVC can't handle":
(https://github.com/bitcoin/bitcoin/pull/28905#issuecomment-1825472700)
> Do we know why this broke again? I guess this is something we'll just have to maintain forever?
I've done a bit more research.
In GHA Windows images, it is an accepted practice to have different versions of MSVC toolsets being installed side-by-side.
For example, the current image 20231115 has the following toolsets installed:
- 14.29.16.11
- 14.37.17.7
- 14.38.17.8
The [`ilammy/msvc-dev-cmd`](https://github.com/ilammy/msvc-dev-cmd) action chooses the latest compiler toolset ver
...
(https://github.com/bitcoin/bitcoin/pull/28905#issuecomment-1825472700)
> Do we know why this broke again? I guess this is something we'll just have to maintain forever?
I've done a bit more research.
In GHA Windows images, it is an accepted practice to have different versions of MSVC toolsets being installed side-by-side.
For example, the current image 20231115 has the following toolsets installed:
- 14.29.16.11
- 14.37.17.7
- 14.38.17.8
The [`ilammy/msvc-dev-cmd`](https://github.com/ilammy/msvc-dev-cmd) action chooses the latest compiler toolset ver
...
💬 theStack commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1404207499)
Good point indeed. To execute the steps build/encrypt/send-message atomically, I think introducing a new lock and acquiring it in the `send_message` method should do the trick? I.e.
```diff
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index a70682a768..59707471a6 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -164,6 +164,7 @@ class P2PConnection(asyncio.Protocol):
# The underlying tra
...
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1404207499)
Good point indeed. To execute the steps build/encrypt/send-message atomically, I think introducing a new lock and acquiring it in the `send_message` method should do the trick? I.e.
```diff
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index a70682a768..59707471a6 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -164,6 +164,7 @@ class P2PConnection(asyncio.Protocol):
# The underlying tra
...
💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1404234919)
+1, good catch! when i tried this, just running `self.test_resource_exhaustion()` in `p2p_invalid messages.py` took too much time.
1. in v1, without locks - 0m22.265s
2. in v2, without locks - won't work
3. in v1, with locks - 0m22.101s
4. in v2, with locks - 8m45.916s
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1404234919)
+1, good catch! when i tried this, just running `self.test_resource_exhaustion()` in `p2p_invalid messages.py` took too much time.
1. in v1, without locks - 0m22.265s
2. in v2, without locks - won't work
3. in v1, with locks - 0m22.101s
4. in v2, with locks - 8m45.916s
💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1404246109)
`*std::get_if` may be UB? In any case, it would be good to submit fuzz inputs that cover everything here.
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1404246109)
`*std::get_if` may be UB? In any case, it would be good to submit fuzz inputs that cover everything here.
💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-1825561375)
Rebased and addressed comments
- https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1155488768: @martinus I took parts from your suggestion and added you as a co-author for the first commit. Thanks for the macro-magic. There's now only the `TRACEPOINT()` macro that works for tracepoints with 0 to 12 args.
- https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1140555021: I clarified unit tests and added a new test for a manual `TRACEPOINT_ACTIVE()` macro.
- https://github.com/bi
...
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-1825561375)
Rebased and addressed comments
- https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1155488768: @martinus I took parts from your suggestion and added you as a co-author for the first commit. Thanks for the macro-magic. There's now only the `TRACEPOINT()` macro that works for tracepoints with 0 to 12 args.
- https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1140555021: I clarified unit tests and added a new test for a manual `TRACEPOINT_ACTIVE()` macro.
- https://github.com/bi
...
📝 maflcko opened a pull request: "fuzz: Faster wallet_notifications target"
(https://github.com/bitcoin/bitcoin/pull/28933)
Avoid read/write from storage to speed the target up.
(https://github.com/bitcoin/bitcoin/pull/28933)
Avoid read/write from storage to speed the target up.
👍 willcl-ark approved a pull request: "Fee Estimator updates from Validation Interface/CScheduler thread"
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1746716540)
ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
Glad to see the fee estimator being decoupled in this way and keen to see more development of additional estimators in the future e.g. as per #27995
I gave the code a review and it seems well thought-out to me at this stage; left a few nits which can be addressed in the case that you re-touch things.
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1746716540)
ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
Glad to see the fee estimator being decoupled in this way and keen to see more development of additional estimators in the future e.g. as per #27995
I gave the code a review and it seems well thought-out to me at this stage; left a few nits which can be addressed in the case that you re-touch things.
💬 willcl-ark commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1403539773)
nit if you retouch bfcd401368fc0dc43827a8969a37b7e038d5ca79:
```suggestion
* Provides the block that was disconnected.
```
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1403539773)
nit if you retouch bfcd401368fc0dc43827a8969a37b7e038d5ca79:
```suggestion
* Provides the block that was disconnected.
```
💬 willcl-ark commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1404174636)
micro nit, if you end up touching 714523918ba2b853fc69bee6b04a33ba0c828bf5 again:
```suggestion
// Drop transactions we were still watching, record fee estimations and unregister
```
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1404174636)
micro nit, if you end up touching 714523918ba2b853fc69bee6b04a33ba0c828bf5 again:
```suggestion
// Drop transactions we were still watching, record fee estimations and unregister
```