💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1157782969)
Looks like this has been implemented as `pub --cleanup`. A quick test shows it working as intended. Will update the doc.
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1157782969)
Looks like this has been implemented as `pub --cleanup`. A quick test shows it working as intended. Will update the doc.
💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1496637445)
I believe I've addressed the comments here. I've chosen to add fresh comments on top rather than squashing to avoid mixing up my changes on top of @jamesob's/@achow101's.
The os/arch filters like `verify.py 22.0-osx` no longer work for me and I'm not quite sure why. Otherwise remote/local verification seem to work ok.
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1496637445)
I believe I've addressed the comments here. I've chosen to add fresh comments on top rather than squashing to avoid mixing up my changes on top of @jamesob's/@achow101's.
The os/arch filters like `verify.py 22.0-osx` no longer work for me and I'm not quite sure why. Otherwise remote/local verification seem to work ok.
💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1496649369)
> Is it not possible to achieve the same with something simpler, like the following? (...)
We'd need one more `if` so that we don't stop advertising onion addresses to `127.0.0.1` peers (inbounds from tor).
But yes, I will try this alternative out.
Alternatively, I also thought of renaming `GetReachabilityFrom` to something like `GetAdvertisingScore` - since we don't use it anywhere else.
> Or why not even expand this logic to all networks and delete `CNetAddr::GetReachabilityFrom()`?
...
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1496649369)
> Is it not possible to achieve the same with something simpler, like the following? (...)
We'd need one more `if` so that we don't stop advertising onion addresses to `127.0.0.1` peers (inbounds from tor).
But yes, I will try this alternative out.
Alternatively, I also thought of renaming `GetReachabilityFrom` to something like `GetAdvertisingScore` - since we don't use it anywhere else.
> Or why not even expand this logic to all networks and delete `CNetAddr::GetReachabilityFrom()`?
...
💬 jonatack commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1157809059)
After removing these two `swap` calls in the last commit bfb9291, should we remove the unused `swap` function?
<details><summary>sample diff</summary><p>
```diff
diff --git a/src/prevector.h b/src/prevector.h
index bcab1ff00cd..2e8510acbe6 100644
--- a/src/prevector.h
+++ b/src/prevector.h
@@ -459,12 +459,6 @@ public:
return *item_ptr(size() - 1);
}
- void swap(prevector<N, T, Size, Diff>& other) noexcept
- {
- std::swap(_union, other._union);
-
...
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1157809059)
After removing these two `swap` calls in the last commit bfb9291, should we remove the unused `swap` function?
<details><summary>sample diff</summary><p>
```diff
diff --git a/src/prevector.h b/src/prevector.h
index bcab1ff00cd..2e8510acbe6 100644
--- a/src/prevector.h
+++ b/src/prevector.h
@@ -459,12 +459,6 @@ public:
return *item_ptr(size() - 1);
}
- void swap(prevector<N, T, Size, Diff>& other) noexcept
- {
- std::swap(_union, other._union);
-
...
💬 jonatack commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496697631)
> I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is CCheckQueueSpeedPrevectorJob goes from 364.56ns down to 346.21ns.
Here are my results with `./src/bench/bench_bitcoin -filter='CCheckQueueSpeedPrevectorJob*.*'`.
master @ 369d4c03b7084de967576759545ba36a17fc18bb
```
| ns/job | job/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----
...
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496697631)
> I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is CCheckQueueSpeedPrevectorJob goes from 364.56ns down to 346.21ns.
Here are my results with `./src/bench/bench_bitcoin -filter='CCheckQueueSpeedPrevectorJob*.*'`.
master @ 369d4c03b7084de967576759545ba36a17fc18bb
```
| ns/job | job/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----
...
💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157862081)
Whoops I meant `int operator() (const OutputGroup& group1, const OutputGroup& group2) const `
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157862081)
Whoops I meant `int operator() (const OutputGroup& group1, const OutputGroup& group2) const `
⚠️ Baronz1 opened an issue: "I forgot the password of one of my wallets on Bitcoin Core App"
(https://github.com/bitcoin/bitcoin/issues/27421)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
There is BTC in there. What can I do?
I use a Macbook
(https://github.com/bitcoin/bitcoin/issues/27421)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
There is BTC in there. What can I do?
I use a Macbook
✅ fanquake closed an issue: "I forgot the password of one of my wallets on Bitcoin Core App"
(https://github.com/bitcoin/bitcoin/issues/27421)
(https://github.com/bitcoin/bitcoin/issues/27421)
💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1496755215)
Rebased ac9fee615a4f0c4d1bbed0d69486c54be4860dcb -> d31816eb5a0518c80f6cc4166fb4246acfc4decd ([`pr/ignoredconf.9`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.9) -> [`pr/ignoredconf.10`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.10), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.9-rebase..pr/ignoredconf.10)) due to conflict with #27254. Also added release note and applied review suggestions
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1496755215)
Rebased ac9fee615a4f0c4d1bbed0d69486c54be4860dcb -> d31816eb5a0518c80f6cc4166fb4246acfc4decd ([`pr/ignoredconf.9`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.9) -> [`pr/ignoredconf.10`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.10), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.9-rebase..pr/ignoredconf.10)) due to conflict with #27254. Also added release note and applied review suggestions
💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157902972)
sure, pushed
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157902972)
sure, pushed
💬 paten12 commented on issue "failure in wallet_basic.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1496777632)
> https://cirrus-ci.com/task/6282049166770176?logs=ci#L3112
>
> ```shell
> node0 2023-03-12T18:59:07.047547Z [scheduler] [wallet/wallet.h:840] [WalletLogPrintf] [default_wallet] AddToWallet 0a94aab7b4dc8ba22b88ccd6b2c9898936c7d36bf8b3e1789f6083b85f6d752a new
> test 2023-03-12T18:59:07.053000Z TestFramework (ERROR): Assertion failed
> Traceback (most recent call last):
> File "/tmp/cirrus-ci-build/ci/scratch/buil
...
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1496777632)
> https://cirrus-ci.com/task/6282049166770176?logs=ci#L3112
>
> ```shell
> node0 2023-03-12T18:59:07.047547Z [scheduler] [wallet/wallet.h:840] [WalletLogPrintf] [default_wallet] AddToWallet 0a94aab7b4dc8ba22b88ccd6b2c9898936c7d36bf8b3e1789f6083b85f6d752a new
> test 2023-03-12T18:59:07.053000Z TestFramework (ERROR): Assertion failed
> Traceback (most recent call last):
> File "/tmp/cirrus-ci-build/ci/scratch/buil
...
💬 ajtowns commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1157916340)
(Would support changing the default to 0sat/vb though)
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1157916340)
(Would support changing the default to 0sat/vb though)
💬 martinus commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496950282)
> The only exception is a slowdown at https://github.com/bitcoin/bitcoin/commit/81f67977f543faca2dcc35846f73e2917375ae79 for the DirectNontrivial benchmark, but all the other improvements look quite significant.
I think the reason why DirectNontrivial got slower when adding `noexcept` is that now the supposedly faster move operations are used when resizing the vector, but since this was implemented as a `swap`, the implementation had to swap the union which I think involves 3 copies behind th
...
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496950282)
> The only exception is a slowdown at https://github.com/bitcoin/bitcoin/commit/81f67977f543faca2dcc35846f73e2917375ae79 for the DirectNontrivial benchmark, but all the other improvements look quite significant.
I think the reason why DirectNontrivial got slower when adding `noexcept` is that now the supposedly faster move operations are used when resizing the vector, but since this was implemented as a `swap`, the implementation had to swap the union which I think involves 3 copies behind th
...
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158081088)
ah, yes. You're right, now I see why SRD will always find a solution if it exists. LGTM
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158081088)
ah, yes. You're right, now I see why SRD will always find a solution if it exists. LGTM
💬 hebasto commented on issue "gen-manpages output depends on build options, so needs to check them":
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1497000975)
> I am not able to understand exactly what this issue is trying to resolve..I have realised that the python script talked about here generates man-pages for each binary in the src/doc/man directory..but I am not able to understand what is meant by the documentation of arguments of build parameters...can anyone please explain me and clear my confusion regarding the same.
For instance, if your system prevents `./configure` from defining `HAVE_SYSTEM`, the `bitcoind -help` output will not contai
...
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1497000975)
> I am not able to understand exactly what this issue is trying to resolve..I have realised that the python script talked about here generates man-pages for each binary in the src/doc/man directory..but I am not able to understand what is meant by the documentation of arguments of build parameters...can anyone please explain me and clear my confusion regarding the same.
For instance, if your system prevents `./configure` from defining `HAVE_SYSTEM`, the `bitcoind -help` output will not contai
...
💬 hebasto commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1497004515)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1497004515)
Concept ACK.
💬 ajtowns commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1497009569)
ACK 552684976b6df34ce563458f73812e6e494e3b0e
Looking at this, it occurs to me that we're actually being a bit wasteful scanning through ~780k historical headers 29 times (or 2.4M headers 29 times for testnet) every time we startup. Replacing `MinBIP9WarningHeight` with `MinBIP9WarningStartTime` (set it to say 2022-07-01 initially), and using it for `WarningBitsConditionChecker::BeginTime()`, and just bumping it at each release might be worthwhile.
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1497009569)
ACK 552684976b6df34ce563458f73812e6e494e3b0e
Looking at this, it occurs to me that we're actually being a bit wasteful scanning through ~780k historical headers 29 times (or 2.4M headers 29 times for testnet) every time we startup. Replacing `MinBIP9WarningHeight` with `MinBIP9WarningStartTime` (set it to say 2022-07-01 initially), and using it for `WarningBitsConditionChecker::BeginTime()`, and just bumping it at each release might be worthwhile.
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158084125)
I think this should be `change_output_size` not `change_spend_size`
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158084125)
I think this should be `change_output_size` not `change_spend_size`
💬 hebasto commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1158131412)
Add a copyright header?
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1158131412)
Add a copyright header?
💬 Satoshingmx commented on issue "Bitcoin.org/https://www.bitcoin.org/www.bitcoin.org":
(https://github.com/bitcoin/bitcoin/issues/27416#issuecomment-1497096744)
Yes what is your question
(https://github.com/bitcoin/bitcoin/issues/27416#issuecomment-1497096744)
Yes what is your question